Ruediger Pluem
Thu, 01 May 2008 12:15:47 -0700
On 04/09/2008 01:46 PM, [EMAIL PROTECTED] 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