httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r646285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_auth_form.xml modules/aaa/config.m4 modules/aaa/mod_auth_form.c
Date Thu, 01 May 2008 19:15:29 GMT


On 04/09/2008 01:46 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Wed Apr  9 04:46:46 2008
> New Revision: 646285
> 
> URL: http://svn.apache.org/viewvc?rev=646285&view=rev
> Log:
> mod_auth_form: Add a module capable of allowing end users to log
> in using an HTML form, storing the credentials within mod_session.
> 
> Added:
>     httpd/httpd/trunk/docs/manual/mod/mod_auth_form.xml   (with props)
>     httpd/httpd/trunk/modules/aaa/mod_auth_form.c   (with props)
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/aaa/config.m4
> 

> Added: httpd/httpd/trunk/modules/aaa/mod_auth_form.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_auth_form.c?rev=646285&view=auto
> ==============================================================================
> --- httpd/httpd/trunk/modules/aaa/mod_auth_form.c (added)
> +++ httpd/httpd/trunk/modules/aaa/mod_auth_form.c Wed Apr  9 04:46:46 2008

> +static int check_site(request_rec * r, const char *site, const char *sent_user, const
char *sent_hash)
> +{
> +
> +    if (site && sent_user && sent_hash) {
> +        const char *hash = ap_md5(r->pool,
> +                      (unsigned char *) apr_pstrcat(r->pool, sent_user, ":", site,
NULL));
> +        
> +//        if (APR_SUCCESS == apr_password_validate(apr_pstrcat(r->pool, sent_user,
":", site, NULL),
> +//                                                 sent_hash)) {

This is a C++ style comment that breaks with ANSI compilers.
I think it would be better to remove the code or use a #if 0 #endif block.

> +        if (!strcmp(sent_hash, hash)) {
> +            return OK;
> +        }
> +        else {
> +            return AUTH_USER_NOT_FOUND;
> +        }
> +    }
> +
> +    return DECLINED;
> +
> +}
> +

> +
> +/**
> + * Must we use form authentication? If so, extract the cookie and run
> + * the authnz hooks to determine if the login is valid.
> + *
> + * If the login is not valid, a 401 Not Authorized will be returned. It
> + * is up to the webmaster to ensure this screen displays a suitable login
> + * form to give the user the opportunity to log in.
> + */
> +static int authenticate_form_user(request_rec * r)
> +{
> +    auth_form_config_rec *conf = ap_get_module_config(r->per_dir_config,
> +                                                      &auth_form_module);
> +    const char *sent_user = NULL, *sent_pw = NULL, *sent_loc = NULL, *sent_method =
NULL,
> +        *sent_mimetype = NULL, *sent_hash = NULL;
> +    const char *current_auth = NULL;
> +    apr_status_t res;
> +    int rv = HTTP_UNAUTHORIZED;
> +    int rv2 = HTTP_UNAUTHORIZED;
> +
> +    /* Are we configured to be Form auth? */
> +    current_auth = ap_auth_type(r);
> +    if (!current_auth || strcasecmp(current_auth, "form")) {
> +        return DECLINED;
> +    }
> +
> +    /*
> +     * XSS security warning: using cookies to store private data only works
> +     * when the administrator has full control over the source website. When
> +     * in forward-proxy mode, websites are public by definition, and so can
> +     * never be secure. Abort the auth attempt in this case.
> +     */
> +    if (PROXYREQ_PROXY == r->proxyreq) {
> +        ap_log_rerror(APLOG_MARK, APLOG_ERR,
> +                      0, r, LOG_PREFIX "form auth cannot be used for proxy "
> +                      "requests due to XSS risk, access denied: %s", r->uri);
> +        return HTTP_INTERNAL_SERVER_ERROR;
> +    }
> +
> +    /* We need an authentication realm. */
> +    if (!ap_auth_name(r)) {
> +        ap_log_rerror(APLOG_MARK, APLOG_ERR,
> +                      0, r, LOG_PREFIX "need AuthName: %s", r->uri);
> +        return HTTP_INTERNAL_SERVER_ERROR;
> +    }
> +
> +    r->ap_auth_type = (char *) current_auth;
> +
> +    /* try get the username and password from a session, if present */
> +    res = get_session_auth(r, &sent_user, &sent_pw, &sent_hash);
> +
> +    /* first test whether the site passphrase matches */
> +    if (APR_SUCCESS == res && sent_user && sent_hash && sent_pw)
{
> +        rv = check_site(r, conf->site, sent_user, sent_hash);
> +        if (OK == rv) {
> +            fake_basic_authentication(r, conf, sent_user, sent_pw);
> +            return OK;
> +        }
> +    }
> +
> +    /* otherwise test for a normal password match */
> +    if (APR_SUCCESS == res && sent_user && sent_pw) {
> +        rv = check_auth(r, sent_user, sent_pw);
> +        if (OK == rv) {
> +            fake_basic_authentication(r, conf, sent_user, sent_pw);
> +            return OK;
> +        }
> +    }
> +
> +    /*
> +     * If we reach this point, the request should fail with access denied,
> +     * except for one potential scenario:
> +     *
> +     * If the request is a POST, and the posted form contains user defined fields
> +     * for a username and a password, and the username and password are correct,
> +     * then return the response obtained by a GET to this URL.
> +     *
> +     * If an additional user defined location field is present in the form,
> +     * instead of a GET of the current URL, redirect the browser to the new
> +     * location.
> +     *
> +     * As a further option, if the user defined fields for the type of request,
> +     * the mime type of the body of the request, and the body of the request
> +     * itself are present, replace this request with a new request of the given
> +     * type and with the given body.
> +     *
> +     * Otherwise access is denied.
> +     */
> +    if (r->method_number == M_POST) {
> +        rv2 = get_form_auth(r, conf->username, conf->password, conf->location,
> +                            conf->method, conf->mimetype, conf->body,
> +                            &sent_user, &sent_pw, &sent_loc, &sent_method,
> +                            &sent_mimetype, conf);
> +        if (OK == rv2) {
> +            rv = check_auth(r, sent_user, sent_pw);
> +            if (OK == rv) {
> +                fake_basic_authentication(r, conf, sent_user, sent_pw);
> +                set_session_auth(r, sent_user, sent_pw, conf->site);
> +            }
> +        }
> +    }
> +
> +    /*
> +     * did the admin prefer to be redirected to the login page on failure
> +     * instead?
> +     */
> +    if (HTTP_UNAUTHORIZED == rv && conf->loginrequired) {
> +        apr_table_set(r->headers_out, "Location", conf->loginrequired);
> +        return HTTP_MOVED_PERMANENTLY;
> +    }
> +
> +    /* did the user ask to be redirected on login success? */
> +    if (sent_loc) {
> +        apr_table_set(r->headers_out, "Location", sent_loc);
> +        rv = HTTP_MOVED_PERMANENTLY;
> +    }
> +
> +    /*
> +     * If the user has submitted a sent method along with their form, switch
> +     * in the redirect handler. The redirect handler will replace the form
> +     * login request with the request given inside the login form.
> +     */
> +    if (OK == rv) {
> +        if (sent_method && sent_mimetype && r->kept_body) {
> +            r->handler = FORM_REDIRECT_HANDLER;
> +        }
> +        else if (sent_method || sent_mimetype || r->kept_body) {
> +            ap_log_rerror(APLOG_MARK, APLOG_ERR,
> +             0, r, LOG_PREFIX "the login form must contain a field for all "
> +                          "three of the method, mimetype and body for the original request
"
> +                          "to be redirected, reverting to GET: %s", r->uri);
> +        }
> +    }
> +
> +    /*
> +     * potential security issue: if we return a login to the browser, we must
> +     * send a no-store to make sure a well behaved browser will not try and
> +     * send the login details a second time if the back button is pressed.
> +     * 
> +     * if the user has full control over the backend, the
> +     * AuthCookieDisableNoStore can be used to turn this off.
> +     */
> +    if (HTTP_UNAUTHORIZED == rv && !conf->disable_no_store) {
> +        apr_table_addn(r->headers_out, "Cache-Control", "no-store");
> +        apr_table_addn(r->err_headers_out, "Cache-Control", "no-store");

Isn't it sufficient to add this r->err_headers_out?

> +    }
> +
> +    return rv;
> +
> +}
> +
> +/**
> + * Handle a login attempt.
> + *
> + * If the login session is either missing or form authnz is unsuccessful, a
> + * 401 Not Authorized will be returned to the browser. The webmaster
> + * is expected to insert a login form into the 401 Not Authorized
> + * error screen.
> + *
> + * If the webmaster wishes, they can point the form submission at this
> + * handler, which will redirect the user to the correct page on success.
> + * On failure, the 401 Not Authorized error screen will be redisplayed,
> + * where the login attempt can be repeated.
> + *
> + */
> +static int authenticate_form_login_handler(request_rec * r)
> +{
> +
> +    auth_form_config_rec *conf = ap_get_module_config(r->per_dir_config,
> +                                                      &auth_form_module);
> +
> +    const char *sent_user = NULL, *sent_pw = NULL, *sent_loc = NULL;
> +    int rv;
> +
> +    if (strcmp(r->handler, FORM_LOGIN_HANDLER)) {
> +        return DECLINED;
> +    }
> +
> +    if (r->method_number != M_POST) {
> +        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, LOG_PREFIX
> +          "the " FORM_LOGIN_HANDLER " only supports the POST method for %s",
> +                      r->uri);
> +        return HTTP_METHOD_NOT_ALLOWED;
> +    }
> +
> +    rv = get_form_auth(r, conf->username, conf->password, conf->location,
> +                       NULL, NULL, NULL,
> +                       &sent_user, &sent_pw, &sent_loc,
> +                       NULL, NULL, conf);
> +    if (OK == rv) {
> +        rv = check_auth(r, sent_user, sent_pw);
> +        if (OK == rv) {
> +            set_session_auth(r, sent_user, sent_pw, conf->site);
> +            if (sent_loc) {
> +                apr_table_set(r->headers_out, "Location", sent_loc);
> +                return HTTP_MOVED_PERMANENTLY;

Shouldn't this be HTTP_TEMPORARY_REDIRECT?

> +            }
> +            if (conf->loginsuccess) {
> +                apr_table_set(r->headers_out, "Location", conf->loginsuccess);
> +                return HTTP_MOVED_PERMANENTLY;

Shouldn't this be HTTP_TEMPORARY_REDIRECT?

> +            }
> +            return HTTP_OK;
> +        }
> +    }
> +
> +    /* did we prefer to be redirected to the login page on failure instead? */
> +    if (HTTP_UNAUTHORIZED == rv && conf->loginrequired) {
> +        apr_table_set(r->headers_out, "Location", conf->loginrequired);
> +        return HTTP_MOVED_PERMANENTLY;

Shouldn't this be HTTP_TEMPORARY_REDIRECT?

> +    }
> +
> +    return rv;
> +
> +}
> +
> +/**
> + * Handle a logout attempt.
> + *
> + * If an attempt is made to access this URL, any username and password
> + * embedded in the session is deleted.
> + *
> + * This has the effect of logging the person out.
> + *
> + * If a logout URI has been specified, this function will create an
> + * internal redirect to this page.
> + */
> +static int authenticate_form_logout_handler(request_rec * r)
> +{
> +
> +    auth_form_config_rec *conf = ap_get_module_config(r->per_dir_config,
> +                                                      &auth_form_module);
> +
> +    if (strcmp(r->handler, FORM_LOGOUT_HANDLER)) {
> +        return DECLINED;
> +    }
> +
> +    /* remove the username and password, effectively logging the user out */
> +    set_session_auth(r, NULL, NULL, NULL);
> +
> +    /*
> +     * make sure the logout page is never cached - otherwise the logout won't
> +     * work!
> +     */
> +    apr_table_addn(r->headers_out, "Cache-Control", "no-store");
> +    apr_table_addn(r->err_headers_out, "Cache-Control", "no-store");

Isn't it sufficient to add this r->headers_out?

> +
> +    /* if set, internal redirect to the logout page */
> +    if (conf->logout) {
> +        apr_table_addn(r->headers_out, "Location", conf->logout);
> +        return HTTP_TEMPORARY_REDIRECT;
> +    }
> +
> +    return HTTP_OK;
> +
> +}
> +
> +/**
> + * Handle a redirect attempt.
> + *
> + * If during a form login, the method, mimetype and request body are
> + * specified, this handler will ensure that this request is included
> + * as an internal redirect.
> + *
> + */
> +static int authenticate_form_redirect_handler(request_rec * r)
> +{
> +
> +    request_rec *rr = NULL;
> +    const char *sent_method = NULL, *sent_mimetype = NULL;
> +
> +    if (strcmp(r->handler, FORM_REDIRECT_HANDLER)) {
> +        return DECLINED;
> +    }
> +
> +    /* get the method and mimetype from the notes */
> +    get_notes_auth(r, NULL, NULL, &sent_method, &sent_mimetype);
> +
> +    if (r->kept_body && sent_method && sent_mimetype) {
> +
> +        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, LOG_PREFIX
> +          "internal redirect to method '%s' and body mimetype '%s' for the "
> +                      "uri: %s", sent_method, sent_mimetype, r->uri);
> +
> +        rr = ap_sub_req_method_uri(sent_method, r->uri, r, r->output_filters);
> +        r->status = ap_run_sub_req(rr);
> +
> +    }
> +    else {
> +        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, LOG_PREFIX
> +        "internal redirect requested but one or all of method, mimetype or "
> +                      "body are NULL: %s", r->uri);
> +        return HTTP_INTERNAL_SERVER_ERROR;
> +    }
> +
> +    /* return the underlying error, or OK on success */
> +    return r->status == HTTP_OK || r->status == OK ? OK : r->status;

Why not returning r->status here directly?

Regards

RĂ¼diger

Mime
View raw message