You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Rodent of Unusual Size <Ke...@Golux.Com> on 1999/05/05 18:07:16 UTC
Re: [PATCH] "responsible party" for requests.
I've played with this patch, and it looks rather interesting.
I've made some tweaks for style and consistency; the updated
patch (against to-day's HEAD) is attached.
I haven't tested this with CGI or suexec; I don't use suexec
at all. Maybe someone else could run it through some suexec
paces?
+1 for 1.3.7.
--
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Software Foundation <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Index: include/http_core.h
===================================================================
RCS file: /home/cvs/apache-1.3/src/include/http_core.h,v
retrieving revision 1.56
diff -u -r1.56 http_core.h
--- http_core.h 1999/04/28 08:35:08 1.56
+++ http_core.h 1999/05/05 16:00:36
@@ -162,6 +162,11 @@
API_EXPORT(file_type_e) ap_get_win32_interpreter(const request_rec *, char*, char **);
#endif
+typedef enum { URI_OWNED_UNSET, URI_OWNED_SERVER, URI_OWNED_UID } uri_owner_e;
+
+API_EXPORT(int) ap_get_uri_owner(const request_rec *r, uid_t *u, gid_t *g);
+API_EXPORT(uri_owner_e) ap_get_uri_owner_type(const request_rec *r);
+
#ifdef CORE_PRIVATE
/*
@@ -263,6 +268,9 @@
/* Where to find interpreter to run scripts */
interpreter_source_e script_interpreter_source;
#endif
+
+ /* Is the resource owned by the UID of the server or the file owner? */
+ uri_owner_e uri_owner;
} core_dir_config;
Index: main/http_core.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_core.c,v
retrieving revision 1.260
diff -u -r1.260 http_core.c
--- http_core.c 1999/04/28 08:35:31 1.260
+++ http_core.c 1999/05/05 16:00:36
@@ -150,6 +150,7 @@
#endif
conf->server_signature = srv_sig_unset;
+ conf->uri_owner = URI_OWNED_UNSET;
return (void *)conf;
}
@@ -278,6 +279,9 @@
conf->server_signature = new->server_signature;
}
+ conf->uri_owner = (new->uri_owner != URI_OWNED_UNSET)
+ ? new->uri_owner : base->uri_owner;
+
return (void*)conf;
}
@@ -904,6 +908,48 @@
}
#endif
+API_EXPORT(int) ap_get_uri_owner(const request_rec *r, uid_t *uid, gid_t *gid)
+{
+ uri_owner_e uri_owner;
+
+ uri_owner = ap_get_uri_owner_type(r);
+
+ if (uri_owner == URI_OWNED_UID) {
+ *uid = r->finfo.st_uid;
+ *gid = r->finfo.st_gid;
+ }
+ else if (uri_owner == URI_OWNED_UNSET && !strncmp(r->uri, "/~", 2)) {
+ struct passwd *pw;
+ char *username, *slash;
+
+ username = ap_pstrdup(r->pool, r->uri+2);
+ if ((slash = strchr(username, '/')) != NULL) {
+ *slash = '\0';
+ }
+ pw = getpwnam(username);
+ if (!pw) {
+ return -1;
+ }
+
+ *uid = pw->pw_uid;
+ *gid = pw->pw_gid;
+ }
+ else {
+ *uid = r->server->server_uid;
+ *gid = r->server->server_gid;
+ }
+ return 0;
+}
+
+API_EXPORT(uri_owner_e) ap_get_uri_owner_type(const request_rec *r)
+{
+ core_dir_config *d;
+
+ d = (core_dir_config *)ap_get_module_config(r->per_dir_config,
+ &core_module);
+ return d->uri_owner;
+}
+
/*****************************************************************
*
* Commands... this module handles almost all of the NCSA httpd.conf
@@ -2653,6 +2699,21 @@
}
#endif
+static const char *set_uri_owner(cmd_parms *cmd, core_dir_config *d,
+ char *arg)
+{
+ if (!strcasecmp(arg, "server")) {
+ d->uri_owner = URI_OWNED_SERVER;
+ }
+ else if (!strcasecmp(arg, "file")) {
+ d->uri_owner = URI_OWNED_UID;
+ }
+ else {
+ return "Argument to URIowner must be either \"server\" or \"file\"";
+ }
+ return NULL;
+}
+
/* Note --- ErrorDocument will now work from .htaccess files.
* The AllowOverride of Fileinfo allows webmasters to turn it off
*/
@@ -2878,6 +2939,8 @@
(void*)XtOffsetOf(core_dir_config, limit_req_body),
OR_ALL, TAKE1,
"Limit (in bytes) on maximum size of request message body" },
+{ "URIOwner", set_uri_owner, NULL, RSRC_CONF|ACCESS_CONF, TAKE1,
+ "Owner of the URL (either 'server' or 'file')" },
{ NULL }
};
Index: main/util_script.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/util_script.c,v
retrieving revision 1.140
diff -u -r1.140 util_script.c
--- util_script.c 1999/04/08 11:36:33 1.140
+++ util_script.c 1999/05/05 16:00:37
@@ -986,11 +986,29 @@
|| (!strncmp("/~", r->uri, 2)))) {
char *execuser, *grpname;
+ uid_t exec_uid;
+ gid_t exec_gid;
struct passwd *pw;
struct group *gr;
- if (!strncmp("/~", r->uri, 2)) {
- gid_t user_gid;
+ if (ap_get_uri_owner(r, &exec_uid, &exec_gid) < 0) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
+ "ap_get_uri_owner: failed on %s", r->uri);
+ return (pid);
+ }
+
+ if ((pw = getpwuid(exec_uid)) == NULL) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
+ "getpwuid: invalid userid %ld",
+ (long) exec_uid);
+ }
+ if ((gr = getgrgid(exec_gid)) == NULL) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
+ "getgrgid: invalid groupid %ld",
+ (long) exec_gid);
+ }
+ if (ap_get_uri_owner_type(r) == URI_OWNED_UNSET
+ && !strncmp("/~", r->uri, 2)) {
char *username = ap_pstrdup(r->pool, r->uri + 2);
char *pos = strchr(username, '/');
@@ -998,20 +1016,19 @@
*pos = '\0';
}
- if ((pw = getpwnam(username)) == NULL) {
+ if (strcmp(username, pw->pw_name) != 0) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
- "getpwnam: invalid username %s", username);
+ "getpwnam: invalid username %s", username);
return (pid);
}
execuser = ap_pstrcat(r->pool, "~", pw->pw_name, NULL);
- user_gid = pw->pw_gid;
- if ((gr = getgrgid(user_gid)) == NULL) {
+ if ((gr = getgrgid(exec_gid)) == NULL) {
if ((grpname = ap_palloc(r->pool, 16)) == NULL) {
return (pid);
}
else {
- ap_snprintf(grpname, 16, "%ld", (long) user_gid);
+ ap_snprintf(grpname, 16, "%ld", (long) exec_gid);
}
}
else {
@@ -1019,20 +1036,7 @@
}
}
else {
- if ((pw = getpwuid(r->server->server_uid)) == NULL) {
- ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
- "getpwuid: invalid userid %ld",
- (long) r->server->server_uid);
- return (pid);
- }
- execuser = ap_pstrdup(r->pool, pw->pw_name);
-
- if ((gr = getgrgid(r->server->server_gid)) == NULL) {
- ap_log_rerror(APLOG_MARK, APLOG_ERR, r,
- "getgrgid: invalid groupid %ld",
- (long) r->server->server_gid);
- return (pid);
- }
+ execuser = pw->pw_name;
grpname = gr->gr_name;
}
Index: modules/standard/mod_log_config.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_log_config.c,v
retrieving revision 1.76
diff -u -r1.76 mod_log_config.c
--- mod_log_config.c 1999/03/04 19:28:40 1.76
+++ mod_log_config.c 1999/05/05 16:00:43
@@ -124,6 +124,8 @@
* %...a: remote IP-address
* %...{Foobar}i: The contents of Foobar: header line(s) in the request
* sent to the client.
+ * %...I: the UID that owns the requested URL
+ * %...G: the GID that owns the requested URL
* %...l: remote logname (from identd, if supplied)
* %...{Foobar}n: The contents of note "Foobar" from another module.
* %...{Foobar}o: The contents of Foobar: header line(s) in the reply.
@@ -350,6 +352,28 @@
return ap_table_get(r->headers_in, a);
}
+static const char *log_request_uid(request_rec *r, char *a)
+{
+ uid_t uid;
+ gid_t gid;
+
+ if (ap_get_uri_owner(r, &uid, &gid) < 0) {
+ return "-";
+ }
+ return ap_psprintf(r->pool, "%u", uid);
+}
+
+static const char *log_request_gid(request_rec *r, char *a)
+{
+ uid_t uid;
+ gid_t gid;
+
+ if (ap_get_uri_owner(r, &uid, &gid) < 0) {
+ return "-";
+ }
+ return ap_psprintf(r->pool, "%u", gid);
+}
+
static const char *log_header_out(request_rec *r, char *a)
{
const char *cp = ap_table_get(r->headers_out, a);
@@ -480,6 +504,12 @@
},
{
'o', log_header_out, 0
+ },
+ {
+ 'I', log_request_uid, 0
+ },
+ {
+ 'G', log_request_gid, 0
},
{
'n', log_note, 0
Re: [PATCH] "responsible party" for requests.
Posted by Aidan Cully <ai...@panix.com>.
After a bit of prodding from Ken Coar, here's an updated version of the
patch, based on his. Unfortunately, I'm not yet able to test out this
version on a running web server.. I wouldn't even have posted this yet
if the 1.3.7/1.4 release cycle weren't coming up.
The only real differences between this version and the version Ken posted
a couple months ago is that a serious bug in the ap_exec_script function
has been fixed, and that we now only use the UID on the file to determine
execution permissions, and run getpwuid() on this uid to figure out the
gid to use. I _have_ tested the "serious bug fix" on our boxes here..
mostly. The other feature I added prodded me into changing my logic a bit.
Both changes are relatively minor differences from the tested version of
this patch, so I don't believe I've broken anything.
Enjoy..
--aidan
--
Aidan Cully "Umm.. Could you call back? My house is on fire."
Panix Staff --Tom Waits
aidan@panix.com
Re: [PATCH] "responsible party" for requests.
Posted by Aidan Cully <ai...@panix.com>.
On Fri, May 07, 1999 at 05:56:01PM, Raymond S Brand said:
> The existing directive is OK (maybe it should be called ExecAsFileOwner); just
> change the behavior in util_script.c so that after the call to
> ap_get_uri_owner(), it does a getpw*() and uses the group from the passwd
> file.
All right, not that it's very much work, but I'll modify the patch to do
this in a month or more. I've only just upgraded our webhosts from Apache
1.2 to 1.3, and I don't like forcing too many changes on our users at once.
--aidan
--
Aidan Cully "You can lead a whore to culture, but you can't make her
Panix Staff think."
aidan@panix.com --Dorothy Parker
Re: [PATCH] "responsible party" for requests.
Posted by Raymond S Brand <rs...@rsbx.net>.
Aidan Cully wrote:
>
> On Fri, May 07, 1999 at 05:16:24PM, Raymond S Brand said:
> > Aidan Cully wrote:
> > > The point is that the hole exists outside of apache/suexec. He can
> > > _always_ chmod g+s on the file and get group privs.
> >
> > Actually, most modern systems insist that the user be a member of the
> > group of the file for chmod g+s to work. So, on those systems, Apache/suexec
> > will be a security hole.
>
> Ah, looks like you're right.. If I added a directive
> URIOwner FileUser
> which did a getpwuid on the st_uid for the file, and returned the group
> from that, would that be acceptable? Or is that too hackish? Maybe it
> would have been better to override the 'User'/'Group' directives than
> to add a new URIOwner?
The existing directive is OK (maybe it should be called ExecAsFileOwner); just
change the behavior in util_script.c so that after the call to
ap_get_uri_owner(), it does a getpw*() and uses the group from the passwd
file.
Raymond S Brand
Re: [PATCH] "responsible party" for requests.
Posted by Aidan Cully <ai...@panix.com>.
On Fri, May 07, 1999 at 05:16:24PM, Raymond S Brand said:
> Aidan Cully wrote:
> > The point is that the hole exists outside of apache/suexec. He can
> > _always_ chmod g+s on the file and get group privs.
>
> Actually, most modern systems insist that the user be a member of the
> group of the file for chmod g+s to work. So, on those systems, Apache/suexec
> will be a security hole.
Ah, looks like you're right.. If I added a directive
URIOwner FileUser
which did a getpwuid on the st_uid for the file, and returned the group
from that, would that be acceptable? Or is that too hackish? Maybe it
would have been better to override the 'User'/'Group' directives than
to add a new URIOwner?
--aidan
--
Aidan Cully "I saw Judas carryin' the body/ Of John Wilkes Booth..
Panix Staff Down there by the train..."
aidan@panix.com -Johnny Cash
Re: [PATCH] "responsible party" for requests.
Posted by Raymond S Brand <rs...@rsbx.net>.
Aidan Cully wrote:
>
> On Fri, May 07, 1999 at 04:38:17PM, Raymond S Brand said:
> > Aidan Cully wrote:
> > > I've thought about this argument, and I don't think I buy it.. If a
> > > file ever gets created that's owned by the user, and in a group the
> > > user isn't in, there's already a security hole.. The user can already
> > > chmod g+s on the file, and then call the setgid script from another
> > > script since SuEXEC doesn't like setid.
> > >
> > > And, of course, if the server isn't configured with this directive,
> > > behaviour shouldn't change.
> > >
> >
> > What about the situation where a user is removed from a group? This is
> > the example where Apache/suexec is the security hole.
>
> The point is that the hole exists outside of apache/suexec. He can
> _always_ chmod g+s on the file and get group privs.
>
Actually, most modern systems insist that the user be a member of the
group of the file for chmod g+s to work. So, on those systems, Apache/suexec
will be a security hole.
Raymond S Brand
Re: [PATCH] "responsible party" for requests.
Posted by Aidan Cully <ai...@panix.com>.
On Fri, May 07, 1999 at 04:38:17PM, Raymond S Brand said:
> Aidan Cully wrote:
> > I've thought about this argument, and I don't think I buy it.. If a
> > file ever gets created that's owned by the user, and in a group the
> > user isn't in, there's already a security hole.. The user can already
> > chmod g+s on the file, and then call the setgid script from another
> > script since SuEXEC doesn't like setid.
> >
> > And, of course, if the server isn't configured with this directive,
> > behaviour shouldn't change.
> >
>
> What about the situation where a user is removed from a group? This is
> the example where Apache/suexec is the security hole.
The point is that the hole exists outside of apache/suexec. He can
_always_ chmod g+s on the file and get group privs.
--aidan
--
Aidan Cully "The 16-day-old moon looks down sadly on a town overrun
Panix Staff with wild dogs."
aidan@panix.com --Abarenbo Shogun
Re: [PATCH] "responsible party" for requests.
Posted by Raymond S Brand <rs...@rsbx.net>.
Aidan Cully wrote:
>
> On Thu, May 06, 1999 at 05:46:04PM, Raymond S Brand said:
> > Aidan Cully wrote:
>
> I've thought about this argument, and I don't think I buy it.. If a
> file ever gets created that's owned by the user, and in a group the
> user isn't in, there's already a security hole.. The user can already
> chmod g+s on the file, and then call the setgid script from another
> script since SuEXEC doesn't like setid.
>
> And, of course, if the server isn't configured with this directive,
> behaviour shouldn't change.
>
What about the situation where a user is removed from a group? This is
the example where Apache/suexec is the security hole.
Raymond S Brand
Re: [PATCH] "responsible party" for requests.
Posted by Aidan Cully <ai...@panix.com>.
On Thu, May 06, 1999 at 05:46:04PM, Raymond S Brand said:
> Aidan Cully wrote:
> >
> > In an effort to make this a slightly easier discussion for people
> > to get involved in, let me explain what the SuEXEC-related changes
> > are supposed to do..
> >
> ...
> >
> > I'd like to know that I solved this PR in a good way.. Can
> > someone else that uses SuEXEC please look at the patch?
> >
>
> I use suexec and, at a previous employer, needed something similar
> to what you seem to be wanting to do. I've looked at the patch,
> though not tried it.
>
> Current Apache/suexec behavior is to execute the CGI as User:Group
> from the config files or as UID:GID of the user from the passwd file.
>
> The patched behavior adds execution as UID:GID of the file to execute.
> The difference is the GIDs and the new behavior is a setGID script.
> This I see as a security hole.
I've thought about this argument, and I don't think I buy it.. If a
file ever gets created that's owned by the user, and in a group the
user isn't in, there's already a security hole.. The user can already
chmod g+s on the file, and then call the setgid script from another
script since SuEXEC doesn't like setid.
And, of course, if the server isn't configured with this directive,
behaviour shouldn't change.
--aidan
--
Aidan Cully "Specialists without spirit, sensualists without heart;
Panix Staff this nullity imagines that it has attained a level of
aidan@panix.com civilization never before achieved." -- Goethe
Re: [PATCH] "responsible party" for requests.
Posted by Raymond S Brand <rs...@rsbx.net>.
Aidan Cull wrote:
>
> In an effort to make this a slightly easier discussion for people
> to get involved in, let me explain what the SuEXEC-related changes
> are supposed to do..
>
...
>
> I'd like to know that I solved this PR in a good way.. Can
> someone else that uses SuEXEC please look at the patch?
>
I use suexec and, at a previous employer, needed something similar
to what you seem to be wanting to do. I've looked at the patch,
though not tried it.
Current Apache/suexec behavior is to execute the CGI as User:Group
from the config files or as UID:GID of the user from the passwd file.
The patched behavior adds execution as UID:GID of the file to execute.
The difference is the GIDs and the new behavior is a setGID script.
This I see as a security hole.
Raymond S Brand
Re: [PATCH] "responsible party" for requests.
Posted by Aidan Cully <ai...@panix.com>.
On Wed, May 05, 1999 at 12:07:16PM, Rodent of Unusual Size said:
> I've played with this patch, and it looks rather interesting.
> I've made some tweaks for style and consistency; the updated
> patch (against to-day's HEAD) is attached.
>
> I haven't tested this with CGI or suexec; I don't use suexec
> at all. Maybe someone else could run it through some suexec
> paces?
>
> +1 for 1.3.7.
In an effort to make this a slightly easier discussion for people
to get involved in, let me explain what the SuEXEC-related changes
are supposed to do..
Basically, I wanted this directive to be able to solve the PR I
submitted, suexec/4069: SuEXEC doesn't work with mod_userdir as
well as it should. The problem (as I'm sure everyone is aware) is
that when a /~user request comes in, apache calls SuEXEC with a
~user argument. When SuEXEC gets that argument, it tries to look
up the username in the password file, chdir ~user/public_html, and
then find the CGI-script to execute relative to that directory.
When using mod_userdir, there's no way to know that the ~userdir
will be located in ~user/public_html, so SuEXEC will never execute
a script for this user.
The only solution I had was to call SuEXEC with a 'user' argument,
instead of a '~user' argument, if the httpd.conf is configured to
do that. If you specify a URIOwner for a file, SuEXEC gets 'user'
instead of '~user'. If you don't specify the URIOwner, SuEXEC
continues to behave in the current fashion.
I've been running my hacked up Apache for about a month, now
(though it's still not running on the default port.. hopefully
that'll be fixed in the next two weeks), and I haven't noticed any
ill effects with our CGIs.. That should probably be taken with a
grain of salt, though, since we were always living with a
functionally equivalent hack.
I'd like to know that I solved this PR in a good way.. Can
someone else that uses SuEXEC please look at the patch?
Thanks,
--aidan
--
Aidan Cully "I'd rather have a bottle in front of me than a frontal
Panix Staff lobotomy."
aidan@panix.com --Dorothy Parker