From 79b9dcc62fe9caf3b5556fccbee63a2dcabccf46 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:08:45 -0500 Subject: [PATCH 01/52] change module name for Apache to be more like format of other modules --- mod_auth_browserid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index ae254d4..a6b0b2b 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -248,7 +248,7 @@ unsigned char c; #define unless(c) if(!(c)) /* apache module name */ -module AP_MODULE_DECLARE_DATA mod_auth_browserid_module; +module AP_MODULE_DECLARE_DATA auth_browserid_module; /* config structure */ typedef struct { From c131b4a443e492a7f08e2ec864ddef2df61faab0 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:15:33 -0500 Subject: [PATCH 02/52] fix typo --- mod_auth_browserid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index a6b0b2b..b0fb8d2 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -838,7 +838,7 @@ static void *create_browserid_config(apr_pool_t *p, char *d) return conf; } -/* apache config fonction of the module */ +/* apache config function of the module */ static const command_rec Auth_browserid_cmds[] = { AP_INIT_TAKE1 ("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, From f42414fb1c687f3dfb86521220c0908eabc93abc Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:20:26 -0500 Subject: [PATCH 03/52] change module name for Apache to be more like format of other modules --- mod_auth_browserid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index b0fb8d2..0e2037b 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -881,7 +881,7 @@ static const command_rec Auth_browserid_cmds[] = }; /* apache module structure */ -module AP_MODULE_DECLARE_DATA mod_auth_browserid_module = +module AP_MODULE_DECLARE_DATA auth_browserid_module = { STANDARD20_MODULE_STUFF, create_browserid_config, /* dir config creator */ From ac1000f568e3b06f5e1368c8f951ab1bdfb147c3 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:22:29 -0500 Subject: [PATCH 04/52] move module def code following module decl code --- mod_auth_browserid.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 0e2037b..1cc1c8e 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -250,6 +250,18 @@ unsigned char c; /* apache module name */ module AP_MODULE_DECLARE_DATA auth_browserid_module; +/* apache module structure */ +module AP_MODULE_DECLARE_DATA auth_browserid_module = +{ + STANDARD20_MODULE_STUFF, + create_browserid_config, /* dir config creator */ + NULL, /* dir merger --- default is to override */ + NULL, /* server config */ + NULL, /* merge server config */ + Auth_browserid_cmds, /* command apr_table_t */ + register_hooks /* register hooks */ +}; + /* config structure */ typedef struct { char *cookieName; @@ -879,15 +891,3 @@ static const command_rec Auth_browserid_cmds[] = {NULL} }; - -/* apache module structure */ -module AP_MODULE_DECLARE_DATA auth_browserid_module = -{ - STANDARD20_MODULE_STUFF, - create_browserid_config, /* dir config creator */ - NULL, /* dir merger --- default is to override */ - NULL, /* server config */ - NULL, /* merge server config */ - Auth_browserid_cmds, /* command apr_table_t */ - register_hooks /* register hooks */ -}; From 04c96de2f321ea45de161a37d5043eed48066d98 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:23:03 -0500 Subject: [PATCH 05/52] add TODO list --- TODO | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 TODO diff --git a/TODO b/TODO new file mode 100644 index 0000000..772da59 --- /dev/null +++ b/TODO @@ -0,0 +1,77 @@ +E-mail to dev-identity list on 28 Mar +(https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.identity/g9yCsiIV_Hs) +===================================================================================== + +This morning I took a closer look at my running server using +mod_browserid, the configuration, and the module source code. +Following are some observations: + +Log errors: +======= + +[Wed Mar 28 01:04:56 2012] [error] [client 92.194.45.48] +Auth_browserID: ap_hook_check_user_id in - Auth_browserid_check_cookie +[Wed Mar 28 01:04:56 2012] [error] [client 92.194.45.48] PHP Notice: +Use of undefined constant php - assumed 'php' in +/home/tbrowde/public_html/mygnus.com/public/id_login/browserid_login.php +on line 1 +[Wed Mar 28 01:05:24 2012] [error] [client 92.194.45.48] +Auth_browserID: ap_hook_check_user_id in - +Auth_browserid_check_cookie, referer: https://mygnus.com/ + +Possible problems with my test setup: +=========================== + ++ directive "AuthBrowserIDAuthoritative" shows "on" in example, but +code says set "yes" or "no" (default) + ++ function "create_browserid_config" doesn't explicitly set all +variables to default values + +Some suggestions: +============= + +2. allow user to establish default max credential life for each sign in + +3. allow mod_browserid an option (directive) to adjust max credential +life for a sign in + +4. explain all directives in README (particularly "authBasicFix"); how +do they affect the example setup (two directories used two different +ways)? + +E-mail to dev-identity list of 29 Mar: +===================================== + ++ fix or document reasons for observed Apache log errors + ++ incorporate other observations in previous e-mail to dev-identity +list on 28 Mar [see above] + ++ code cleanup + - minor typos + - rearrange some functions for easier maintenance (e.g., keep those + modifying struct BrowserIDConfigRec closer to it) + ++ add capability to use a dbm for authorized user e-mail addresses + ++ more examples + ++ add bits to docs (e.g., ensure all user options and inputs are documented) + +#========= COMPLETED ===================== + +1. module name for Apache2: [DONE] + +current: + +/* apache module name */ +module AP_MODULE_DECLARE_DATA mod_auth_browserid_module; + +suggested: +auth_browserid_module; + +Rationale: More like other module names in existing set shipping with Apache2. + +Example of a current one: +LoadModule auth_digest_module /usr/lib/apache2/modules/mod_auth_digest.so From 396d93358c66bd728ce72d9af86f10ccbf374557 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:24:30 -0500 Subject: [PATCH 06/52] move config functions following config struct definition --- mod_auth_browserid.c | 118 +++++++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 1cc1c8e..63ac133 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -275,6 +275,65 @@ typedef struct { char *serverSecret; } BrowserIDConfigRec; +/************************************************************************************ + * Apache CONFIG Phase: + ************************************************************************************/ +static void *create_browserid_config(apr_pool_t *p, char *d) +{ + BrowserIDConfigRec *conf = apr_palloc(p, sizeof(*conf)); + + conf->cookieName = apr_pstrdup(p,"BrowserID"); + conf->submitPath = "/mod_browserid_submit"; + conf->serverSecret = "BrowserIDSecret"; + conf->logoutPath = NULL; + conf->authoritative = 0; /* not by default */ + conf->authBasicFix = 0; /* do not fix header for php auth by default */ + conf->forwardedRequestHeader = NULL; /* pass the authenticated user, signed, as an HTTP header */ + return conf; +} + +/* apache config function of the module */ +static const command_rec Auth_browserid_cmds[] = +{ + AP_INIT_TAKE1 ("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), + OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default"), + + AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), + OR_AUTHCFG, "Name of cookie to set"), + + AP_INIT_FLAG ("AuthBrowserIDAuthoritative", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), + OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default"), + + AP_INIT_FLAG ("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), + OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username"), + + AP_INIT_TAKE1 ("AuthBrowserIDSubmitPath", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), + OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'"), + + AP_INIT_TAKE1 ("AuthBrowserIDLogoutPath", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, logoutPath), + OR_AUTHCFG, "Path to which logout requests will be submitted. An optional 'returnto' parameter will be used for a redirection, if provided."), + + AP_INIT_TAKE1 ("AuthBrowserIDVerificationServerURL", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, verificationServerURL), + OR_AUTHCFG, "URL of the BrowserID verification server."), + + AP_INIT_FLAG ("AuthBrowserIDVerifyLocally", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, verifyLocally), + OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set"), + + AP_INIT_TAKE1 ("AuthBrowserIDSecret", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, serverSecret), + OR_AUTHCFG, "Server secret for authentication cookie."), + + {NULL} +}; + /* Look through the 'Cookie' headers for the indicated cookie; extract it * and URL-unescape it. Return the cookie on success, NULL on failure. */ static char * extract_cookie(request_rec *r, const char *szCookie_name) @@ -832,62 +891,3 @@ static void register_hooks(apr_pool_t *p) ap_hook_auth_checker(Auth_browserid_check_auth, NULL, NULL, APR_HOOK_FIRST); ap_hook_fixups(Auth_browserid_fixups, NULL, NULL, APR_HOOK_FIRST); } - -/************************************************************************************ - * Apache CONFIG Phase: - ************************************************************************************/ -static void *create_browserid_config(apr_pool_t *p, char *d) -{ - BrowserIDConfigRec *conf = apr_palloc(p, sizeof(*conf)); - - conf->cookieName = apr_pstrdup(p,"BrowserID"); - conf->submitPath = "/mod_browserid_submit"; - conf->serverSecret = "BrowserIDSecret"; - conf->logoutPath = NULL; - conf->authoritative = 0; /* not by default */ - conf->authBasicFix = 0; /* do not fix header for php auth by default */ - conf->forwardedRequestHeader = NULL; /* pass the authenticated user, signed, as an HTTP header */ - return conf; -} - -/* apache config function of the module */ -static const command_rec Auth_browserid_cmds[] = -{ - AP_INIT_TAKE1 ("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), - OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default"), - - AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), - OR_AUTHCFG, "Name of cookie to set"), - - AP_INIT_FLAG ("AuthBrowserIDAuthoritative", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), - OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default"), - - AP_INIT_FLAG ("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), - OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username"), - - AP_INIT_TAKE1 ("AuthBrowserIDSubmitPath", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), - OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'"), - - AP_INIT_TAKE1 ("AuthBrowserIDLogoutPath", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, logoutPath), - OR_AUTHCFG, "Path to which logout requests will be submitted. An optional 'returnto' parameter will be used for a redirection, if provided."), - - AP_INIT_TAKE1 ("AuthBrowserIDVerificationServerURL", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, verificationServerURL), - OR_AUTHCFG, "URL of the BrowserID verification server."), - - AP_INIT_FLAG ("AuthBrowserIDVerifyLocally", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, verifyLocally), - OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set"), - - AP_INIT_TAKE1 ("AuthBrowserIDSecret", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, serverSecret), - OR_AUTHCFG, "Server secret for authentication cookie."), - - {NULL} -}; From c6d1c45dd56f0f8be707d411d3632e3b8ada6f21 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:25:18 -0500 Subject: [PATCH 07/52] align var names --- mod_auth_browserid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 63ac133..7458dfa 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -267,7 +267,7 @@ typedef struct { char *cookieName; int authoritative; int authBasicFix; - char *forwardedRequestHeader; + char *forwardedRequestHeader; char *submitPath; char *logoutPath; char *verificationServerURL; From b158081c70928d60171276be5d2567152c653370 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:28:06 -0500 Subject: [PATCH 08/52] sort var names alphabetically --- mod_auth_browserid.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 7458dfa..03bc2b6 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -264,15 +264,15 @@ module AP_MODULE_DECLARE_DATA auth_browserid_module = /* config structure */ typedef struct { - char *cookieName; - int authoritative; int authBasicFix; + int authoritative; + char *cookieName; char *forwardedRequestHeader; - char *submitPath; char *logoutPath; + char *serverSecret; + char *submitPath; char *verificationServerURL; int verifyLocally; - char *serverSecret; } BrowserIDConfigRec; /************************************************************************************ From 71f394a9a509f2774c9d6052635dd490fe4f15ad Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:28:35 -0500 Subject: [PATCH 09/52] sort var names alphabetically --- mod_auth_browserid.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 03bc2b6..0e0afbd 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -282,13 +282,13 @@ static void *create_browserid_config(apr_pool_t *p, char *d) { BrowserIDConfigRec *conf = apr_palloc(p, sizeof(*conf)); - conf->cookieName = apr_pstrdup(p,"BrowserID"); - conf->submitPath = "/mod_browserid_submit"; - conf->serverSecret = "BrowserIDSecret"; - conf->logoutPath = NULL; - conf->authoritative = 0; /* not by default */ conf->authBasicFix = 0; /* do not fix header for php auth by default */ + conf->authoritative = 0; /* not by default */ + conf->cookieName = apr_pstrdup(p,"BrowserID"); conf->forwardedRequestHeader = NULL; /* pass the authenticated user, signed, as an HTTP header */ + conf->logoutPath = NULL; + conf->serverSecret = "BrowserIDSecret"; + conf->submitPath = "/mod_browserid_submit"; return conf; } From 0f75c4ab95e11d3414254604591ba5df1208d56d Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:31:35 -0500 Subject: [PATCH 10/52] ensure all conf struct vars are explicitly initialized --- mod_auth_browserid.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 0e0afbd..0909213 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -289,6 +289,8 @@ static void *create_browserid_config(apr_pool_t *p, char *d) conf->logoutPath = NULL; conf->serverSecret = "BrowserIDSecret"; conf->submitPath = "/mod_browserid_submit"; + conf->verificationServerURL = NULL; + conf->verifyLocally = 0; return conf; } From 4882ac8ddd591699573dacbc0f58ef653579bdcf Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:33:16 -0500 Subject: [PATCH 11/52] align = signs for tidyness --- mod_auth_browserid.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 0909213..2173c05 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -280,17 +280,17 @@ typedef struct { ************************************************************************************/ static void *create_browserid_config(apr_pool_t *p, char *d) { - BrowserIDConfigRec *conf = apr_palloc(p, sizeof(*conf)); + BrowserIDConfigRec *conf = apr_palloc(p, sizeof(*conf)); - conf->authBasicFix = 0; /* do not fix header for php auth by default */ - conf->authoritative = 0; /* not by default */ - conf->cookieName = apr_pstrdup(p,"BrowserID"); + conf->authBasicFix = 0; /* do not fix header for php auth by default */ + conf->authoritative = 0; /* not by default */ + conf->cookieName = apr_pstrdup(p,"BrowserID"); conf->forwardedRequestHeader = NULL; /* pass the authenticated user, signed, as an HTTP header */ - conf->logoutPath = NULL; - conf->serverSecret = "BrowserIDSecret"; - conf->submitPath = "/mod_browserid_submit"; - conf->verificationServerURL = NULL; - conf->verifyLocally = 0; + conf->logoutPath = NULL; + conf->serverSecret = "BrowserIDSecret"; + conf->submitPath = "/mod_browserid_submit"; + conf->verificationServerURL = NULL; + conf->verifyLocally = 0; return conf; } From b9f2fc8e620e866c4c078e9f0fa879872213a73d Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:37:35 -0500 Subject: [PATCH 12/52] list cmds in conf struct order --- mod_auth_browserid.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 2173c05..8aaf8d3 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -297,30 +297,34 @@ static void *create_browserid_config(apr_pool_t *p, char *d) /* apache config function of the module */ static const command_rec Auth_browserid_cmds[] = { - AP_INIT_TAKE1 ("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), - OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default"), - - AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), - OR_AUTHCFG, "Name of cookie to set"), + AP_INIT_FLAG ("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), + OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username"), AP_INIT_FLAG ("AuthBrowserIDAuthoritative", ap_set_flag_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default"), - AP_INIT_FLAG ("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), - OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username"), + AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), + OR_AUTHCFG, "Name of cookie to set"), - AP_INIT_TAKE1 ("AuthBrowserIDSubmitPath", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), - OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'"), + AP_INIT_TAKE1 ("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), + OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default"), AP_INIT_TAKE1 ("AuthBrowserIDLogoutPath", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, logoutPath), OR_AUTHCFG, "Path to which logout requests will be submitted. An optional 'returnto' parameter will be used for a redirection, if provided."), + AP_INIT_TAKE1 ("AuthBrowserIDSecret", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, serverSecret), + OR_AUTHCFG, "Server secret for authentication cookie."), + + AP_INIT_TAKE1 ("AuthBrowserIDSubmitPath", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), + OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'"), + AP_INIT_TAKE1 ("AuthBrowserIDVerificationServerURL", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, verificationServerURL), OR_AUTHCFG, "URL of the BrowserID verification server."), @@ -329,10 +333,6 @@ static const command_rec Auth_browserid_cmds[] = (void *)APR_OFFSETOF(BrowserIDConfigRec, verifyLocally), OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set"), - AP_INIT_TAKE1 ("AuthBrowserIDSecret", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, serverSecret), - OR_AUTHCFG, "Server secret for authentication cookie."), - {NULL} }; From ab1e98e2782770adab5fe1e2d38af1412197a6eb Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:46:20 -0500 Subject: [PATCH 13/52] US English spelling; try for more consistent code style (at least with functions) --- mod_auth_browserid.c | 48 ++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 8aaf8d3..69d94fc 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -496,51 +496,59 @@ static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCook **************************************************/ static int Auth_browserid_check_cookie(request_rec *r) { - BrowserIDConfigRec *conf=NULL; - char *szCookieValue=NULL; - char *szRemoteIP=NULL; + BrowserIDConfigRec *conf = NULL; + char *szCookieValue = NULL; + char *szRemoteIP = NULL; - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0,r,ERRTAG "ap_hook_check_user_id in - Auth_browserid_check_cookie"); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "ap_hook_check_user_id in - Auth_browserid_check_cookie"); /* get apache config */ conf = ap_get_module_config(r->per_dir_config, &mod_auth_browserid_module); unless(conf->authoritative) - return DECLINED; + return DECLINED; - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG "AuthType are '%s'", ap_auth_type(r)); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,ERRTAG + "AuthType are '%s'", ap_auth_type(r)); unless(strncmp("BrowserID",ap_auth_type(r),9)==0) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG "Auth type must be 'BrowserID'"); - return HTTP_UNAUTHORIZED; + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + "Auth type must be 'BrowserID'"); + return HTTP_UNAUTHORIZED; } unless(conf->cookieName) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG "No Auth_browserid_CookieName specified"); - return HTTP_UNAUTHORIZED; + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "No Auth_browserid_CookieName specified"); + return HTTP_UNAUTHORIZED; } /* get cookie who are named cookieName */ - unless(szCookieValue = extract_cookie(r, conf->cookieName)) - { - ap_log_rerror(APLOG_MARK, APLOG_INFO|APLOG_NOERRNO, 0, r, ERRTAG "BrowserID cookie not found; not authorized! RemoteIP:%s",szRemoteIP); - return HTTP_UNAUTHORIZED; + unless(szCookieValue = extract_cookie(r, conf->cookieName)) { + ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG + "BrowserID cookie not found; not authorized! RemoteIP:%s",szRemoteIP); + return HTTP_UNAUTHORIZED; } - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG "got cookie; value is %s", szCookieValue); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "got cookie; value is %s", szCookieValue); /* Check cookie validity */ if (validateCookie(r, conf, szCookieValue)) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING|APLOG_NOERRNO, 0, r, ERRTAG "Invalid BrowserID cookie: %s", szCookieValue); + ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r, ERRTAG + "Invalid BrowserID cookie: %s", szCookieValue); return HTTP_UNAUTHORIZED; } /* set REMOTE_USER var for scripts language */ - apr_table_setn(r->subprocess_env,"REMOTE_USER",r->user); + apr_table_setn(r->subprocess_env, "REMOTE_USER", r->user); - /* log authorisation ok */ - ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, ERRTAG "BrowserID authentication ok"); + /* log authorization ok */ + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "BrowserID authentication ok"); /* fix http header for php */ - if (conf->authBasicFix) fix_headers_in(r,"browserid"); + if (conf->authBasicFix) + fix_headers_in(r, "browserid"); /* if all is ok return auth ok */ return OK; From 522714f2b2ca2317281bb2026d3596d3cc57aff0 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:47:49 -0500 Subject: [PATCH 14/52] ensure all vars are initialized --- mod_auth_browserid.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 69d94fc..98b5c7c 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -565,13 +565,13 @@ static int Auth_browserid_check_cookie(request_rec *r) static int Auth_browserid_check_auth(request_rec *r) { BrowserIDConfigRec *conf=NULL; - char *szUser; + char *szUser = NULL; const apr_array_header_t *reqs_arr=NULL; require_line *reqs=NULL; - register int x; - const char *szRequireLine; - char *szFileName; - char *szRequire_cmd; + register int x = 0; + const char *szRequireLine = NULL; + char *szFileName = NULL; + char *szRequire_cmd = NULL; /* get apache config */ conf = ap_get_module_config(r->per_dir_config, &mod_auth_browserid_module); From 36a761aaa16ce4b5eb412e83eaa2b4a972a88d9a Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 16:49:05 -0500 Subject: [PATCH 15/52] align = signs --- mod_auth_browserid.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 98b5c7c..11ba4c2 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -564,14 +564,14 @@ static int Auth_browserid_check_cookie(request_rec *r) **************************************************/ static int Auth_browserid_check_auth(request_rec *r) { - BrowserIDConfigRec *conf=NULL; - char *szUser = NULL; - const apr_array_header_t *reqs_arr=NULL; - require_line *reqs=NULL; - register int x = 0; - const char *szRequireLine = NULL; - char *szFileName = NULL; - char *szRequire_cmd = NULL; + BrowserIDConfigRec *conf = NULL; + char *szUser = NULL; + const apr_array_header_t *reqs_arr = NULL; + require_line *reqs = NULL; + register int x = 0; + const char *szRequireLine = NULL; + char *szFileName = NULL; + char *szRequire_cmd = NULL; /* get apache config */ conf = ap_get_module_config(r->per_dir_config, &mod_auth_browserid_module); From 7741c165a4417a51e40e1c01bad4e4c408adf4c0 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 20:34:14 -0500 Subject: [PATCH 16/52] add function decls; some style fixes; rename old mod_auth_browserid_module to new auth_browserid_module; rearrange functions for (hopefully) easier maintenance --- mod_auth_browserid.c | 103 ++++++++++++++++++++++++++----------------- 1 file changed, 62 insertions(+), 41 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 11ba4c2..4968654 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -247,21 +247,6 @@ unsigned char c; #define VERSION "1.0.0" #define unless(c) if(!(c)) -/* apache module name */ -module AP_MODULE_DECLARE_DATA auth_browserid_module; - -/* apache module structure */ -module AP_MODULE_DECLARE_DATA auth_browserid_module = -{ - STANDARD20_MODULE_STUFF, - create_browserid_config, /* dir config creator */ - NULL, /* dir merger --- default is to override */ - NULL, /* server config */ - NULL, /* merge server config */ - Auth_browserid_cmds, /* command apr_table_t */ - register_hooks /* register hooks */ -}; - /* config structure */ typedef struct { int authBasicFix; @@ -275,25 +260,6 @@ typedef struct { int verifyLocally; } BrowserIDConfigRec; -/************************************************************************************ - * Apache CONFIG Phase: - ************************************************************************************/ -static void *create_browserid_config(apr_pool_t *p, char *d) -{ - BrowserIDConfigRec *conf = apr_palloc(p, sizeof(*conf)); - - conf->authBasicFix = 0; /* do not fix header for php auth by default */ - conf->authoritative = 0; /* not by default */ - conf->cookieName = apr_pstrdup(p,"BrowserID"); - conf->forwardedRequestHeader = NULL; /* pass the authenticated user, signed, as an HTTP header */ - conf->logoutPath = NULL; - conf->serverSecret = "BrowserIDSecret"; - conf->submitPath = "/mod_browserid_submit"; - conf->verificationServerURL = NULL; - conf->verifyLocally = 0; - return conf; -} - /* apache config function of the module */ static const command_rec Auth_browserid_cmds[] = { @@ -336,6 +302,58 @@ static const command_rec Auth_browserid_cmds[] = {NULL} }; +/* function declarations */ +static void *create_browserid_config(apr_pool_t *p, char *d); +static int Auth_browserid_check_auth(request_rec *r); +static int Auth_browserid_check_cookie(request_rec *r); +static int Auth_browserid_fixups(request_rec *r); +static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char *identity); +static char * extract_cookie(request_rec *r, const char *szCookie_name); +static void fix_headers_in(request_rec *r,char*szPassword); +static char *generateSignature(request_rec *r, BrowserIDConfigRec *conf, char *userAddress); +//apr_table_t *parseArgs(request_rec *r, char *argStr); +static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf); +static int processLogout(request_rec *r, BrowserIDConfigRec *conf); +static void register_hooks(apr_pool_t *p); +static int user_in_file(request_rec *r, char *username, char *filename); +static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCookieValue); +static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, char *assertionText); + +/* apache module name */ +module AP_MODULE_DECLARE_DATA auth_browserid_module; + +/* apache module structure */ +module AP_MODULE_DECLARE_DATA auth_browserid_module = +{ + STANDARD20_MODULE_STUFF, + create_browserid_config, /* dir config creator */ + NULL, /* dir merger --- default is to override */ + NULL, /* server config */ + NULL, /* merge server config */ + Auth_browserid_cmds, /* command apr_table_t */ + register_hooks /* register hooks */ +}; + + +/************************************************************************************ + * Apache CONFIG Phase: + ************************************************************************************/ +static void *create_browserid_config(apr_pool_t *p, char *d) +{ + BrowserIDConfigRec *conf = apr_palloc(p, sizeof(*conf)); + + conf->authBasicFix = 0; /* do not fix header for php auth by default */ + conf->authoritative = 0; /* not by default */ + conf->cookieName = apr_pstrdup(p,"BrowserID"); + conf->forwardedRequestHeader = NULL; /* pass the authenticated user, signed, as an HTTP header */ + conf->logoutPath = NULL; + conf->serverSecret = "BrowserIDSecret"; + conf->submitPath = "/mod_browserid_submit"; + conf->verificationServerURL = NULL; + conf->verifyLocally = 0; + return conf; +} + /* Look through the 'Cookie' headers for the indicated cookie; extract it * and URL-unescape it. Return the cookie on success, NULL on failure. */ static char * extract_cookie(request_rec *r, const char *szCookie_name) @@ -504,7 +522,7 @@ static int Auth_browserid_check_cookie(request_rec *r) "ap_hook_check_user_id in - Auth_browserid_check_cookie"); /* get apache config */ - conf = ap_get_module_config(r->per_dir_config, &mod_auth_browserid_module); + conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); unless(conf->authoritative) return DECLINED; @@ -574,7 +592,7 @@ static int Auth_browserid_check_auth(request_rec *r) char *szRequire_cmd = NULL; /* get apache config */ - conf = ap_get_module_config(r->per_dir_config, &mod_auth_browserid_module); + conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); /* check if this module is authoritative */ unless(conf->authoritative) @@ -585,7 +603,8 @@ static int Auth_browserid_check_auth(request_rec *r) reqs = reqs_arr ? (require_line *) reqs_arr->elts : NULL; /* decline if no require line found */ - if (!reqs_arr) return DECLINED; + if (!reqs_arr) + return DECLINED; /* walk through the array to check each require command */ for (x = 0; x < reqs_arr->nelts; x++) { @@ -616,18 +635,20 @@ static int Auth_browserid_check_auth(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_INFO|APLOG_NOERRNO, 0, r ,ERRTAG "user '%s' is authorized",r->user); return OK; } - /* check for users in a file */ + /* check for users in a file */ else if (!strcmp("userfile",szRequire_cmd)) { szFileName = ap_getword_conf(r->pool, &szRequireLine); if (!user_in_file(r, r->user, szFileName)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r ,ERRTAG "user '%s' is not in username list at '%s'",r->user,szFileName); return HTTP_FORBIDDEN; - } else { - return OK; + } + else { + return OK; } } } ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r ,ERRTAG "user '%s' is not authorized",r->user); + /* forbid by default */ return HTTP_FORBIDDEN; } @@ -878,7 +899,7 @@ static int Auth_browserid_fixups(request_rec *r) BrowserIDConfigRec *conf=NULL; /* get apache config */ - conf = ap_get_module_config(r->per_dir_config, &mod_auth_browserid_module); + conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); if (conf->submitPath && !strcmp(r->uri, conf->submitPath)) { return processAssertionFormSubmit(r, conf); From bcc0574bbf15f5ef77f8e5a94a1f806108aefc8b Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 20:47:53 -0500 Subject: [PATCH 17/52] some style fixes --- mod_auth_browserid.c | 151 +++++++++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 63 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 4968654..0f04945 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -311,7 +311,7 @@ static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char * static char * extract_cookie(request_rec *r, const char *szCookie_name); static void fix_headers_in(request_rec *r,char*szPassword); static char *generateSignature(request_rec *r, BrowserIDConfigRec *conf, char *userAddress); -//apr_table_t *parseArgs(request_rec *r, char *argStr); +apr_table_t *parseArgs(request_rec *r, char *argStr); static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf); static int processLogout(request_rec *r, BrowserIDConfigRec *conf); static void register_hooks(apr_pool_t *p); @@ -366,7 +366,8 @@ static char * extract_cookie(request_rec *r, const char *szCookie_name) /* loop to search cookie name in cookie header */ do { - ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, ERRTAG "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); /* search cookie name in cookie string */ unless (szRaw_cookie =strstr(szRaw_cookie, szCookie_name)) return 0; @@ -386,7 +387,8 @@ static char * extract_cookie(request_rec *r, const char *szCookie_name) /* unescape the value string */ unless (ap_unescape_url(szCookie) == 0) return 0; - ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, ERRTAG "finished cookie scan, returning %s", szCookie); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, + ERRTAG "finished cookie scan, returning %s", szCookie); return szCookie; } @@ -428,7 +430,7 @@ static int user_in_file(request_rec *r, char *username, char *filename) application. e.g. php uses the Authorization header when logging the request in apache and not r->user (like it ought to). It is applied after the request has been authenticated. */ -static void fix_headers_in(request_rec *r,char*szPassword) +static void fix_headers_in(request_rec *r, char*szPassword) { char *szUser=NULL; /* Set an Authorization header in the input request table for php and @@ -436,18 +438,21 @@ static void fix_headers_in(request_rec *r,char*szPassword) apache logging of php scripts). We only set this if there is no header already present. */ - if (apr_table_get(r->headers_in,"Authorization")==NULL) - { - ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, ERRTAG "fixing apache Authorization header for this request using user: %s",r->user); + if (apr_table_get(r->headers_in,"Authorization") == NULL) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "fixing apache Authorization header for this request using user: %s",r->user); /* concat username and ':' */ - if (szPassword!=NULL) szUser=(char*)apr_pstrcat(r->pool,r->user,":",szPassword,NULL); - else szUser=(char*)apr_pstrcat(r->pool,r->user,":",NULL); + if (szPassword!=NULL) + szUser=(char*)apr_pstrcat(r->pool, r->user, ":", szPassword, NULL); + else + szUser=(char*)apr_pstrcat(r->pool, r->user, ":", NULL); /* alloc memory for the estimated encode size of the username */ char *szB64_enc_user=(char*)apr_palloc(r->pool,apr_base64_encode_len(strlen(szUser))+1); unless (szB64_enc_user) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG "memory alloc failed!"); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "memory alloc failed!"); return; } @@ -461,7 +466,7 @@ static void fix_headers_in(request_rec *r,char*szPassword) r->ap_auth_type=apr_pstrdup(r->pool,"Basic"); } - return; + return; } /** Generates a signature with the given inputs, returning a Base64-encoded @@ -487,12 +492,14 @@ static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCook char *sig = NULL; char *addr = apr_strtok(szCookieValue, "|", &sig); if (!addr) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG "malformed BrowserID cookie"); + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + "malformed BrowserID cookie"); return 1; } char *digest64 = generateSignature(r, conf, addr); - ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, ERRTAG "Got cookie: email is %s; expected digest is %s; got digest %s", + ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, ERRTAG + "Got cookie: email is %s; expected digest is %s; got digest %s", addr, digest64, sig); /* paranoia indicates that we should use a time-invariant compare here */ @@ -525,36 +532,37 @@ static int Auth_browserid_check_cookie(request_rec *r) conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); unless(conf->authoritative) - return DECLINED; + return DECLINED; ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,ERRTAG "AuthType are '%s'", ap_auth_type(r)); + unless(strncmp("BrowserID",ap_auth_type(r),9)==0) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG - "Auth type must be 'BrowserID'"); - return HTTP_UNAUTHORIZED; + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + "Auth type must be 'BrowserID'"); + return HTTP_UNAUTHORIZED; } unless(conf->cookieName) { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "No Auth_browserid_CookieName specified"); - return HTTP_UNAUTHORIZED; + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "No Auth_browserid_CookieName specified"); + return HTTP_UNAUTHORIZED; } /* get cookie who are named cookieName */ unless(szCookieValue = extract_cookie(r, conf->cookieName)) { - ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG + ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG "BrowserID cookie not found; not authorized! RemoteIP:%s",szRemoteIP); - return HTTP_UNAUTHORIZED; + return HTTP_UNAUTHORIZED; } ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "got cookie; value is %s", szCookieValue); /* Check cookie validity */ if (validateCookie(r, conf, szCookieValue)) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r, ERRTAG - "Invalid BrowserID cookie: %s", szCookieValue); - return HTTP_UNAUTHORIZED; + ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r, ERRTAG + "Invalid BrowserID cookie: %s", szCookieValue); + return HTTP_UNAUTHORIZED; } /* set REMOTE_USER var for scripts language */ @@ -566,7 +574,7 @@ static int Auth_browserid_check_cookie(request_rec *r) /* fix http header for php */ if (conf->authBasicFix) - fix_headers_in(r, "browserid"); + fix_headers_in(r, "browserid"); /* if all is ok return auth ok */ return OK; @@ -614,32 +622,38 @@ static int Auth_browserid_check_auth(request_rec *r) /* get require line */ szRequireLine = reqs[x].requirement; - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG "Require Line is '%s'", szRequireLine); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Require Line is '%s'", szRequireLine); /* get the first word in require line */ szRequire_cmd = ap_getword_white(r->pool, &szRequireLine); - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG "Require Cmd is '%s'", szRequire_cmd); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Require Cmd is '%s'", szRequire_cmd); /* if require cmd are valid-user, they are already authenticated than allow and return OK */ if (!strcmp("valid-user",szRequire_cmd)) { - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG "Require Cmd valid-user"); + ap_log_rerror(APLOG_MARK,APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Require Cmd valid-user"); return OK; } /* check the required user */ else if (!strcmp("user",szRequire_cmd)) { szUser = ap_getword_conf(r->pool, &szRequireLine); if (strcmp(r->user, szUser)) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r ,ERRTAG "user '%s' is not the required user '%s'",r->user, szUser); + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, ERRTAG + "user '%s' is not the required user '%s'",r->user, szUser); return HTTP_FORBIDDEN; } - ap_log_rerror(APLOG_MARK, APLOG_INFO|APLOG_NOERRNO, 0, r ,ERRTAG "user '%s' is authorized",r->user); + ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG + "user '%s' is authorized",r->user); return OK; } /* check for users in a file */ else if (!strcmp("userfile",szRequire_cmd)) { szFileName = ap_getword_conf(r->pool, &szRequireLine); if (!user_in_file(r, r->user, szFileName)) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r ,ERRTAG "user '%s' is not in username list at '%s'",r->user,szFileName); + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, ERRTAG + "user '%s' is not in username list at '%s'", r->user,szFileName); return HTTP_FORBIDDEN; } else { @@ -647,14 +661,13 @@ static int Auth_browserid_check_auth(request_rec *r) } } } - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r ,ERRTAG "user '%s' is not authorized",r->user); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "user '%s' is not authorized", r->user); /* forbid by default */ return HTTP_FORBIDDEN; } - - /* Helper struct for CURL response */ struct MemoryStruct { char *memory; @@ -690,8 +703,8 @@ static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, cha curl_easy_setopt(curl, CURLOPT_URL, conf->verificationServerURL); curl_easy_setopt(curl, CURLOPT_POST, 1); - ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r , - ERRTAG "Requeting verification with audience %s", r->server->server_hostname); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Requeting verification with audience %s", r->server->server_hostname); char *body = apr_psprintf(r->pool, "assertion=%s&audience=%s", assertionText, r->server->server_hostname); @@ -709,8 +722,8 @@ static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, cha CURLcode result = curl_easy_perform(curl); if (result != 0) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r , - ERRTAG "Error while communicating with BrowserID verification server: %s", + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error while communicating with BrowserID verification server: %s", curl_easy_strerror(result)); curl_easy_cleanup(curl); return NULL; @@ -718,8 +731,8 @@ static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, cha long responseCode; curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &responseCode); if (responseCode != 200) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r , - ERRTAG "Error while communicating with BrowserID verification server: result code %ld", responseCode); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error while communicating with BrowserID verification server: result code %ld", responseCode); curl_easy_cleanup(curl); return NULL; } @@ -737,9 +750,9 @@ apr_table_t *parseArgs(request_rec *r, char *argStr) apr_table_t *vars = apr_table_make(r->pool, 10) ; char *delim = "&"; - for ( pair = apr_strtok(r->args, delim, &last) ; - pair ; - pair = apr_strtok(NULL, delim, &last) ) + for (pair = apr_strtok(r->args, delim, &last) ; + pair ; + pair = apr_strtok(NULL, delim, &last)) { for (eq = pair ; *eq ; ++eq) if ( *eq == '+' ) @@ -748,10 +761,11 @@ apr_table_t *parseArgs(request_rec *r, char *argStr) ap_unescape_url(pair) ; eq = strchr(pair, '=') ; - if ( eq ) { + if (eq) { *eq++ = 0 ; apr_table_merge(vars, pair, eq) ; - } else { + } + else { apr_table_merge(vars, pair, "") ; } } @@ -776,7 +790,8 @@ static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char * */ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) { - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG "Submission to BrowserID form handler"); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Submission to BrowserID form handler"); /* parse the form and extract the assertion */ if (r->method_number == M_GET) { @@ -788,9 +803,9 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) apr_table_t *vars = parseArgs(r, r->args); const char *assertionParsed = apr_table_get(vars, "assertion") ; const char *returnto = apr_table_get(vars, "returnto") ; - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "In post_read_request; parsed assertion as %s", assertionParsed); - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "In post_read_request; parsed returnto as %s", returnto); /* verify the assertion... */ @@ -801,17 +816,20 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) char errorBuffer[256]; parsed_result = yajl_tree_parse(assertionResult, errorBuffer, 255); if (!parsed_result) { - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0,r,ERRTAG "Error parsing BrowserID verification response: malformed payload: %s", errorBuffer); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error parsing BrowserID verification response: malformed payload: %s", errorBuffer); return DECLINED; } - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG - "In post_read_request; parsed JSON from verification server: %s", assertionResult); - } else { - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0,r,ERRTAG - "Unable to verify assertion; communication error with verification server"); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "In post_read_request; parsed JSON from verification server: %s", assertionResult); + } + else { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Unable to verify assertion; communication error with verification server"); return DECLINED; } - } else { + } + else { if (conf->verifyLocally) { char *hdr=NULL, *payload=NULL, *sig=NULL; char *assertion = apr_pstrdup(r->pool, assertionParsed); @@ -826,14 +844,17 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) char errorBuffer[256]; parsed_result = yajl_tree_parse(payloadDecode, errorBuffer, 255); if (!parsed_result) { - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0,r,ERRTAG "Error parsing BrowserID login: malformed payload: %s", errorBuffer); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error parsing BrowserID login: malformed payload: %s", errorBuffer); return DECLINED; } /** XXX more local validation required!!! Check timestamp, audience **/ } } - } else { - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0,r,ERRTAG "Cannot verify BrowserID login: no verification server configured!"); + } + else { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Cannot verify BrowserID login: no verification server configured!"); return DECLINED; } } @@ -846,10 +867,12 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) /** XXX if we don't have an email, something went wrong. Should pull the error code properly! This will * probably require refactoring this function since the local path is different. ***/ if (!foundEmail || foundEmail->type != yajl_t_string) { - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0,r,ERRTAG "Error parsing BrowserID login: no email in payload"); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error parsing BrowserID login: no email in payload"); return DECLINED; } - ap_log_rerror(APLOG_MARK,APLOG_DEBUG|APLOG_NOERRNO, 0,r,ERRTAG "In post_read_request; got email %s", foundEmail->u.string); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "In post_read_request; got email %s", foundEmail->u.string); createSessionCookie(r, conf, foundEmail->u.string); /* redirect to the requested resource */ @@ -858,8 +881,10 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) return HTTP_TEMPORARY_REDIRECT; } } - } else { - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0,r,ERRTAG "In post_read_request; this is a POST - skipping it for now"); + } + else { + ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + "In post_read_request; this is a POST - skipping it for now"); } return DECLINED; } From 9efbb21509f09c6d83e7dd1a9351b25ee756f95b Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 20:51:00 -0500 Subject: [PATCH 18/52] update TODO --- TODO | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/TODO b/TODO index 772da59..ae487af 100644 --- a/TODO +++ b/TODO @@ -48,11 +48,6 @@ E-mail to dev-identity list of 29 Mar: + incorporate other observations in previous e-mail to dev-identity list on 28 Mar [see above] -+ code cleanup - - minor typos - - rearrange some functions for easier maintenance (e.g., keep those - modifying struct BrowserIDConfigRec closer to it) - + add capability to use a dbm for authorized user e-mail addresses + more examples @@ -75,3 +70,8 @@ Rationale: More like other module names in existing set shipping with Apache2. Example of a current one: LoadModule auth_digest_module /usr/lib/apache2/modules/mod_auth_digest.so + ++ code cleanup [mostly DONE ongoing] + - minor typos + - rearrange some functions for easier maintenance (e.g., keep those + modifying struct BrowserIDConfigRec closer to it) From 53ea22172450ede3882448565bb494067c9a0cf3 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 29 Mar 2012 20:54:31 -0500 Subject: [PATCH 19/52] correct syntax --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 01f7d53..cae6a0f 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,8 @@ httpd.conf: /id_login/browserid_login.php: ``` - + Authentication @@ -126,6 +127,7 @@ httpd.conf: }); } + ?> ``` /usr/local/apache2/htdocs/id_demo_users: From 9c03aa00f16142730c7f75290b39c254f1d4b9f9 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Fri, 30 Mar 2012 05:56:26 -0500 Subject: [PATCH 20/52] add periods for consistency --- mod_auth_browserid.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 0f04945..0931e45 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -265,19 +265,19 @@ static const command_rec Auth_browserid_cmds[] = { AP_INIT_FLAG ("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), - OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username"), + OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username."), AP_INIT_FLAG ("AuthBrowserIDAuthoritative", ap_set_flag_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), - OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default"), + OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default."), AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), - OR_AUTHCFG, "Name of cookie to set"), + OR_AUTHCFG, "Name of cookie to set."), AP_INIT_TAKE1 ("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), - OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default"), + OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default."), AP_INIT_TAKE1 ("AuthBrowserIDLogoutPath", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, logoutPath), @@ -289,7 +289,7 @@ static const command_rec Auth_browserid_cmds[] = AP_INIT_TAKE1 ("AuthBrowserIDSubmitPath", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), - OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'"), + OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'."), AP_INIT_TAKE1 ("AuthBrowserIDVerificationServerURL", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, verificationServerURL), @@ -297,7 +297,7 @@ static const command_rec Auth_browserid_cmds[] = AP_INIT_FLAG ("AuthBrowserIDVerifyLocally", ap_set_flag_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, verifyLocally), - OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set"), + OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set."), {NULL} }; From e049d8208a5ff28d11d913c08cc35660937a75fe Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Fri, 30 Mar 2012 06:14:20 -0500 Subject: [PATCH 21/52] settle on two spaces for indents --- mod_auth_browserid.c | 57 ++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 0931e45..861a5c8 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -263,43 +263,44 @@ typedef struct { /* apache config function of the module */ static const command_rec Auth_browserid_cmds[] = { - AP_INIT_FLAG ("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), - OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username."), - AP_INIT_FLAG ("AuthBrowserIDAuthoritative", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), - OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default."), + AP_INIT_FLAG("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), + OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username."), - AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), - OR_AUTHCFG, "Name of cookie to set."), + AP_INIT_FLAG("AuthBrowserIDAuthoritative", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), + OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default."), - AP_INIT_TAKE1 ("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), - OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default."), + AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), + OR_AUTHCFG, "Name of cookie to set."), - AP_INIT_TAKE1 ("AuthBrowserIDLogoutPath", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, logoutPath), - OR_AUTHCFG, "Path to which logout requests will be submitted. An optional 'returnto' parameter will be used for a redirection, if provided."), + AP_INIT_TAKE1("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), + OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default."), - AP_INIT_TAKE1 ("AuthBrowserIDSecret", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, serverSecret), - OR_AUTHCFG, "Server secret for authentication cookie."), + AP_INIT_TAKE1("AuthBrowserIDLogoutPath", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, logoutPath), + OR_AUTHCFG, "Path to which logout requests will be submitted. An optional 'returnto' parameter will be used for a redirection, if provided."), - AP_INIT_TAKE1 ("AuthBrowserIDSubmitPath", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), - OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'."), + AP_INIT_TAKE1("AuthBrowserIDSecret", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, serverSecret), + OR_AUTHCFG, "Server secret for authentication cookie."), - AP_INIT_TAKE1 ("AuthBrowserIDVerificationServerURL", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, verificationServerURL), - OR_AUTHCFG, "URL of the BrowserID verification server."), + AP_INIT_TAKE1("AuthBrowserIDSubmitPath", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), + OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'."), - AP_INIT_FLAG ("AuthBrowserIDVerifyLocally", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, verifyLocally), - OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set."), + AP_INIT_TAKE1("AuthBrowserIDVerificationServerURL", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, verificationServerURL), + OR_AUTHCFG, "URL of the BrowserID verification server."), - {NULL} + AP_INIT_FLAG("AuthBrowserIDVerifyLocally", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, verifyLocally), + OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set."), + + {NULL} }; /* function declarations */ From a03e506ba43489699bff825f6473dbb308f9e673 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Fri, 30 Mar 2012 06:44:12 -0500 Subject: [PATCH 22/52] add comment --- mod_auth_browserid.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 861a5c8..2af82d0 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -264,6 +264,8 @@ typedef struct { static const command_rec Auth_browserid_cmds[] = { + /* from http_config.h: AP_INIT_FLAG This configuration directive + takes a flag (on/off) [the flag is an int, probably a macro] as a argument */ AP_INIT_FLAG("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username."), @@ -272,10 +274,12 @@ static const command_rec Auth_browserid_cmds[] = (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default."), + /* from http_config.h: AP_INIT_TAKE1 This configuration directive takes 1 argument */ AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), OR_AUTHCFG, "Name of cookie to set."), + /* TB: doesn't appear to be used at the moment, looks like it could be a flag instead */ AP_INIT_TAKE1("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default."), @@ -893,8 +897,8 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) static int processLogout(request_rec *r, BrowserIDConfigRec *conf) { apr_table_set(r->err_headers_out, "Set-Cookie", - apr_psprintf(r->pool, "%s=; Path=/; Expires=Thu, 01-Jan-1970 00:00:01 GMT", - conf->cookieName)); + apr_psprintf(r->pool, "%s=; Path=/; Expires=Thu, 01-Jan-1970 00:00:01 GMT", + conf->cookieName)); if (r->args) { if ( strlen(r->args) > 16384 ) { @@ -922,20 +926,20 @@ static int processLogout(request_rec *r, BrowserIDConfigRec *conf) */ static int Auth_browserid_fixups(request_rec *r) { - BrowserIDConfigRec *conf=NULL; + BrowserIDConfigRec *conf=NULL; - /* get apache config */ - conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); + /* get apache config */ + conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); - if (conf->submitPath && !strcmp(r->uri, conf->submitPath)) { - return processAssertionFormSubmit(r, conf); - } - else if (conf->logoutPath && !strcmp(r->uri, conf->logoutPath)) { - return processLogout(r, conf); - } - - /* otherwise we don't care */ - return DECLINED; + if (conf->submitPath && !strcmp(r->uri, conf->submitPath)) { + return processAssertionFormSubmit(r, conf); + } + else if (conf->logoutPath && !strcmp(r->uri, conf->logoutPath)) { + return processLogout(r, conf); + } + + /* otherwise we don't care */ + return DECLINED; } @@ -944,7 +948,7 @@ static int Auth_browserid_fixups(request_rec *r) **************************************************/ static void register_hooks(apr_pool_t *p) { - ap_hook_check_user_id(Auth_browserid_check_cookie, NULL, NULL, APR_HOOK_FIRST); - ap_hook_auth_checker(Auth_browserid_check_auth, NULL, NULL, APR_HOOK_FIRST); - ap_hook_fixups(Auth_browserid_fixups, NULL, NULL, APR_HOOK_FIRST); + ap_hook_check_user_id(Auth_browserid_check_cookie, NULL, NULL, APR_HOOK_FIRST); + ap_hook_auth_checker(Auth_browserid_check_auth, NULL, NULL, APR_HOOK_FIRST); + ap_hook_fixups(Auth_browserid_fixups, NULL, NULL, APR_HOOK_FIRST); } From e330abf4150e5c24c95845d1bf1e9bb079ec0f64 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Fri, 30 Mar 2012 06:52:04 -0500 Subject: [PATCH 23/52] style: macros should be upper case for clarity --- mod_auth_browserid.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 2af82d0..152e08e 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -245,7 +245,7 @@ unsigned char c; #define ERRTAG "Auth_browserID: " #define VERSION "1.0.0" -#define unless(c) if(!(c)) +#define UNLESS(c) if(!(c)) /* config structure */ typedef struct { @@ -361,13 +361,13 @@ static void *create_browserid_config(apr_pool_t *p, char *d) /* Look through the 'Cookie' headers for the indicated cookie; extract it * and URL-unescape it. Return the cookie on success, NULL on failure. */ -static char * extract_cookie(request_rec *r, const char *szCookie_name) +static char * extract_cookie(request_rec *r, const char *szCookie_name) { char *szRaw_cookie_start=NULL, *szRaw_cookie_end; char *szCookie; /* get cookie string */ char*szRaw_cookie = (char*)apr_table_get( r->headers_in, "Cookie"); - unless(szRaw_cookie) return 0; + UNLESS(szRaw_cookie) return 0; /* loop to search cookie name in cookie header */ do { @@ -375,22 +375,22 @@ static char * extract_cookie(request_rec *r, const char *szCookie_name) "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); /* search cookie name in cookie string */ - unless (szRaw_cookie =strstr(szRaw_cookie, szCookie_name)) return 0; + UNLESS (szRaw_cookie =strstr(szRaw_cookie, szCookie_name)) return 0; szRaw_cookie_start=szRaw_cookie; /* search '=' */ - unless (szRaw_cookie = strchr(szRaw_cookie, '=')) return 0; + UNLESS (szRaw_cookie = strchr(szRaw_cookie, '=')) return 0; } while (strncmp(szCookie_name,szRaw_cookie_start,szRaw_cookie-szRaw_cookie_start)!=0); /* skip '=' */ szRaw_cookie++; /* search end of cookie name value: ';' or end of cookie strings */ - unless ((szRaw_cookie_end = strchr(szRaw_cookie, ';')) || (szRaw_cookie_end = strchr(szRaw_cookie, '\0'))) return 0; + UNLESS ((szRaw_cookie_end = strchr(szRaw_cookie, ';')) || (szRaw_cookie_end = strchr(szRaw_cookie, '\0'))) return 0; /* dup the value string found in apache pool and set the result pool ptr to szCookie ptr */ - unless (szCookie = apr_pstrndup(r->pool, szRaw_cookie, szRaw_cookie_end-szRaw_cookie)) return 0; - /* unescape the value string */ - unless (ap_unescape_url(szCookie) == 0) return 0; + UNLESS (szCookie = apr_pstrndup(r->pool, szRaw_cookie, szRaw_cookie_end-szRaw_cookie)) return 0; + /* unescape the value string */ + UNLESS (ap_unescape_url(szCookie) == 0) return 0; ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "finished cookie scan, returning %s", szCookie); @@ -415,12 +415,12 @@ static int user_in_file(request_rec *r, char *username, char *filename) int found = 0; while (!(ap_cfg_getline(l, MAX_STRING_LEN, f))) { - + /* Skip # or blank lines. */ if ((l[0] == '#') || (!l[0])) { continue; } - + if (!strcmp(username, l)) { found = 1; break; @@ -455,7 +455,7 @@ static void fix_headers_in(request_rec *r, char*szPassword) /* alloc memory for the estimated encode size of the username */ char *szB64_enc_user=(char*)apr_palloc(r->pool,apr_base64_encode_len(strlen(szUser))+1); - unless (szB64_enc_user) { + UNLESS (szB64_enc_user) { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "memory alloc failed!"); return; @@ -484,7 +484,7 @@ static char *generateSignature(request_rec *r, BrowserIDConfigRec *conf, char *u SHA1Update(&context, (unsigned char*)conf->serverSecret, strlen(conf->serverSecret)); unsigned char digest[20]; SHA1Final(digest, &context); - + char *digest64 = apr_palloc(r->pool, apr_base64_encode_len(20)); apr_base64_encode(digest64, (char*)digest, 20); return digest64; @@ -513,7 +513,7 @@ static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCook free(digest64); return 1; } - + /* Cookie is good: set r->user */ r->user = (char*)addr; return 0; @@ -536,26 +536,26 @@ static int Auth_browserid_check_cookie(request_rec *r) /* get apache config */ conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); - unless(conf->authoritative) + UNLESS(conf->authoritative) return DECLINED; ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,ERRTAG "AuthType are '%s'", ap_auth_type(r)); - unless(strncmp("BrowserID",ap_auth_type(r),9)==0) { + UNLESS(strncmp("BrowserID",ap_auth_type(r),9)==0) { ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG "Auth type must be 'BrowserID'"); return HTTP_UNAUTHORIZED; } - unless(conf->cookieName) { + UNLESS(conf->cookieName) { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "No Auth_browserid_CookieName specified"); return HTTP_UNAUTHORIZED; } /* get cookie who are named cookieName */ - unless(szCookieValue = extract_cookie(r, conf->cookieName)) { + UNLESS(szCookieValue = extract_cookie(r, conf->cookieName)) { ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG "BrowserID cookie not found; not authorized! RemoteIP:%s",szRemoteIP); return HTTP_UNAUTHORIZED; @@ -603,12 +603,12 @@ static int Auth_browserid_check_auth(request_rec *r) const char *szRequireLine = NULL; char *szFileName = NULL; char *szRequire_cmd = NULL; - + /* get apache config */ conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); /* check if this module is authoritative */ - unless(conf->authoritative) + UNLESS(conf->authoritative) return DECLINED; /* get require line */ From 7ca4a2f5ddefa3836ff5f40d8ed1fe4a80e842db Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Fri, 30 Mar 2012 06:56:00 -0500 Subject: [PATCH 24/52] add entries --- TODO | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/TODO b/TODO index ae487af..1245edb 100644 --- a/TODO +++ b/TODO @@ -1,3 +1,7 @@ +query Mike and Dan about intent of "forwardedRequestHeader"--it appears not to be used + +query about Apache use of flags: is there a macro? + E-mail to dev-identity list on 28 Mar (https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.identity/g9yCsiIV_Hs) ===================================================================================== From 754436419593ce16376d9102601d0b189df1318a Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 05:57:05 -0500 Subject: [PATCH 25/52] use leading cap --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index cae6a0f..cda488f 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ httpd.conf: # to redirect unauthorized users to the login page ErrorDocument 401 "/id_login/browserid_login.php" - require userfile /usr/local/apache2/htdocs/id_demo_users + Require userfile /usr/local/apache2/htdocs/id_demo_users ``` From fe0b0c77be8c76082c93d4d0ce13d2101bc7568d Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 06:11:50 -0500 Subject: [PATCH 26/52] trying to standardize format and ws, need an astyle options file --- mod_auth_browserid.c | 152 +++++++++++++++++++++---------------------- 1 file changed, 76 insertions(+), 76 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 152e08e..33dc6d2 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -99,11 +99,11 @@ void SHA1Final(unsigned char digest[20], SHA1_CTX* context); void SHA1Transform(u_int32_t state[5], const unsigned char buffer[64]) { -u_int32_t a, b, c, d, e; -typedef union { - unsigned char c[64]; - u_int32_t l[16]; -} CHAR64LONG16; + u_int32_t a, b, c, d, e; + typedef union { + unsigned char c[64]; + u_int32_t l[16]; + } CHAR64LONG16; #ifdef SHA1HANDSOFF CHAR64LONG16 block[1]; /* use array to appear as a pointer */ memcpy(block, buffer, 64); @@ -113,7 +113,7 @@ CHAR64LONG16 block[1]; /* use array to appear as a pointer */ * And the result is written through. I threw a "const" in, hoping * this will cause a diagnostic. */ -CHAR64LONG16* block = (const CHAR64LONG16*)buffer; + CHAR64LONG16* block = (const CHAR64LONG16*)buffer; #endif /* Copy context->state[] to working vars */ a = state[0]; @@ -174,8 +174,8 @@ void SHA1Init(SHA1_CTX* context) void SHA1Update(SHA1_CTX* context, const unsigned char* data, u_int32_t len) { -u_int32_t i; -u_int32_t j; + u_int32_t i; + u_int32_t j; j = context->count[0]; if ((context->count[0] += len << 3) < j) @@ -371,14 +371,14 @@ static char * extract_cookie(request_rec *r, const char *szCookie_name) /* loop to search cookie name in cookie header */ do { - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); - /* search cookie name in cookie string */ - UNLESS (szRaw_cookie =strstr(szRaw_cookie, szCookie_name)) return 0; - szRaw_cookie_start=szRaw_cookie; - /* search '=' */ - UNLESS (szRaw_cookie = strchr(szRaw_cookie, '=')) return 0; + /* search cookie name in cookie string */ + UNLESS (szRaw_cookie =strstr(szRaw_cookie, szCookie_name)) return 0; + szRaw_cookie_start=szRaw_cookie; + /* search '=' */ + UNLESS (szRaw_cookie = strchr(szRaw_cookie, '=')) return 0; } while (strncmp(szCookie_name,szRaw_cookie_start,szRaw_cookie-szRaw_cookie_start)!=0); /* skip '=' */ @@ -408,23 +408,23 @@ static int user_in_file(request_rec *r, char *username, char *filename) ap_configfile_t *f; status = ap_pcfg_openfile(&f, r->pool, filename); if (status != APR_SUCCESS) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, - "Could not open user file: %s", filename); - return 0; + ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, + "Could not open user file: %s", filename); + return 0; } int found = 0; while (!(ap_cfg_getline(l, MAX_STRING_LEN, f))) { - /* Skip # or blank lines. */ - if ((l[0] == '#') || (!l[0])) { - continue; - } + /* Skip # or blank lines. */ + if ((l[0] == '#') || (!l[0])) { + continue; + } - if (!strcmp(username, l)) { - found = 1; - break; - } + if (!strcmp(username, l)) { + found = 1; + break; + } } ap_cfg_closefile(f); return found; @@ -437,41 +437,41 @@ static int user_in_file(request_rec *r, char *username, char *filename) has been authenticated. */ static void fix_headers_in(request_rec *r, char*szPassword) { - char *szUser=NULL; - /* Set an Authorization header in the input request table for php and - other applications that use it to obtain the username (mainly to fix - apache logging of php scripts). We only set this if there is no header - already present. */ + char *szUser = NULL; + /* Set an Authorization header in the input request table for php + and other applications that use it to obtain the username + (mainly to fix apache logging of php scripts). We only set this + if there is no header already present. */ - if (apr_table_get(r->headers_in,"Authorization") == NULL) { - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "fixing apache Authorization header for this request using user: %s",r->user); - - /* concat username and ':' */ - if (szPassword!=NULL) - szUser=(char*)apr_pstrcat(r->pool, r->user, ":", szPassword, NULL); - else - szUser=(char*)apr_pstrcat(r->pool, r->user, ":", NULL); - - /* alloc memory for the estimated encode size of the username */ - char *szB64_enc_user=(char*)apr_palloc(r->pool,apr_base64_encode_len(strlen(szUser))+1); - UNLESS (szB64_enc_user) { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "memory alloc failed!"); - return; - } + if (apr_table_get(r->headers_in,"Authorization") == NULL) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "fixing apache Authorization header for this request using user: %s",r->user); - /* encode username in base64 format */ - apr_base64_encode(szB64_enc_user,szUser,strlen(szUser)); + /* concat username and ':' */ + if (szPassword!=NULL) + szUser=(char*)apr_pstrcat(r->pool, r->user, ":", szPassword, NULL); + else + szUser=(char*)apr_pstrcat(r->pool, r->user, ":", NULL); - /* set authorization header */ - apr_table_set(r->headers_in,"Authorization", (char*)apr_pstrcat(r->pool,"Basic ",szB64_enc_user,NULL)); + /* alloc memory for the estimated encode size of the username */ + char *szB64_enc_user=(char*)apr_palloc(r->pool,apr_base64_encode_len(strlen(szUser))+1); + UNLESS (szB64_enc_user) { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "memory alloc failed!"); + return; + } - /* force auth type to basic */ - r->ap_auth_type=apr_pstrdup(r->pool,"Basic"); - } + /* encode username in base64 format */ + apr_base64_encode(szB64_enc_user,szUser,strlen(szUser)); - return; + /* set authorization header */ + apr_table_set(r->headers_in,"Authorization", (char*)apr_pstrcat(r->pool,"Basic ",szB64_enc_user,NULL)); + + /* force auth type to basic */ + r->ap_auth_type=apr_pstrdup(r->pool,"Basic"); + } + + return; } /** Generates a signature with the given inputs, returning a Base64-encoded @@ -497,9 +497,9 @@ static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCook char *sig = NULL; char *addr = apr_strtok(szCookieValue, "|", &sig); if (!addr) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG - "malformed BrowserID cookie"); - return 1; + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + "malformed BrowserID cookie"); + return 1; } char *digest64 = generateSignature(r, conf, addr); @@ -509,9 +509,9 @@ static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCook /* paranoia indicates that we should use a time-invariant compare here */ if (strcmp(digest64, sig)) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG "invalid BrowserID cookie"); - free(digest64); - return 1; + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG "invalid BrowserID cookie"); + free(digest64); + return 1; } /* Cookie is good: set r->user */ @@ -542,32 +542,32 @@ static int Auth_browserid_check_cookie(request_rec *r) ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,ERRTAG "AuthType are '%s'", ap_auth_type(r)); - UNLESS(strncmp("BrowserID",ap_auth_type(r),9)==0) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG - "Auth type must be 'BrowserID'"); - return HTTP_UNAUTHORIZED; + UNLESS(strncmp("BrowserID",ap_auth_type(r),9) == 0) { + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + "Auth type must be 'BrowserID'"); + return HTTP_UNAUTHORIZED; } UNLESS(conf->cookieName) { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "No Auth_browserid_CookieName specified"); - return HTTP_UNAUTHORIZED; + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "No Auth_browserid_CookieName specified"); + return HTTP_UNAUTHORIZED; } /* get cookie who are named cookieName */ UNLESS(szCookieValue = extract_cookie(r, conf->cookieName)) { - ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG - "BrowserID cookie not found; not authorized! RemoteIP:%s",szRemoteIP); - return HTTP_UNAUTHORIZED; + ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG + "BrowserID cookie not found; not authorized! RemoteIP:%s",szRemoteIP); + return HTTP_UNAUTHORIZED; } ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "got cookie; value is %s", szCookieValue); /* Check cookie validity */ if (validateCookie(r, conf, szCookieValue)) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r, ERRTAG - "Invalid BrowserID cookie: %s", szCookieValue); - return HTTP_UNAUTHORIZED; + ap_log_rerror(APLOG_MARK, APLOG_WARNING | APLOG_NOERRNO, 0, r, ERRTAG + "Invalid BrowserID cookie: %s", szCookieValue); + return HTTP_UNAUTHORIZED; } /* set REMOTE_USER var for scripts language */ @@ -579,7 +579,7 @@ static int Auth_browserid_check_cookie(request_rec *r) /* fix http header for php */ if (conf->authBasicFix) - fix_headers_in(r, "browserid"); + fix_headers_in(r, "browserid"); /* if all is ok return auth ok */ return OK; From 20d1c967aebdae759e00b5462dc42665d44bcba2 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 06:25:06 -0500 Subject: [PATCH 27/52] add astylr options and a script to drive it --- astyle.opt | 26 ++++++++++++++++++++++++++ format.sh | 8 ++++++++ 2 files changed, 34 insertions(+) create mode 100644 astyle.opt create mode 100755 format.sh diff --git a/astyle.opt b/astyle.opt new file mode 100644 index 0000000..dc4a4e1 --- /dev/null +++ b/astyle.opt @@ -0,0 +1,26 @@ +# Use k&r styling +style=k&r + +# Enable support for mixed tabs and spaces +indent=force-tab-x + +# Indent goto labels rather than making them flush on the left +indent-labels + +# Indent switch cases +indent-switches + +# We don't want to add additional indentation to conditionals +min-conditional-indent=0 + +# Allow deeper indenting of statements +max-instatement-indent=120 + +# Pad operators +#pad-oper + +# Tighten parenthesis (unless overridden by other options such as pad-header) +unpad-paren + +# Keep the space between if, while, etc. and the first paren +pad-header diff --git a/format.sh b/format.sh new file mode 100755 index 0000000..bd1f520 --- /dev/null +++ b/format.sh @@ -0,0 +1,8 @@ +#!/bin/sh + +if [ -z $1 ] ; then + echo "Usage: $0 " + exit +fi + +astyle --options=astyle.opt $1 From 03fc1a25b359dcdeb789db4271506b811e95ad3e Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 06:36:56 -0500 Subject: [PATCH 28/52] set standards --- astyle.opt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/astyle.opt b/astyle.opt index dc4a4e1..bb5a479 100644 --- a/astyle.opt +++ b/astyle.opt @@ -1,8 +1,11 @@ # Use k&r styling style=k&r +# set standard spacing +indent=4 + # Enable support for mixed tabs and spaces -indent=force-tab-x +indent=force-tab # Indent goto labels rather than making them flush on the left indent-labels @@ -14,7 +17,7 @@ indent-switches min-conditional-indent=0 # Allow deeper indenting of statements -max-instatement-indent=120 +max-instatement-indent=79 # Pad operators #pad-oper From aea8c10b61b4951793a6840852cca620345c2e2d Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 06:49:37 -0500 Subject: [PATCH 29/52] set style options --- astyle.opt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/astyle.opt b/astyle.opt index bb5a479..7fc942d 100644 --- a/astyle.opt +++ b/astyle.opt @@ -2,10 +2,7 @@ style=k&r # set standard spacing -indent=4 - -# Enable support for mixed tabs and spaces -indent=force-tab +indent=spaces=4 # Indent goto labels rather than making them flush on the left indent-labels From d8a803383467c5aecab657b79dc1a5de235192e1 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 06:50:02 -0500 Subject: [PATCH 30/52] first pass with astyle --- mod_auth_browserid.c | 896 +++++++++++++++++++++++-------------------- 1 file changed, 472 insertions(+), 424 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 33dc6d2..50136a7 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -105,7 +105,7 @@ void SHA1Transform(u_int32_t state[5], const unsigned char buffer[64]) u_int32_t l[16]; } CHAR64LONG16; #ifdef SHA1HANDSOFF -CHAR64LONG16 block[1]; /* use array to appear as a pointer */ + CHAR64LONG16 block[1]; /* use array to appear as a pointer */ memcpy(block, buffer, 64); #else /* The following had better never be used because it causes the @@ -122,26 +122,86 @@ CHAR64LONG16 block[1]; /* use array to appear as a pointer */ d = state[3]; e = state[4]; /* 4 rounds of 20 operations each. Loop unrolled. */ - R0(a,b,c,d,e, 0); R0(e,a,b,c,d, 1); R0(d,e,a,b,c, 2); R0(c,d,e,a,b, 3); - R0(b,c,d,e,a, 4); R0(a,b,c,d,e, 5); R0(e,a,b,c,d, 6); R0(d,e,a,b,c, 7); - R0(c,d,e,a,b, 8); R0(b,c,d,e,a, 9); R0(a,b,c,d,e,10); R0(e,a,b,c,d,11); - R0(d,e,a,b,c,12); R0(c,d,e,a,b,13); R0(b,c,d,e,a,14); R0(a,b,c,d,e,15); - R1(e,a,b,c,d,16); R1(d,e,a,b,c,17); R1(c,d,e,a,b,18); R1(b,c,d,e,a,19); - R2(a,b,c,d,e,20); R2(e,a,b,c,d,21); R2(d,e,a,b,c,22); R2(c,d,e,a,b,23); - R2(b,c,d,e,a,24); R2(a,b,c,d,e,25); R2(e,a,b,c,d,26); R2(d,e,a,b,c,27); - R2(c,d,e,a,b,28); R2(b,c,d,e,a,29); R2(a,b,c,d,e,30); R2(e,a,b,c,d,31); - R2(d,e,a,b,c,32); R2(c,d,e,a,b,33); R2(b,c,d,e,a,34); R2(a,b,c,d,e,35); - R2(e,a,b,c,d,36); R2(d,e,a,b,c,37); R2(c,d,e,a,b,38); R2(b,c,d,e,a,39); - R3(a,b,c,d,e,40); R3(e,a,b,c,d,41); R3(d,e,a,b,c,42); R3(c,d,e,a,b,43); - R3(b,c,d,e,a,44); R3(a,b,c,d,e,45); R3(e,a,b,c,d,46); R3(d,e,a,b,c,47); - R3(c,d,e,a,b,48); R3(b,c,d,e,a,49); R3(a,b,c,d,e,50); R3(e,a,b,c,d,51); - R3(d,e,a,b,c,52); R3(c,d,e,a,b,53); R3(b,c,d,e,a,54); R3(a,b,c,d,e,55); - R3(e,a,b,c,d,56); R3(d,e,a,b,c,57); R3(c,d,e,a,b,58); R3(b,c,d,e,a,59); - R4(a,b,c,d,e,60); R4(e,a,b,c,d,61); R4(d,e,a,b,c,62); R4(c,d,e,a,b,63); - R4(b,c,d,e,a,64); R4(a,b,c,d,e,65); R4(e,a,b,c,d,66); R4(d,e,a,b,c,67); - R4(c,d,e,a,b,68); R4(b,c,d,e,a,69); R4(a,b,c,d,e,70); R4(e,a,b,c,d,71); - R4(d,e,a,b,c,72); R4(c,d,e,a,b,73); R4(b,c,d,e,a,74); R4(a,b,c,d,e,75); - R4(e,a,b,c,d,76); R4(d,e,a,b,c,77); R4(c,d,e,a,b,78); R4(b,c,d,e,a,79); + R0(a,b,c,d,e, 0); + R0(e,a,b,c,d, 1); + R0(d,e,a,b,c, 2); + R0(c,d,e,a,b, 3); + R0(b,c,d,e,a, 4); + R0(a,b,c,d,e, 5); + R0(e,a,b,c,d, 6); + R0(d,e,a,b,c, 7); + R0(c,d,e,a,b, 8); + R0(b,c,d,e,a, 9); + R0(a,b,c,d,e,10); + R0(e,a,b,c,d,11); + R0(d,e,a,b,c,12); + R0(c,d,e,a,b,13); + R0(b,c,d,e,a,14); + R0(a,b,c,d,e,15); + R1(e,a,b,c,d,16); + R1(d,e,a,b,c,17); + R1(c,d,e,a,b,18); + R1(b,c,d,e,a,19); + R2(a,b,c,d,e,20); + R2(e,a,b,c,d,21); + R2(d,e,a,b,c,22); + R2(c,d,e,a,b,23); + R2(b,c,d,e,a,24); + R2(a,b,c,d,e,25); + R2(e,a,b,c,d,26); + R2(d,e,a,b,c,27); + R2(c,d,e,a,b,28); + R2(b,c,d,e,a,29); + R2(a,b,c,d,e,30); + R2(e,a,b,c,d,31); + R2(d,e,a,b,c,32); + R2(c,d,e,a,b,33); + R2(b,c,d,e,a,34); + R2(a,b,c,d,e,35); + R2(e,a,b,c,d,36); + R2(d,e,a,b,c,37); + R2(c,d,e,a,b,38); + R2(b,c,d,e,a,39); + R3(a,b,c,d,e,40); + R3(e,a,b,c,d,41); + R3(d,e,a,b,c,42); + R3(c,d,e,a,b,43); + R3(b,c,d,e,a,44); + R3(a,b,c,d,e,45); + R3(e,a,b,c,d,46); + R3(d,e,a,b,c,47); + R3(c,d,e,a,b,48); + R3(b,c,d,e,a,49); + R3(a,b,c,d,e,50); + R3(e,a,b,c,d,51); + R3(d,e,a,b,c,52); + R3(c,d,e,a,b,53); + R3(b,c,d,e,a,54); + R3(a,b,c,d,e,55); + R3(e,a,b,c,d,56); + R3(d,e,a,b,c,57); + R3(c,d,e,a,b,58); + R3(b,c,d,e,a,59); + R4(a,b,c,d,e,60); + R4(e,a,b,c,d,61); + R4(d,e,a,b,c,62); + R4(c,d,e,a,b,63); + R4(b,c,d,e,a,64); + R4(a,b,c,d,e,65); + R4(e,a,b,c,d,66); + R4(d,e,a,b,c,67); + R4(c,d,e,a,b,68); + R4(b,c,d,e,a,69); + R4(a,b,c,d,e,70); + R4(e,a,b,c,d,71); + R4(d,e,a,b,c,72); + R4(c,d,e,a,b,73); + R4(b,c,d,e,a,74); + R4(a,b,c,d,e,75); + R4(e,a,b,c,d,76); + R4(d,e,a,b,c,77); + R4(c,d,e,a,b,78); + R4(b,c,d,e,a,79); /* Add the working vars back into context.state[] */ state[0] += a; state[1] += b; @@ -179,18 +239,17 @@ void SHA1Update(SHA1_CTX* context, const unsigned char* data, u_int32_t len) j = context->count[0]; if ((context->count[0] += len << 3) < j) - context->count[1]++; + context->count[1]++; context->count[1] += (len>>29); j = (j >> 3) & 63; if ((j + len) > 63) { memcpy(&context->buffer[j], data, (i = 64-j)); SHA1Transform(context->state, context->buffer); - for ( ; i + 63 < len; i += 64) { + for (; i + 63 < len; i += 64) { SHA1Transform(context->state, &data[i]); } j = 0; - } - else i = 0; + } else i = 0; memcpy(&context->buffer[j], &data[i], len - i); } @@ -199,9 +258,9 @@ void SHA1Update(SHA1_CTX* context, const unsigned char* data, u_int32_t len) void SHA1Final(unsigned char digest[20], SHA1_CTX* context) { -unsigned i; -unsigned char finalcount[8]; -unsigned char c; + unsigned i; + unsigned char finalcount[8]; + unsigned char c; #if 0 /* untested "improvement" by DHR */ /* Convert context->count to a sequence of bytes @@ -211,30 +270,29 @@ unsigned char c; */ unsigned char *fcp = &finalcount[8]; - for (i = 0; i < 2; i++) - { - u_int32_t t = context->count[i]; - int j; + for (i = 0; i < 2; i++) { + u_int32_t t = context->count[i]; + int j; - for (j = 0; j < 4; t >>= 8, j++) - *--fcp = (unsigned char) t - } + for (j = 0; j < 4; t >>= 8, j++) + *--fcp = (unsigned char) t + } #else for (i = 0; i < 8; i++) { finalcount[i] = (unsigned char)((context->count[(i >= 4 ? 0 : 1)] - >> ((3-(i & 3)) * 8) ) & 255); /* Endian independent */ + >> ((3-(i & 3)) * 8)) & 255); /* Endian independent */ } #endif - c = 0200; + c = 0200; SHA1Update(context, &c, 1); while ((context->count[0] & 504) != 448) { - c = 0000; + c = 0000; SHA1Update(context, &c, 1); } SHA1Update(context, finalcount, 8); /* Should cause a SHA1Transform() */ for (i = 0; i < 20; i++) { digest[i] = (unsigned char) - ((context->state[i>>2] >> ((3-(i & 3)) * 8) ) & 255); + ((context->state[i>>2] >> ((3-(i & 3)) * 8)) & 255); } /* Wipe variables */ memset(context, '\0', sizeof(*context)); @@ -249,62 +307,61 @@ unsigned char c; /* config structure */ typedef struct { - int authBasicFix; - int authoritative; - char *cookieName; - char *forwardedRequestHeader; - char *logoutPath; - char *serverSecret; - char *submitPath; - char *verificationServerURL; - int verifyLocally; + int authBasicFix; + int authoritative; + char *cookieName; + char *forwardedRequestHeader; + char *logoutPath; + char *serverSecret; + char *submitPath; + char *verificationServerURL; + int verifyLocally; } BrowserIDConfigRec; /* apache config function of the module */ -static const command_rec Auth_browserid_cmds[] = -{ +static const command_rec Auth_browserid_cmds[] = { - /* from http_config.h: AP_INIT_FLAG This configuration directive - takes a flag (on/off) [the flag is an int, probably a macro] as a argument */ - AP_INIT_FLAG("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), - OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username."), + /* from http_config.h: AP_INIT_FLAG This configuration directive + takes a flag (on/off) [the flag is an int, probably a macro] as a argument */ + AP_INIT_FLAG("AuthBrowserIDSimulateAuthBasic", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, authBasicFix), + OR_AUTHCFG, "Set to 'yes' to enable creation of a synthetic Basic Authorization header containing the username."), - AP_INIT_FLAG("AuthBrowserIDAuthoritative", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), - OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default."), + AP_INIT_FLAG("AuthBrowserIDAuthoritative", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, authoritative), + OR_AUTHCFG, "Set to 'yes' to allow access control to be passed along to lower modules; set to 'no' by default."), - /* from http_config.h: AP_INIT_TAKE1 This configuration directive takes 1 argument */ - AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), - OR_AUTHCFG, "Name of cookie to set."), + /* from http_config.h: AP_INIT_TAKE1 This configuration directive takes 1 argument */ + AP_INIT_TAKE1("AuthBrowserIDCookieName", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, cookieName), + OR_AUTHCFG, "Name of cookie to set."), - /* TB: doesn't appear to be used at the moment, looks like it could be a flag instead */ - AP_INIT_TAKE1("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), - OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default."), + /* TB: doesn't appear to be used at the moment, looks like it could be a flag instead */ + AP_INIT_TAKE1("AuthBrowserIDSetHTTPHeader", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, forwardedRequestHeader), + OR_AUTHCFG, "Set to 'yes' to forward a signed HTTP header containing the verified identity; set to 'no' by default."), - AP_INIT_TAKE1("AuthBrowserIDLogoutPath", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, logoutPath), - OR_AUTHCFG, "Path to which logout requests will be submitted. An optional 'returnto' parameter will be used for a redirection, if provided."), + AP_INIT_TAKE1("AuthBrowserIDLogoutPath", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, logoutPath), + OR_AUTHCFG, "Path to which logout requests will be submitted. An optional 'returnto' parameter will be used for a redirection, if provided."), - AP_INIT_TAKE1("AuthBrowserIDSecret", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, serverSecret), - OR_AUTHCFG, "Server secret for authentication cookie."), + AP_INIT_TAKE1("AuthBrowserIDSecret", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, serverSecret), + OR_AUTHCFG, "Server secret for authentication cookie."), - AP_INIT_TAKE1("AuthBrowserIDSubmitPath", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), - OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'."), + AP_INIT_TAKE1("AuthBrowserIDSubmitPath", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, submitPath), + OR_AUTHCFG, "Path to which login forms will be submitted. Form must contain a field named 'assertion'."), - AP_INIT_TAKE1("AuthBrowserIDVerificationServerURL", ap_set_string_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, verificationServerURL), - OR_AUTHCFG, "URL of the BrowserID verification server."), + AP_INIT_TAKE1("AuthBrowserIDVerificationServerURL", ap_set_string_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, verificationServerURL), + OR_AUTHCFG, "URL of the BrowserID verification server."), - AP_INIT_FLAG("AuthBrowserIDVerifyLocally", ap_set_flag_slot, - (void *)APR_OFFSETOF(BrowserIDConfigRec, verifyLocally), - OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set."), + AP_INIT_FLAG("AuthBrowserIDVerifyLocally", ap_set_flag_slot, + (void *)APR_OFFSETOF(BrowserIDConfigRec, verifyLocally), + OR_AUTHCFG, "Set to 'yes' to verify assertions locally; ignored if VerificationServerURL is set."), - {NULL} + {NULL} }; /* function declarations */ @@ -328,8 +385,7 @@ static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, cha module AP_MODULE_DECLARE_DATA auth_browserid_module; /* apache module structure */ -module AP_MODULE_DECLARE_DATA auth_browserid_module = -{ +module AP_MODULE_DECLARE_DATA auth_browserid_module = { STANDARD20_MODULE_STUFF, create_browserid_config, /* dir config creator */ NULL, /* dir merger --- default is to override */ @@ -363,39 +419,39 @@ static void *create_browserid_config(apr_pool_t *p, char *d) * and URL-unescape it. Return the cookie on success, NULL on failure. */ static char * extract_cookie(request_rec *r, const char *szCookie_name) { - char *szRaw_cookie_start=NULL, *szRaw_cookie_end; - char *szCookie; - /* get cookie string */ - char*szRaw_cookie = (char*)apr_table_get( r->headers_in, "Cookie"); - UNLESS(szRaw_cookie) return 0; - - /* loop to search cookie name in cookie header */ - do { - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); - - /* search cookie name in cookie string */ - UNLESS (szRaw_cookie =strstr(szRaw_cookie, szCookie_name)) return 0; - szRaw_cookie_start=szRaw_cookie; - /* search '=' */ - UNLESS (szRaw_cookie = strchr(szRaw_cookie, '=')) return 0; - } while (strncmp(szCookie_name,szRaw_cookie_start,szRaw_cookie-szRaw_cookie_start)!=0); - - /* skip '=' */ - szRaw_cookie++; - - /* search end of cookie name value: ';' or end of cookie strings */ - UNLESS ((szRaw_cookie_end = strchr(szRaw_cookie, ';')) || (szRaw_cookie_end = strchr(szRaw_cookie, '\0'))) return 0; - - /* dup the value string found in apache pool and set the result pool ptr to szCookie ptr */ - UNLESS (szCookie = apr_pstrndup(r->pool, szRaw_cookie, szRaw_cookie_end-szRaw_cookie)) return 0; - /* unescape the value string */ - UNLESS (ap_unescape_url(szCookie) == 0) return 0; - - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, - ERRTAG "finished cookie scan, returning %s", szCookie); - - return szCookie; + char *szRaw_cookie_start=NULL, *szRaw_cookie_end; + char *szCookie; + /* get cookie string */ + char*szRaw_cookie = (char*)apr_table_get(r->headers_in, "Cookie"); + UNLESS(szRaw_cookie) return 0; + + /* loop to search cookie name in cookie header */ + do { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); + + /* search cookie name in cookie string */ + UNLESS(szRaw_cookie =strstr(szRaw_cookie, szCookie_name)) return 0; + szRaw_cookie_start=szRaw_cookie; + /* search '=' */ + UNLESS(szRaw_cookie = strchr(szRaw_cookie, '=')) return 0; + } while (strncmp(szCookie_name,szRaw_cookie_start,szRaw_cookie-szRaw_cookie_start)!=0); + + /* skip '=' */ + szRaw_cookie++; + + /* search end of cookie name value: ';' or end of cookie strings */ + UNLESS((szRaw_cookie_end = strchr(szRaw_cookie, ';')) || (szRaw_cookie_end = strchr(szRaw_cookie, '\0'))) return 0; + + /* dup the value string found in apache pool and set the result pool ptr to szCookie ptr */ + UNLESS(szCookie = apr_pstrndup(r->pool, szRaw_cookie, szRaw_cookie_end-szRaw_cookie)) return 0; + /* unescape the value string */ + UNLESS(ap_unescape_url(szCookie) == 0) return 0; + + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, + ERRTAG "finished cookie scan, returning %s", szCookie); + + return szCookie; } /** Given a filename and username, open the file (using normal Apache @@ -403,31 +459,31 @@ static char * extract_cookie(request_rec *r, const char *szCookie_name) * in it (as a newline-seaparated list) */ static int user_in_file(request_rec *r, char *username, char *filename) { - apr_status_t status; - char l[MAX_STRING_LEN]; - ap_configfile_t *f; - status = ap_pcfg_openfile(&f, r->pool, filename); - if (status != APR_SUCCESS) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, - "Could not open user file: %s", filename); - return 0; - } - - int found = 0; - while (!(ap_cfg_getline(l, MAX_STRING_LEN, f))) { - - /* Skip # or blank lines. */ - if ((l[0] == '#') || (!l[0])) { - continue; - } - - if (!strcmp(username, l)) { - found = 1; - break; - } - } - ap_cfg_closefile(f); - return found; + apr_status_t status; + char l[MAX_STRING_LEN]; + ap_configfile_t *f; + status = ap_pcfg_openfile(&f, r->pool, filename); + if (status != APR_SUCCESS) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, + "Could not open user file: %s", filename); + return 0; + } + + int found = 0; + while (!(ap_cfg_getline(l, MAX_STRING_LEN, f))) { + + /* Skip # or blank lines. */ + if ((l[0] == '#') || (!l[0])) { + continue; + } + + if (!strcmp(username, l)) { + found = 1; + break; + } + } + ap_cfg_closefile(f); + return found; } @@ -455,7 +511,7 @@ static void fix_headers_in(request_rec *r, char*szPassword) /* alloc memory for the estimated encode size of the username */ char *szB64_enc_user=(char*)apr_palloc(r->pool,apr_base64_encode_len(strlen(szUser))+1); - UNLESS (szB64_enc_user) { + UNLESS(szB64_enc_user) { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "memory alloc failed!"); return; @@ -505,7 +561,7 @@ static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCook char *digest64 = generateSignature(r, conf, addr); ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, ERRTAG "Got cookie: email is %s; expected digest is %s; got digest %s", - addr, digest64, sig); + addr, digest64, sig); /* paranoia indicates that we should use a time-invariant compare here */ if (strcmp(digest64, sig)) { @@ -537,7 +593,7 @@ static int Auth_browserid_check_cookie(request_rec *r) conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); UNLESS(conf->authoritative) - return DECLINED; + return DECLINED; ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,ERRTAG "AuthType are '%s'", ap_auth_type(r)); @@ -595,197 +651,194 @@ static int Auth_browserid_check_cookie(request_rec *r) **************************************************/ static int Auth_browserid_check_auth(request_rec *r) { - BrowserIDConfigRec *conf = NULL; - char *szUser = NULL; - const apr_array_header_t *reqs_arr = NULL; - require_line *reqs = NULL; - register int x = 0; - const char *szRequireLine = NULL; - char *szFileName = NULL; - char *szRequire_cmd = NULL; - - /* get apache config */ - conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); - - /* check if this module is authoritative */ - UNLESS(conf->authoritative) - return DECLINED; + BrowserIDConfigRec *conf = NULL; + char *szUser = NULL; + const apr_array_header_t *reqs_arr = NULL; + require_line *reqs = NULL; + register int x = 0; + const char *szRequireLine = NULL; + char *szFileName = NULL; + char *szRequire_cmd = NULL; - /* get require line */ - reqs_arr = ap_requires(r); - reqs = reqs_arr ? (require_line *) reqs_arr->elts : NULL; + /* get apache config */ + conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); - /* decline if no require line found */ - if (!reqs_arr) + /* check if this module is authoritative */ + UNLESS(conf->authoritative) return DECLINED; - /* walk through the array to check each require command */ - for (x = 0; x < reqs_arr->nelts; x++) { + /* get require line */ + reqs_arr = ap_requires(r); + reqs = reqs_arr ? (require_line *) reqs_arr->elts : NULL; - if (!(reqs[x].method_mask & (AP_METHOD_BIT << r->method_number))) - continue; + /* decline if no require line found */ + if (!reqs_arr) + return DECLINED; - /* get require line */ - szRequireLine = reqs[x].requirement; - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "Require Line is '%s'", szRequireLine); + /* walk through the array to check each require command */ + for (x = 0; x < reqs_arr->nelts; x++) { - /* get the first word in require line */ - szRequire_cmd = ap_getword_white(r->pool, &szRequireLine); - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "Require Cmd is '%s'", szRequire_cmd); - - /* if require cmd are valid-user, they are already authenticated than allow and return OK */ - if (!strcmp("valid-user",szRequire_cmd)) { - ap_log_rerror(APLOG_MARK,APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "Require Cmd valid-user"); - return OK; - } - /* check the required user */ - else if (!strcmp("user",szRequire_cmd)) { - szUser = ap_getword_conf(r->pool, &szRequireLine); - if (strcmp(r->user, szUser)) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, ERRTAG - "user '%s' is not the required user '%s'",r->user, szUser); - return HTTP_FORBIDDEN; - } - ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG - "user '%s' is authorized",r->user); - return OK; - } - /* check for users in a file */ - else if (!strcmp("userfile",szRequire_cmd)) { - szFileName = ap_getword_conf(r->pool, &szRequireLine); - if (!user_in_file(r, r->user, szFileName)) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, ERRTAG - "user '%s' is not in username list at '%s'", r->user,szFileName); - return HTTP_FORBIDDEN; - } - else { - return OK; - } + if (!(reqs[x].method_mask & (AP_METHOD_BIT << r->method_number))) + continue; + + /* get require line */ + szRequireLine = reqs[x].requirement; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Require Line is '%s'", szRequireLine); + + /* get the first word in require line */ + szRequire_cmd = ap_getword_white(r->pool, &szRequireLine); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Require Cmd is '%s'", szRequire_cmd); + + /* if require cmd are valid-user, they are already authenticated than allow and return OK */ + if (!strcmp("valid-user",szRequire_cmd)) { + ap_log_rerror(APLOG_MARK,APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Require Cmd valid-user"); + return OK; + } + /* check the required user */ + else if (!strcmp("user",szRequire_cmd)) { + szUser = ap_getword_conf(r->pool, &szRequireLine); + if (strcmp(r->user, szUser)) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, ERRTAG + "user '%s' is not the required user '%s'",r->user, szUser); + return HTTP_FORBIDDEN; + } + ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG + "user '%s' is authorized",r->user); + return OK; + } + /* check for users in a file */ + else if (!strcmp("userfile",szRequire_cmd)) { + szFileName = ap_getword_conf(r->pool, &szRequireLine); + if (!user_in_file(r, r->user, szFileName)) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, ERRTAG + "user '%s' is not in username list at '%s'", r->user,szFileName); + return HTTP_FORBIDDEN; + } else { + return OK; + } + } } - } - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "user '%s' is not authorized", r->user); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "user '%s' is not authorized", r->user); - /* forbid by default */ - return HTTP_FORBIDDEN; + /* forbid by default */ + return HTTP_FORBIDDEN; } /* Helper struct for CURL response */ struct MemoryStruct { - char *memory; - size_t size; - size_t realsize; - request_rec *r; + char *memory; + size_t size; + size_t realsize; + request_rec *r; }; - + /** Callback function for streaming CURL response */ static size_t WriteMemoryCallback(void *contents, size_t size, size_t nmemb, void *userp) { - size_t realsize = size * nmemb; - struct MemoryStruct *mem = (struct MemoryStruct *)userp; - - if (mem->size + realsize >= mem->realsize) { - mem->realsize = mem->size + realsize + 256; - void *tmp = apr_palloc(mem->r->pool, mem->size + realsize + 256); - memcpy(tmp, mem->memory, mem->size); - mem->memory = tmp; - } - - memcpy(&(mem->memory[mem->size]), contents, realsize); - mem->size += realsize; - mem->memory[mem->size] = 0; - return realsize; + size_t realsize = size * nmemb; + struct MemoryStruct *mem = (struct MemoryStruct *)userp; + + if (mem->size + realsize >= mem->realsize) { + mem->realsize = mem->size + realsize + 256; + void *tmp = apr_palloc(mem->r->pool, mem->size + realsize + 256); + memcpy(tmp, mem->memory, mem->size); + mem->memory = tmp; + } + + memcpy(&(mem->memory[mem->size]), contents, realsize); + mem->size += realsize; + mem->memory[mem->size] = 0; + return realsize; } /* Pass the assertion to the verification service defined in the config, * and return the result to the caller */ static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, char *assertionText) { - CURL *curl = curl_easy_init(); - curl_easy_setopt(curl, CURLOPT_URL, conf->verificationServerURL); - curl_easy_setopt(curl, CURLOPT_POST, 1); - - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "Requeting verification with audience %s", r->server->server_hostname); - - char *body = apr_psprintf(r->pool, "assertion=%s&audience=%s", - assertionText, r->server->server_hostname); - curl_easy_setopt(curl, CURLOPT_POSTFIELDS, body); - /** XXX set certificate for SSL negotiation */ - - struct MemoryStruct chunk; - chunk.memory = apr_pcalloc(r->pool, 1024); - chunk.size = 0; - chunk.realsize = 1024; - chunk.r = r; - curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteMemoryCallback); - curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk); - curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-mod_browserid-agent/1.0"); - - CURLcode result = curl_easy_perform(curl); - if (result != 0) { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "Error while communicating with BrowserID verification server: %s", - curl_easy_strerror(result)); - curl_easy_cleanup(curl); - return NULL; - } - long responseCode; - curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &responseCode); - if (responseCode != 200) { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "Error while communicating with BrowserID verification server: result code %ld", responseCode); + CURL *curl = curl_easy_init(); + curl_easy_setopt(curl, CURLOPT_URL, conf->verificationServerURL); + curl_easy_setopt(curl, CURLOPT_POST, 1); + + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Requeting verification with audience %s", r->server->server_hostname); + + char *body = apr_psprintf(r->pool, "assertion=%s&audience=%s", + assertionText, r->server->server_hostname); + curl_easy_setopt(curl, CURLOPT_POSTFIELDS, body); + /** XXX set certificate for SSL negotiation */ + + struct MemoryStruct chunk; + chunk.memory = apr_pcalloc(r->pool, 1024); + chunk.size = 0; + chunk.realsize = 1024; + chunk.r = r; + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteMemoryCallback); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk); + curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-mod_browserid-agent/1.0"); + + CURLcode result = curl_easy_perform(curl); + if (result != 0) { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error while communicating with BrowserID verification server: %s", + curl_easy_strerror(result)); + curl_easy_cleanup(curl); + return NULL; + } + long responseCode; + curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &responseCode); + if (responseCode != 200) { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error while communicating with BrowserID verification server: result code %ld", responseCode); + curl_easy_cleanup(curl); + return NULL; + } curl_easy_cleanup(curl); - return NULL; - } - curl_easy_cleanup(curl); - return chunk.memory; + return chunk.memory; } /* Parse x-www-url-formencoded args */ apr_table_t *parseArgs(request_rec *r, char *argStr) { - char* pair ; - char* last = NULL ; - char* eq ; - - apr_table_t *vars = apr_table_make(r->pool, 10) ; - char *delim = "&"; - - for (pair = apr_strtok(r->args, delim, &last) ; - pair ; - pair = apr_strtok(NULL, delim, &last)) - { - for (eq = pair ; *eq ; ++eq) - if ( *eq == '+' ) - *eq = ' ' ; - - ap_unescape_url(pair) ; - eq = strchr(pair, '=') ; - - if (eq) { - *eq++ = 0 ; - apr_table_merge(vars, pair, eq) ; - } - else { - apr_table_merge(vars, pair, "") ; + char* pair ; + char* last = NULL ; + char* eq ; + + apr_table_t *vars = apr_table_make(r->pool, 10) ; + char *delim = "&"; + + for (pair = apr_strtok(r->args, delim, &last) ; + pair ; + pair = apr_strtok(NULL, delim, &last)) { + for (eq = pair ; *eq ; ++eq) + if (*eq == '+') + *eq = ' ' ; + + ap_unescape_url(pair) ; + eq = strchr(pair, '=') ; + + if (eq) { + *eq++ = 0 ; + apr_table_merge(vars, pair, eq) ; + } else { + apr_table_merge(vars, pair, "") ; + } } - } - return vars; + return vars; } /** Create a session cookie with a given identity */ static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char *identity) { - char *digest64 = generateSignature(r, conf, identity); - - /* syntax of cookie is identity|signature */ - apr_table_set(r->err_headers_out, "Set-Cookie", - apr_psprintf(r->pool, "%s=%s|%s; Path=/", - conf->cookieName, identity, digest64)); + char *digest64 = generateSignature(r, conf, identity); + + /* syntax of cookie is identity|signature */ + apr_table_set(r->err_headers_out, "Set-Cookie", + apr_psprintf(r->pool, "%s=%s|%s; Path=/", + conf->cookieName, identity, digest64)); } /* Called from the fixup_handler when we receive a form submission. @@ -795,125 +848,121 @@ static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char * */ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) { - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "Submission to BrowserID form handler"); - - /* parse the form and extract the assertion */ - if (r->method_number == M_GET) { - if ( r->args ) { - if ( strlen(r->args) > 16384 ) { - return HTTP_REQUEST_URI_TOO_LARGE ; - } - - apr_table_t *vars = parseArgs(r, r->args); - const char *assertionParsed = apr_table_get(vars, "assertion") ; - const char *returnto = apr_table_get(vars, "returnto") ; - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "In post_read_request; parsed assertion as %s", assertionParsed); - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "In post_read_request; parsed returnto as %s", returnto); - - /* verify the assertion... */ - yajl_val parsed_result = NULL; - if (conf->verificationServerURL) { - char *assertionResult = verifyAssertionRemote(r, conf, (char*)assertionParsed); - if (assertionResult) { - char errorBuffer[256]; - parsed_result = yajl_tree_parse(assertionResult, errorBuffer, 255); - if (!parsed_result) { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "Error parsing BrowserID verification response: malformed payload: %s", errorBuffer); - return DECLINED; - } - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "In post_read_request; parsed JSON from verification server: %s", assertionResult); - } - else { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "Unable to verify assertion; communication error with verification server"); - return DECLINED; - } - } - else { - if (conf->verifyLocally) { - char *hdr=NULL, *payload=NULL, *sig=NULL; - char *assertion = apr_pstrdup(r->pool, assertionParsed); - hdr= apr_strtok(assertion, ".", &payload); - if (hdr) { - payload= apr_strtok(payload, ".", &sig); - if (sig) { - int len = apr_base64_decode_len(payload); - char *payloadDecode = apr_pcalloc(r->pool, len+1); - apr_base64_decode(payloadDecode, payload); - - char errorBuffer[256]; - parsed_result = yajl_tree_parse(payloadDecode, errorBuffer, 255); - if (!parsed_result) { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "Error parsing BrowserID login: malformed payload: %s", errorBuffer); - return DECLINED; - } - /** XXX more local validation required!!! Check timestamp, audience **/ + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "Submission to BrowserID form handler"); + + /* parse the form and extract the assertion */ + if (r->method_number == M_GET) { + if (r->args) { + if (strlen(r->args) > 16384) { + return HTTP_REQUEST_URI_TOO_LARGE ; + } + + apr_table_t *vars = parseArgs(r, r->args); + const char *assertionParsed = apr_table_get(vars, "assertion") ; + const char *returnto = apr_table_get(vars, "returnto") ; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "In post_read_request; parsed assertion as %s", assertionParsed); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "In post_read_request; parsed returnto as %s", returnto); + + /* verify the assertion... */ + yajl_val parsed_result = NULL; + if (conf->verificationServerURL) { + char *assertionResult = verifyAssertionRemote(r, conf, (char*)assertionParsed); + if (assertionResult) { + char errorBuffer[256]; + parsed_result = yajl_tree_parse(assertionResult, errorBuffer, 255); + if (!parsed_result) { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error parsing BrowserID verification response: malformed payload: %s", errorBuffer); + return DECLINED; + } + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "In post_read_request; parsed JSON from verification server: %s", assertionResult); + } else { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Unable to verify assertion; communication error with verification server"); + return DECLINED; + } + } else { + if (conf->verifyLocally) { + char *hdr=NULL, *payload=NULL, *sig=NULL; + char *assertion = apr_pstrdup(r->pool, assertionParsed); + hdr= apr_strtok(assertion, ".", &payload); + if (hdr) { + payload= apr_strtok(payload, ".", &sig); + if (sig) { + int len = apr_base64_decode_len(payload); + char *payloadDecode = apr_pcalloc(r->pool, len+1); + apr_base64_decode(payloadDecode, payload); + + char errorBuffer[256]; + parsed_result = yajl_tree_parse(payloadDecode, errorBuffer, 255); + if (!parsed_result) { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error parsing BrowserID login: malformed payload: %s", errorBuffer); + return DECLINED; + } + /** XXX more local validation required!!! Check timestamp, audience **/ + } + } + } else { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Cannot verify BrowserID login: no verification server configured!"); + return DECLINED; + } + } + if (parsed_result) { + char *parsePath[2]; + parsePath[0] = "email"; + parsePath[1] = NULL; + yajl_val foundEmail = yajl_tree_get(parsed_result, (const char**)parsePath, yajl_t_any); + + /** XXX if we don't have an email, something went wrong. Should pull the error code properly! This will + * probably require refactoring this function since the local path is different. ***/ + if (!foundEmail || foundEmail->type != yajl_t_string) { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG + "Error parsing BrowserID login: no email in payload"); + return DECLINED; + } + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + "In post_read_request; got email %s", foundEmail->u.string); + createSessionCookie(r, conf, foundEmail->u.string); + + /* redirect to the requested resource */ + apr_table_set(r->headers_out,"Location", returnto); + + return HTTP_TEMPORARY_REDIRECT; } - } - } - else { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "Cannot verify BrowserID login: no verification server configured!"); - return DECLINED; - } - } - if (parsed_result) { - char *parsePath[2]; - parsePath[0] = "email"; - parsePath[1] = NULL; - yajl_val foundEmail = yajl_tree_get(parsed_result, (const char**)parsePath, yajl_t_any); - - /** XXX if we don't have an email, something went wrong. Should pull the error code properly! This will - * probably require refactoring this function since the local path is different. ***/ - if (!foundEmail || foundEmail->type != yajl_t_string) { - ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG - "Error parsing BrowserID login: no email in payload"); - return DECLINED; } - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "In post_read_request; got email %s", foundEmail->u.string); - createSessionCookie(r, conf, foundEmail->u.string); - - /* redirect to the requested resource */ - apr_table_set(r->headers_out,"Location", returnto); - - return HTTP_TEMPORARY_REDIRECT; - } + } else { + ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + "In post_read_request; this is a POST - skipping it for now"); } - } - else { - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG - "In post_read_request; this is a POST - skipping it for now"); - } - return DECLINED; + return DECLINED; } static int processLogout(request_rec *r, BrowserIDConfigRec *conf) { - apr_table_set(r->err_headers_out, "Set-Cookie", - apr_psprintf(r->pool, "%s=; Path=/; Expires=Thu, 01-Jan-1970 00:00:01 GMT", - conf->cookieName)); + apr_table_set(r->err_headers_out, "Set-Cookie", + apr_psprintf(r->pool, "%s=; Path=/; Expires=Thu, 01-Jan-1970 00:00:01 GMT", + conf->cookieName)); - if (r->args) { - if ( strlen(r->args) > 16384 ) { - return HTTP_REQUEST_URI_TOO_LARGE ; - } + if (r->args) { + if (strlen(r->args) > 16384) { + return HTTP_REQUEST_URI_TOO_LARGE ; + } - apr_table_t *vars = parseArgs(r, r->args); - const char *returnto = apr_table_get(vars, "returnto") ; - if (returnto) { - apr_table_set(r->headers_out,"Location", returnto); - return HTTP_TEMPORARY_REDIRECT; + apr_table_t *vars = parseArgs(r, r->args); + const char *returnto = apr_table_get(vars, "returnto") ; + if (returnto) { + apr_table_set(r->headers_out,"Location", returnto); + return HTTP_TEMPORARY_REDIRECT; + } } - } - apr_table_set(r->headers_out,"Location", "/"); - return HTTP_TEMPORARY_REDIRECT; + apr_table_set(r->headers_out,"Location", "/"); + return HTTP_TEMPORARY_REDIRECT; } /* @@ -926,20 +975,19 @@ static int processLogout(request_rec *r, BrowserIDConfigRec *conf) */ static int Auth_browserid_fixups(request_rec *r) { - BrowserIDConfigRec *conf=NULL; + BrowserIDConfigRec *conf=NULL; - /* get apache config */ - conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); + /* get apache config */ + conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); - if (conf->submitPath && !strcmp(r->uri, conf->submitPath)) { - return processAssertionFormSubmit(r, conf); - } - else if (conf->logoutPath && !strcmp(r->uri, conf->logoutPath)) { - return processLogout(r, conf); - } + if (conf->submitPath && !strcmp(r->uri, conf->submitPath)) { + return processAssertionFormSubmit(r, conf); + } else if (conf->logoutPath && !strcmp(r->uri, conf->logoutPath)) { + return processLogout(r, conf); + } - /* otherwise we don't care */ - return DECLINED; + /* otherwise we don't care */ + return DECLINED; } @@ -948,7 +996,7 @@ static int Auth_browserid_fixups(request_rec *r) **************************************************/ static void register_hooks(apr_pool_t *p) { - ap_hook_check_user_id(Auth_browserid_check_cookie, NULL, NULL, APR_HOOK_FIRST); - ap_hook_auth_checker(Auth_browserid_check_auth, NULL, NULL, APR_HOOK_FIRST); - ap_hook_fixups(Auth_browserid_fixups, NULL, NULL, APR_HOOK_FIRST); + ap_hook_check_user_id(Auth_browserid_check_cookie, NULL, NULL, APR_HOOK_FIRST); + ap_hook_auth_checker(Auth_browserid_check_auth, NULL, NULL, APR_HOOK_FIRST); + ap_hook_fixups(Auth_browserid_fixups, NULL, NULL, APR_HOOK_FIRST); } From 3ac93978bab1c7d919111492b4ea1ffc318e0174 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:25:35 -0500 Subject: [PATCH 31/52] use modified style --- astyle.opt | 8 +- mod_auth_browserid.c | 304 +++++++++++++++++++++++-------------------- 2 files changed, 169 insertions(+), 143 deletions(-) diff --git a/astyle.opt b/astyle.opt index 7fc942d..ebb035c 100644 --- a/astyle.opt +++ b/astyle.opt @@ -7,6 +7,12 @@ indent=spaces=4 # Indent goto labels rather than making them flush on the left indent-labels +# no cuddled else's! +break-closing-brackets + +# use brackets for one-line conditionals +add-brackets + # Indent switch cases indent-switches @@ -17,7 +23,7 @@ min-conditional-indent=0 max-instatement-indent=79 # Pad operators -#pad-oper +pad-oper # Tighten parenthesis (unless overridden by other options such as pad-header) unpad-paren diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 50136a7..b3a22e6 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -122,86 +122,86 @@ void SHA1Transform(u_int32_t state[5], const unsigned char buffer[64]) d = state[3]; e = state[4]; /* 4 rounds of 20 operations each. Loop unrolled. */ - R0(a,b,c,d,e, 0); - R0(e,a,b,c,d, 1); - R0(d,e,a,b,c, 2); - R0(c,d,e,a,b, 3); - R0(b,c,d,e,a, 4); - R0(a,b,c,d,e, 5); - R0(e,a,b,c,d, 6); - R0(d,e,a,b,c, 7); - R0(c,d,e,a,b, 8); - R0(b,c,d,e,a, 9); - R0(a,b,c,d,e,10); - R0(e,a,b,c,d,11); - R0(d,e,a,b,c,12); - R0(c,d,e,a,b,13); - R0(b,c,d,e,a,14); - R0(a,b,c,d,e,15); - R1(e,a,b,c,d,16); - R1(d,e,a,b,c,17); - R1(c,d,e,a,b,18); - R1(b,c,d,e,a,19); - R2(a,b,c,d,e,20); - R2(e,a,b,c,d,21); - R2(d,e,a,b,c,22); - R2(c,d,e,a,b,23); - R2(b,c,d,e,a,24); - R2(a,b,c,d,e,25); - R2(e,a,b,c,d,26); - R2(d,e,a,b,c,27); - R2(c,d,e,a,b,28); - R2(b,c,d,e,a,29); - R2(a,b,c,d,e,30); - R2(e,a,b,c,d,31); - R2(d,e,a,b,c,32); - R2(c,d,e,a,b,33); - R2(b,c,d,e,a,34); - R2(a,b,c,d,e,35); - R2(e,a,b,c,d,36); - R2(d,e,a,b,c,37); - R2(c,d,e,a,b,38); - R2(b,c,d,e,a,39); - R3(a,b,c,d,e,40); - R3(e,a,b,c,d,41); - R3(d,e,a,b,c,42); - R3(c,d,e,a,b,43); - R3(b,c,d,e,a,44); - R3(a,b,c,d,e,45); - R3(e,a,b,c,d,46); - R3(d,e,a,b,c,47); - R3(c,d,e,a,b,48); - R3(b,c,d,e,a,49); - R3(a,b,c,d,e,50); - R3(e,a,b,c,d,51); - R3(d,e,a,b,c,52); - R3(c,d,e,a,b,53); - R3(b,c,d,e,a,54); - R3(a,b,c,d,e,55); - R3(e,a,b,c,d,56); - R3(d,e,a,b,c,57); - R3(c,d,e,a,b,58); - R3(b,c,d,e,a,59); - R4(a,b,c,d,e,60); - R4(e,a,b,c,d,61); - R4(d,e,a,b,c,62); - R4(c,d,e,a,b,63); - R4(b,c,d,e,a,64); - R4(a,b,c,d,e,65); - R4(e,a,b,c,d,66); - R4(d,e,a,b,c,67); - R4(c,d,e,a,b,68); - R4(b,c,d,e,a,69); - R4(a,b,c,d,e,70); - R4(e,a,b,c,d,71); - R4(d,e,a,b,c,72); - R4(c,d,e,a,b,73); - R4(b,c,d,e,a,74); - R4(a,b,c,d,e,75); - R4(e,a,b,c,d,76); - R4(d,e,a,b,c,77); - R4(c,d,e,a,b,78); - R4(b,c,d,e,a,79); + R0(a, b, c, d, e, 0); + R0(e, a, b, c, d, 1); + R0(d, e, a, b, c, 2); + R0(c, d, e, a, b, 3); + R0(b, c, d, e, a, 4); + R0(a, b, c, d, e, 5); + R0(e, a, b, c, d, 6); + R0(d, e, a, b, c, 7); + R0(c, d, e, a, b, 8); + R0(b, c, d, e, a, 9); + R0(a, b, c, d, e, 10); + R0(e, a, b, c, d, 11); + R0(d, e, a, b, c, 12); + R0(c, d, e, a, b, 13); + R0(b, c, d, e, a, 14); + R0(a, b, c, d, e, 15); + R1(e, a, b, c, d, 16); + R1(d, e, a, b, c, 17); + R1(c, d, e, a, b, 18); + R1(b, c, d, e, a, 19); + R2(a, b, c, d, e, 20); + R2(e, a, b, c, d, 21); + R2(d, e, a, b, c, 22); + R2(c, d, e, a, b, 23); + R2(b, c, d, e, a, 24); + R2(a, b, c, d, e, 25); + R2(e, a, b, c, d, 26); + R2(d, e, a, b, c, 27); + R2(c, d, e, a, b, 28); + R2(b, c, d, e, a, 29); + R2(a, b, c, d, e, 30); + R2(e, a, b, c, d, 31); + R2(d, e, a, b, c, 32); + R2(c, d, e, a, b, 33); + R2(b, c, d, e, a, 34); + R2(a, b, c, d, e, 35); + R2(e, a, b, c, d, 36); + R2(d, e, a, b, c, 37); + R2(c, d, e, a, b, 38); + R2(b, c, d, e, a, 39); + R3(a, b, c, d, e, 40); + R3(e, a, b, c, d, 41); + R3(d, e, a, b, c, 42); + R3(c, d, e, a, b, 43); + R3(b, c, d, e, a, 44); + R3(a, b, c, d, e, 45); + R3(e, a, b, c, d, 46); + R3(d, e, a, b, c, 47); + R3(c, d, e, a, b, 48); + R3(b, c, d, e, a, 49); + R3(a, b, c, d, e, 50); + R3(e, a, b, c, d, 51); + R3(d, e, a, b, c, 52); + R3(c, d, e, a, b, 53); + R3(b, c, d, e, a, 54); + R3(a, b, c, d, e, 55); + R3(e, a, b, c, d, 56); + R3(d, e, a, b, c, 57); + R3(c, d, e, a, b, 58); + R3(b, c, d, e, a, 59); + R4(a, b, c, d, e, 60); + R4(e, a, b, c, d, 61); + R4(d, e, a, b, c, 62); + R4(c, d, e, a, b, 63); + R4(b, c, d, e, a, 64); + R4(a, b, c, d, e, 65); + R4(e, a, b, c, d, 66); + R4(d, e, a, b, c, 67); + R4(c, d, e, a, b, 68); + R4(b, c, d, e, a, 69); + R4(a, b, c, d, e, 70); + R4(e, a, b, c, d, 71); + R4(d, e, a, b, c, 72); + R4(c, d, e, a, b, 73); + R4(b, c, d, e, a, 74); + R4(a, b, c, d, e, 75); + R4(e, a, b, c, d, 76); + R4(d, e, a, b, c, 77); + R4(c, d, e, a, b, 78); + R4(b, c, d, e, a, 79); /* Add the working vars back into context.state[] */ state[0] += a; state[1] += b; @@ -238,18 +238,22 @@ void SHA1Update(SHA1_CTX* context, const unsigned char* data, u_int32_t len) u_int32_t j; j = context->count[0]; - if ((context->count[0] += len << 3) < j) + if ((context->count[0] += len << 3) < j) { context->count[1]++; - context->count[1] += (len>>29); + } + context->count[1] += (len >> 29); j = (j >> 3) & 63; if ((j + len) > 63) { - memcpy(&context->buffer[j], data, (i = 64-j)); + memcpy(&context->buffer[j], data, (i = 64 - j)); SHA1Transform(context->state, context->buffer); for (; i + 63 < len; i += 64) { SHA1Transform(context->state, &data[i]); } j = 0; - } else i = 0; + } + else { + i = 0; + } memcpy(&context->buffer[j], &data[i], len - i); } @@ -274,16 +278,17 @@ void SHA1Final(unsigned char digest[20], SHA1_CTX* context) u_int32_t t = context->count[i]; int j; - for (j = 0; j < 4; t >>= 8, j++) + for (j = 0; j < 4; t >>= 8, j++) { *--fcp = (unsigned char) t - } + } + } #else for (i = 0; i < 8; i++) { finalcount[i] = (unsigned char)((context->count[(i >= 4 ? 0 : 1)] - >> ((3-(i & 3)) * 8)) & 255); /* Endian independent */ + >> ((3 - (i & 3)) * 8)) & 255); /* Endian independent */ } #endif - c = 0200; + c = 0200; SHA1Update(context, &c, 1); while ((context->count[0] & 504) != 448) { c = 0000; @@ -292,7 +297,7 @@ void SHA1Final(unsigned char digest[20], SHA1_CTX* context) SHA1Update(context, finalcount, 8); /* Should cause a SHA1Transform() */ for (i = 0; i < 20; i++) { digest[i] = (unsigned char) - ((context->state[i>>2] >> ((3-(i & 3)) * 8)) & 255); + ((context->state[i>>2] >> ((3 - (i & 3)) * 8)) & 255); } /* Wipe variables */ memset(context, '\0', sizeof(*context)); @@ -371,7 +376,7 @@ static int Auth_browserid_check_cookie(request_rec *r); static int Auth_browserid_fixups(request_rec *r); static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char *identity); static char * extract_cookie(request_rec *r, const char *szCookie_name); -static void fix_headers_in(request_rec *r,char*szPassword); +static void fix_headers_in(request_rec *r, char*szPassword); static char *generateSignature(request_rec *r, BrowserIDConfigRec *conf, char *userAddress); apr_table_t *parseArgs(request_rec *r, char *argStr); static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf); @@ -405,7 +410,7 @@ static void *create_browserid_config(apr_pool_t *p, char *d) conf->authBasicFix = 0; /* do not fix header for php auth by default */ conf->authoritative = 0; /* not by default */ - conf->cookieName = apr_pstrdup(p,"BrowserID"); + conf->cookieName = apr_pstrdup(p, "BrowserID"); conf->forwardedRequestHeader = NULL; /* pass the authenticated user, signed, as an HTTP header */ conf->logoutPath = NULL; conf->serverSecret = "BrowserIDSecret"; @@ -419,7 +424,7 @@ static void *create_browserid_config(apr_pool_t *p, char *d) * and URL-unescape it. Return the cookie on success, NULL on failure. */ static char * extract_cookie(request_rec *r, const char *szCookie_name) { - char *szRaw_cookie_start=NULL, *szRaw_cookie_end; + char *szRaw_cookie_start = NULL, *szRaw_cookie_end; char *szCookie; /* get cookie string */ char*szRaw_cookie = (char*)apr_table_get(r->headers_in, "Cookie"); @@ -431,11 +436,12 @@ static char * extract_cookie(request_rec *r, const char *szCookie_name) "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); /* search cookie name in cookie string */ - UNLESS(szRaw_cookie =strstr(szRaw_cookie, szCookie_name)) return 0; - szRaw_cookie_start=szRaw_cookie; + UNLESS(szRaw_cookie = strstr(szRaw_cookie, szCookie_name)) return 0; + szRaw_cookie_start = szRaw_cookie; /* search '=' */ UNLESS(szRaw_cookie = strchr(szRaw_cookie, '=')) return 0; - } while (strncmp(szCookie_name,szRaw_cookie_start,szRaw_cookie-szRaw_cookie_start)!=0); + } + while (strncmp(szCookie_name, szRaw_cookie_start, szRaw_cookie - szRaw_cookie_start) != 0); /* skip '=' */ szRaw_cookie++; @@ -444,7 +450,7 @@ static char * extract_cookie(request_rec *r, const char *szCookie_name) UNLESS((szRaw_cookie_end = strchr(szRaw_cookie, ';')) || (szRaw_cookie_end = strchr(szRaw_cookie, '\0'))) return 0; /* dup the value string found in apache pool and set the result pool ptr to szCookie ptr */ - UNLESS(szCookie = apr_pstrndup(r->pool, szRaw_cookie, szRaw_cookie_end-szRaw_cookie)) return 0; + UNLESS(szCookie = apr_pstrndup(r->pool, szRaw_cookie, szRaw_cookie_end - szRaw_cookie)) return 0; /* unescape the value string */ UNLESS(ap_unescape_url(szCookie) == 0) return 0; @@ -499,18 +505,20 @@ static void fix_headers_in(request_rec *r, char*szPassword) (mainly to fix apache logging of php scripts). We only set this if there is no header already present. */ - if (apr_table_get(r->headers_in,"Authorization") == NULL) { + if (apr_table_get(r->headers_in, "Authorization") == NULL) { ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG - "fixing apache Authorization header for this request using user: %s",r->user); + "fixing apache Authorization header for this request using user: %s", r->user); /* concat username and ':' */ - if (szPassword!=NULL) - szUser=(char*)apr_pstrcat(r->pool, r->user, ":", szPassword, NULL); - else - szUser=(char*)apr_pstrcat(r->pool, r->user, ":", NULL); + if (szPassword != NULL) { + szUser = (char*)apr_pstrcat(r->pool, r->user, ":", szPassword, NULL); + } + else { + szUser = (char*)apr_pstrcat(r->pool, r->user, ":", NULL); + } /* alloc memory for the estimated encode size of the username */ - char *szB64_enc_user=(char*)apr_palloc(r->pool,apr_base64_encode_len(strlen(szUser))+1); + char *szB64_enc_user = (char*)apr_palloc(r->pool, apr_base64_encode_len(strlen(szUser)) + 1); UNLESS(szB64_enc_user) { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "memory alloc failed!"); @@ -518,13 +526,13 @@ static void fix_headers_in(request_rec *r, char*szPassword) } /* encode username in base64 format */ - apr_base64_encode(szB64_enc_user,szUser,strlen(szUser)); + apr_base64_encode(szB64_enc_user, szUser, strlen(szUser)); /* set authorization header */ - apr_table_set(r->headers_in,"Authorization", (char*)apr_pstrcat(r->pool,"Basic ",szB64_enc_user,NULL)); + apr_table_set(r->headers_in, "Authorization", (char*)apr_pstrcat(r->pool, "Basic ", szB64_enc_user, NULL)); /* force auth type to basic */ - r->ap_auth_type=apr_pstrdup(r->pool,"Basic"); + r->ap_auth_type = apr_pstrdup(r->pool, "Basic"); } return; @@ -553,19 +561,19 @@ static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCook char *sig = NULL; char *addr = apr_strtok(szCookieValue, "|", &sig); if (!addr) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "malformed BrowserID cookie"); return 1; } char *digest64 = generateSignature(r, conf, addr); - ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, ERRTAG + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "Got cookie: email is %s; expected digest is %s; got digest %s", addr, digest64, sig); /* paranoia indicates that we should use a time-invariant compare here */ if (strcmp(digest64, sig)) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG "invalid BrowserID cookie"); + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "invalid BrowserID cookie"); free(digest64); return 1; } @@ -592,14 +600,15 @@ static int Auth_browserid_check_cookie(request_rec *r) /* get apache config */ conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); - UNLESS(conf->authoritative) - return DECLINED; + UNLESS(conf->authoritative) { + return DECLINED; + } - ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r,ERRTAG + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "AuthType are '%s'", ap_auth_type(r)); - UNLESS(strncmp("BrowserID",ap_auth_type(r),9) == 0) { - ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + UNLESS(strncmp("BrowserID", ap_auth_type(r), 9) == 0) { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "Auth type must be 'BrowserID'"); return HTTP_UNAUTHORIZED; } @@ -613,7 +622,7 @@ static int Auth_browserid_check_cookie(request_rec *r) /* get cookie who are named cookieName */ UNLESS(szCookieValue = extract_cookie(r, conf->cookieName)) { ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG - "BrowserID cookie not found; not authorized! RemoteIP:%s",szRemoteIP); + "BrowserID cookie not found; not authorized! RemoteIP:%s", szRemoteIP); return HTTP_UNAUTHORIZED; } ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG @@ -634,8 +643,9 @@ static int Auth_browserid_check_cookie(request_rec *r) "BrowserID authentication ok"); /* fix http header for php */ - if (conf->authBasicFix) + if (conf->authBasicFix) { fix_headers_in(r, "browserid"); + } /* if all is ok return auth ok */ return OK; @@ -672,14 +682,16 @@ static int Auth_browserid_check_auth(request_rec *r) reqs = reqs_arr ? (require_line *) reqs_arr->elts : NULL; /* decline if no require line found */ - if (!reqs_arr) + if (!reqs_arr) { return DECLINED; + } /* walk through the array to check each require command */ for (x = 0; x < reqs_arr->nelts; x++) { - if (!(reqs[x].method_mask & (AP_METHOD_BIT << r->method_number))) + if (!(reqs[x].method_mask & (AP_METHOD_BIT << r->method_number))) { continue; + } /* get require line */ szRequireLine = reqs[x].requirement; @@ -692,31 +704,32 @@ static int Auth_browserid_check_auth(request_rec *r) "Require Cmd is '%s'", szRequire_cmd); /* if require cmd are valid-user, they are already authenticated than allow and return OK */ - if (!strcmp("valid-user",szRequire_cmd)) { - ap_log_rerror(APLOG_MARK,APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG + if (!strcmp("valid-user", szRequire_cmd)) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "Require Cmd valid-user"); return OK; } /* check the required user */ - else if (!strcmp("user",szRequire_cmd)) { + else if (!strcmp("user", szRequire_cmd)) { szUser = ap_getword_conf(r->pool, &szRequireLine); if (strcmp(r->user, szUser)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, ERRTAG - "user '%s' is not the required user '%s'",r->user, szUser); + "user '%s' is not the required user '%s'", r->user, szUser); return HTTP_FORBIDDEN; } ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG - "user '%s' is authorized",r->user); + "user '%s' is authorized", r->user); return OK; } /* check for users in a file */ - else if (!strcmp("userfile",szRequire_cmd)) { + else if (!strcmp("userfile", szRequire_cmd)) { szFileName = ap_getword_conf(r->pool, &szRequireLine); if (!user_in_file(r, r->user, szFileName)) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, ERRTAG - "user '%s' is not in username list at '%s'", r->user,szFileName); + "user '%s' is not in username list at '%s'", r->user, szFileName); return HTTP_FORBIDDEN; - } else { + } + else { return OK; } } @@ -814,8 +827,9 @@ apr_table_t *parseArgs(request_rec *r, char *argStr) pair ; pair = apr_strtok(NULL, delim, &last)) { for (eq = pair ; *eq ; ++eq) - if (*eq == '+') + if (*eq == '+') { *eq = ' ' ; + } ap_unescape_url(pair) ; eq = strchr(pair, '=') ; @@ -823,7 +837,8 @@ apr_table_t *parseArgs(request_rec *r, char *argStr) if (eq) { *eq++ = 0 ; apr_table_merge(vars, pair, eq) ; - } else { + } + else { apr_table_merge(vars, pair, "") ; } } @@ -880,21 +895,23 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) } ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "In post_read_request; parsed JSON from verification server: %s", assertionResult); - } else { + } + else { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "Unable to verify assertion; communication error with verification server"); return DECLINED; } - } else { + } + else { if (conf->verifyLocally) { - char *hdr=NULL, *payload=NULL, *sig=NULL; + char *hdr = NULL, *payload = NULL, *sig = NULL; char *assertion = apr_pstrdup(r->pool, assertionParsed); - hdr= apr_strtok(assertion, ".", &payload); + hdr = apr_strtok(assertion, ".", &payload); if (hdr) { - payload= apr_strtok(payload, ".", &sig); + payload = apr_strtok(payload, ".", &sig); if (sig) { int len = apr_base64_decode_len(payload); - char *payloadDecode = apr_pcalloc(r->pool, len+1); + char *payloadDecode = apr_pcalloc(r->pool, len + 1); apr_base64_decode(payloadDecode, payload); char errorBuffer[256]; @@ -907,7 +924,8 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) /** XXX more local validation required!!! Check timestamp, audience **/ } } - } else { + } + else { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "Cannot verify BrowserID login: no verification server configured!"); return DECLINED; @@ -931,13 +949,14 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) createSessionCookie(r, conf, foundEmail->u.string); /* redirect to the requested resource */ - apr_table_set(r->headers_out,"Location", returnto); + apr_table_set(r->headers_out, "Location", returnto); return HTTP_TEMPORARY_REDIRECT; } } - } else { - ap_log_rerror(APLOG_MARK,APLOG_ERR|APLOG_NOERRNO, 0, r, ERRTAG + } + else { + ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "In post_read_request; this is a POST - skipping it for now"); } return DECLINED; @@ -957,11 +976,11 @@ static int processLogout(request_rec *r, BrowserIDConfigRec *conf) apr_table_t *vars = parseArgs(r, r->args); const char *returnto = apr_table_get(vars, "returnto") ; if (returnto) { - apr_table_set(r->headers_out,"Location", returnto); + apr_table_set(r->headers_out, "Location", returnto); return HTTP_TEMPORARY_REDIRECT; } } - apr_table_set(r->headers_out,"Location", "/"); + apr_table_set(r->headers_out, "Location", "/"); return HTTP_TEMPORARY_REDIRECT; } @@ -975,14 +994,15 @@ static int processLogout(request_rec *r, BrowserIDConfigRec *conf) */ static int Auth_browserid_fixups(request_rec *r) { - BrowserIDConfigRec *conf=NULL; + BrowserIDConfigRec *conf = NULL; /* get apache config */ conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); if (conf->submitPath && !strcmp(r->uri, conf->submitPath)) { return processAssertionFormSubmit(r, conf); - } else if (conf->logoutPath && !strcmp(r->uri, conf->logoutPath)) { + } + else if (conf->logoutPath && !strcmp(r->uri, conf->logoutPath)) { return processLogout(r, conf); } From 931ff71dc423ae1580c3cb863c440e410e4f2dc5 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:33:09 -0500 Subject: [PATCH 32/52] add pointer style --- astyle.opt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/astyle.opt b/astyle.opt index ebb035c..3f6b85c 100644 --- a/astyle.opt +++ b/astyle.opt @@ -4,6 +4,9 @@ style=k&r # set standard spacing indent=spaces=4 +# set pointer format to existing +align-pointer=name + # Indent goto labels rather than making them flush on the left indent-labels From 253e91348e9aa15e69c42811bdf45dbca3587b72 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:36:13 -0500 Subject: [PATCH 33/52] use modified style --- astyle.opt | 2 +- mod_auth_browserid.c | 56 ++++++++++++++++++++++---------------------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/astyle.opt b/astyle.opt index 3f6b85c..676587a 100644 --- a/astyle.opt +++ b/astyle.opt @@ -4,7 +4,7 @@ style=k&r # set standard spacing indent=spaces=4 -# set pointer format to existing +# set pointer format to existing predominat style align-pointer=name # Indent goto labels rather than making them flush on the left diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index b3a22e6..c1b1f84 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -56,9 +56,9 @@ typedef struct { } SHA1_CTX; void SHA1Transform(u_int32_t state[5], const unsigned char buffer[64]); -void SHA1Init(SHA1_CTX* context); -void SHA1Update(SHA1_CTX* context, const unsigned char* data, u_int32_t len); -void SHA1Final(unsigned char digest[20], SHA1_CTX* context); +void SHA1Init(SHA1_CTX *context); +void SHA1Update(SHA1_CTX *context, const unsigned char *data, u_int32_t len); +void SHA1Final(unsigned char digest[20], SHA1_CTX *context); /* ================ sha1.c ================ */ @@ -113,7 +113,7 @@ void SHA1Transform(u_int32_t state[5], const unsigned char buffer[64]) * And the result is written through. I threw a "const" in, hoping * this will cause a diagnostic. */ - CHAR64LONG16* block = (const CHAR64LONG16*)buffer; + CHAR64LONG16 *block = (const CHAR64LONG16 *)buffer; #endif /* Copy context->state[] to working vars */ a = state[0]; @@ -218,7 +218,7 @@ void SHA1Transform(u_int32_t state[5], const unsigned char buffer[64]) /* SHA1Init - Initialize new context */ -void SHA1Init(SHA1_CTX* context) +void SHA1Init(SHA1_CTX *context) { /* SHA1 initialization constants */ context->state[0] = 0x67452301; @@ -232,7 +232,7 @@ void SHA1Init(SHA1_CTX* context) /* Run your data through this. */ -void SHA1Update(SHA1_CTX* context, const unsigned char* data, u_int32_t len) +void SHA1Update(SHA1_CTX *context, const unsigned char *data, u_int32_t len) { u_int32_t i; u_int32_t j; @@ -260,7 +260,7 @@ void SHA1Update(SHA1_CTX* context, const unsigned char* data, u_int32_t len) /* Add padding and return the message digest. */ -void SHA1Final(unsigned char digest[20], SHA1_CTX* context) +void SHA1Final(unsigned char digest[20], SHA1_CTX *context) { unsigned i; unsigned char finalcount[8]; @@ -280,15 +280,15 @@ void SHA1Final(unsigned char digest[20], SHA1_CTX* context) for (j = 0; j < 4; t >>= 8, j++) { *--fcp = (unsigned char) t - } - } + } + } #else for (i = 0; i < 8; i++) { finalcount[i] = (unsigned char)((context->count[(i >= 4 ? 0 : 1)] >> ((3 - (i & 3)) * 8)) & 255); /* Endian independent */ } #endif - c = 0200; + c = 0200; SHA1Update(context, &c, 1); while ((context->count[0] & 504) != 448) { c = 0000; @@ -375,8 +375,8 @@ static int Auth_browserid_check_auth(request_rec *r); static int Auth_browserid_check_cookie(request_rec *r); static int Auth_browserid_fixups(request_rec *r); static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char *identity); -static char * extract_cookie(request_rec *r, const char *szCookie_name); -static void fix_headers_in(request_rec *r, char*szPassword); +static char *extract_cookie(request_rec *r, const char *szCookie_name); +static void fix_headers_in(request_rec *r, char *szPassword); static char *generateSignature(request_rec *r, BrowserIDConfigRec *conf, char *userAddress); apr_table_t *parseArgs(request_rec *r, char *argStr); static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf); @@ -422,12 +422,12 @@ static void *create_browserid_config(apr_pool_t *p, char *d) /* Look through the 'Cookie' headers for the indicated cookie; extract it * and URL-unescape it. Return the cookie on success, NULL on failure. */ -static char * extract_cookie(request_rec *r, const char *szCookie_name) +static char *extract_cookie(request_rec *r, const char *szCookie_name) { char *szRaw_cookie_start = NULL, *szRaw_cookie_end; char *szCookie; /* get cookie string */ - char*szRaw_cookie = (char*)apr_table_get(r->headers_in, "Cookie"); + char *szRaw_cookie = (char *)apr_table_get(r->headers_in, "Cookie"); UNLESS(szRaw_cookie) return 0; /* loop to search cookie name in cookie header */ @@ -497,7 +497,7 @@ static int user_in_file(request_rec *r, char *username, char *filename) application. e.g. php uses the Authorization header when logging the request in apache and not r->user (like it ought to). It is applied after the request has been authenticated. */ -static void fix_headers_in(request_rec *r, char*szPassword) +static void fix_headers_in(request_rec *r, char *szPassword) { char *szUser = NULL; /* Set an Authorization header in the input request table for php @@ -511,14 +511,14 @@ static void fix_headers_in(request_rec *r, char*szPassword) /* concat username and ':' */ if (szPassword != NULL) { - szUser = (char*)apr_pstrcat(r->pool, r->user, ":", szPassword, NULL); + szUser = (char *)apr_pstrcat(r->pool, r->user, ":", szPassword, NULL); } else { - szUser = (char*)apr_pstrcat(r->pool, r->user, ":", NULL); + szUser = (char *)apr_pstrcat(r->pool, r->user, ":", NULL); } /* alloc memory for the estimated encode size of the username */ - char *szB64_enc_user = (char*)apr_palloc(r->pool, apr_base64_encode_len(strlen(szUser)) + 1); + char *szB64_enc_user = (char *)apr_palloc(r->pool, apr_base64_encode_len(strlen(szUser)) + 1); UNLESS(szB64_enc_user) { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "memory alloc failed!"); @@ -529,7 +529,7 @@ static void fix_headers_in(request_rec *r, char*szPassword) apr_base64_encode(szB64_enc_user, szUser, strlen(szUser)); /* set authorization header */ - apr_table_set(r->headers_in, "Authorization", (char*)apr_pstrcat(r->pool, "Basic ", szB64_enc_user, NULL)); + apr_table_set(r->headers_in, "Authorization", (char *)apr_pstrcat(r->pool, "Basic ", szB64_enc_user, NULL)); /* force auth type to basic */ r->ap_auth_type = apr_pstrdup(r->pool, "Basic"); @@ -544,13 +544,13 @@ static char *generateSignature(request_rec *r, BrowserIDConfigRec *conf, char *u { SHA1_CTX context; SHA1Init(&context); - SHA1Update(&context, (unsigned char*)userAddress, strlen(userAddress)); - SHA1Update(&context, (unsigned char*)conf->serverSecret, strlen(conf->serverSecret)); + SHA1Update(&context, (unsigned char *)userAddress, strlen(userAddress)); + SHA1Update(&context, (unsigned char *)conf->serverSecret, strlen(conf->serverSecret)); unsigned char digest[20]; SHA1Final(digest, &context); char *digest64 = apr_palloc(r->pool, apr_base64_encode_len(20)); - apr_base64_encode(digest64, (char*)digest, 20); + apr_base64_encode(digest64, (char *)digest, 20); return digest64; } @@ -579,7 +579,7 @@ static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCook } /* Cookie is good: set r->user */ - r->user = (char*)addr; + r->user = (char *)addr; return 0; } @@ -816,9 +816,9 @@ static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, cha /* Parse x-www-url-formencoded args */ apr_table_t *parseArgs(request_rec *r, char *argStr) { - char* pair ; - char* last = NULL ; - char* eq ; + char *pair ; + char *last = NULL ; + char *eq ; apr_table_t *vars = apr_table_make(r->pool, 10) ; char *delim = "&"; @@ -884,7 +884,7 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) /* verify the assertion... */ yajl_val parsed_result = NULL; if (conf->verificationServerURL) { - char *assertionResult = verifyAssertionRemote(r, conf, (char*)assertionParsed); + char *assertionResult = verifyAssertionRemote(r, conf, (char *)assertionParsed); if (assertionResult) { char errorBuffer[256]; parsed_result = yajl_tree_parse(assertionResult, errorBuffer, 255); @@ -935,7 +935,7 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) char *parsePath[2]; parsePath[0] = "email"; parsePath[1] = NULL; - yajl_val foundEmail = yajl_tree_get(parsed_result, (const char**)parsePath, yajl_t_any); + yajl_val foundEmail = yajl_tree_get(parsed_result, (const char **)parsePath, yajl_t_any); /** XXX if we don't have an email, something went wrong. Should pull the error code properly! This will * probably require refactoring this function since the local path is different. ***/ From 26f3d2685f484bb9fa1133a3d595aa5d39d7b656 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:37:58 -0500 Subject: [PATCH 34/52] modify comment --- mod_auth_browserid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index c1b1f84..e05d119 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -369,7 +369,7 @@ static const command_rec Auth_browserid_cmds[] = { {NULL} }; -/* function declarations */ +/* local function declarations */ static void *create_browserid_config(apr_pool_t *p, char *d); static int Auth_browserid_check_auth(request_rec *r); static int Auth_browserid_check_cookie(request_rec *r); From 48c6e1fc161e3de820d042d9270c3582a652fb12 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:38:45 -0500 Subject: [PATCH 35/52] align function names for ease of reading --- mod_auth_browserid.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index e05d119..d3fc22e 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -371,19 +371,19 @@ static const command_rec Auth_browserid_cmds[] = { /* local function declarations */ static void *create_browserid_config(apr_pool_t *p, char *d); -static int Auth_browserid_check_auth(request_rec *r); -static int Auth_browserid_check_cookie(request_rec *r); -static int Auth_browserid_fixups(request_rec *r); -static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char *identity); +static int Auth_browserid_check_auth(request_rec *r); +static int Auth_browserid_check_cookie(request_rec *r); +static int Auth_browserid_fixups(request_rec *r); +static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char *identity); static char *extract_cookie(request_rec *r, const char *szCookie_name); -static void fix_headers_in(request_rec *r, char *szPassword); +static void fix_headers_in(request_rec *r, char *szPassword); static char *generateSignature(request_rec *r, BrowserIDConfigRec *conf, char *userAddress); apr_table_t *parseArgs(request_rec *r, char *argStr); -static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf); -static int processLogout(request_rec *r, BrowserIDConfigRec *conf); -static void register_hooks(apr_pool_t *p); -static int user_in_file(request_rec *r, char *username, char *filename); -static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCookieValue); +static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf); +static int processLogout(request_rec *r, BrowserIDConfigRec *conf); +static void register_hooks(apr_pool_t *p); +static int user_in_file(request_rec *r, char *username, char *filename); +static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCookieValue); static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, char *assertionText); /* apache module name */ From ac378cee130c348813a977bfb8c5764a2793583a Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:39:40 -0500 Subject: [PATCH 36/52] order function names for ease of reference --- mod_auth_browserid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index d3fc22e..4df8147 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -370,10 +370,10 @@ static const command_rec Auth_browserid_cmds[] = { }; /* local function declarations */ -static void *create_browserid_config(apr_pool_t *p, char *d); static int Auth_browserid_check_auth(request_rec *r); static int Auth_browserid_check_cookie(request_rec *r); static int Auth_browserid_fixups(request_rec *r); +static void *create_browserid_config(apr_pool_t *p, char *d); static void createSessionCookie(request_rec *r, BrowserIDConfigRec *conf, char *identity); static char *extract_cookie(request_rec *r, const char *szCookie_name); static void fix_headers_in(request_rec *r, char *szPassword); From dd6d8ec155abc938db2a7e774641730bf74e0b24 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:41:16 -0500 Subject: [PATCH 37/52] align names for ease of reference --- mod_auth_browserid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index 4df8147..a8f8359 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -312,8 +312,8 @@ void SHA1Final(unsigned char digest[20], SHA1_CTX *context) /* config structure */ typedef struct { - int authBasicFix; - int authoritative; + int authBasicFix; + int authoritative; char *cookieName; char *forwardedRequestHeader; char *logoutPath; From d4afb71fb08c23365d90e061a9c4e87d527a06d0 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:45:01 -0500 Subject: [PATCH 38/52] standardize semi after no space --- mod_auth_browserid.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index a8f8359..c0dab6f 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -816,30 +816,32 @@ static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, cha /* Parse x-www-url-formencoded args */ apr_table_t *parseArgs(request_rec *r, char *argStr) { - char *pair ; - char *last = NULL ; - char *eq ; + char *pair; + char *last = NULL; + char *eq; - apr_table_t *vars = apr_table_make(r->pool, 10) ; + apr_table_t *vars = apr_table_make(r->pool, 10); char *delim = "&"; - for (pair = apr_strtok(r->args, delim, &last) ; - pair ; + for (pair = apr_strtok(r->args, delim, &last); + pair; pair = apr_strtok(NULL, delim, &last)) { - for (eq = pair ; *eq ; ++eq) + + for (eq = pair; *eq; ++eq) { if (*eq == '+') { - *eq = ' ' ; + *eq = ' '; } + } - ap_unescape_url(pair) ; - eq = strchr(pair, '=') ; + ap_unescape_url(pair); + eq = strchr(pair, '='); if (eq) { - *eq++ = 0 ; - apr_table_merge(vars, pair, eq) ; + *eq++ = 0; + apr_table_merge(vars, pair, eq); } else { - apr_table_merge(vars, pair, "") ; + apr_table_merge(vars, pair, ""); } } return vars; @@ -870,12 +872,12 @@ static int processAssertionFormSubmit(request_rec *r, BrowserIDConfigRec *conf) if (r->method_number == M_GET) { if (r->args) { if (strlen(r->args) > 16384) { - return HTTP_REQUEST_URI_TOO_LARGE ; + return HTTP_REQUEST_URI_TOO_LARGE; } apr_table_t *vars = parseArgs(r, r->args); - const char *assertionParsed = apr_table_get(vars, "assertion") ; - const char *returnto = apr_table_get(vars, "returnto") ; + const char *assertionParsed = apr_table_get(vars, "assertion"); + const char *returnto = apr_table_get(vars, "returnto"); ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "In post_read_request; parsed assertion as %s", assertionParsed); ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG @@ -970,11 +972,11 @@ static int processLogout(request_rec *r, BrowserIDConfigRec *conf) if (r->args) { if (strlen(r->args) > 16384) { - return HTTP_REQUEST_URI_TOO_LARGE ; + return HTTP_REQUEST_URI_TOO_LARGE; } apr_table_t *vars = parseArgs(r, r->args); - const char *returnto = apr_table_get(vars, "returnto") ; + const char *returnto = apr_table_get(vars, "returnto"); if (returnto) { apr_table_set(r->headers_out, "Location", returnto); return HTTP_TEMPORARY_REDIRECT; From 5ea506a506a373d23db884ae8e4f7528534818f1 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 07:53:32 -0500 Subject: [PATCH 39/52] remove non-standard UNLESS convenience macro --- mod_auth_browserid.c | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index c0dab6f..f055093 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -428,7 +428,9 @@ static char *extract_cookie(request_rec *r, const char *szCookie_name) char *szCookie; /* get cookie string */ char *szRaw_cookie = (char *)apr_table_get(r->headers_in, "Cookie"); - UNLESS(szRaw_cookie) return 0; + if (!szRaw_cookie) { + return 0; + } /* loop to search cookie name in cookie header */ do { @@ -436,10 +438,14 @@ static char *extract_cookie(request_rec *r, const char *szCookie_name) "Checking cookie %s, looking for %s", szRaw_cookie, szCookie_name); /* search cookie name in cookie string */ - UNLESS(szRaw_cookie = strstr(szRaw_cookie, szCookie_name)) return 0; + if (!(szRaw_cookie = strstr(szRaw_cookie, szCookie_name))) { + return 0; + } szRaw_cookie_start = szRaw_cookie; /* search '=' */ - UNLESS(szRaw_cookie = strchr(szRaw_cookie, '=')) return 0; + if (!(szRaw_cookie = strchr(szRaw_cookie, '='))) { + return 0; + } } while (strncmp(szCookie_name, szRaw_cookie_start, szRaw_cookie - szRaw_cookie_start) != 0); @@ -447,12 +453,19 @@ static char *extract_cookie(request_rec *r, const char *szCookie_name) szRaw_cookie++; /* search end of cookie name value: ';' or end of cookie strings */ - UNLESS((szRaw_cookie_end = strchr(szRaw_cookie, ';')) || (szRaw_cookie_end = strchr(szRaw_cookie, '\0'))) return 0; + if (!((szRaw_cookie_end = strchr(szRaw_cookie, ';')) || (szRaw_cookie_end = strchr(szRaw_cookie, '\0')))) { + return 0; + } /* dup the value string found in apache pool and set the result pool ptr to szCookie ptr */ - UNLESS(szCookie = apr_pstrndup(r->pool, szRaw_cookie, szRaw_cookie_end - szRaw_cookie)) return 0; + if (!(szCookie = apr_pstrndup(r->pool, szRaw_cookie, szRaw_cookie_end - szRaw_cookie))) { + return 0; + } + /* unescape the value string */ - UNLESS(ap_unescape_url(szCookie) == 0) return 0; + if (!(ap_unescape_url(szCookie) == 0)) { + return 0; + } ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "finished cookie scan, returning %s", szCookie); @@ -519,7 +532,7 @@ static void fix_headers_in(request_rec *r, char *szPassword) /* alloc memory for the estimated encode size of the username */ char *szB64_enc_user = (char *)apr_palloc(r->pool, apr_base64_encode_len(strlen(szUser)) + 1); - UNLESS(szB64_enc_user) { + if (!szB64_enc_user) { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "memory alloc failed!"); return; @@ -600,27 +613,27 @@ static int Auth_browserid_check_cookie(request_rec *r) /* get apache config */ conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); - UNLESS(conf->authoritative) { + if (!conf->authoritative) { return DECLINED; } ap_log_rerror(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r, ERRTAG "AuthType are '%s'", ap_auth_type(r)); - UNLESS(strncmp("BrowserID", ap_auth_type(r), 9) == 0) { + if (!(strncmp("BrowserID", ap_auth_type(r), 9) == 0)) { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "Auth type must be 'BrowserID'"); return HTTP_UNAUTHORIZED; } - UNLESS(conf->cookieName) { + if (!conf->cookieName) { ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r, ERRTAG "No Auth_browserid_CookieName specified"); return HTTP_UNAUTHORIZED; } /* get cookie who are named cookieName */ - UNLESS(szCookieValue = extract_cookie(r, conf->cookieName)) { + if (!(szCookieValue = extract_cookie(r, conf->cookieName))) { ap_log_rerror(APLOG_MARK, APLOG_INFO | APLOG_NOERRNO, 0, r, ERRTAG "BrowserID cookie not found; not authorized! RemoteIP:%s", szRemoteIP); return HTTP_UNAUTHORIZED; @@ -674,8 +687,9 @@ static int Auth_browserid_check_auth(request_rec *r) conf = ap_get_module_config(r->per_dir_config, &auth_browserid_module); /* check if this module is authoritative */ - UNLESS(conf->authoritative) - return DECLINED; + if (!conf->authoritative) { + return DECLINED; + } /* get require line */ reqs_arr = ap_requires(r); From 29a16a412ee264050772cac9e24b9336d288267b Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 08:45:37 -0500 Subject: [PATCH 40/52] tidy example spacing --- README.md | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index cda488f..4f27ed5 100644 --- a/README.md +++ b/README.md @@ -80,24 +80,26 @@ httpd.conf: LoadModule mod_auth_browserid_module modules/mod_auth_browserid.so - AuthBrowserIDCookieName myauthcookie - AuthBrowserIDSubmitPath "/id_login/submit" - AuthBrowserIDVerificationServerURL "https://browserid.org/verify" + AuthBrowserIDCookieName myauthcookie + AuthBrowserIDSubmitPath "/id_login/submit" + AuthBrowserIDVerificationServerURL "https://browserid.org/verify" - AuthType BrowserID - AuthBrowserIDAuthoritative on - AuthBrowserIDCookieName myauthcookie - AuthBrowserIDVerificationServerURL "https://browserid.org/verify" + AuthType BrowserID + AuthBrowserIDAuthoritative on + AuthBrowserIDCookieName myauthcookie + AuthBrowserIDSecret + AuthBrowserIDVerificationServerURL "https://browserid.org/verify" - # must be set (apache mandatory) but not used by the module - AuthName "My Login" + # must be set (apache mandatory) but not used by the module + AuthName "My Login" - # to redirect unauthorized users to the login page - ErrorDocument 401 "/id_login/browserid_login.php" + # to redirect unauthorized users to the login page + ErrorDocument 401 "/id_login/browserid_login.php" - Require userfile /usr/local/apache2/htdocs/id_demo_users + # file with authorized user names (e-mail addresses) + Require userfile /usr/local/apache2/htdocs/id_demo_users ``` From 8609c791c5848821d8b149f20e4e86679bb318a6 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Thu, 5 Jul 2012 08:50:54 -0500 Subject: [PATCH 41/52] add missing directive --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4f27ed5..943c5c4 100644 --- a/README.md +++ b/README.md @@ -89,12 +89,12 @@ httpd.conf: AuthType BrowserID AuthBrowserIDAuthoritative on AuthBrowserIDCookieName myauthcookie - AuthBrowserIDSecret + AuthBrowserIDSecret aaz5R2w42^24A3uM&75Z822M79xQ82 AuthBrowserIDVerificationServerURL "https://browserid.org/verify" - + # must be set (apache mandatory) but not used by the module AuthName "My Login" - + # to redirect unauthorized users to the login page ErrorDocument 401 "/id_login/browserid_login.php" From ea238d69f0362617af9615a3ff0da21e9a9f1686 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Fri, 6 Jul 2012 06:23:07 -0500 Subject: [PATCH 42/52] start database capability --- mod_auth_browserid.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index f055093..b7e9530 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -21,6 +21,10 @@ * * SHA-1 implementation by Steve Reid, steve@edmweb.com, in * public domain. + * + * Database usage is based on the Apache2 standard module + * "mod_authn_dbd" with directives renamed for this module. + * */ #include @@ -30,6 +34,7 @@ #include "apr_strings.h" #include "apr_uuid.h" #include "apr_tables.h" +#include "apr_dbd.h" /* for SQL DB use */ #include "httpd.h" #include "http_config.h" @@ -505,6 +510,37 @@ static int user_in_file(request_rec *r, char *username, char *filename) return found; } +/** Given a database name and table of usernames, query the + * presence of the username (see mod_dbd) */ +static int user_in_db(request_rec *r, char *username, char *filename) +{ + apr_status_t status; + char l[MAX_STRING_LEN]; + ap_configfile_t *f; + status = ap_pcfg_openfile(&f, r->pool, filename); + if (status != APR_SUCCESS) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, + "Could not open user file: %s", filename); + return 0; + } + + int found = 0; + while (!(ap_cfg_getline(l, MAX_STRING_LEN, f))) { + + /* Skip # or blank lines. */ + if ((l[0] == '#') || (!l[0])) { + continue; + } + + if (!strcmp(username, l)) { + found = 1; + break; + } + } + ap_cfg_closefile(f); + return found; +} + /* function to fix any headers in the input request that may be relied on by an application. e.g. php uses the Authorization header when logging the request From 43d39d1bb6ea1773d7b516cf0b47b9476d8add74 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Fri, 6 Jul 2012 06:24:35 -0500 Subject: [PATCH 43/52] add reference to database capability --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 943c5c4..ab91b8e 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -mod_browserid is a module for Apache 2.0 or later that implements Apache authentication for the BrowserID protocol. +mod_browserid is a module for Apache 2.0 or later that implements Apache authentication for the BrowserID protocol. Building and Installing ======================= @@ -58,6 +58,9 @@ Once authentication is set up, the "require" directive can be used with one of t * `require user `: a specific identity must be presented * `require userfile `: the BrowserID presented by the user must be the newline-separated list of identities found in this file +Instead of a file of authorized identities a database may be used. In that case several other directives must be used along with the `require valid-user` directive: + + NOT YET IMPLEMENTED ------------------- @@ -100,6 +103,7 @@ httpd.conf: # file with authorized user names (e-mail addresses) Require userfile /usr/local/apache2/htdocs/id_demo_users + ``` From f50aa19295311848fcc828bcb48e23bdbd5eda08 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Fri, 6 Jul 2012 06:35:04 -0500 Subject: [PATCH 44/52] update add a git ignore file --- .gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..f31f507 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +NOTES From 668c68a4035df800fa2f41998e0ef1b227a7f2aa Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Sat, 7 Jul 2012 09:02:35 -0500 Subject: [PATCH 45/52] update correct install target command for apxs path --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 38d913b..b499011 100644 --- a/Makefile +++ b/Makefile @@ -18,7 +18,7 @@ all: mod_auth_browserid.la install: mod_auth_browserid.la @echo "-"$*"-" "-"$?"-" "-"$%"-" "-"$@"-" "-"$<"-" - $(MY_APXS) -i $? + $(APXS_PATH) -i $? clean: -rm -f *.o *.lo *.la *.slo From cea3845e5e6cef59364a2ce6225240a9626791a8 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Sat, 7 Jul 2012 09:23:27 -0500 Subject: [PATCH 46/52] update make more robust for using apxs --- Makefile | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b499011..8819076 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,14 @@ CC=gcc + +# if user has not defined the apxs path, try to set +# it here +ifeq ($(APXS_PATH),) + APXS_PATH := $(shell which apxs) +endif + +# check again, abort on error ifeq ($(APXS_PATH),) -APXS_PATH=/usr/sbin/apxs + $(error Cannot find Apache utility program 'apxs') endif MY_LDFLAGS=-lcurl -lyajl From 4c6de4a04124b9abe05df7fd0b0457d4f8d20893 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Sat, 7 Jul 2012 09:26:38 -0500 Subject: [PATCH 47/52] update add info on git commands for my set up --- TOMS-NOTES | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 TOMS-NOTES diff --git a/TOMS-NOTES b/TOMS-NOTES new file mode 100644 index 0000000..884e1cb --- /dev/null +++ b/TOMS-NOTES @@ -0,0 +1,31 @@ +Base dbd use on standard module mod_authn_dbd. + +git hints for local repo clone: + +$ git remote -v +origin ssh://git@github.com/tbrowder/mod_browserid.git (fetch) +origin ssh://git@github.com/tbrowder/mod_browserid.git (push) +upstream git@github.com:mozilla/mod_browserid.git (fetch) +upstream git@github.com:mozilla/mod_browserid.git (push) + + +# update my repo on git hub from the local repo: +$ git push origin master # alias: gip + +# update my local repo from mozilla repo +$ git fetch upstream # alias giu or gif + +From Mozilla's Mike Hanson: +========================== + +If this is new territory for you, check out +https://help.github.com/articles/using-pull-requests - it's a good +explanation of how these things work. + +If you're working on an actual clone of mozilla/modbrowserid, you may +want to back up a step, fork to your own account, and commit your +changes on that repo. Then the github.com "pull request" button does +everything you want, including a nice diff view, comment system, etc. + +[cool--it works] + From 204ec1d34fb4f59cf00206dbf3f6e2cb9493ee15 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Sat, 7 Jul 2012 09:33:42 -0500 Subject: [PATCH 48/52] update remove info on database use for now --- README.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/README.md b/README.md index ab91b8e..0919926 100644 --- a/README.md +++ b/README.md @@ -57,9 +57,6 @@ Once authentication is set up, the "require" directive can be used with one of t * `require valid-user`: a valid BrowserID identity must have been presented * `require user `: a specific identity must be presented * `require userfile `: the BrowserID presented by the user must be the newline-separated list of identities found in this file - -Instead of a file of authorized identities a database may be used. In that case several other directives must be used along with the `require valid-user` directive: - NOT YET IMPLEMENTED ------------------- @@ -140,4 +137,4 @@ httpd.conf: ``` user@site.com otheruser@site.com -``` \ No newline at end of file +``` From 9104a542d0006219788b9ce371aa7a591bb788ad Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Sat, 7 Jul 2012 09:46:22 -0500 Subject: [PATCH 49/52] update remove unused dbd function for now --- mod_auth_browserid.c | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index b7e9530..cb7e477 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -510,38 +510,6 @@ static int user_in_file(request_rec *r, char *username, char *filename) return found; } -/** Given a database name and table of usernames, query the - * presence of the username (see mod_dbd) */ -static int user_in_db(request_rec *r, char *username, char *filename) -{ - apr_status_t status; - char l[MAX_STRING_LEN]; - ap_configfile_t *f; - status = ap_pcfg_openfile(&f, r->pool, filename); - if (status != APR_SUCCESS) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, - "Could not open user file: %s", filename); - return 0; - } - - int found = 0; - while (!(ap_cfg_getline(l, MAX_STRING_LEN, f))) { - - /* Skip # or blank lines. */ - if ((l[0] == '#') || (!l[0])) { - continue; - } - - if (!strcmp(username, l)) { - found = 1; - break; - } - } - ap_cfg_closefile(f); - return found; -} - - /* function to fix any headers in the input request that may be relied on by an application. e.g. php uses the Authorization header when logging the request in apache and not r->user (like it ought to). It is applied after the request From c562f0bd9149ac436494457f63303b715aee98af Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Sat, 7 Jul 2012 09:54:12 -0500 Subject: [PATCH 50/52] update add new ignore exprs --- .gitignore | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index f31f507..6ce9e19 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,13 @@ -NOTES +.libs +*.new +*.orig +my-pull.log +update-vh1.sh +*~ +*.lo +*.slo +*.la +t +tt +ttt +tttt From 5a6697c6600d29d340c5bbde328018a7bce38411 Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Sun, 8 Jul 2012 16:14:44 -0500 Subject: [PATCH 51/52] update expand comment to clarify module naming conventions --- mod_auth_browserid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod_auth_browserid.c b/mod_auth_browserid.c index cb7e477..4176076 100644 --- a/mod_auth_browserid.c +++ b/mod_auth_browserid.c @@ -391,7 +391,7 @@ static int user_in_file(request_rec *r, char *username, char *filename); static int validateCookie(request_rec *r, BrowserIDConfigRec *conf, char *szCookieValue); static char *verifyAssertionRemote(request_rec *r, BrowserIDConfigRec *conf, char *assertionText); -/* apache module name */ +/* apache module name (used in LoadModule) */ module AP_MODULE_DECLARE_DATA auth_browserid_module; /* apache module structure */ From e47585aa4f2138460f0397131a374b1c9a7868fb Mon Sep 17 00:00:00 2001 From: Tom Browder Date: Sun, 8 Jul 2012 16:17:51 -0500 Subject: [PATCH 52/52] update create macro for module name; define module name respecting apsx convention; assure all make targets use the module name explicitly to allow for future use of multiple files for the module's source (and changing the module name for eventual rebranding) --- Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 8819076..6ecaddf 100644 --- a/Makefile +++ b/Makefile @@ -16,9 +16,12 @@ MY_LDFLAGS=-lcurl -lyajl # Note that gcc flags are passed through apxs, so preface with -Wc MY_CFLAGS=-Wc,-I. -Wc,-Wall +# note apsx adds "_module" to the name +MODULE_NAME := auth_browserid + .SUFFIXES: .c .o .la .c.la: - $(APXS_PATH) $(MY_LDFLAGS) $(MY_CFLAGS) -c $< + $(APXS_PATH) $(MY_LDFLAGS) $(MY_CFLAGS) -c $< -n $(MODULE_NAME) .c.o: $(CC) -c $< @@ -26,7 +29,7 @@ all: mod_auth_browserid.la install: mod_auth_browserid.la @echo "-"$*"-" "-"$?"-" "-"$%"-" "-"$@"-" "-"$<"-" - $(APXS_PATH) -i $? + $(APXS_PATH) -i -n $(MODULE_NAME) -a $? clean: -rm -f *.o *.lo *.la *.slo