httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From chr...@apache.org
Subject svn commit: r1362317 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID modules/fcgid/mod_fcgid.c
Date Tue, 17 Jul 2012 00:02:27 GMT
Author: chrisd
Date: Tue Jul 17 00:02:26 2012
New Revision: 1362317

URL: http://svn.apache.org/viewvc?rev=1362317&view=rev
Log:
Merge access control hook function logic into a common base, with
improved comments and logging, to reduce code duplication and simplify
future maintenance.

Modified:
    httpd/mod_fcgid/trunk/CHANGES-FCGID
    httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c

Modified: httpd/mod_fcgid/trunk/CHANGES-FCGID
URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/CHANGES-FCGID?rev=1362317&r1=1362316&r2=1362317&view=diff
==============================================================================
--- httpd/mod_fcgid/trunk/CHANGES-FCGID [utf8] (original)
+++ httpd/mod_fcgid/trunk/CHANGES-FCGID [utf8] Tue Jul 17 00:02:26 2012
@@ -6,6 +6,10 @@ Changes with mod_fcgid 2.3.8
      treat Location headers returned by scripts as an error since
      redirections are not meaningful in this mode.  [Chris Darroch]
 
+  *) Merge access control hook function logic into a common base, with
+     improved comments and logging, to reduce code duplication and
+     simplify future maintenance.  [Chris Darroch]
+
 Changes with mod_fcgid 2.3.7
 
   *) Introduce FcgidWin32PreventOrphans directive on Windows to use OS

Modified: httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c
URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c?rev=1362317&r1=1362316&r2=1362317&view=diff
==============================================================================
--- httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c (original)
+++ httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c Tue Jul 17 00:02:26 2012
@@ -46,6 +46,12 @@ enum fcgid_procnode_type {
     FCGID_PROCNODE_TYPE_ERROR,
 };
 
+enum fcgid_auth_check_mode {
+    FCGID_AUTH_CHECK_AUTHN,
+    FCGID_AUTH_CHECK_AUTHZ,
+    FCGID_AUTH_CHECK_ACCESS
+};
+
 /* Stolen from mod_cgi.c */
 /* KLUDGE --- for back-compatibility, we don't have to check ExecCGI
  * in ScriptAliased directories, which means we need to know if this
@@ -477,107 +483,78 @@ static int mod_fcgid_modify_auth_header(
     return 1;
 }
 
-static int mod_fcgid_authenticator(request_rec * r)
+static int mod_fcgid_check_auth(request_rec *r,
+                                enum fcgid_auth_check_mode auth_check_mode)
 {
     int res = 0;
     const char *password = NULL;
     apr_table_t *saved_subprocess_env = NULL;
-    fcgid_cmd_conf *authenticator_info;
+    fcgid_cmd_conf *auth_cmd_info = NULL;
     int authoritative;
+    const char *auth_role = NULL;
+    const char *role_log_msg = NULL;
+    const char *user_log_msg = "";
+
+    /* Because we don't function as authn/z providers, integration with
+     * the standard httpd authn/z modules is somewhat problematic.
+     *
+     * With httpd 2.4 in particular, our hook functions may be
+     * circumvented by mod_authz_core's check_access_ex hook, unless
+     * Require directives specify that user-based authn/z is needed.
+     *
+     * Even then, APR_HOOK_MIDDLE may cause our authentication hook to be
+     * ordered after mod_auth_basic's check_authn hook, in which case it
+     * will be skipped unless AuthBasicAuthoritative is Off and no authn
+     * provider recognizes the user or outright denies the request.
+     *
+     * Also, when acting as an authenticator, we don't have a mechanism to
+     * set r->user based on the script response, so scripts can't implement
+     * a private authentication scheme; instead we use ap_get_basic_auth_pw()
+     * and only support Basic HTTP authentication.
+     *
+     * It is possible to act reliably as both authenticator and authorizer
+     * if mod_authn_core is loaded to support AuthType and AuthName, but
+     * mod_authz_core and mod_auth_basic are not loaded.  However, in this
+     * case the Require directive is not available, which defeats many
+     * common configuration tropes.
+     */
 
-    authenticator_info = get_authenticator_info(r, &authoritative);
+    switch (auth_check_mode) {
+    case FCGID_AUTH_CHECK_AUTHN:
+        auth_cmd_info = get_authenticator_info(r, &authoritative);
+        auth_role = "AUTHENTICATOR";
+        role_log_msg = "Authentication";
+        break;
+
+    case FCGID_AUTH_CHECK_AUTHZ:
+        auth_cmd_info = get_authorizer_info(r, &authoritative);
+        auth_role = "AUTHORIZER";
+        role_log_msg = "Authorization";
+        break;
+
+    case FCGID_AUTH_CHECK_ACCESS:
+        auth_cmd_info = get_access_info(r, &authoritative);
+        auth_role = "ACCESS_CHECKER";
+        role_log_msg = "Access check";
+        break;
+    }
 
-    /* Is authenticator enable? */
-    if (authenticator_info == NULL)
+    /* Is this auth check command enabled? */
+    if (auth_cmd_info == NULL)
         return DECLINED;
 
     /* Get the user password */
-    if ((res = ap_get_basic_auth_pw(r, &password)) != OK)
+    if (auth_check_mode == FCGID_AUTH_CHECK_AUTHN
+        && (res = ap_get_basic_auth_pw(r, &password)) != OK) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                      "mod_fcgid: authenticator requires "
+                      "basic HTTP auth credentials");
         return res;
-
-    /* Save old process environment */
-    saved_subprocess_env = apr_table_copy(r->pool, r->subprocess_env);
-
-    /* Add some environment variables */
-    ap_add_common_vars(r);
-    ap_add_cgi_vars(r);
-    fcgid_add_cgi_vars(r);
-    apr_table_setn(r->subprocess_env, "REMOTE_PASSWD", password);
-    apr_table_setn(r->subprocess_env, "FCGI_APACHE_ROLE", "AUTHENTICATOR");
-
-    /* Drop the variables CONTENT_LENGTH, PATH_INFO, PATH_TRANSLATED,
-     * SCRIPT_NAME and most Hop-By-Hop headers - EXCEPT we will pass
-     * PROXY_AUTH to allow CGI to perform proxy auth for httpd
-     */
-    apr_table_unset(r->subprocess_env, "CONTENT_LENGTH");
-    apr_table_unset(r->subprocess_env, "PATH_INFO");
-    apr_table_unset(r->subprocess_env, "PATH_TRANSLATED");
-    apr_table_unset(r->subprocess_env, "SCRIPT_NAME");
-    apr_table_unset(r->subprocess_env, "HTTP_KEEP_ALIVE");
-    apr_table_unset(r->subprocess_env, "HTTP_TE");
-    apr_table_unset(r->subprocess_env, "HTTP_TRAILER");
-    apr_table_unset(r->subprocess_env, "HTTP_TRANSFER_ENCODING");
-    apr_table_unset(r->subprocess_env, "HTTP_UPGRADE");
-
-    /* Connection hop-by-hop header to prevent the CGI from hanging */
-    apr_table_set(r->subprocess_env, "HTTP_CONNECTION", "close");
-
-    /* Handle the request */
-    res = bridge_request(r, FCGI_AUTHORIZER, authenticator_info);
-
-    /* Restore r->subprocess_env */
-    r->subprocess_env = saved_subprocess_env;
-
-    if (res == OK && r->status == 200
-        && apr_table_get(r->headers_out, "Location") == NULL)
-    {
-        /* Pass */
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,
-                      "mod_fcgid: user %s authentication pass", r->user);
-
-        /* Modify headers: An Authorizer application's 200 response may include headers
-           whose names are prefixed with Variable-.  */
-        apr_table_do(mod_fcgid_modify_auth_header, r->subprocess_env,
-                     r->err_headers_out, NULL);
-
-        return OK;
-    } else {
-        /* Print error info first */
-        if (res != OK)
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s authentication failed, respond %d, URI %s",
-                          r->user, res, r->uri);
-        else if (r->status != 200)
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s authentication failed, status %d, URI %s",
-                          r->user, r->status, r->uri);
-        else
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s authentication failed, redirected is not allowed",
-                          r->user);
-
-        /* Handle error */
-        if (!authoritative)
-            return DECLINED;
-        else {
-            ap_note_basic_auth_failure(r);
-            return (res == OK) ? HTTP_UNAUTHORIZED : res;
-        }
     }
-}
 
-static int mod_fcgid_authorizer(request_rec * r)
-{
-    int res = 0;
-    apr_table_t *saved_subprocess_env = NULL;
-    fcgid_cmd_conf *authorizer_info;
-    int authoritative;
-
-    authorizer_info = get_authorizer_info(r, &authoritative);
-
-    /* Is authenticator enable? */
-    if (authorizer_info == NULL)
-        return DECLINED;
+    if (auth_check_mode != FCGID_AUTH_CHECK_ACCESS) {
+        user_log_msg = apr_psprintf(r->pool, " of user %s", r->user);
+    }
 
     /* Save old process environment */
     saved_subprocess_env = apr_table_copy(r->pool, r->subprocess_env);
@@ -586,7 +563,10 @@ static int mod_fcgid_authorizer(request_
     ap_add_common_vars(r);
     ap_add_cgi_vars(r);
     fcgid_add_cgi_vars(r);
-    apr_table_setn(r->subprocess_env, "FCGI_APACHE_ROLE", "AUTHORIZER");
+    if (auth_check_mode == FCGID_AUTH_CHECK_AUTHN) {
+        apr_table_setn(r->subprocess_env, "REMOTE_PASSWD", password);
+    }
+    apr_table_setn(r->subprocess_env, "FCGI_APACHE_ROLE", auth_role);
 
     /* Drop the variables CONTENT_LENGTH, PATH_INFO, PATH_TRANSLATED,
      * SCRIPT_NAME and most Hop-By-Hop headers - EXCEPT we will pass
@@ -606,17 +586,17 @@ static int mod_fcgid_authorizer(request_
     apr_table_set(r->subprocess_env, "HTTP_CONNECTION", "close");
 
     /* Handle the request */
-    res = bridge_request(r, FCGI_AUTHORIZER, authorizer_info);
+    res = bridge_request(r, FCGI_AUTHORIZER, auth_cmd_info);
 
     /* Restore r->subprocess_env */
     r->subprocess_env = saved_subprocess_env;
 
-    if (res == OK && r->status == 200
-        && apr_table_get(r->headers_out, "Location") == NULL)
-    {
+    if (res == OK && r->status == HTTP_OK
+        && apr_table_get(r->headers_out, "Location") == NULL) {
         /* Pass */
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,
-                      "mod_fcgid: access granted (authorization)");
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+                      "mod_fcgid: %s%s to access %s succeeded",
+                      role_log_msg, user_log_msg, r->uri);
 
         /* Modify headers: An Authorizer application's 200 response may include headers
            whose names are prefixed with Variable-.  */
@@ -624,112 +604,53 @@ static int mod_fcgid_authorizer(request_
                      r->err_headers_out, NULL);
 
         return OK;
-    } else {
+    }
+    else {
+        const char *add_err_msg = "";
+
         /* Print error info first */
-        if (res != OK)
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s authorization failed, respond %d, URI %s",
-                          r->user, res, r->uri);
-        else if (r->status != 200)
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s authorization failed, status %d, URI %s",
-                          r->user, r->status, r->uri);
-        else
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s authorization failed, redirected is not allowed",
-                          r->user);
+        if (res != OK) {
+            add_err_msg =
+                apr_psprintf(r->pool, "; error or unexpected condition "
+                                      "while parsing response (%d)", res);
+        }
+        else if (r->status == HTTP_OK) {
+            add_err_msg = "; internal redirection not allowed";
+        }
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                      "mod_fcgid: %s%s to access %s failed, reason: "
+                      "script returned status %d%s",
+                      role_log_msg, user_log_msg, r->uri, r->status,
+                      add_err_msg);
 
         /* Handle error */
-        if (!authoritative)
+        if (!authoritative) {
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+                          "mod_fcgid: not authoritative");
             return DECLINED;
+        }
         else {
-            ap_note_basic_auth_failure(r);
+            if (auth_check_mode != FCGID_AUTH_CHECK_ACCESS) {
+                ap_note_basic_auth_failure(r);
+            }
             return (res == OK) ? HTTP_UNAUTHORIZED : res;
         }
     }
 }
 
-static int mod_fcgid_check_access(request_rec * r)
+static int mod_fcgid_authenticator(request_rec *r)
 {
-    int res = 0;
-    apr_table_t *saved_subprocess_env = NULL;
-    fcgid_cmd_conf *access_info;
-    int authoritative;
-
-    access_info = get_access_info(r, &authoritative);
-
-    /* Is access check enable? */
-    if (access_info == NULL)
-        return DECLINED;
-
-    /* Save old process environment */
-    saved_subprocess_env = apr_table_copy(r->pool, r->subprocess_env);
-
-    /* Add some environment variables */
-    ap_add_common_vars(r);
-    ap_add_cgi_vars(r);
-    fcgid_add_cgi_vars(r);
-    apr_table_setn(r->subprocess_env, "FCGI_APACHE_ROLE",
-                   "ACCESS_CHECKER");
-
-    /* Drop the variables CONTENT_LENGTH, PATH_INFO, PATH_TRANSLATED,
-     * SCRIPT_NAME and most Hop-By-Hop headers - EXCEPT we will pass
-     * PROXY_AUTH to allow CGI to perform proxy auth for httpd
-     */
-    apr_table_unset(r->subprocess_env, "CONTENT_LENGTH");
-    apr_table_unset(r->subprocess_env, "PATH_INFO");
-    apr_table_unset(r->subprocess_env, "PATH_TRANSLATED");
-    apr_table_unset(r->subprocess_env, "SCRIPT_NAME");
-    apr_table_unset(r->subprocess_env, "HTTP_KEEP_ALIVE");
-    apr_table_unset(r->subprocess_env, "HTTP_TE");
-    apr_table_unset(r->subprocess_env, "HTTP_TRAILER");
-    apr_table_unset(r->subprocess_env, "HTTP_TRANSFER_ENCODING");
-    apr_table_unset(r->subprocess_env, "HTTP_UPGRADE");
-
-    /* Connection hop-by-hop header to prevent the CGI from hanging */
-    apr_table_set(r->subprocess_env, "HTTP_CONNECTION", "close");
-
-    /* Handle the request */
-    res = bridge_request(r, FCGI_AUTHORIZER, access_info);
-
-    /* Restore r->subprocess_env */
-    r->subprocess_env = saved_subprocess_env;
-
-    if (res == OK && r->status == 200
-        && apr_table_get(r->headers_out, "Location") == NULL)
-    {
-        /* Pass */
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,
-                      "mod_fcgid: access check pass");
-
-        /* Modify headers: An Authorizer application's 200 response may include headers
-           whose names are prefixed with Variable-.  */
-        apr_table_do(mod_fcgid_modify_auth_header, r->subprocess_env,
-                     r->err_headers_out, NULL);
+    return mod_fcgid_check_auth(r, FCGID_AUTH_CHECK_AUTHN);
+}
 
-        return OK;
-    } else {
-        /* Print error info first */
-        if (res != OK)
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s access check failed, respond %d, URI %s",
-                          r->user, res, r->uri);
-        else if (r->status != 200)
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s access check failed, status %d, URI %s",
-                          r->user, r->status, r->uri);
-        else
-            ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r,
-                          "mod_fcgid: user %s access check failed, redirected is not allowed",
-                          r->user);
+static int mod_fcgid_authorizer(request_rec *r)
+{
+    return mod_fcgid_check_auth(r, FCGID_AUTH_CHECK_AUTHZ);
+}
 
-        /* Handle error */
-        if (!authoritative)
-            return DECLINED;
-        else {
-            return (res == OK) ? HTTP_UNAUTHORIZED : res;
-        }
-    }
+static int mod_fcgid_check_access(request_rec *r)
+{
+    return mod_fcgid_check_auth(r, FCGID_AUTH_CHECK_ACCESS);
 }
 
 static void initialize_child(apr_pool_t * pchild, server_rec * main_server)



Mime
View raw message