httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Colm MacCarthaigh <c...@stdlib.net>
Subject Documentation for mod_cgid suexec patch
Date Tue, 24 Jan 2006 21:02:40 GMT

I'm conscious that cgid not many people deal with the cgid code, to the
point that before the Autumn, I think the last major change to it was a
patch I submitted 4 years ago, so it's not fresh in people's minds. 

Nevertheless I'm also conscious that it's used by a lot of people, and
some of them want to use it with third party modules that implement the
suexec hook, and also that the httpd API should really work as
advertised.

So, in a last effort to get the patch into 2.0.x, here's some verbose
documentation on what it does :) If the patch doesn't get in, I suggest
we declare the API broken in 2.0.x and just leave it at that. It's fixed
in 2.2.x and we can point people there.

The patch I'm talking about is:

	http://people.apache.org/~colm/2.0.x-suexec-cgid.patch

Background:

	mod_cgid forks off a daemon process at startup. I'm going to 
	refer to this process as "cgid", distinct from the "httpd"
	side of the code. Before the fork a unix socket is created
	and both the httpd side and the cgid maintain a reference
	to this.

	When mod_cgid handles a request - within httpd - it presently
	works out relevant metadata concerning the process to be
	executed and writes that accross to cgid, which forks and
	execs the neccessary process.

The problem:

	Right now, part of the metadata that we send to cgid is the
	"mod_userdir_user" note from r->notes. So that when the
	mod_userdir suexec identity hook is run on the cgid side
	it doesn't segfault, and we get the right user to run the
	process as.

	In essence, this means we have a mod_userdir-specific hack
	in cgid to make the suexec identity hook work. When other
	modules implement this hook, and they get run within cgid
	they don't have their usual environment available. r->notes
	is meaningless to them, other places they search may be
	unavailable, and so on. So the hook simply doesn't work.

The solution:

	Instead of just running the hook within cgid, the patch
	changes is so that the hook is run within httpd, where all
	of the environment is available, r->notes is set-up properly
	and so on.

The details:

	On the httpd side, after we've run the hook, where we used
	to send the userdir note from r->notes, now we send the
	ap_unix_identity_t structure. If the resulf of running the
	hook was NULL (ie don't use suexec), then we send over
	a magic instance of the structure, corresponding to:

	  static ap_unix_identity_t empty_ugid = {(uid_t)-1, (gid_t)-1, -1 };

	Although -1 is a possible (though unlikely) value for the first
	two elements. -1 is not a valid element for the third element
	(int suexec_enabled; see os/unix/unixd.h) so there is no
	danger of confusing a valid suexec identity structure with our
	magic constants.

	Patched, cgid has it's own suexec identity doer. It's
	cgid_suexec_id_doer(). It's a simple doer, and just assumes that
	the entire request config is the ap_unix_identity_t structure.
	When cgid starts we add this doer with the REALLY_FIRST
	instruction, and then do an explicit apr_hook_sort_all(); This
	guarantees that - within cgid - our own private doer will be run
	first amongst the registered doers for the hook.

	When cgid receives its metadata, it blindly does:

	ap_set_module_config(r->request_config, &cgid_module, (void *)&req->ugid);

	At this point that ugid may be a valid one, or it may be
	identical to the empty_ugid magic constant. However the request
	config is now set-up, and is something our own private doer can
	return. 

	Lastly, to ensure that our doer is never called when the
	identity is really empty; cgid now checks to see if the ugid
	is the empty one, if so it will not call
	ap_os_create_privileged_process() (which runs the hook), and
	instead calls regular apr_proc_create() (which does not run
	the hook).

	The design decision on that last point was this: it may 
	appear tempting to instead have to doer do something like:

		if(!memcmp(ugid, empty_ugid, sizeof(ugid)))
		    return NULL;

	but if our doer ever return'd null, we'd have a problem
	as the next doer registered with the hook would be run. And
	we'd back to the original problem. Although I guess an
	alternative would be to use apr_hook_deregister_all() and
	only register our own hook.

That's it, the patch should be reasnoably clear. If anyone does get a
chance or have an inclination, it's appreciated. If not, we can mark the
API as broken.

-- 
Colm MacCárthaigh                        Public Key: colm+pgp@stdlib.net

Mime
View raw message