From 970e6426703f883a35d0d1741bb0a3002de84257 Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Sun, 20 May 2012 18:46:36 -0400 Subject: [PATCH 01/11] PR35 isalnum feedback --- src/mod_auth_cas.c | 12 ++++++++++-- src/mod_auth_cas.h | 1 + tests/mod_auth_cas_test.c | 10 ++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index d4e5358..e272e6a 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -667,7 +667,7 @@ apr_byte_t validCASTicketFormat(const char *ticket) state = postfix; break; case postfix: - if (*ticket != '-' && *ticket != '.' && !isalnum(*ticket)) + if (*ticket != '-' && *ticket != '.' && !cas_isalnum(*ticket)) goto bail; break; default: @@ -1710,13 +1710,21 @@ char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket) return (apr_pstrndup(r->pool, curlBuffer.buf, strlen(curlBuffer.buf))); } +/* locale independent version of isalnum() */ +apr_byte_t cas_isalnum(char c) { + if ((c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z')) + return TRUE; + return FALSE; +} + /* convert a character to a normalized representation, as for using as * an environment variable. Perform the same character transformation * as http2env() from server/util_script.c at e.g. * */ int cas_char_to_env(int c) { - return apr_isalnum(c) ? apr_toupper(c) : '_'; + return cas_isalnum(c) ? apr_toupper(c) : '_'; } /* Compare two strings based on how they would be converted to an diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index 0548af1..103afed 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -169,6 +169,7 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value); char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket); apr_byte_t validCASTicketFormat(const char *ticket); apr_byte_t isValidCASTicket(request_rec *r, cas_cfg *c, char *ticket, char **user, cas_saml_attr **attrs); +apr_byte_t cas_isalnum(char c); int cas_char_to_env(int c); int cas_strnenvcmp(const char *a, const char *b, int len); apr_table_t *cas_scrub_headers(apr_pool_t *p, const char *const attr_prefix, diff --git a/tests/mod_auth_cas_test.c b/tests/mod_auth_cas_test.c index ac2b751..722b43d 100644 --- a/tests/mod_auth_cas_test.c +++ b/tests/mod_auth_cas_test.c @@ -102,6 +102,15 @@ START_TEST(isSSL_test) { } END_TEST + +START_TEST(cas_isalnum_test) { + fail_unless(cas_isalnum('a') == TRUE); + fail_unless(cas_isalnum('A') == TRUE); + fail_unless(cas_isalnum('5') == TRUE); + fail_if(cas_isalnum('^') == TRUE); +} +END_TEST + START_TEST(cas_char_to_env_test) { int i; for (i = 0; i < 255; i++) { @@ -1030,6 +1039,7 @@ Suite *mod_auth_cas_suite(void) { tcase_add_checked_fixture(tc_core, core_setup, core_teardown); tcase_add_test(tc_core, escapeString_test); tcase_add_test(tc_core, isSSL_test); + tcase_add_test(tc_core, cas_isalnum_test); tcase_add_test(tc_core, cas_char_to_env_test); tcase_add_test(tc_core, cas_scrub_headers_test); tcase_add_test(tc_core, cas_strnenvcmp_test); From c37dc5057e53845c18f024a22240d33a0dcf4755 Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Tue, 14 Aug 2012 21:15:53 -0400 Subject: [PATCH 02/11] Refactor configuration parsing, remove obsolete/deprecated parameter. --- README | 8 - src/mod_auth_cas.c | 311 +++++++++++++++++++------------------- src/mod_auth_cas.h | 4 + tests/mod_auth_cas_test.c | 31 ++++ 4 files changed, 188 insertions(+), 166 deletions(-) diff --git a/README b/README index 53b676b..3504244 100644 --- a/README +++ b/README @@ -179,14 +179,6 @@ Default: 9 Description: This directive will set the maximum depth for chained certificate validation. The default (according to OpenSSL documentation) is 9. -Directive: CASAllowWildcardCert -Default: Off -Description: This directive determines whether a wildcard certificate can be trusted - to verify the CAS server. For instance, if the CAS server presents a - certificate for *.example.com and the hostname portion of the CASValidateURL - is 'cas.login.example.com', this directive (if enabled) will accept that - certificate. - Directive: CASCertificatePath Default: /etc/ssl/certs/ Description: The path to the X509 certificate of the Certificate Authority for diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index 27ce044..ff535c9 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -31,6 +31,7 @@ #include #include +#include #include "httpd.h" #include "http_config.h" @@ -213,174 +214,168 @@ void *cas_merge_dir_config(apr_pool_t *pool, void *BASE, void *ADD) return(c); } -const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) -{ - cas_cfg *c = (cas_cfg *) ap_get_module_config(cmd->server->module_config, &auth_cas_module); - apr_finfo_t f; - size_t sz, limit; - int i; - char d; - - /* cases determined from valid_cmds in mod_auth_cas.h - the config at this point is initialized to default values */ - switch((size_t) cmd->info) { - case cmd_version: - i = atoi(value); - if(i > 0) - c->CASVersion = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CAS version (%s) specified", value)); - break; - case cmd_debug: - /* if atoi() is used on value here with AP_INIT_FLAG, it works but results in a compile warning, so we use TAKE1 to avoid it */ - if(apr_strnatcasecmp(value, "On") == 0) - c->CASDebug = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASDebug = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASDebug - must be 'On' or 'Off'")); - break; - case cmd_validate_server: - /* if atoi() is used on value here with AP_INIT_FLAG, it works but results in a compile warning, so we use TAKE1 to avoid it */ - if(apr_strnatcasecmp(value, "On") == 0) - c->CASValidateServer = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASValidateServer = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASValidateServer - must be 'On' or 'Off'")); - break; - case cmd_validate_saml: - if(apr_strnatcasecmp(value, "On") == 0) - c->CASValidateSAML = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASValidateSAML = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASValidateSAML - must be 'On' or 'Off'")); - break; - case cmd_attribute_delimiter: - c->CASAttributeDelimiter = apr_pstrdup(cmd->pool, value); - break; - case cmd_attribute_prefix: - c->CASAttributePrefix = apr_pstrdup(cmd->pool, value); - break; - case cmd_wildcard_cert: - // XXX this feature is broken now - /* if atoi() is used on value here with AP_INIT_FLAG, it works but results in a compile warning, so we use TAKE1 to avoid it */ - if(apr_strnatcasecmp(value, "On") == 0) - c->CASAllowWildcardCert = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASAllowWildcardCert = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASAllowWildcardCert - must be 'On' or 'Off'")); - break; +const char *cas_read_onoff(apr_pool_t *pool, const char *directive, + const char *value, unsigned int *cfg) { + if (apr_strnatcasecmp(value, "On") == 0) + *cfg = TRUE; + else if (apr_strnatcasecmp(value, "Off") == 0) + *cfg = FALSE; + else + return apr_psprintf(pool, + "MOD_AUTH_CAS: Invalid argument to %s (must be" + "\"On\" or \"Off\")", directive); + return NULL; +} - case cmd_ca_path: - if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Could not find Certificate Authority file '%s'", value)); +const char *cas_read_int(apr_pool_t *pool, const char *directive, + const char *value, unsigned int *cfg) { + long l; + char *eptr; - if(f.filetype != APR_REG && f.filetype != APR_DIR) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Certificate Authority file '%s' is not a regular file or directory", value)); - c->CASCertificatePath = apr_pstrdup(cmd->pool, value); - break; - case cmd_validate_depth: - i = atoi(value); - if(i > 0) - c->CASValidateDepth = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASValidateDepth (%s) specified", value)); - break; + errno = 0; + l = strtol(value, &eptr, 10); - case cmd_cookie_path: - /* this is probably redundant since the same check is performed in cas_post_config */ - if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Could not find CASCookiePath '%s'", value)); + if ((errno == ERANGE && (l == LONG_MAX || l == LONG_MIN)) || + (errno != 0 && l == 0) || l < 0 || *eptr != '\0') { + return apr_psprintf(pool, "MOD_AUTH_CAS: Invalid argument to %s (must be " + "greater than zero", directive); + } - if(f.filetype != APR_DIR || value[strlen(value)-1] != '/') - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: CASCookiePath '%s' is not a directory or does not end in a trailing '/'!", value)); + *cfg = (unsigned int) l; - c->CASCookiePath = apr_pstrdup(cmd->pool, value); - break; + return NULL; +} +const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) +{ + cas_cfg *c = (cas_cfg *) + ap_get_module_config(cmd->server->module_config, &auth_cas_module); + apr_finfo_t f; + size_t sz, limit; + char d; + const char *rv; + /* + * cases determined from valid_cmds in mod_auth_cas.h + */ + switch((size_t) cmd->info) { + case cmd_version: + rv = cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASVersion); + if (rv) + return rv; + else if (c->CASVersion != 1 && c->CASVersion != 2) + return(apr_psprintf(cmd->pool, + "MOD_AUTH_CAS: Invalid CAS version (%s) specified", + value)); + break; + case cmd_debug: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASDebug); + case cmd_validate_server: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASValidateServer); + case cmd_validate_saml: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASValidateSAML); + case cmd_attribute_delimiter: + c->CASAttributeDelimiter = apr_pstrdup(cmd->pool, value); + break; + case cmd_attribute_prefix: + c->CASAttributePrefix = apr_pstrdup(cmd->pool, value); + break; + case cmd_ca_path: + if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) + return(apr_psprintf(cmd->pool, + "MOD_AUTH_CAS: Could not find Certificate " + "Authority file '%s'", value)); + + if(f.filetype != APR_REG && f.filetype != APR_DIR) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Certificate " + "Authority file '%s' is not a regular file " + "or directory", value)); + c->CASCertificatePath = apr_pstrdup(cmd->pool, value); + break; + case cmd_validate_depth: + rv = cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASValidateDepth); + if (rv) + return rv; + break; + case cmd_cookie_path: + /* + * this is probably redundant since the same check is performed in + * cas_post_config + */ + if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Could not find " + "CASCookiePath '%s'", value)); + + if(f.filetype != APR_DIR || value[strlen(value)-1] != '/') + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: CASCookiePath '%s' " + "is not a directory or does not end in a " + "trailing '/'!", value)); + c->CASCookiePath = apr_pstrdup(cmd->pool, value); + break; case cmd_loginurl: - if(cas_setURL(cmd->pool, &(c->CASLoginURL), value) != TRUE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Login URL '%s' could not be parsed!", value)); - break; + if(cas_setURL(cmd->pool, &(c->CASLoginURL), value) != TRUE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Login URL '%s' " + "could not be parsed!", value)); + break; case cmd_validateurl: - if(cas_setURL(cmd->pool, &(c->CASValidateURL), value) != TRUE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Validation URL '%s' could not be parsed!", value)); - break; - case cmd_proxyurl: - if(cas_setURL(cmd->pool, &(c->CASProxyValidateURL), value) != TRUE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Proxy Validation URL '%s' could not be parsed!", value)); - break; - case cmd_root_proxied_as: - if(cas_setURL(cmd->pool, &(c->CASRootProxiedAs), value) != TRUE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Root Proxy URL '%s' could not be parsed!", value)); - break; - case cmd_cookie_entropy: - i = atoi(value); - if(i > 0) - c->CASCookieEntropy = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASCookieEntropy (%s) specified - must be numeric", value)); - break; - case cmd_session_timeout: - i = atoi(value); - if(i >= 0) - c->CASTimeout = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASTimeout (%s) specified - must be numeric", value)); - break; - case cmd_idle_timeout: - i = atoi(value); - if(i > 0) - c->CASIdleTimeout = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASIdleTimeout (%s) specified - must be numeric", value)); - break; - - case cmd_cache_interval: - i = atoi(value); - if(i > 0) - c->CASCacheCleanInterval = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASCacheCleanInterval (%s) specified - must be numeric", value)); - break; - case cmd_cookie_domain: - limit = strlen(value); - for(sz = 0; sz < limit; sz++) { - d = value[sz]; - if( (d < '0' || d > '9') && - (d < 'a' || d > 'z') && - (d < 'A' || d > 'Z') && - d != '.' && d != '-') { - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid character (%c) in CASCookieDomain", d)); - } - } - c->CASCookieDomain = apr_pstrdup(cmd->pool, value); - break; - case cmd_cookie_httponly: - if(apr_strnatcasecmp(value, "On") == 0) - c->CASCookieHttpOnly = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASCookieHttpOnly = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASCookieHttpOnly - must be 'On' or 'Off'")); - + if(cas_setURL(cmd->pool, &(c->CASValidateURL), value) != TRUE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Validation URL '%s' " + "could not be parsed!", value)); break; + case cmd_proxyurl: + if(cas_setURL(cmd->pool, &(c->CASProxyValidateURL), value) != TRUE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Proxy Validation URL " + "'%s' could not be parsed!", value)); + break; + case cmd_root_proxied_as: + if(cas_setURL(cmd->pool, &(c->CASRootProxiedAs), value) != TRUE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Root Proxy URL '%s' " + "could not be parsed!", value)); + break; + case cmd_cookie_entropy: + return cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASCookieEntropy); + case cmd_session_timeout: + return cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASTimeout); + case cmd_idle_timeout: + return cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASIdleTimeout); + case cmd_cache_interval: + return cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASCacheCleanInterval); + break; + case cmd_cookie_domain: + limit = strlen(value); + for(sz = 0; sz < limit; sz++) { + d = value[sz]; + if( (d < '0' || d > '9') && + (d < 'a' || d > 'z') && + (d < 'A' || d > 'Z') && + d != '.' && d != '-') { + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid character " + "(%c) in CASCookieDomain", d)); + } + } + c->CASCookieDomain = apr_pstrdup(cmd->pool, value); + break; + case cmd_cookie_httponly: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASCookieHttpOnly); case cmd_sso: - if(apr_strnatcasecmp(value, "On") == 0) - c->CASSSOEnabled = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASSSOEnabled = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASSSOEnabled - must be 'On' or 'Off'")); - break; - default: - /* should not happen */ - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: invalid command '%s'", cmd->directive->directive)); - break; - } - return NULL; + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASSSOEnabled); + default: + /* should not happen */ + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: invalid command '%s'", + cmd->directive->directive)); + break; + } + return NULL; } /* utility functions to set/retrieve values from the configuration */ diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index 103afed..cfed4c9 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -165,6 +165,10 @@ void *cas_create_server_config(apr_pool_t *pool, server_rec *svr); void *cas_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD); void *cas_create_dir_config(apr_pool_t *pool, char *path); void *cas_merge_dir_config(apr_pool_t *pool, void *BASE, void *ADD); +const char *cas_read_onoff(apr_pool_t *pool, const char *directive, + const char *value, unsigned int *cfg); +const char *cas_read_int(apr_pool_t *pool, const char *directive, + const char *value, unsigned int *cfg); const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value); char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket); apr_byte_t validCASTicketFormat(const char *ticket); diff --git a/tests/mod_auth_cas_test.c b/tests/mod_auth_cas_test.c index 2761140..1fbc374 100644 --- a/tests/mod_auth_cas_test.c +++ b/tests/mod_auth_cas_test.c @@ -74,6 +74,35 @@ START_TEST(cas_merge_dir_config_test) { } END_TEST +START_TEST(cas_read_onoff_test) { + unsigned int u = 0; + fail_unless(cas_read_onoff(request->pool, "CASFoo", + "On", &u) == NULL); + fail_unless(u == 1); + + fail_unless(cas_read_onoff(request->pool, "CASFoo", + "Off", &u) == NULL); + fail_unless(u == 0); + + fail_unless(cas_read_onoff(request->pool, "CASFoo", + "Qwe", &u) != NULL); +} +END_TEST + +START_TEST(cas_read_int_test) { + unsigned int i = 0; + fail_unless(cas_read_int(request->pool, "CASFoo", + "1", &i) == NULL); + fail_unless(i == 1); + + fail_unless(cas_read_int(request->pool, "CASFoo", + "Qwe", &i) != NULL); + + fail_unless(cas_read_int(request->pool, "CASFoo", + "12Qwe", &i) != NULL); +} +END_TEST + START_TEST(cas_setURL_test) { const char *url1 = "http://www.example.com/"; const char *url2 = "http://www.example.com:8080/foo.html"; @@ -1061,6 +1090,8 @@ Suite *mod_auth_cas_suite(void) { tcase_add_test(tc_core, normalizeHeaderName_test); tcase_add_test(tc_core, cas_merge_server_config_test); tcase_add_test(tc_core, cas_merge_dir_config_test); + tcase_add_test(tc_core, cas_read_onoff_test); + tcase_add_test(tc_core, cas_read_int_test); tcase_add_test(tc_core, cas_setURL_test); tcase_add_test(tc_core, getCASPath_test); tcase_add_test(tc_core, getCASScope_test); From 3e7b5e096959ad0ef538ac4bad04a77eff6f916a Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Tue, 14 Aug 2012 21:18:12 -0400 Subject: [PATCH 03/11] Remove last remnants of wildcard cert directive. --- src/mod_auth_cas.c | 2 -- src/mod_auth_cas.h | 10 +++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index ff535c9..ca8ec2c 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -83,7 +83,6 @@ void *cas_create_server_config(apr_pool_t *pool, server_rec *svr) c->CASDebug = CAS_DEFAULT_DEBUG; c->CASValidateServer = CAS_DEFAULT_VALIDATE_SERVER; c->CASValidateDepth = CAS_DEFAULT_VALIDATE_DEPTH; - c->CASAllowWildcardCert = CAS_DEFAULT_ALLOW_WILDCARD_CERT; c->CASCertificatePath = CAS_DEFAULT_CA_PATH; c->CASCookiePath = CAS_DEFAULT_COOKIE_PATH; c->CASCookieEntropy = CAS_DEFAULT_COOKIE_ENTROPY; @@ -2325,7 +2324,6 @@ const command_rec cas_cmds [] = { /* ssl related options */ AP_INIT_TAKE1("CASValidateServer", cfg_readCASParameter, (void *) cmd_validate_server, RSRC_CONF, "Require validation of CAS server SSL certificate for successful authentication (On or Off)"), AP_INIT_TAKE1("CASValidateDepth", cfg_readCASParameter, (void *) cmd_validate_depth, RSRC_CONF, "Define the number of chained certificates required for a successful validation"), - AP_INIT_TAKE1("CASAllowWildcardCert", cfg_readCASParameter, (void *) cmd_wildcard_cert, RSRC_CONF, "Allow wildcards in certificates when performing validation (e.g. *.example.com) (On or Off)"), AP_INIT_TAKE1("CASCertificatePath", cfg_readCASParameter, (void *) cmd_ca_path, RSRC_CONF, "Path to the X509 certificate for the CASServer Certificate Authority"), /* pertinent CAS urls */ diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index cfed4c9..d0b6211 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -71,7 +71,6 @@ #define CAS_DEFAULT_ATTRIBUTE_DELIMITER "," #define CAS_DEFAULT_ATTRIBUTE_PREFIX "CAS_" #define CAS_DEFAULT_VALIDATE_DEPTH 9 -#define CAS_DEFAULT_ALLOW_WILDCARD_CERT 0 #define CAS_DEFAULT_CA_PATH "/etc/ssl/certs/" #define CAS_DEFAULT_COOKIE_PATH "/dev/null" #define CAS_DEFAULT_LOGIN_URL NULL @@ -153,10 +152,11 @@ typedef struct cas_curl_buffer { } cas_curl_buffer; typedef enum { - cmd_version, cmd_debug, cmd_validate_server, cmd_validate_depth, cmd_wildcard_cert, - cmd_ca_path, cmd_cookie_path, cmd_loginurl, cmd_validateurl, cmd_proxyurl, cmd_cookie_entropy, - cmd_session_timeout, cmd_idle_timeout, cmd_cache_interval, cmd_cookie_domain, cmd_cookie_httponly, - cmd_sso, cmd_validate_saml, cmd_attribute_delimiter, cmd_attribute_prefix, cmd_root_proxied_as + cmd_version, cmd_debug, cmd_validate_server, cmd_validate_depth, + cmd_ca_path, cmd_cookie_path, cmd_loginurl, cmd_validateurl, cmd_proxyurl, + cmd_cookie_entropy, cmd_session_timeout, cmd_idle_timeout, cmd_cache_interval, + cmd_cookie_domain, cmd_cookie_httponly, cmd_sso, cmd_validate_saml, + cmd_attribute_delimiter, cmd_attribute_prefix, cmd_root_proxied_as } valid_cmds; module AP_MODULE_DECLARE_DATA auth_cas_module; From cd5bb67f2b59413ce82e9238bbf62e54d87620aa Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Tue, 14 Aug 2012 22:02:01 -0400 Subject: [PATCH 04/11] Fix botched merge of cmd_authoritative directive. --- src/mod_auth_cas.c | 6 ++++-- src/mod_auth_cas.h | 5 +++-- tests/mod_auth_cas_test.c | 15 ++++++++------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index c25a59d..ffe4754 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -118,7 +118,6 @@ void *cas_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) c->CASDebug = (add->CASDebug != CAS_DEFAULT_DEBUG ? add->CASDebug : base->CASDebug); c->CASValidateServer = (add->CASValidateServer != CAS_DEFAULT_VALIDATE_SERVER ? add->CASValidateServer : base->CASValidateServer); c->CASValidateDepth = (add->CASValidateDepth != CAS_DEFAULT_VALIDATE_DEPTH ? add->CASValidateDepth : base->CASValidateDepth); - c->CASAllowWildcardCert = (add->CASAllowWildcardCert != CAS_DEFAULT_ALLOW_WILDCARD_CERT ? add->CASAllowWildcardCert : base->CASAllowWildcardCert); c->CASCertificatePath = (apr_strnatcasecmp(add->CASCertificatePath,CAS_DEFAULT_CA_PATH) != 0 ? add->CASCertificatePath : base->CASCertificatePath); c->CASCookiePath = (apr_strnatcasecmp(add->CASCookiePath, CAS_DEFAULT_COOKIE_PATH) != 0 ? add->CASCookiePath : base->CASCookiePath); c->CASCookieEntropy = (add->CASCookieEntropy != CAS_DEFAULT_COOKIE_ENTROPY ? add->CASCookieEntropy : base->CASCookieEntropy); @@ -367,9 +366,12 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) case cmd_cookie_httponly: return cas_read_onoff(cmd->pool, cmd->directive->directive, value, &c->CASCookieHttpOnly); - case cmd_sso: + case cmd_sso: return cas_read_onoff(cmd->pool, cmd->directive->directive, value, &c->CASSSOEnabled); + case cmd_authoritative: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASAuthoritative); default: /* should not happen */ return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: invalid command '%s'", diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index 1c6cc3a..948660d 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -162,8 +162,9 @@ typedef enum { cmd_version, cmd_debug, cmd_validate_server, cmd_validate_depth, cmd_ca_path, cmd_cookie_path, cmd_loginurl, cmd_validateurl, cmd_proxyurl, cmd_cookie_entropy, cmd_session_timeout, cmd_idle_timeout, cmd_cache_interval, - cmd_cookie_domain, cmd_cookie_httponly, cmd_sso, cmd_validate_saml, - cmd_attribute_delimiter, cmd_attribute_prefix, cmd_root_proxied_as + cmd_cookie_domain, cmd_cookie_httponly, cmd_sso, cmd_authoritative, + cmd_validate_saml, cmd_attribute_delimiter, cmd_attribute_prefix, + cmd_root_proxied_as } valid_cmds; module AP_MODULE_DECLARE_DATA auth_cas_module; diff --git a/tests/mod_auth_cas_test.c b/tests/mod_auth_cas_test.c index 07a7388..2975f19 100644 --- a/tests/mod_auth_cas_test.c +++ b/tests/mod_auth_cas_test.c @@ -934,11 +934,11 @@ START_TEST(cas_attribute_authz_test) { * apply different combinations of them in the tests which * follow. */ r = &(require_line_array[0]); - r->method_mask = AP_METHOD_BIT; + r->method_mask = AP_METHOD_BIT << M_POST; r->requirement = apr_pstrdup(pool, "cas-attribute hopefully:fail"); r = &(require_line_array[1]); - r->method_mask = AP_METHOD_BIT; + r->method_mask = AP_METHOD_BIT << M_POST; r->requirement = apr_pstrdup(pool, "cas-attribute should:succeed"); /* When mod_auth_cas is authoritative, an attribute payload which @@ -965,11 +965,11 @@ START_TEST(cas_attribute_authz_test) { c->CASAuthoritative = 0; should_decline2 = cas_authorize_worker(request, attrs, NULL, 0, c); - fail_unless((should_fail == HTTP_UNAUTHORIZED) && - (should_succeed1 == OK) && - (should_succeed2 == OK) && - (should_decline1 == DECLINED) && - (should_decline2 == DECLINED)); + fail_unless(should_fail == HTTP_UNAUTHORIZED); + fail_unless(should_succeed1 == OK); + fail_unless(should_succeed2 == OK); + fail_unless(should_decline1 == DECLINED); + fail_unless(should_decline2 == DECLINED); } END_TEST @@ -1112,6 +1112,7 @@ void core_setup(void) { apr_pool_create(&pool, NULL); request->pool = pool; /* set up the request */ + request->method_number = M_POST; request->headers_in = apr_table_make(request->pool, 0); request->headers_out = apr_table_make(request->pool, 0); request->err_headers_out = apr_table_make(request->pool, 0); From 71146c56175c66258e567611666b9eee2d10531c Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Sun, 20 May 2012 18:46:36 -0400 Subject: [PATCH 05/11] PR35 isalnum feedback --- src/mod_auth_cas.c | 12 ++++++++++-- src/mod_auth_cas.h | 1 + tests/mod_auth_cas_test.c | 10 ++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index 03279e3..30e2f48 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -685,7 +685,7 @@ apr_byte_t validCASTicketFormat(const char *ticket) state = postfix; break; case postfix: - if (*ticket != '-' && *ticket != '.' && !isalnum(*ticket)) + if (*ticket != '-' && *ticket != '.' && !cas_isalnum(*ticket)) goto bail; break; default: @@ -1735,13 +1735,21 @@ char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket) return rv; } +/* locale independent version of isalnum() */ +apr_byte_t cas_isalnum(char c) { + if ((c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z')) + return TRUE; + return FALSE; +} + /* convert a character to a normalized representation, as for using as * an environment variable. Perform the same character transformation * as http2env() from server/util_script.c at e.g. * */ int cas_char_to_env(int c) { - return apr_isalnum(c) ? apr_toupper(c) : '_'; + return cas_isalnum(c) ? apr_toupper(c) : '_'; } /* Compare two strings based on how they would be converted to an diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index bf5ccf8..88b0c02 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -177,6 +177,7 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value); char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket); apr_byte_t validCASTicketFormat(const char *ticket); apr_byte_t isValidCASTicket(request_rec *r, cas_cfg *c, char *ticket, char **user, cas_saml_attr **attrs); +apr_byte_t cas_isalnum(char c); int cas_char_to_env(int c); int cas_strnenvcmp(const char *a, const char *b, int len); apr_table_t *cas_scrub_headers(apr_pool_t *p, const char *const attr_prefix, diff --git a/tests/mod_auth_cas_test.c b/tests/mod_auth_cas_test.c index cc37429..cc8e160 100644 --- a/tests/mod_auth_cas_test.c +++ b/tests/mod_auth_cas_test.c @@ -112,6 +112,15 @@ START_TEST(isSSL_test) { } END_TEST + +START_TEST(cas_isalnum_test) { + fail_unless(cas_isalnum('a') == TRUE); + fail_unless(cas_isalnum('A') == TRUE); + fail_unless(cas_isalnum('5') == TRUE); + fail_if(cas_isalnum('^') == TRUE); +} +END_TEST + START_TEST(cas_char_to_env_test) { int i; for (i = 0; i < 255; i++) { @@ -1181,6 +1190,7 @@ Suite *mod_auth_cas_suite(void) { tcase_add_checked_fixture(tc_core, core_setup, core_teardown); tcase_add_test(tc_core, escapeString_test); tcase_add_test(tc_core, isSSL_test); + tcase_add_test(tc_core, cas_isalnum_test); tcase_add_test(tc_core, cas_char_to_env_test); tcase_add_test(tc_core, cas_scrub_headers_test); tcase_add_test(tc_core, cas_strnenvcmp_test); From ba2ec8a3223ae977401adaab9979d7141cb4ae72 Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Tue, 14 Aug 2012 21:15:53 -0400 Subject: [PATCH 06/11] Refactor configuration parsing, remove obsolete/deprecated parameter. Remove last remnants of wildcard cert directive. --- README | 8 - src/mod_auth_cas.c | 321 ++++++++++++++++++-------------------- src/mod_auth_cas.h | 15 +- tests/mod_auth_cas_test.c | 31 ++++ 4 files changed, 193 insertions(+), 182 deletions(-) diff --git a/README b/README index 068873f..999ad8b 100644 --- a/README +++ b/README @@ -230,14 +230,6 @@ Default: 9 Description: This directive will set the maximum depth for chained certificate validation. The default (according to OpenSSL documentation) is 9. -Directive: CASAllowWildcardCert -Default: Off -Description: This directive determines whether a wildcard certificate can be trusted - to verify the CAS server. For instance, if the CAS server presents a - certificate for *.example.com and the hostname portion of the CASValidateURL - is 'cas.login.example.com', this directive (if enabled) will accept that - certificate. - Directive: CASCertificatePath Default: /etc/ssl/certs/ Description: The path to the X509 certificate of the Certificate Authority for diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index 30e2f48..c19761e 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -31,6 +31,7 @@ #include #include +#include #include "httpd.h" #include "http_config.h" @@ -83,7 +84,6 @@ void *cas_create_server_config(apr_pool_t *pool, server_rec *svr) c->CASDebug = CAS_DEFAULT_DEBUG; c->CASValidateServer = CAS_DEFAULT_VALIDATE_SERVER; c->CASValidateDepth = CAS_DEFAULT_VALIDATE_DEPTH; - c->CASAllowWildcardCert = CAS_DEFAULT_ALLOW_WILDCARD_CERT; c->CASCertificatePath = CAS_DEFAULT_CA_PATH; c->CASCookiePath = CAS_DEFAULT_COOKIE_PATH; c->CASCookieEntropy = CAS_DEFAULT_COOKIE_ENTROPY; @@ -216,182 +216,168 @@ void *cas_merge_dir_config(apr_pool_t *pool, void *BASE, void *ADD) return(c); } -const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) -{ - cas_cfg *c = (cas_cfg *) ap_get_module_config(cmd->server->module_config, &auth_cas_module); - apr_finfo_t f; - size_t sz, limit; - int i; - char d; - - /* cases determined from valid_cmds in mod_auth_cas.h - the config at this point is initialized to default values */ - switch((size_t) cmd->info) { - case cmd_version: - i = atoi(value); - if(i > 0) - c->CASVersion = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CAS version (%s) specified", value)); - break; - case cmd_debug: - /* if atoi() is used on value here with AP_INIT_FLAG, it works but results in a compile warning, so we use TAKE1 to avoid it */ - if(apr_strnatcasecmp(value, "On") == 0) - c->CASDebug = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASDebug = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASDebug - must be 'On' or 'Off'")); - break; - case cmd_validate_server: - /* if atoi() is used on value here with AP_INIT_FLAG, it works but results in a compile warning, so we use TAKE1 to avoid it */ - if(apr_strnatcasecmp(value, "On") == 0) - c->CASValidateServer = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASValidateServer = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASValidateServer - must be 'On' or 'Off'")); - break; - case cmd_validate_saml: - if(apr_strnatcasecmp(value, "On") == 0) - c->CASValidateSAML = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASValidateSAML = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASValidateSAML - must be 'On' or 'Off'")); - break; - case cmd_attribute_delimiter: - c->CASAttributeDelimiter = apr_pstrdup(cmd->pool, value); - break; - case cmd_attribute_prefix: - c->CASAttributePrefix = apr_pstrdup(cmd->pool, value); - break; - case cmd_wildcard_cert: - // XXX this feature is broken now - /* if atoi() is used on value here with AP_INIT_FLAG, it works but results in a compile warning, so we use TAKE1 to avoid it */ - if(apr_strnatcasecmp(value, "On") == 0) - c->CASAllowWildcardCert = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASAllowWildcardCert = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASAllowWildcardCert - must be 'On' or 'Off'")); - break; +const char *cas_read_onoff(apr_pool_t *pool, const char *directive, + const char *value, unsigned int *cfg) { + if (apr_strnatcasecmp(value, "On") == 0) + *cfg = TRUE; + else if (apr_strnatcasecmp(value, "Off") == 0) + *cfg = FALSE; + else + return apr_psprintf(pool, + "MOD_AUTH_CAS: Invalid argument to %s (must be" + "\"On\" or \"Off\")", directive); + return NULL; +} - case cmd_ca_path: - if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Could not find Certificate Authority file '%s'", value)); +const char *cas_read_int(apr_pool_t *pool, const char *directive, + const char *value, unsigned int *cfg) { + long l; + char *eptr; - if(f.filetype != APR_REG && f.filetype != APR_DIR) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Certificate Authority file '%s' is not a regular file or directory", value)); - c->CASCertificatePath = apr_pstrdup(cmd->pool, value); - break; - case cmd_validate_depth: - i = atoi(value); - if(i > 0) - c->CASValidateDepth = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASValidateDepth (%s) specified", value)); - break; + errno = 0; + l = strtol(value, &eptr, 10); - case cmd_cookie_path: - /* this is probably redundant since the same check is performed in cas_post_config */ - if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Could not find CASCookiePath '%s'", value)); + if ((errno == ERANGE && (l == LONG_MAX || l == LONG_MIN)) || + (errno != 0 && l == 0) || l < 0 || *eptr != '\0') { + return apr_psprintf(pool, "MOD_AUTH_CAS: Invalid argument to %s (must be " + "greater than zero", directive); + } - if(f.filetype != APR_DIR || value[strlen(value)-1] != '/') - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: CASCookiePath '%s' is not a directory or does not end in a trailing '/'!", value)); + *cfg = (unsigned int) l; - c->CASCookiePath = apr_pstrdup(cmd->pool, value); - break; + return NULL; +} +const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) +{ + cas_cfg *c = (cas_cfg *) + ap_get_module_config(cmd->server->module_config, &auth_cas_module); + apr_finfo_t f; + size_t sz, limit; + char d; + const char *rv; + /* + * cases determined from valid_cmds in mod_auth_cas.h + */ + switch((size_t) cmd->info) { + case cmd_version: + rv = cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASVersion); + if (rv) + return rv; + else if (c->CASVersion != 1 && c->CASVersion != 2) + return(apr_psprintf(cmd->pool, + "MOD_AUTH_CAS: Invalid CAS version (%s) specified", + value)); + break; + case cmd_debug: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASDebug); + case cmd_validate_server: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASValidateServer); + case cmd_validate_saml: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASValidateSAML); + case cmd_attribute_delimiter: + c->CASAttributeDelimiter = apr_pstrdup(cmd->pool, value); + break; + case cmd_attribute_prefix: + c->CASAttributePrefix = apr_pstrdup(cmd->pool, value); + break; + case cmd_ca_path: + if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) + return(apr_psprintf(cmd->pool, + "MOD_AUTH_CAS: Could not find Certificate " + "Authority file '%s'", value)); + + if(f.filetype != APR_REG && f.filetype != APR_DIR) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Certificate " + "Authority file '%s' is not a regular file " + "or directory", value)); + c->CASCertificatePath = apr_pstrdup(cmd->pool, value); + break; + case cmd_validate_depth: + rv = cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASValidateDepth); + if (rv) + return rv; + break; + case cmd_cookie_path: + /* + * this is probably redundant since the same check is performed in + * cas_post_config + */ + if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Could not find " + "CASCookiePath '%s'", value)); + + if(f.filetype != APR_DIR || value[strlen(value)-1] != '/') + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: CASCookiePath '%s' " + "is not a directory or does not end in a " + "trailing '/'!", value)); + c->CASCookiePath = apr_pstrdup(cmd->pool, value); + break; case cmd_loginurl: - if(cas_setURL(cmd->pool, &(c->CASLoginURL), value) != TRUE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Login URL '%s' could not be parsed!", value)); - break; + if(cas_setURL(cmd->pool, &(c->CASLoginURL), value) != TRUE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Login URL '%s' " + "could not be parsed!", value)); + break; case cmd_validateurl: - if(cas_setURL(cmd->pool, &(c->CASValidateURL), value) != TRUE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Validation URL '%s' could not be parsed!", value)); - break; - case cmd_proxyurl: - if(cas_setURL(cmd->pool, &(c->CASProxyValidateURL), value) != TRUE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Proxy Validation URL '%s' could not be parsed!", value)); - break; - case cmd_root_proxied_as: - if(cas_setURL(cmd->pool, &(c->CASRootProxiedAs), value) != TRUE) - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Root Proxy URL '%s' could not be parsed!", value)); - break; - case cmd_cookie_entropy: - i = atoi(value); - if(i > 0) - c->CASCookieEntropy = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASCookieEntropy (%s) specified - must be numeric", value)); - break; - case cmd_session_timeout: - i = atoi(value); - if(i >= 0) - c->CASTimeout = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASTimeout (%s) specified - must be numeric", value)); - break; - case cmd_idle_timeout: - i = atoi(value); - if(i > 0) - c->CASIdleTimeout = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASIdleTimeout (%s) specified - must be numeric", value)); - break; - - case cmd_cache_interval: - i = atoi(value); - if(i > 0) - c->CASCacheCleanInterval = i; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid CASCacheCleanInterval (%s) specified - must be numeric", value)); - break; - case cmd_cookie_domain: - limit = strlen(value); - for(sz = 0; sz < limit; sz++) { - d = value[sz]; - if( (d < '0' || d > '9') && - (d < 'a' || d > 'z') && - (d < 'A' || d > 'Z') && - d != '.' && d != '-') { - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid character (%c) in CASCookieDomain", d)); - } - } - c->CASCookieDomain = apr_pstrdup(cmd->pool, value); - break; - case cmd_cookie_httponly: - if(apr_strnatcasecmp(value, "On") == 0) - c->CASCookieHttpOnly = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASCookieHttpOnly = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASCookieHttpOnly - must be 'On' or 'Off'")); - + if(cas_setURL(cmd->pool, &(c->CASValidateURL), value) != TRUE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Validation URL '%s' " + "could not be parsed!", value)); break; + case cmd_proxyurl: + if(cas_setURL(cmd->pool, &(c->CASProxyValidateURL), value) != TRUE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Proxy Validation URL " + "'%s' could not be parsed!", value)); + break; + case cmd_root_proxied_as: + if(cas_setURL(cmd->pool, &(c->CASRootProxiedAs), value) != TRUE) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Root Proxy URL '%s' " + "could not be parsed!", value)); + break; + case cmd_cookie_entropy: + return cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASCookieEntropy); + case cmd_session_timeout: + return cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASTimeout); + case cmd_idle_timeout: + return cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASIdleTimeout); + case cmd_cache_interval: + return cas_read_int(cmd->pool, cmd->directive->directive, value, + &c->CASCacheCleanInterval); + break; + case cmd_cookie_domain: + limit = strlen(value); + for(sz = 0; sz < limit; sz++) { + d = value[sz]; + if( (d < '0' || d > '9') && + (d < 'a' || d > 'z') && + (d < 'A' || d > 'Z') && + d != '.' && d != '-') { + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid character " + "(%c) in CASCookieDomain", d)); + } + } + c->CASCookieDomain = apr_pstrdup(cmd->pool, value); + break; + case cmd_cookie_httponly: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASCookieHttpOnly); case cmd_sso: - if(apr_strnatcasecmp(value, "On") == 0) - c->CASSSOEnabled = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASSSOEnabled = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASSSOEnabled - must be 'On' or 'Off'")); - break; - case cmd_authoritative: - if(apr_strnatcasecmp(value, "On") == 0) - c->CASAuthoritative = TRUE; - else if(apr_strnatcasecmp(value, "Off") == 0) - c->CASAuthoritative = FALSE; - else - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid argument to CASAuthoritative - must be 'On' or 'Off'")); - break; - default: - /* should not happen */ - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: invalid command '%s'", cmd->directive->directive)); - break; - } - return NULL; + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASSSOEnabled); + default: + /* should not happen */ + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: invalid command '%s'", + cmd->directive->directive)); + break; + } + return NULL; } /* utility functions to set/retrieve values from the configuration */ @@ -2600,7 +2586,6 @@ const command_rec cas_cmds [] = { /* ssl related options */ AP_INIT_TAKE1("CASValidateServer", cfg_readCASParameter, (void *) cmd_validate_server, RSRC_CONF, "Require validation of CAS server SSL certificate for successful authentication (On or Off)"), AP_INIT_TAKE1("CASValidateDepth", cfg_readCASParameter, (void *) cmd_validate_depth, RSRC_CONF, "Define the number of chained certificates required for a successful validation"), - AP_INIT_TAKE1("CASAllowWildcardCert", cfg_readCASParameter, (void *) cmd_wildcard_cert, RSRC_CONF, "Allow wildcards in certificates when performing validation (e.g. *.example.com) (On or Off)"), AP_INIT_TAKE1("CASCertificatePath", cfg_readCASParameter, (void *) cmd_ca_path, RSRC_CONF, "Path to the X509 certificate for the CASServer Certificate Authority"), /* pertinent CAS urls */ diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index 88b0c02..1c6cc3a 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -73,7 +73,6 @@ #define CAS_DEFAULT_ATTRIBUTE_DELIMITER "," #define CAS_DEFAULT_ATTRIBUTE_PREFIX "CAS_" #define CAS_DEFAULT_VALIDATE_DEPTH 9 -#define CAS_DEFAULT_ALLOW_WILDCARD_CERT 0 #define CAS_DEFAULT_CA_PATH "/etc/ssl/certs/" #define CAS_DEFAULT_COOKIE_PATH "/dev/null" #define CAS_DEFAULT_LOGIN_URL NULL @@ -160,11 +159,11 @@ typedef struct cas_curl_buffer { } cas_curl_buffer; typedef enum { - cmd_version, cmd_debug, cmd_validate_server, cmd_validate_depth, cmd_wildcard_cert, - cmd_ca_path, cmd_cookie_path, cmd_loginurl, cmd_validateurl, cmd_proxyurl, cmd_cookie_entropy, - cmd_session_timeout, cmd_idle_timeout, cmd_cache_interval, cmd_cookie_domain, cmd_cookie_httponly, - cmd_sso, cmd_validate_saml, cmd_attribute_delimiter, cmd_attribute_prefix, cmd_root_proxied_as, - cmd_authoritative + cmd_version, cmd_debug, cmd_validate_server, cmd_validate_depth, + cmd_ca_path, cmd_cookie_path, cmd_loginurl, cmd_validateurl, cmd_proxyurl, + cmd_cookie_entropy, cmd_session_timeout, cmd_idle_timeout, cmd_cache_interval, + cmd_cookie_domain, cmd_cookie_httponly, cmd_sso, cmd_validate_saml, + cmd_attribute_delimiter, cmd_attribute_prefix, cmd_root_proxied_as } valid_cmds; module AP_MODULE_DECLARE_DATA auth_cas_module; @@ -173,6 +172,10 @@ void *cas_create_server_config(apr_pool_t *pool, server_rec *svr); void *cas_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD); void *cas_create_dir_config(apr_pool_t *pool, char *path); void *cas_merge_dir_config(apr_pool_t *pool, void *BASE, void *ADD); +const char *cas_read_onoff(apr_pool_t *pool, const char *directive, + const char *value, unsigned int *cfg); +const char *cas_read_int(apr_pool_t *pool, const char *directive, + const char *value, unsigned int *cfg); const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value); char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket); apr_byte_t validCASTicketFormat(const char *ticket); diff --git a/tests/mod_auth_cas_test.c b/tests/mod_auth_cas_test.c index cc8e160..3c9b6fa 100644 --- a/tests/mod_auth_cas_test.c +++ b/tests/mod_auth_cas_test.c @@ -84,6 +84,35 @@ START_TEST(cas_merge_dir_config_test) { } END_TEST +START_TEST(cas_read_onoff_test) { + unsigned int u = 0; + fail_unless(cas_read_onoff(request->pool, "CASFoo", + "On", &u) == NULL); + fail_unless(u == 1); + + fail_unless(cas_read_onoff(request->pool, "CASFoo", + "Off", &u) == NULL); + fail_unless(u == 0); + + fail_unless(cas_read_onoff(request->pool, "CASFoo", + "Qwe", &u) != NULL); +} +END_TEST + +START_TEST(cas_read_int_test) { + unsigned int i = 0; + fail_unless(cas_read_int(request->pool, "CASFoo", + "1", &i) == NULL); + fail_unless(i == 1); + + fail_unless(cas_read_int(request->pool, "CASFoo", + "Qwe", &i) != NULL); + + fail_unless(cas_read_int(request->pool, "CASFoo", + "12Qwe", &i) != NULL); +} +END_TEST + START_TEST(cas_setURL_test) { const char *url1 = "http://www.example.com/"; const char *url2 = "http://www.example.com:8080/foo.html"; @@ -1197,6 +1226,8 @@ Suite *mod_auth_cas_suite(void) { tcase_add_test(tc_core, normalizeHeaderName_test); tcase_add_test(tc_core, cas_merge_server_config_test); tcase_add_test(tc_core, cas_merge_dir_config_test); + tcase_add_test(tc_core, cas_read_onoff_test); + tcase_add_test(tc_core, cas_read_int_test); tcase_add_test(tc_core, cas_setURL_test); tcase_add_test(tc_core, getCASPath_test); tcase_add_test(tc_core, getCASScope_test); From f2c5129afa14d527559771497d5ba5340e404815 Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Tue, 14 Aug 2012 22:02:01 -0400 Subject: [PATCH 07/11] Fix botched merge of cmd_authoritative directive. --- src/mod_auth_cas.c | 6 ++++-- src/mod_auth_cas.h | 5 +++-- tests/mod_auth_cas_test.c | 23 ++++++++++++----------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index c19761e..7beb003 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -119,7 +119,6 @@ void *cas_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) c->CASDebug = (add->CASDebug != CAS_DEFAULT_DEBUG ? add->CASDebug : base->CASDebug); c->CASValidateServer = (add->CASValidateServer != CAS_DEFAULT_VALIDATE_SERVER ? add->CASValidateServer : base->CASValidateServer); c->CASValidateDepth = (add->CASValidateDepth != CAS_DEFAULT_VALIDATE_DEPTH ? add->CASValidateDepth : base->CASValidateDepth); - c->CASAllowWildcardCert = (add->CASAllowWildcardCert != CAS_DEFAULT_ALLOW_WILDCARD_CERT ? add->CASAllowWildcardCert : base->CASAllowWildcardCert); c->CASCertificatePath = (apr_strnatcasecmp(add->CASCertificatePath,CAS_DEFAULT_CA_PATH) != 0 ? add->CASCertificatePath : base->CASCertificatePath); c->CASCookiePath = (apr_strnatcasecmp(add->CASCookiePath, CAS_DEFAULT_COOKIE_PATH) != 0 ? add->CASCookiePath : base->CASCookiePath); c->CASCookieEntropy = (add->CASCookieEntropy != CAS_DEFAULT_COOKIE_ENTROPY ? add->CASCookieEntropy : base->CASCookieEntropy); @@ -368,9 +367,12 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) case cmd_cookie_httponly: return cas_read_onoff(cmd->pool, cmd->directive->directive, value, &c->CASCookieHttpOnly); - case cmd_sso: + case cmd_sso: return cas_read_onoff(cmd->pool, cmd->directive->directive, value, &c->CASSSOEnabled); + case cmd_authoritative: + return cas_read_onoff(cmd->pool, cmd->directive->directive, + value, &c->CASAuthoritative); default: /* should not happen */ return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: invalid command '%s'", diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index 1c6cc3a..948660d 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -162,8 +162,9 @@ typedef enum { cmd_version, cmd_debug, cmd_validate_server, cmd_validate_depth, cmd_ca_path, cmd_cookie_path, cmd_loginurl, cmd_validateurl, cmd_proxyurl, cmd_cookie_entropy, cmd_session_timeout, cmd_idle_timeout, cmd_cache_interval, - cmd_cookie_domain, cmd_cookie_httponly, cmd_sso, cmd_validate_saml, - cmd_attribute_delimiter, cmd_attribute_prefix, cmd_root_proxied_as + cmd_cookie_domain, cmd_cookie_httponly, cmd_sso, cmd_authoritative, + cmd_validate_saml, cmd_attribute_delimiter, cmd_attribute_prefix, + cmd_root_proxied_as } valid_cmds; module AP_MODULE_DECLARE_DATA auth_cas_module; diff --git a/tests/mod_auth_cas_test.c b/tests/mod_auth_cas_test.c index 3c9b6fa..74e6daf 100644 --- a/tests/mod_auth_cas_test.c +++ b/tests/mod_auth_cas_test.c @@ -945,11 +945,11 @@ START_TEST(cas_attribute_authz_test) { * apply different combinations of them in the tests which * follow. */ r = &(require_line_array[0]); - r->method_mask = AP_METHOD_BIT; + r->method_mask = AP_METHOD_BIT << M_POST; r->requirement = apr_pstrdup(pool, "cas-attribute hopefully:fail"); r = &(require_line_array[1]); - r->method_mask = AP_METHOD_BIT; + r->method_mask = AP_METHOD_BIT << M_POST; r->requirement = apr_pstrdup(pool, "cas-attribute should:succeed"); r = &(require_line_array[2]); @@ -1002,15 +1002,15 @@ START_TEST(cas_attribute_authz_test) { request->method = old_method; request->method_number = old_method_number; - fail_unless((should_fail1 == HTTP_UNAUTHORIZED) && - (should_fail2 == HTTP_UNAUTHORIZED) && - (should_succeed1 == OK) && - (should_succeed2 == OK) && - (should_succeed3 == OK) && - (should_succeed4 == OK) && - (should_succeed5 == OK) && - (should_decline1 == DECLINED) && - (should_decline2 == DECLINED)); + fail_unless(should_fail1 == HTTP_UNAUTHORIZED); + fail_unless(should_fail2 == HTTP_UNAUTHORIZED); + fail_unless(should_succeed1 == OK); + fail_unless(should_succeed2 == OK); + fail_unless(should_succeed3 == OK); + fail_unless(should_succeed4 == OK); + fail_unless(should_succeed5 == OK); + fail_unless(should_decline1 == DECLINED); + fail_unless(should_decline2 == DECLINED); } END_TEST @@ -1153,6 +1153,7 @@ void core_setup(void) { apr_pool_create(&pool, NULL); request->pool = pool; /* set up the request */ + request->method_number = M_POST; request->headers_in = apr_table_make(request->pool, 0); request->headers_out = apr_table_make(request->pool, 0); request->err_headers_out = apr_table_make(request->pool, 0); From 07422255ed2bdc3664ed973d542d0424952c7519 Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Wed, 7 Nov 2012 21:11:42 -0500 Subject: [PATCH 08/11] PR47 feedback. --- src/mod_auth_cas.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index 7beb003..5f692d1 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -236,8 +236,7 @@ const char *cas_read_int(apr_pool_t *pool, const char *directive, errno = 0; l = strtol(value, &eptr, 10); - if ((errno == ERANGE && (l == LONG_MAX || l == LONG_MIN)) || - (errno != 0 && l == 0) || l < 0 || *eptr != '\0') { + if (errno != 0 || *eptr != '\0' || l < 0) { return apr_psprintf(pool, "MOD_AUTH_CAS: Invalid argument to %s (must be " "greater than zero", directive); } @@ -249,7 +248,7 @@ const char *cas_read_int(apr_pool_t *pool, const char *directive, const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) { - cas_cfg *c = (cas_cfg *) + cas_cfg *c = ap_get_module_config(cmd->server->module_config, &auth_cas_module); apr_finfo_t f; size_t sz, limit; @@ -285,7 +284,7 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) c->CASAttributePrefix = apr_pstrdup(cmd->pool, value); break; case cmd_ca_path: - if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) + if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) != APR_SUCCESS) return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Could not find Certificate " "Authority file '%s'", value)); @@ -307,7 +306,7 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) * this is probably redundant since the same check is performed in * cas_post_config */ - if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) == APR_INCOMPLETE) + if(apr_stat(&f, value, APR_FINFO_TYPE, cmd->temp_pool) != APR_SUCCESS) return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Could not find " "CASCookiePath '%s'", value)); From 1abb6933ae1004391c7716de7a8391cd7242c9d6 Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Wed, 7 Nov 2012 21:27:44 -0500 Subject: [PATCH 09/11] Fix failing test. --- src/mod_auth_cas.c | 4 +++- tests/mod_auth_cas_test.c | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index 5f692d1..6c875ff 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -2232,7 +2232,9 @@ int cas_authorize(request_rec *r) /* Pulled out from cas_authorize to enable unit-testing */ -int cas_authorize_worker(request_rec *r, const cas_saml_attr *const attrs, const require_line *const reqs, int nelts, const cas_cfg *const c) +int cas_authorize_worker(request_rec *r, const cas_saml_attr *const attrs, + const require_line *const reqs, int nelts, + const cas_cfg *const c) { const int m = r->method_number; const char *token; diff --git a/tests/mod_auth_cas_test.c b/tests/mod_auth_cas_test.c index 74e6daf..257e372 100644 --- a/tests/mod_auth_cas_test.c +++ b/tests/mod_auth_cas_test.c @@ -945,11 +945,11 @@ START_TEST(cas_attribute_authz_test) { * apply different combinations of them in the tests which * follow. */ r = &(require_line_array[0]); - r->method_mask = AP_METHOD_BIT << M_POST; + r->method_mask = AP_METHOD_BIT; r->requirement = apr_pstrdup(pool, "cas-attribute hopefully:fail"); r = &(require_line_array[1]); - r->method_mask = AP_METHOD_BIT << M_POST; + r->method_mask = AP_METHOD_BIT; r->requirement = apr_pstrdup(pool, "cas-attribute should:succeed"); r = &(require_line_array[2]); From ae3f9fa920a613dcdbfd32358aa614159399c1e1 Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Wed, 7 Nov 2012 21:40:44 -0500 Subject: [PATCH 10/11] *Actually* fix the failing test. Refactor valid domain check and adjust cast of cmd_parm info member. --- src/mod_auth_cas.c | 34 ++++++++++++++++++++-------------- src/mod_auth_cas.h | 1 + tests/mod_auth_cas_test.c | 12 ++++++++++-- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index 6c875ff..c61fe74 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -251,13 +251,11 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) cas_cfg *c = ap_get_module_config(cmd->server->module_config, &auth_cas_module); apr_finfo_t f; - size_t sz, limit; - char d; const char *rv; /* * cases determined from valid_cmds in mod_auth_cas.h */ - switch((size_t) cmd->info) { + switch((valid_cmds) cmd->info) { case cmd_version: rv = cas_read_int(cmd->pool, cmd->directive->directive, value, &c->CASVersion); @@ -350,17 +348,9 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) &c->CASCacheCleanInterval); break; case cmd_cookie_domain: - limit = strlen(value); - for(sz = 0; sz < limit; sz++) { - d = value[sz]; - if( (d < '0' || d > '9') && - (d < 'a' || d > 'z') && - (d < 'A' || d > 'Z') && - d != '.' && d != '-') { - return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid character " - "(%c) in CASCookieDomain", d)); - } - } + if (!cas_valid_domain(value)) + return(apr_psprintf(cmd->pool, "MOD_AUTH_CAS: Invalid character in " + "CASCookieDomain")); c->CASCookieDomain = apr_pstrdup(cmd->pool, value); break; case cmd_cookie_httponly: @@ -1730,6 +1720,22 @@ apr_byte_t cas_isalnum(char c) { return FALSE; } +apr_byte_t cas_valid_domain(const char *test) { + char c; + if (!*test) + return FALSE; + + while (*test) { + c = *test++; + if((c < '0' || c > '9') && + (c < 'a' || c > 'z') && + (c < 'A' || c > 'Z') && + c != '.' && c != '-') + return FALSE; + } + return TRUE; +} + /* convert a character to a normalized representation, as for using as * an environment variable. Perform the same character transformation * as http2env() from server/util_script.c at e.g. diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index 948660d..5b1727a 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -182,6 +182,7 @@ char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket); apr_byte_t validCASTicketFormat(const char *ticket); apr_byte_t isValidCASTicket(request_rec *r, cas_cfg *c, char *ticket, char **user, cas_saml_attr **attrs); apr_byte_t cas_isalnum(char c); +apr_byte_t cas_valid_domain(const char *test); int cas_char_to_env(int c); int cas_strnenvcmp(const char *a, const char *b, int len); apr_table_t *cas_scrub_headers(apr_pool_t *p, const char *const attr_prefix, diff --git a/tests/mod_auth_cas_test.c b/tests/mod_auth_cas_test.c index 74e6daf..7da1550 100644 --- a/tests/mod_auth_cas_test.c +++ b/tests/mod_auth_cas_test.c @@ -150,6 +150,13 @@ START_TEST(cas_isalnum_test) { } END_TEST +START_TEST(cas_valid_domain_test) { + fail_unless(cas_valid_domain("example.com") == TRUE); + fail_unless(cas_valid_domain("") == FALSE); + fail_unless(cas_valid_domain("http://www.example.com") == FALSE); +} +END_TEST + START_TEST(cas_char_to_env_test) { int i; for (i = 0; i < 255; i++) { @@ -945,11 +952,11 @@ START_TEST(cas_attribute_authz_test) { * apply different combinations of them in the tests which * follow. */ r = &(require_line_array[0]); - r->method_mask = AP_METHOD_BIT << M_POST; + r->method_mask = AP_METHOD_BIT; r->requirement = apr_pstrdup(pool, "cas-attribute hopefully:fail"); r = &(require_line_array[1]); - r->method_mask = AP_METHOD_BIT << M_POST; + r->method_mask = AP_METHOD_BIT; r->requirement = apr_pstrdup(pool, "cas-attribute should:succeed"); r = &(require_line_array[2]); @@ -1221,6 +1228,7 @@ Suite *mod_auth_cas_suite(void) { tcase_add_test(tc_core, escapeString_test); tcase_add_test(tc_core, isSSL_test); tcase_add_test(tc_core, cas_isalnum_test); + tcase_add_test(tc_core, cas_valid_domain_test); tcase_add_test(tc_core, cas_char_to_env_test); tcase_add_test(tc_core, cas_scrub_headers_test); tcase_add_test(tc_core, cas_strnenvcmp_test); From 336d4a2c656d228ecbb98fad15e9cbb1eb20f2b1 Mon Sep 17 00:00:00 2001 From: Phil Ames Date: Tue, 13 May 2014 20:36:33 -0400 Subject: [PATCH 11/11] Remove CASValidateServer directive --- README | 12 +++++------- src/mod_auth_cas.c | 20 ++++++-------------- src/mod_auth_cas.h | 4 +--- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/README b/README index 999ad8b..47b6141 100644 --- a/README +++ b/README @@ -86,6 +86,11 @@ NEW FEATURES AND FUNCTIONS IN THIS RELEASE * Improved automake suport. +* CASValidateServer functionality has been removed. Users must + either fix their SSL certificate configuration using the + CASCertificatePath directive, or switch to communicating with + the CAS server over standard HTTP. + ==================================================================== BUG FIXES ==================================================================== @@ -218,13 +223,6 @@ Description: Enable or disable debugging mode for troubleshooting. Please note that LogLevel must be set to Debug for the VirtualHost in order for these logs to be visible. -Directive: CASValidateServer -Default: On -Description: If set to 'On', mod_auth_cas will validate that the certificate - presented by the server specified in CASLoginURL is both - signed by the Certificate Authority specified in CASCertificatePath - and that the hostname matches the Common Name of the certificate. - Directive: CASValidateDepth Default: 9 Description: This directive will set the maximum depth for chained certificate diff --git a/src/mod_auth_cas.c b/src/mod_auth_cas.c index c61fe74..cf5926c 100644 --- a/src/mod_auth_cas.c +++ b/src/mod_auth_cas.c @@ -82,7 +82,6 @@ void *cas_create_server_config(apr_pool_t *pool, server_rec *svr) c->merged = FALSE; c->CASVersion = CAS_DEFAULT_VERSION; c->CASDebug = CAS_DEFAULT_DEBUG; - c->CASValidateServer = CAS_DEFAULT_VALIDATE_SERVER; c->CASValidateDepth = CAS_DEFAULT_VALIDATE_DEPTH; c->CASCertificatePath = CAS_DEFAULT_CA_PATH; c->CASCookiePath = CAS_DEFAULT_COOKIE_PATH; @@ -117,7 +116,6 @@ void *cas_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) c->merged = TRUE; c->CASVersion = (add->CASVersion != CAS_DEFAULT_VERSION ? add->CASVersion : base->CASVersion); c->CASDebug = (add->CASDebug != CAS_DEFAULT_DEBUG ? add->CASDebug : base->CASDebug); - c->CASValidateServer = (add->CASValidateServer != CAS_DEFAULT_VALIDATE_SERVER ? add->CASValidateServer : base->CASValidateServer); c->CASValidateDepth = (add->CASValidateDepth != CAS_DEFAULT_VALIDATE_DEPTH ? add->CASValidateDepth : base->CASValidateDepth); c->CASCertificatePath = (apr_strnatcasecmp(add->CASCertificatePath,CAS_DEFAULT_CA_PATH) != 0 ? add->CASCertificatePath : base->CASCertificatePath); c->CASCookiePath = (apr_strnatcasecmp(add->CASCookiePath, CAS_DEFAULT_COOKIE_PATH) != 0 ? add->CASCookiePath : base->CASCookiePath); @@ -269,9 +267,6 @@ const char *cfg_readCASParameter(cmd_parms *cmd, void *cfg, const char *value) case cmd_debug: return cas_read_onoff(cmd->pool, cmd->directive->directive, value, &c->CASDebug); - case cmd_validate_server: - return cas_read_onoff(cmd->pool, cmd->directive->directive, - value, &c->CASValidateServer); case cmd_validate_saml: return cas_read_onoff(cmd->pool, cmd->directive->directive, value, &c->CASValidateSAML); @@ -1602,13 +1597,12 @@ size_t cas_curl_write(const void *ptr, size_t size, size_t nmemb, void *stream) CURLcode cas_curl_ssl_ctx(CURL *curl, void *sslctx, void *parm) { - SSL_CTX *ctx = (SSL_CTX *) sslctx; - cas_cfg *c = (cas_cfg *)parm; + SSL_CTX *ctx = (SSL_CTX *) sslctx; + cas_cfg *c = (cas_cfg *) parm; - if(c->CASValidateServer != FALSE) - SSL_CTX_set_verify_depth(ctx, c->CASValidateDepth); + SSL_CTX_set_verify_depth(ctx, c->CASValidateDepth); - return CURLE_OK; + return CURLE_OK; } char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket) @@ -1653,7 +1647,7 @@ char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket) curl_easy_setopt(curl, CURLOPT_PROTOCOLS, CURLPROTO_HTTP|CURLPROTO_HTTPS); #endif - curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, (c->CASValidateServer != FALSE ? 1L : 0L)); + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 1L); if(apr_stat(&f, c->CASCertificatePath, APR_FINFO_TYPE, r->pool) == APR_INCOMPLETE) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "MOD_AUTH_CAS: Could not load CA certificate: %s", c->CASCertificatePath); @@ -1668,8 +1662,7 @@ char *getResponseFromServer (request_rec *r, cas_cfg *c, char *ticket) goto out; } - curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, (c->CASValidateServer != FALSE ? 2L : 0L)); - + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2L); curl_easy_setopt(curl, CURLOPT_USERAGENT, "mod_auth_cas 1.0.10"); if(c->CASValidateSAML == TRUE) { @@ -2593,7 +2586,6 @@ const command_rec cas_cmds [] = { AP_INIT_TAKE1("CASAttributePrefix", cfg_readCASParameter, (void *) cmd_attribute_prefix, RSRC_CONF, "The prefix to use when setting attributes in the HTTP headers"), /* ssl related options */ - AP_INIT_TAKE1("CASValidateServer", cfg_readCASParameter, (void *) cmd_validate_server, RSRC_CONF, "Require validation of CAS server SSL certificate for successful authentication (On or Off)"), AP_INIT_TAKE1("CASValidateDepth", cfg_readCASParameter, (void *) cmd_validate_depth, RSRC_CONF, "Define the number of chained certificates required for a successful validation"), AP_INIT_TAKE1("CASCertificatePath", cfg_readCASParameter, (void *) cmd_ca_path, RSRC_CONF, "Path to the X509 certificate for the CASServer Certificate Authority"), diff --git a/src/mod_auth_cas.h b/src/mod_auth_cas.h index 5b1727a..25c82b4 100644 --- a/src/mod_auth_cas.h +++ b/src/mod_auth_cas.h @@ -109,7 +109,6 @@ typedef struct cas_cfg { unsigned int merged; unsigned int CASVersion; unsigned int CASDebug; - unsigned int CASValidateServer; unsigned int CASValidateDepth; unsigned int CASAllowWildcardCert; unsigned int CASCacheCleanInterval; @@ -159,12 +158,11 @@ typedef struct cas_curl_buffer { } cas_curl_buffer; typedef enum { - cmd_version, cmd_debug, cmd_validate_server, cmd_validate_depth, + cmd_version, cmd_debug, cmd_validate_depth, cmd_root_proxied_as, cmd_ca_path, cmd_cookie_path, cmd_loginurl, cmd_validateurl, cmd_proxyurl, cmd_cookie_entropy, cmd_session_timeout, cmd_idle_timeout, cmd_cache_interval, cmd_cookie_domain, cmd_cookie_httponly, cmd_sso, cmd_authoritative, cmd_validate_saml, cmd_attribute_delimiter, cmd_attribute_prefix, - cmd_root_proxied_as } valid_cmds; module AP_MODULE_DECLARE_DATA auth_cas_module;