From 8d5aadc77ea8b00558101f0258d7494ebe65c292 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Thu, 18 Jun 2009 19:13:56 +0000 Subject: A bunch of fixes that make OpenID authentication work for the first time. --- module/consumer.cc | 150 ++++++++++++++++++++++++--------------------- module/mod_auth_singleid.c | 93 +++++++++++++++++++++++----- module/mod_auth_singleid.h | 3 +- 3 files changed, 161 insertions(+), 85 deletions(-) (limited to 'module') diff --git a/module/consumer.cc b/module/consumer.cc index 91ab773..9933eeb 100644 --- a/module/consumer.cc +++ b/module/consumer.cc @@ -15,6 +15,7 @@ using opkele::exception; using opkele::failed_discovery; using opkele::failed_lookup; using opkele::failed_xri_resolution; +using opkele::id_res_bad_nonce; using opkele::no_endpoint; using opkele::openid_endpoint_t; using opkele::openid_message_t; @@ -34,21 +35,6 @@ public: { sid_shared_unlock (); } }; -class AdaptorFix : - public params_t -{ -public: - AdaptorFix(params_t& params) - { _params = params; }; - virtual bool has_field(const string& n) const - { return _params.has_param("openid." + n); } - virtual const string& get_field(const string& n) const - { return _params.get_param("openid." + n); } -private: - params_t _params; -}; - - class Consumer : public prequeue_RP { @@ -57,7 +43,7 @@ private: // types public: // interface Consumer(const char *url, sid_storage_t *store) - : _url(url), _store(store), _index(0) + : _store(store), _url(url), _index(0) { } public: // overrides @@ -202,11 +188,15 @@ Consumer::invalidate_assoc(const string& server, const string& handle) void Consumer::check_nonce(const string& server, const string& nonce) { - if (!_store) - throw dumb_RP("no storage initialized"); + int res = 0; - LockShared lock; - sid_storage_check_nonce (_store, server.c_str(), nonce.c_str()); + if (_store) { + LockShared lock; + res = sid_storage_check_nonce (_store, server.c_str(), nonce.c_str()); + } + + if (res == 1) + throw id_res_bad_nonce ("nonce is too old, or has already been used"); } /* ----------------------------------------------------------------------- @@ -214,11 +204,13 @@ Consumer::check_nonce(const string& server, const string& nonce) */ static void -filter_openid_params (params_t ¶ms) +filter_openid_params (params_t ¶ms, params_t &openid) { for (params_t::iterator it = params.begin(); it != params.end(); ) { const string& name = it->first; if (name.find ("openid.") == 0) { + openid[name.substr(7)] = it->second; + /* We erase an increment together, must use post-increment operator */ params.erase (it++); } else { @@ -234,7 +226,10 @@ parse_query_string (const char *qs, params_t ¶ms) string pair, key, value; const char *at; - while (qs && *qs) { + if (qs == NULL) + return; + + while (*qs != 0) { at = strchr (qs, '&'); if (at == NULL) at = qs + strlen (qs); @@ -249,7 +244,9 @@ parse_query_string (const char *qs, params_t ¶ms) } params[opkele::util::url_decode(key)] = opkele::util::url_decode(value); - qs = at; + if (*at == 0) + break; + qs = at + 1; } } @@ -257,67 +254,75 @@ static void begin_auth (sid_request_t *req, Consumer &consumer, params_t ¶ms, const string& trust_root, const string &identity) { - /* Remove all openid params, and stash away extensions */ - filter_openid_params (params); - string return_to = params.append_query (consumer.get_this_url(), ""); - - params_t result; - string redirect; - - try { - openid_message_t cm; - consumer.initiate (identity); - consumer.checkid_ (cm, opkele::mode_checkid_setup, return_to, trust_root); - redirect = cm.append_query (consumer.get_endpoint().uri); - - } catch (failed_xri_resolution &ex) { - sid_request_respond (req, 503, "Invalid Identifier", NULL, NULL); - sid_request_log_error (req, "failed xri resolution while while discovering identity provider", - ex.what ()); - return; + params_t openid; - } catch (failed_discovery &ex) { - sid_request_respond (req, 503, "Invalid Identifier", NULL, NULL); - sid_request_log_error (req, "failed discovery while while discovering identity provider", - ex.what ()); - return; + /* Remove all openid params, and stash away extensions */ + filter_openid_params (params, openid); + string return_to = consumer.get_this_url(); + if (!params.empty()) + return_to = params.append_query (return_to, ""); - } catch (bad_input &ex) { - sid_request_respond (req, 503, "Invalid Identifier", NULL, NULL); - sid_request_log_error (req, "bad input while while discovering identity provider", - ex.what()); - return; + params_t result; + string redirect; - } catch (exception &ex) { - sid_request_respond (req, 500, NULL, NULL, NULL); - sid_request_log_error (req, "error while while discovering identity provider", - ex.what()); + try { + openid_message_t cm; + consumer.initiate (identity); + consumer.checkid_ (cm, opkele::mode_checkid_setup, return_to, trust_root); + redirect = cm.append_query (consumer.get_endpoint().uri); + + } catch (failed_xri_resolution &ex) { + sid_request_respond (req, 503, "Invalid Identifier", NULL); + sid_request_log_error (req, "failed xri resolution while while discovering identity provider", + ex.what ()); + return; + + } catch (failed_discovery &ex) { + sid_request_respond (req, 503, "Invalid Identifier", NULL); + sid_request_log_error (req, "failed discovery while while discovering identity provider", + ex.what ()); return; - } - sid_request_respond (req, 307, "Moved Temporarily", NULL, - "Location", redirect.c_str(), - "Cache-Control", "no-cache", - NULL); + } catch (bad_input &ex) { + sid_request_respond (req, 503, "Invalid Identifier", NULL); + sid_request_log_error (req, "bad input while while discovering identity provider", + ex.what()); + return; + + } catch (exception &ex) { + sid_request_respond (req, 500, NULL, NULL); + sid_request_log_error (req, "error while while discovering identity provider", + ex.what()); + return; + } + + sid_request_respond (req, 307, "Moved Temporarily", + "Location", redirect.c_str(), + "Cache-Control", "no-cache", + NULL); } static void complete_auth (sid_request_t *req, Consumer &consumer, params_t ¶ms) { + params_t openid; + + filter_openid_params (params, openid); + try { - consumer.id_res(AdaptorFix(params)); + consumer.id_res(openid); string identity = consumer.get_claimed_id(); sid_request_authenticated (req, identity.c_str()); } catch (exception &ex) { - sid_request_respond (req, 500, NULL, NULL, NULL); - sid_request_log_error (req, "error while completing authentication: %s", ex.what()); + sid_request_respond (req, 500, NULL, NULL); + sid_request_log_error (req, "error while completing authentication", ex.what()); } } static void cancelled_auth (sid_request_t *req, Consumer &consumer, params_t ¶ms) { - sid_request_respond (req, 401, "Authentication Required", NULL, NULL); + sid_request_respond (req, 401, "Authentication Required", NULL); } void @@ -327,18 +332,25 @@ sid_consumer_authenticate(sid_request_t *req, sid_storage_t *store, params_t params; assert (req); - assert (store); const char *qs = sid_request_qs (req); parse_query_string (qs, params); - const char *url = sid_request_url (req); + const char *url = sid_request_url (req, 1); Consumer consumer(url, store); - if (params.has_param("openid.assoc_handle")) + /* Returning (hopefully successful) authentication */ + if (params.has_param("openid.assoc_handle")) { complete_auth (req, consumer, params); - else if (params.has_param("openid.mode") && params.get_param("openid.mode") == "cancel") + + /* Returning cancelled authentication */ + } else if (params.has_param("openid.mode") && params.get_param("openid.mode") == "cancel") { cancelled_auth (req, consumer, params); - else + + /* Begin a new authentication */ + } else { + if (!trust_root) + trust_root = sid_request_url (req, 0); begin_auth (req, consumer, params, trust_root, identity); + } } diff --git a/module/mod_auth_singleid.c b/module/mod_auth_singleid.c index 3b6304a..9c38434 100644 --- a/module/mod_auth_singleid.c +++ b/module/mod_auth_singleid.c @@ -92,8 +92,8 @@ static int shared_initialize (apr_pool_t *p, server_rec *s) { apr_file_t *file = NULL; + char *lock_name = NULL; const char *tmpdir; - char *lock_name; int rc; /* This may be called more than once */ @@ -324,7 +324,7 @@ session_cookie_value (request_rec *r, const char *name) return NULL; while (*cookies) { - pair = ap_get_token (r->pool, &cookies, 0); + pair = ap_get_token (r->pool, &cookies, 1); if (!pair) break; if (pair[0] == '$') @@ -338,6 +338,7 @@ session_cookie_value (request_rec *r, const char *name) if (*value != '=') continue; + ++value; while (isspace (*value)) ++value; @@ -380,22 +381,22 @@ session_load_info (request_rec *r) char *token, *sig, *end; char *identifier; long expiry; + size_t len; value = session_cookie_value (r, "mod-auth-single-id"); if (!value) return NULL; - sig = ap_get_token (r->pool, &value, 1); + sig = ap_get_token (r->pool, &value, 0); + if (!session_validate_sig (r->pool, sig, value)) + return NULL; /* The version of the session info, only 1 supported for now */ - token = ap_get_token (r->pool, &value, 1); + token = ap_get_token (r->pool, &value, 0); if (strcmp (token, "1") != 0) return NULL; - if (!session_validate_sig (r->pool, sig, value)) - return NULL; - - token = ap_get_token (r->pool, &value, 1); + token = ap_get_token (r->pool, &value, 0); expiry = strtol (token, &end, 10); if (*end != '\0') return NULL; @@ -405,7 +406,13 @@ session_load_info (request_rec *r) return NULL; /* The identifier */ - identifier = ap_get_token (r->pool, &value, 1); + identifier = ap_get_token (r->pool, &value, 0); + len = strlen (identifier); + if (identifier[0] == '"' && identifier[len - 1] == '"') { + identifier[len - 1] = 0; + ++identifier; + } + if (!ap_is_url (identifier)) return NULL; @@ -466,7 +473,7 @@ sid_request_qs (sid_request_t *req) } const char* -sid_request_url (sid_request_t *req) +sid_request_url (sid_request_t *req, int with_path) { /* function to determine if a connection is using https */ static APR_OPTIONAL_FN_TYPE(ssl_is_https) *using_https = NULL; @@ -487,7 +494,7 @@ sid_request_url (sid_request_t *req) host = req->rec->hostname ? req->rec->hostname : ap_get_server_name (req->rec); scheme = is_ssl ? "https" : "http"; port = ap_get_server_port (req->rec); - uri = req->rec->uri ? req->rec->uri : ""; + uri = with_path && req->rec->uri ? req->rec->uri : ""; /* Default ports? */ if ((port == 80 && !is_ssl) || (port == 443 && is_ssl)) @@ -573,8 +580,8 @@ hook_authenticate (request_rec* r) if (!(authtype = ap_auth_type (r)) || strcasecmp (SID_AUTHTYPE, authtype) != 0) return DECLINED; - ctx = (sid_context_t*)ap_get_module_config(r->per_dir_config, &auth_singleid_module); - if(ctx->identifier == NULL) + ctx = (sid_context_t*)ap_get_module_config (r->per_dir_config, &auth_singleid_module); + if (ctx->identifier == NULL) return DECLINED; mainreq = r; @@ -611,14 +618,70 @@ hook_authenticate (request_rec* r) return req.result; } +static int +hook_access(request_rec *r) +{ + sid_context_t* ctx; + const char* authtype; + char *user = r->user; + int m = r->method_number; + int method_restricted = 0; + register int x; + const char *t, *w; + const apr_array_header_t *reqs_arr; + require_line *reqs; + + /* Make sure it's for us */ + if (!(authtype = ap_auth_type (r)) || strcasecmp (SID_AUTHTYPE, authtype) != 0) + return DECLINED; + + ctx = (sid_context_t*)ap_get_module_config (r->per_dir_config, &auth_singleid_module); + + reqs_arr = ap_requires (r); + if (!reqs_arr) + return OK; + + reqs = (require_line *)reqs_arr->elts; + for (x = 0; x < reqs_arr->nelts; x++) { + if (!(reqs[x].method_mask & (AP_METHOD_BIT << m))) + continue; + + method_restricted = 1; + + t = reqs[x].requirement; + w = ap_getword_white (r->pool, &t); + if (!strcmp (w, "valid-user")) { + return OK; + } else if (!strcmp (w, "user")) { + while (t[0]) { + w = ap_getword_conf (r->pool, &t); + if (!strcmp (user, w)) { + return OK; + } + } + } else { + ap_log_rerror (APLOG_MARK, APLOG_ERR, 0, r, + "access to %s failed, reason: unknown require " + "directive:\"%s\"", r->uri, reqs[x].requirement); + } + } + + if (!method_restricted) + return OK; + + ap_log_rerror (APLOG_MARK, APLOG_ERR, 0, r, + "access to %s failed, reason: user %s not allowed access", + r->uri, user); + return HTTP_UNAUTHORIZED; +} + static void register_hooks(apr_pool_t *p) { - ap_log_perror (APLOG_MARK, APLOG_ERR, 0, p, "mod_auth_singleid registering hooks"); - ap_hook_post_config (hook_initialize, NULL, NULL, APR_HOOK_MIDDLE); ap_hook_child_init (hook_child, NULL, NULL, APR_HOOK_MIDDLE); ap_hook_check_user_id (hook_authenticate, NULL, NULL, APR_HOOK_MIDDLE); + ap_hook_auth_checker (hook_access, NULL, NULL, APR_HOOK_MIDDLE); } module AP_MODULE_DECLARE_DATA auth_singleid_module = { diff --git a/module/mod_auth_singleid.h b/module/mod_auth_singleid.h index eb6d7f7..038e23e 100644 --- a/module/mod_auth_singleid.h +++ b/module/mod_auth_singleid.h @@ -19,7 +19,8 @@ void sid_request_log_error (sid_request_t *req, const char* sid_request_qs (sid_request_t *req); -const char* sid_request_url (sid_request_t *req); +const char* sid_request_url (sid_request_t *req, + int with_path); void sid_request_respond (sid_request_t *req, int code, -- cgit v1.2.3