httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r1362317 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID modules/fcgid/mod_fcgid.c
Date Wed, 18 Jul 2012 12:35:38 GMT
On Mon, Jul 16, 2012 at 8:02 PM,  <chrisd@apache.org> wrote:
> 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]

Most of that is not user-visible stuff for CHANGES.  (CHANGES would be
barely usable by users if every refactor or other code improvement was
described therein.)

"Improved logging for AAA handling" perhaps?

> +
>  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)
>
>



-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Mime
View raw message