You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ch...@apache.org on 2008/10/16 23:09:27 UTC

svn commit: r705361 - in /httpd/httpd/trunk/modules/aaa: mod_authz_dbd.c mod_authz_dbm.c mod_authz_groupfile.c mod_authz_owner.c mod_authz_user.c

Author: chrisd
Date: Thu Oct 16 14:09:27 2008
New Revision: 705361

URL: http://svn.apache.org/viewvc?rev=705361&view=rev
Log:
Prior to authn/z refactoring in r368027, if authorization Require
directives had no matching AuthType and associated authentication
directives, requests would generally fall through in the
check_user_id hook to mod_authn_default.c's authentication_no_user()
handler, which returned DECLINED if ap_auth_type() was not set.
The ap_process_request_internal() function in request.c would handle
this case by logging an "AuthType not set!" error and returning
HTTP_INTERNAL_SERVER_ERROR.

The refactoring removes this error handling in request.c, so
individual modules will need to test for a lack of authentication,
as necessary.  Since some modules such as mod_authz_host.c support
Require directives that do not need any authentication, the
mod_authn_default.c handler no longer returns DECLINED if ap_auth_type()
is not set.  (Also, mod_authn_default can be compiled out with
--disable-authn-default, so it can't be relied upon to exist.)

Since r->user may now be NULL, individual handlers must test for that
case when necessary.  Otherwise, most Require directives in the
absence of AuthType directives cause handlers to crash while performing
strcmp() and friends on a NULL r->user value.

NOTE: I can't test mod_authnz_ldap.c myself, so I'm not sure if it
needs similar fixes.  On the one hand, a NULL r->user in the authz
handlers always generates a log message.  However, it appears that
authn_ldap_build_filter() will sometimes then be called, perform no
action, which may result in a possibly uninitialized filtbuf buffer
being passed to util_ldap_cache_getuserdn().  I don't know if that
could cause problems in the LDAP cache code.  If someone familiar with
LDAP authz could take a look, that would be much appreciated.

Modified:
    httpd/httpd/trunk/modules/aaa/mod_authz_dbd.c
    httpd/httpd/trunk/modules/aaa/mod_authz_dbm.c
    httpd/httpd/trunk/modules/aaa/mod_authz_groupfile.c
    httpd/httpd/trunk/modules/aaa/mod_authz_owner.c
    httpd/httpd/trunk/modules/aaa/mod_authz_user.c

Modified: httpd/httpd/trunk/modules/aaa/mod_authz_dbd.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authz_dbd.c?rev=705361&r1=705360&r2=705361&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/aaa/mod_authz_dbd.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_authz_dbd.c Thu Oct 16 14:09:27 2008
@@ -253,6 +253,12 @@
     authz_dbd_cfg *cfg = ap_get_module_config(r->per_dir_config,
                                               &authz_dbd_module);
 
+    if (!r->user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     if (groups == NULL) {
         groups = apr_array_make(r->pool, 4, sizeof(const char*));
         rv = authz_dbd_group_query(r, cfg, groups);
@@ -280,6 +286,12 @@
     authz_dbd_cfg *cfg = ap_get_module_config(r->per_dir_config,
                                               &authz_dbd_module);
 
+    if (!r->user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     return (authz_dbd_login(r, cfg, "login") == OK ? AUTHZ_GRANTED : AUTHZ_DENIED);
 }
 
@@ -289,6 +301,12 @@
     authz_dbd_cfg *cfg = ap_get_module_config(r->per_dir_config,
                                               &authz_dbd_module);
 
+    if (!r->user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     return (authz_dbd_login(r, cfg, "logout") == OK ? AUTHZ_GRANTED : AUTHZ_DENIED);
 }
 

Modified: httpd/httpd/trunk/modules/aaa/mod_authz_dbm.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authz_dbm.c?rev=705361&r1=705360&r2=705361&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/aaa/mod_authz_dbm.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_authz_dbm.c Thu Oct 16 14:09:27 2008
@@ -143,6 +143,12 @@
     const char *groups;
     char *v;
 
+    if (!user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     if (!conf->grpfile) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
                         "No group file was specified in the configuration");
@@ -209,6 +215,12 @@
     const char *groups;
     char *v;
 
+    if (!user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     if (!conf->grpfile) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
                         "No group file was specified in the configuration");

Modified: httpd/httpd/trunk/modules/aaa/mod_authz_groupfile.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authz_groupfile.c?rev=705361&r1=705360&r2=705361&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/aaa/mod_authz_groupfile.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_authz_groupfile.c Thu Oct 16 14:09:27 2008
@@ -147,6 +147,12 @@
     apr_table_t *grpstatus = NULL;
     apr_status_t status;
 
+    if (!user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     /* If there is no group file - then we are not
      * configured. So decline.
      */
@@ -202,6 +208,12 @@
     apr_status_t status;
     const char *filegroup = NULL;
 
+    if (!user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     /* If there is no group file - then we are not
      * configured. So decline.
      */

Modified: httpd/httpd/trunk/modules/aaa/mod_authz_owner.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authz_owner.c?rev=705361&r1=705360&r2=705361&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/aaa/mod_authz_owner.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_authz_owner.c Thu Oct 16 14:09:27 2008
@@ -54,6 +54,12 @@
     char *owner = NULL;
     apr_finfo_t finfo;
 
+    if (!r->user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     if (!r->filename) {
         reason = "no filename available";
         ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,

Modified: httpd/httpd/trunk/modules/aaa/mod_authz_user.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authz_user.c?rev=705361&r1=705360&r2=705361&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/aaa/mod_authz_user.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_authz_user.c Thu Oct 16 14:09:27 2008
@@ -50,6 +50,12 @@
 {
     const char *t, *w;
 
+    if (!r->user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     t = require_args;
     while ((w = ap_getword_conf(r->pool, &t)) && w[0]) {
         if (!strcmp(r->user, w)) {
@@ -67,6 +73,12 @@
 
 static authz_status validuser_check_authorization(request_rec *r, const char *require_line)
 {
+    if (!r->user) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+            "access to %s failed, reason: no authenticated user", r->uri);
+        return AUTHZ_DENIED;
+    }
+
     return AUTHZ_GRANTED;
 }
 



Re: svn commit: r705361 - in /httpd/httpd/trunk/modules/aaa: mod_authz_dbd.c mod_authz_dbm.c mod_authz_groupfile.c mod_authz_owner.c mod_authz_user.c

Posted by Chris Darroch <ch...@pearsoncmg.com>.
Eric Covener wrote:

> Authorization in LDAP has a special path for when authentication
> wasn't handled by mod_authnz_ldap, but r->user still may be mappable
> to an DN on the LDAP server. Net, it can't do anything useful without
> r->user.  This short-circuit should be possible well before the
> problematic functions you mention.

   OK, I'll look at adding the same !r->user test, unless others
think it's unnecessary.  It looks like it might be necessary to
me, but I'm not 100% sure.

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B


Re: svn commit: r705361 - in /httpd/httpd/trunk/modules/aaa: mod_authz_dbd.c mod_authz_dbm.c mod_authz_groupfile.c mod_authz_owner.c mod_authz_user.c

Posted by Eric Covener <co...@gmail.com>.
On Thu, Oct 16, 2008 at 5:09 PM,  <ch...@apache.org> wrote:

> NOTE: I can't test mod_authnz_ldap.c myself, so I'm not sure if it
> needs similar fixes.  On the one hand, a NULL r->user in the authz
> handlers always generates a log message.  However, it appears that
> authn_ldap_build_filter() will sometimes then be called, perform no
> action, which may result in a possibly uninitialized filtbuf buffer
> being passed to util_ldap_cache_getuserdn().  I don't know if that
> could cause problems in the LDAP cache code.  If someone familiar with
> LDAP authz could take a look, that would be much appreciated.

IIUC mod_authnz_ldap would follow the pattern of the modules in your
commit and not mod_authz_host.

Authorization in LDAP has a special path for when authentication
wasn't handled by mod_authnz_ldap, but r->user still may be mappable
to an DN on the LDAP server. Net, it can't do anything useful without
r->user.  This short-circuit should be possible well before the
problematic functions you mention.

-- 
Eric Covener
covener@gmail.com