From e6414be3e2dc57e66d907fee49f5f63b0ecb7f33 Mon Sep 17 00:00:00 2001 From: Rafal Wijata Date: Tue, 28 Mar 2017 19:06:03 +0200 Subject: [PATCH 1/4] Add support for GoogleAuthCookieDomain GoogleAuthCookiePath GoogleAuthCookieSecure configuration --- googleauth.conf | 12 ++++++++++++ mod_authn_google.c | 23 ++++++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/googleauth.conf b/googleauth.conf index 60999be..343e552 100644 --- a/googleauth.conf +++ b/googleauth.conf @@ -13,6 +13,18 @@ Loadmodule authn_google_module modules/mod_authn_google.so # are to last. Not specifying this disables authentication cookies, meaning # "sessions" will only last about a minute before needed re-authentication. # +# GoogleAuthCookieDomain specifies domain name under witch the cookie is valid. +# It will use current url host if unspecified, which in turn may cause problems +# if protected website redirects to upper or sibling host/domain. +# +# GoogleAuthCookiePath specifies url path under witch the cookie is valid. +# It will use current url path if unspecified, which in turn may cause problems +# if protected website redirects to upper or sibling directories. +# +# GoogleAuthCookieSecure specifies that this cookie should be sent +# only via https connection. Note that most browsers will not accept such +# cookie unless it's set via https as well. +# # EntryWindow specifies the "leeway" in how exact the time has to be for a # proper authentication to occur. "0" would mean it must be exact. "1" # would mean it will except one entry newer or older. (About +/- 30 seconds) diff --git a/mod_authn_google.c b/mod_authn_google.c index c659551..3ef7f6e 100644 --- a/mod_authn_google.c +++ b/mod_authn_google.c @@ -51,7 +51,10 @@ ap_regex_t *cookie_regexp; ap_regex_t *passwd_regexp; typedef struct { char *pwfile; + char *cookieDomain; + char *cookiePath; int cookieLife; + int cookieSecure; int entryWindow; int debugLevel; } authn_google_config_rec; @@ -90,7 +93,10 @@ static void *create_authn_google_dir_config(apr_pool_t *p, char *d) authn_google_config_rec *conf = apr_palloc(p, sizeof(*conf)); conf->pwfile = NULL; /* just to illustrate the default really */ + conf->cookieDomain = NULL; + conf->cookiePath = NULL; conf->cookieLife=0; + conf->cookieSecure=0; conf->entryWindow=0; conf->debugLevel=0; return conf; @@ -116,9 +122,18 @@ static const command_rec authn_google_cmds[] = AP_INIT_TAKE12("GoogleAuthUserPath", set_authn_google_slot, (void *)APR_OFFSETOF(authn_google_config_rec, pwfile), OR_AUTHCFG, "Directory containing Google Authenticator credential files"), + AP_INIT_TAKE1("GoogleAuthCookieDomain", ap_set_string_slot, + (void *)APR_OFFSETOF(authn_google_config_rec, cookieDomain), + OR_AUTHCFG, "Cookie Domain to set(if any)"), + AP_INIT_TAKE1("GoogleAuthCookiePath", ap_set_string_slot, + (void *)APR_OFFSETOF(authn_google_config_rec, cookiePath), + OR_AUTHCFG, "Cookie Path to set(if any)"), AP_INIT_TAKE1("GoogleAuthCookieLife", set_authn_set_int, (void *)APR_OFFSETOF(authn_google_config_rec, cookieLife), OR_AUTHCFG, "Life (in seconds) authentication cookie before revalidation required"), + AP_INIT_TAKE1("GoogleAuthCookieSecure", set_authn_set_int, + (void *)APR_OFFSETOF(authn_google_config_rec, cookieSecure), + OR_AUTHCFG, "Mark cookie secure (https only)"), AP_INIT_TAKE1("GoogleAuthLogLevel", set_authn_set_int, (void *)APR_OFFSETOF(authn_google_config_rec, debugLevel), OR_AUTHCFG, "Verbosity level of debug output (zero=off)"), @@ -339,7 +354,13 @@ static void addCookie(request_rec *r, uint8_t *secret, int secretLen) { unsigned long exp = (apr_time_now() / (1000000) ) + conf->cookieLife; char *h = hash_cookie(r->pool,secret,secretLen,exp); - char * cookie = apr_psprintf(r->pool,"google_authn=%s:%lu:%s",r->user,exp,h); + char * cookie = apr_psprintf(r->pool, + "google_authn=%s:%lu:%s%s%s%s%s%s", + r->user, exp, h, + conf->cookieSecure ? "; Secure" : "", + conf->cookiePath ? "; Path=" : "", conf->cookiePath ? conf->cookiePath : "", + conf->cookieDomain ? "; Domain=" : "", conf->cookieDomain ? conf->cookieDomain : "" + ); if (conf->debugLevel) ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, From 8856360bcec2fa64c0d78917f065a3cfe9c4f171 Mon Sep 17 00:00:00 2001 From: Rafal Wijata Date: Tue, 28 Mar 2017 19:06:33 +0200 Subject: [PATCH 2/4] validate username, fix https://github.com/M1Sports20/google-authenticator-apache-module/issues/7 --- mod_authn_google.c | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/mod_authn_google.c b/mod_authn_google.c index 3ef7f6e..7e04687 100644 --- a/mod_authn_google.c +++ b/mod_authn_google.c @@ -145,6 +145,27 @@ static const command_rec authn_google_cmds[] = module AP_MODULE_DECLARE_DATA authn_google_module; +static int bad_username(const char *username) +{ + for(; *username; ++username) + { + if ( *username <= ' ' //we simply don't like it + || *username == ':' //cookie value separator + || *username == ';' //cookie options separator + // some from https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words + || *username == '/' + || *username == '\\' + || *username == '?' + || *username == '*' + || *username == '|' + || *username == '"' + || *username == '<' + || *username == '>' + ) + return 1; + } + return 0; +} static char * hash_cookie(apr_pool_t *p, uint8_t *secret,int secretLen,unsigned long expires) { unsigned char hash[SHA1_DIGEST_LENGTH]; @@ -209,16 +230,13 @@ if (conf->debugLevel) static uint8_t *getUserSecret(request_rec *r, const char *username, int *secretLen, char **static_pw) { authn_google_config_rec *conf = ap_get_module_config(r->per_dir_config, &authn_google_module); + + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "getUserSecret with username \"%s\"\n",username); + + if (bad_username(username)) return 0L; + char *ga_filename = apr_psprintf(r->pool,"%s/%s",conf->pwfile,username); - char *sharedKey; - -ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, -"getUserSecret with username \"%s\"\n",username); - sharedKey = getSharedKey(r,ga_filename,static_pw); - if (!sharedKey) - return 0L; - - + char *sharedKey = getSharedKey(r,ga_filename,static_pw); if (!sharedKey) return 0L; @@ -472,11 +490,13 @@ if (conf->debugLevel) unsigned char *hash = apr_palloc(r->pool,APR_MD5_DIGESTSIZE); - + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "GetRealmHash called for sharedkey \"%s/%s\"\n", conf->pwfile, user); + + if (bad_username(user)) return AUTH_USER_NOT_FOUND; + ga_filename = apr_psprintf(r->pool,"%s/%s",conf->pwfile,user); - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, -"GetRealmHash called for sharedkey \"%s\"\n",ga_filename); sharedKey = getSharedKey(r,ga_filename,&static_pw); if (!sharedKey) From b734e188719ddb3aa4d27d7e793e666f35ca49f3 Mon Sep 17 00:00:00 2001 From: Rafal Wijata Date: Tue, 28 Mar 2017 19:06:56 +0200 Subject: [PATCH 3/4] put some logs under debugLevel condition --- mod_authn_google.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mod_authn_google.c b/mod_authn_google.c index 7e04687..4feaafc 100644 --- a/mod_authn_google.c +++ b/mod_authn_google.c @@ -185,8 +185,8 @@ static char *getSharedKey(request_rec *r,char *filename, char **static_pw) { authn_google_config_rec *conf = ap_get_module_config(r->per_dir_config, &authn_google_module); status = ap_pcfg_openfile(&f, r->pool, filename); -ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, -"OPENING FILENAME %s",filename); + if (conf->debugLevel) + ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, "OPENING FILENAME %s",filename); if (status != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, @@ -231,7 +231,8 @@ static uint8_t *getUserSecret(request_rec *r, const char *username, int *secretL authn_google_config_rec *conf = ap_get_module_config(r->per_dir_config, &authn_google_module); - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "getUserSecret with username \"%s\"\n",username); + if (conf->debugLevel) + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "getUserSecret with username \"%s\"\n",username); if (bad_username(username)) return 0L; From 801980bc47d938be76e5d779fba7f7a6958645fe Mon Sep 17 00:00:00 2001 From: Rafal Wijata Date: Wed, 29 Mar 2017 08:39:40 +0200 Subject: [PATCH 4/4] validate cookie username as well, fix https://github.com/M1Sports20/google-authenticator-apache-module/issues/7 --- mod_authn_google.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/mod_authn_google.c b/mod_authn_google.c index 4feaafc..90f6dea 100644 --- a/mod_authn_google.c +++ b/mod_authn_google.c @@ -47,8 +47,8 @@ #define DEBUG -ap_regex_t *cookie_regexp; -ap_regex_t *passwd_regexp; +static ap_regex_t *cookie_regexp; +static ap_regex_t *passwd_regexp; typedef struct { char *pwfile; char *cookieDomain; @@ -147,6 +147,10 @@ module AP_MODULE_DECLARE_DATA authn_google_module; static int bad_username(const char *username) { + if (*username == 0) + { + return 1; + } for(; *username; ++username) { if ( *username <= ' ' //we simply don't like it @@ -271,7 +275,7 @@ static int find_cookie(request_rec *r,char **user,uint8_t *secret,int secretLen) ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "Found cookie \"%s\"",cookie); if (!ap_regexec(cookie_regexp, cookie, AP_MAX_REG_MATCH, regmatch, 0)) { - if (user) *user = ap_pregsub(r->pool, "$2", cookie,AP_MAX_REG_MATCH,regmatch); + *user = ap_pregsub(r->pool, "$2", cookie,AP_MAX_REG_MATCH,regmatch); cookie_expire = ap_pregsub(r->pool, "$3", cookie,AP_MAX_REG_MATCH,regmatch); cookie_valid = ap_pregsub(r->pool, "$4", cookie,AP_MAX_REG_MATCH,regmatch); @@ -279,7 +283,7 @@ if (conf->debugLevel) ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "Found cookie Expires \"%s\" Valid \"%s\"",cookie_expire,cookie_valid); - if (cookie_expire && cookie_valid && *user) { + if (cookie_expire && cookie_valid && (!bad_username(*user))) { long unsigned int exp = apr_atoi64(cookie_expire); long unsigned int now = apr_time_now()/1000000; if (exp < now) {