You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Brad Nicholes <BN...@novell.com> on 2006/01/12 02:05:14 UTC

Re: Merging branch authz-dev - Authorization and Access Control 2.3vs. 2.2

>>> On 1/11/2006 at 3:43:36 pm, in message <43...@apache.org>,
rpluem@apache.org wrote:

> 
> On 01/11/2006 11:23 PM, Roy T. Fielding wrote:
>> On Jan 11, 2006, at 7:19 AM, Joshua Slive wrote:
>> 
>>> [Your merge today prompted me to dig out a response I started but
>>> never finished.]
>>>
>>> I am still worried that we are underestimating the pain that this will
>>> cause.  In my opinion, a config change that requires substantial
>>> changes to every httpd.conf and many .htaccess files requires a major
>>> version bump (to 3.0) unless it can, in some way, be made seamless to
>>> the end user.  And there is no way to deny that this will put a large
>>> roadblock in the way of upgraders.
>> 
>> 
>> It isn't just your opinion -- incompatible configuration changes
>> means third-parties have to change their source code, which means a
>> major version bump is required.  So either somebody gets busy on
>> implementing backward-compatibility or this stuff gets bumped to 3.x.
> 
> +1
> 
> Regards
> 
> RĂ¼diger

I'm looking for help.  I obviously don't have all of the answers :)

Brad


Re: Merging branch authz-dev - Authorization and Access Control 2.3vs. 2.2

Posted by Brad Nicholes <BN...@novell.com>.
>>> On 1/13/2006 at 8:19:39 am, in message
<e4...@mail.gmail.com>,
joshua@slive.ca 
wrote:
> I would consider moving this compat code into mod_access_compat or
> something of the like so that it would be easy for people to select
> whether they want the clean new system or a mix of the new and old
> system.  Since this change will not come until at least 2.4, I think
> it is acceptable to have module name changes (as we did from 2.0 to
> 2.2).
> 
> Joshua.

I'm OK with that.  I will rework the patch into a mod_access_compat
module.

Brad

Re: Merging branch authz-dev - Authorization and Access Control 2.3vs. 2.2

Posted by Joshua Slive <jo...@slive.ca>.
On 1/12/06, Brad Nicholes <bn...@novell.com> wrote:
>   OK, try this on for size.  Since Order,Allow,Deny are all hooked at
> the access_checker stage, we should be able to add these directives back
> in and allow them to function normally.  The real problem is 'Satisfy'
> because it had its fingers into the middle of
> ap_process_request_internal().  So to get around this problem, I added
> the directive back into mod_authz_host (along with Order,Allow,Deny) and
> let it do it thing also.  'Satisfy All' is the default (as it was
> before) meaning that if the access_checker fails or the authz_checker
> fails, then the entire request fails.  But if 'Satisfy Any' is specified
> then if the access_checker fails, it makes a note of that fact in the
> request_rec->notes and defers to the authz_checker.  If the
> authz_checker fails, obviously the request fails.  But if the request
> makes it all the way to the mod_authz_default handler, this handler
> checks the note and determines whether or not to authorize or reject the
> request based on what the current state of both access control and
> authz.
>
>    So what this means is that Order,Allow,Deny,Satisfy are back and
> *should* function as before along side of the new authz model.  This
> should resolve the backward compatibility issue with the following
> caveat.  Both mod_authn_default and mod_authz_default modules must be
> loaded.  These module implement the catch-all handlers that allow things
> to work if no authn or authz is implemented for a <Directory> or
> <Location>.  Otherwise access is automatically denied. What I would like
> to see is the above  *should* confirmed to be a *do*.  In other words, I
> am looking for some help testing this.  Any takers?  So far my testing
> shows that things are good.

The idea seems right to me.  I'll try to find time to do some testing,
but I'm pretty backed-up at the moment.

I would consider moving this compat code into mod_access_compat or
something of the like so that it would be easy for people to select
whether they want the clean new system or a mix of the new and old
system.  Since this change will not come until at least 2.4, I think
it is acceptable to have module name changes (as we did from 2.0 to
2.2).

Joshua.

Re: Merging branch authz-dev - Authorization and Access Control 2.3vs. 2.2

Posted by Brad Nicholes <bn...@novell.com>.
  OK, try this on for size.  Since Order,Allow,Deny are all hooked at
the access_checker stage, we should be able to add these directives back
in and allow them to function normally.  The real problem is 'Satisfy'
because it had its fingers into the middle of
ap_process_request_internal().  So to get around this problem, I added
the directive back into mod_authz_host (along with Order,Allow,Deny) and
let it do it thing also.  'Satisfy All' is the default (as it was
before) meaning that if the access_checker fails or the authz_checker
fails, then the entire request fails.  But if 'Satisfy Any' is specified
then if the access_checker fails, it makes a note of that fact in the
request_rec->notes and defers to the authz_checker.  If the
authz_checker fails, obviously the request fails.  But if the request
makes it all the way to the mod_authz_default handler, this handler
checks the note and determines whether or not to authorize or reject the
request based on what the current state of both access control and
authz.  
   
   So what this means is that Order,Allow,Deny,Satisfy are back and
*should* function as before along side of the new authz model.  This
should resolve the backward compatibility issue with the following
caveat.  Both mod_authn_default and mod_authz_default modules must be
loaded.  These module implement the catch-all handlers that allow things
to work if no authn or authz is implemented for a <Directory> or
<Location>.  Otherwise access is automatically denied. What I would like
to see is the above  *should* confirmed to be a *do*.  In other words, I
am looking for some help testing this.  Any takers?  So far my testing
shows that things are good.

  I'll commit this patch to trunk of nobody sees any glaring issues.
 
Brad

Index: modules/aaa/mod_authz_host.c
===================================================================
--- modules/aaa/mod_authz_host.c        (revision 368081)
+++ modules/aaa/mod_authz_host.c        (working copy)
@@ -44,22 +44,175 @@
 #include <netinet/in.h>
 #endif

+enum allowdeny_type {
+    T_ENV,
+    T_ALL,
+    T_IP,
+    T_HOST,
+    T_FAIL
+};
+
 typedef struct {
-       int dummy;  /* just here to stop compiler warnings for now. */
+    apr_int64_t limited;
+    union {
+        char *from;
+        apr_ipsubnet_t *ip;
+    } x;
+    enum allowdeny_type type;
+} allowdeny;
+
+/* things in the 'order' array */
+#define DENY_THEN_ALLOW 0
+#define ALLOW_THEN_DENY 1
+#define MUTUAL_FAILURE 2
+
+/** all of the requirements must be met */
+#define SATISFY_ALL 0
+/**  any of the requirements must be met */
+#define SATISFY_ANY 1
+/** There are no applicable satisfy lines */
+#define SATISFY_NOSPEC 2
+
+
+typedef struct {
+    int order[METHODS];
+    apr_array_header_t *allows;
+    apr_array_header_t *denys;
+    int *satisfy; /* for every method one */
 } authz_host_dir_conf;

 module AP_MODULE_DECLARE_DATA authz_host_module;

 static void *create_authz_host_dir_config(apr_pool_t *p, char *dummy)
 {
+    int i;
     authz_host_dir_conf *conf =
         (authz_host_dir_conf *)apr_pcalloc(p,
sizeof(authz_host_dir_conf));

+    for (i = 0; i < METHODS; ++i) {
+        conf->order[i] = DENY_THEN_ALLOW;
+    }
+    conf->allows = apr_array_make(p, 1, sizeof(allowdeny));
+    conf->denys = apr_array_make(p, 1, sizeof(allowdeny));
+    conf->satisfy = apr_palloc(p, sizeof(*conf->satisfy) * METHODS);
+    for (i = 0; i < METHODS; ++i) {
+        conf->satisfy[i] = SATISFY_NOSPEC;
+    }
+
     return (void *)conf;
 }

+static const char *order(cmd_parms *cmd, void *dv, const char *arg)
+{
+    authz_host_dir_conf *d = (authz_host_dir_conf *) dv;
+    int i, o;
+
+    if (!strcasecmp(arg, "allow,deny"))
+        o = ALLOW_THEN_DENY;
+    else if (!strcasecmp(arg, "deny,allow"))
+        o = DENY_THEN_ALLOW;
+    else if (!strcasecmp(arg, "mutual-failure"))
+        o = MUTUAL_FAILURE;
+    else
+        return "unknown order";
+
+    for (i = 0; i < METHODS; ++i)
+        if (cmd->limited & (AP_METHOD_BIT << i))
+            d->order[i] = o;
+
+    return NULL;
+}
+
+static const char *satisfy(cmd_parms *cmd, void *dv, const char *arg)
+{
+    authz_host_dir_conf *d = (authz_host_dir_conf *) dv;
+    int satisfy = SATISFY_NOSPEC;
+    int i;
+
+    if (!strcasecmp(arg, "all")) {
+        satisfy = SATISFY_ALL;
+    }
+    else if (!strcasecmp(arg, "any")) {
+        satisfy = SATISFY_ANY;
+    }
+    else {
+        return "Satisfy either 'any' or 'all'.";
+    }
+
+    for (i = 0; i < METHODS; ++i) {
+        if (cmd->limited & (AP_METHOD_BIT << i)) {
+            d->satisfy[i] = satisfy;
+        }
+    }
+
+    return NULL;
+}
+
+static const char *allow_cmd(cmd_parms *cmd, void *dv, const char
*from,
+                             const char *where_c)
+{
+    authz_host_dir_conf *d = (authz_host_dir_conf *) dv;
+    allowdeny *a;
+    char *where = apr_pstrdup(cmd->pool, where_c);
+    char *s;
+    char msgbuf[120];
+    apr_status_t rv;
+
+    if (strcasecmp(from, "from"))
+        return "allow and deny must be followed by 'from'";
+
+    a = (allowdeny *) apr_array_push(cmd->info ? d->allows :
d->denys);
+    a->x.from = where;
+    a->limited = cmd->limited;
+
+    if (!strncasecmp(where, "env=", 4)) {
+        a->type = T_ENV;
+        a->x.from += 4;
+
+    }
+    else if (!strcasecmp(where, "all")) {
+        a->type = T_ALL;
+    }
+    else if ((s = ap_strchr(where, '/'))) {
+        *s++ = '\0';
+        rv = apr_ipsubnet_create(&a->x.ip, where, s, cmd->pool);
+        if(APR_STATUS_IS_EINVAL(rv)) {
+            /* looked nothing like an IP address */
+            return "An IP address was expected";
+        }
+        else if (rv != APR_SUCCESS) {
+            apr_strerror(rv, msgbuf, sizeof msgbuf);
+            return apr_pstrdup(cmd->pool, msgbuf);
+        }
+        a->type = T_IP;
+    }
+    else if (!APR_STATUS_IS_EINVAL(rv = apr_ipsubnet_create(&a->x.ip,
where,
+                                                            NULL,
cmd->pool))) {
+        if (rv != APR_SUCCESS) {
+            apr_strerror(rv, msgbuf, sizeof msgbuf);
+            return apr_pstrdup(cmd->pool, msgbuf);
+        }
+        a->type = T_IP;
+    }
+    else { /* no slash, didn't look like an IP address => must be a
host */
+        a->type = T_HOST;
+    }
+
+    return NULL;
+}
+
+static char its_an_allow;
+
 static const command_rec authz_host_cmds[] =
 {
+    AP_INIT_TAKE1("order", order, NULL, OR_LIMIT,
+                  "'allow,deny', 'deny,allow', or 'mutual-failure'"),
+    AP_INIT_ITERATE2("allow", allow_cmd, &its_an_allow, OR_LIMIT,
+                     "'from' followed by hostnames or IP-address
wildcards"),
+    AP_INIT_ITERATE2("deny", allow_cmd, NULL, OR_LIMIT,
+                     "'from' followed by hostnames or IP-address
wildcards"),
+    AP_INIT_TAKE1("Satisfy", satisfy, NULL, OR_AUTHCFG,
+                  "access policy if both allow and require used ('all'
or 'any')"),
     {NULL}
 };

@@ -90,6 +243,135 @@
     }
 }

+static int find_allowdeny(request_rec *r, apr_array_header_t *a, int
method)
+{
+
+    allowdeny *ap = (allowdeny *) a->elts;
+    apr_int64_t mmask = (AP_METHOD_BIT << method);
+    int i;
+    int gothost = 0;
+    const char *remotehost = NULL;
+
+    for (i = 0; i < a->nelts; ++i) {
+        if (!(mmask & ap[i].limited)) {
+            continue;
+        }
+
+        switch (ap[i].type) {
+        case T_ENV:
+            if (apr_table_get(r->subprocess_env, ap[i].x.from)) {
+                return 1;
+            }
+            break;
+
+        case T_ALL:
+            return 1;
+
+        case T_IP:
+            if (apr_ipsubnet_test(ap[i].x.ip,
r->connection->remote_addr)) {
+                return 1;
+            }
+            break;
+
+        case T_HOST:
+            if (!gothost) {
+                int remotehost_is_ip;
+
+                remotehost = ap_get_remote_host(r->connection,
+                                                r->per_dir_config,
+                                                REMOTE_DOUBLE_REV,
+                                                &remotehost_is_ip);
+
+                if ((remotehost == NULL) || remotehost_is_ip) {
+                    gothost = 1;
+                }
+                else {
+                    gothost = 2;
+                }
+            }
+
+            if ((gothost == 2) && in_domain(ap[i].x.from, remotehost))
{
+                return 1;
+            }
+            break;
+
+        case T_FAIL:
+            /* do nothing? */
+            break;
+        }
+    }
+
+    return 0;
+}
+
+static int ap_satisfies(request_rec *r)
+{
+    authz_host_dir_conf *conf = (authz_host_dir_conf *)
+        ap_get_module_config(r->per_dir_config, &authz_host_module);
+
+    return conf->satisfy[r->method_number];
+}
+
+static int check_dir_access(request_rec *r)
+{
+    int method = r->method_number;
+    int ret = OK;
+    authz_host_dir_conf *a = (authz_host_dir_conf *)
+        ap_get_module_config(r->per_dir_config, &authz_host_module);
+
+    if (a->order[method] == ALLOW_THEN_DENY) {
+        ret = HTTP_FORBIDDEN;
+        if (find_allowdeny(r, a->allows, method)) {
+            ret = OK;
+        }
+        if (find_allowdeny(r, a->denys, method)) {
+            ret = HTTP_FORBIDDEN;
+        }
+    }
+    else if (a->order[method] == DENY_THEN_ALLOW) {
+        if (find_allowdeny(r, a->denys, method)) {
+            ret = HTTP_FORBIDDEN;
+        }
+        if (find_allowdeny(r, a->allows, method)) {
+            ret = OK;
+        }
+    }
+    else {
+        if (find_allowdeny(r, a->allows, method)
+            && !find_allowdeny(r, a->denys, method)) {
+            ret = OK;
+        }
+        else {
+            ret = HTTP_FORBIDDEN;
+        }
+    }
+
+       /*
+    if (ret == HTTP_FORBIDDEN
+        && (ap_satisfies(r) != SATISFY_ANY ||
!ap_some_auth_required(r))) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "client denied by server configuration: %s",
+            r->filename);
+    }
+       */
+
+       if (ret == HTTP_FORBIDDEN) {
+               /* If Satisfy is Any and authorization is required,
then make a
+                  note of it and defer to the authorization stage */
+               if ((ap_satisfies(r) == SATISFY_ANY) &&
ap_some_auth_required(r)) {
+                       apr_table_setn(r->notes,
AUTHZ_ACCESS_FAILED_NOTE, "Y");
+                       return OK;
+               }
+               else {
+                       ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                                                 "client denied by
server configuration: %s",
+                                                 r->filename);
+               }
+       }
+
+    return ret;
+}
+
 static authz_status env_check_authorization(request_rec *r, const char
*require_line)
 {
     const char *t, *w;
@@ -252,6 +534,9 @@
                          &authz_host_provider);
     ap_register_provider(p, AUTHZ_PROVIDER_GROUP, "all", "0",
                          &authz_all_provider);
+
+    /* This can be access checker since we don't require r->user to be
set. */
+   
ap_hook_access_checker(check_dir_access,NULL,NULL,APR_HOOK_MIDDLE);
 }

 module AP_MODULE_DECLARE_DATA authz_host_module =
Index: modules/aaa/mod_authz_default.c
===================================================================
--- modules/aaa/mod_authz_default.c     (revision 368081)
+++ modules/aaa/mod_authz_default.c     (working copy)
@@ -25,6 +25,9 @@
 #include "http_protocol.h"
 #include "http_request.h"

+#include "mod_auth.h"
+
+
 typedef struct {
     int authoritative;
 } authz_default_config_rec;
@@ -53,7 +56,14 @@
 {
     authz_default_config_rec *conf =
ap_get_module_config(r->per_dir_config,
                                                 
&authz_default_module);
+       const char *note = apr_table_get(r->notes,
AUTHZ_ACCESS_FAILED_NOTE);

+       /* If we got here and there isn't any authz required and there
is no
+          note from the access checker that it failed, assume access
is OK */
+       if (!ap_some_auth_required(r) && note) {
+               return OK;
+       }
+
     if (!(conf->authoritative)) {
         return DECLINED;
     }
Index: modules/aaa/mod_auth.h
===================================================================
--- modules/aaa/mod_auth.h      (revision 368081)
+++ modules/aaa/mod_auth.h      (working copy)
@@ -42,6 +42,7 @@
 #define AUTHZ_GROUP_NOTE "authz_group_note"
 #define AUTHN_PROVIDER_NAME_NOTE "authn_provider_name"
 #define AUTHZ_PROVIDER_NAME_NOTE "authz_provider_name"
+#define AUTHZ_ACCESS_FAILED_NOTE "authz_access_failed"

 typedef enum {
     AUTH_DENIED,