diff --git a/README b/README index 3553275..638329c 100644 --- a/README +++ b/README @@ -531,6 +531,23 @@ MellonPostCount 100 # sending an ECP PAOS message to an ECP client. MellonECPSendIDPList Off + # List of domains that we allow redirects to. + # The special name "[self]" means the domain of the current request. + # The domain names can also use wildcards. + # + # Example: + # * Allow redirects to example.com and all subdomains: + # MellonRedirectDomains example.com *.example.com + # * Allow redirects to the host running mod_auth_mellon, as well as the + # web page at www.example.com: + # MellonRedirectDomains [self] www.example.com + # * Allow redirects to all domains: + # MellonRedirectDomains * + # + # Default: + # MellonRedirectDomains [self] + MellonRedirectDomains [self] + diff --git a/auth_mellon.h b/auth_mellon.h index b73f8fd..11ea1f9 100644 --- a/auth_mellon.h +++ b/auth_mellon.h @@ -244,6 +244,10 @@ typedef struct am_dir_cfg_rec { /* Whether to send an ECP client a list of IdP's */ int ecp_send_idplist; + + /* List of domains we can redirect to. */ + const char * const *redirect_domains; + } am_dir_cfg_rec; /* Bitmask for PAOS service options */ @@ -387,6 +391,7 @@ void am_delete_request_session(request_rec *r, am_cache_entry_t *session); char *am_reconstruct_url(request_rec *r); +int am_validate_redirect_url(request_rec *r, const char *url); int am_check_permissions(request_rec *r, am_cache_entry_t *session); void am_set_cache_control_headers(request_rec *r); int am_read_post_data(request_rec *r, char **data, apr_size_t *length); diff --git a/auth_mellon_config.c b/auth_mellon_config.c index 4fa12a1..51849cf 100644 --- a/auth_mellon_config.c +++ b/auth_mellon_config.c @@ -85,6 +85,9 @@ static const int default_env_vars_index_start = -1; */ static const int default_env_vars_count_in_n = -1; +/* The default list of trusted redirect domains. */ +static const char * const default_redirect_domains[] = { "[self]", NULL }; + /* This function handles configuration directives which set a * multivalued string slot in the module configuration (the destination * strucure is a hash). @@ -895,6 +898,43 @@ static const char *am_set_merge_env_vars(cmd_parms *cmd, return NULL; } +/* Handle MellonRedirectDomains option. + * + * Parameters: + * cmd_parms *cmd The command structure for this configuration + * directive. + * void *struct_ptr Pointer to the current directory configuration. + * NULL if we are not in a directory configuration. + * int argc Number of redirect domains. + * char *const argv[] List of redirect domains. + * + * Returns: + * NULL on success, or errror string on failure. + */ +static const char *am_set_redirect_domains(cmd_parms *cmd, + void *struct_ptr, + int argc, + char *const argv[]) +{ + am_dir_cfg_rec *cfg = (am_dir_cfg_rec *)struct_ptr; + const char **redirect_domains; + int i; + + if (argc < 1) + return apr_psprintf(cmd->pool, "%s takes at least one arguments", + cmd->cmd->name); + + redirect_domains = apr_palloc(cmd->pool, sizeof(const char *) * (argc + 1)); + for (i = 0; i < argc; i++) { + redirect_domains[i] = argv[i]; + } + redirect_domains[argc] = NULL; + + cfg->redirect_domains = redirect_domains; + + return NULL; +} + /* This array contains all the configuration directive which are handled * by auth_mellon. */ @@ -1295,6 +1335,13 @@ const command_rec auth_mellon_commands[] = { OR_AUTHCFG, "Whether to send an ECP client a list of IdP's. Default is off." ), + AP_INIT_TAKE_ARGV( + "MellonRedirectDomains", + am_set_redirect_domains, + NULL, + OR_AUTHCFG, + "List of domains we can redirect to." + ), {NULL} }; @@ -1391,6 +1438,7 @@ void *auth_mellon_dir_config(apr_pool_t *p, char *d) dir->subject_confirmation_data_address_check = inherit_subject_confirmation_data_address_check; dir->do_not_verify_logout_signature = apr_hash_make(p); dir->post_replay = inherit_post_replay; + dir->redirect_domains = default_redirect_domains; dir->ecp_send_idplist = inherit_ecp_send_idplist; @@ -1625,6 +1673,11 @@ void *auth_mellon_dir_merge(apr_pool_t *p, void *base, void *add) new_cfg->ecp_send_idplist = CFG_MERGE(add_cfg, base_cfg, ecp_send_idplist); + new_cfg->redirect_domains = + (add_cfg->redirect_domains != default_redirect_domains ? + add_cfg->redirect_domains : + base_cfg->redirect_domains); + return new_cfg; } diff --git a/auth_mellon_handler.c b/auth_mellon_handler.c index 0113401..7f4b73a 100644 --- a/auth_mellon_handler.c +++ b/auth_mellon_handler.c @@ -837,6 +837,14 @@ static int am_handle_logout_response(request_rec *r, LassoLogout *logout) return rc; } + /* Make sure that it is a valid redirect URL. */ + rc = am_validate_redirect_url(r, return_to); + if (rc != OK) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Invalid target domain in logout response RelayState parameter."); + return rc; + } + apr_table_setn(r->headers_out, "Location", return_to); return HTTP_SEE_OTHER; } @@ -875,6 +883,13 @@ static int am_init_logout_request(request_rec *r, LassoLogout *logout) return HTTP_BAD_REQUEST; } + rc = am_validate_redirect_url(r, return_to); + if (rc != OK) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Invalid target domain in logout request ReturnTo parameter."); + return rc; + } + /* Disable the the local session (in case the IdP doesn't respond). */ mellon_session = am_get_request_session(r); if(mellon_session != NULL) { @@ -1859,6 +1874,13 @@ static int am_handle_reply_common(request_rec *r, LassoLogin *login, return rc; } + rc = am_validate_redirect_url(r, relay_state); + if (rc != OK) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Invalid target domain in logout response RelayState parameter."); + return rc; + } + apr_table_setn(r->headers_out, "Location", relay_state); @@ -2353,6 +2375,7 @@ static int am_handle_repost(request_rec *r) char *output; char *return_url; const char *(*post_mkform)(request_rec *, const char *); + int rc; if (am_cookie_get(r) == NULL) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, @@ -2435,6 +2458,13 @@ static int am_handle_repost(request_rec *r) return HTTP_BAD_REQUEST; } + rc = am_validate_redirect_url(r, return_url); + if (rc != OK) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Invalid target domain in repost request ReturnTo parameter."); + return rc; + } + psf_filename = apr_psprintf(r->pool, "%s/%s", mod_cfg->post_dir, psf_id); post_data = am_getfile(r->pool, r->server, psf_filename); if (post_data == NULL) { @@ -3094,6 +3124,13 @@ static int am_handle_login(request_rec *r) return ret; } + ret = am_validate_redirect_url(r, return_to); + if(ret != OK) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Invalid target domain in login request ReturnTo parameter."); + return ret; + } + idp_param = am_extract_query_parameter(r->pool, r->args, "IdP"); if(idp_param != NULL) { ret = am_urldecode(idp_param); @@ -3213,6 +3250,13 @@ static int am_handle_probe_discovery(request_rec *r) { return HTTP_BAD_REQUEST; } + ret = am_validate_redirect_url(r, return_to); + if (ret != OK) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Invalid target domain in probe discovery return parameter."); + return ret; + } + idp_param = am_extract_query_parameter(r->pool, r->args, "returnIDParam"); if(idp_param == NULL) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, diff --git a/auth_mellon_util.c b/auth_mellon_util.c index c88b844..5d6850e 100644 --- a/auth_mellon_util.c +++ b/auth_mellon_util.c @@ -51,6 +51,98 @@ char *am_reconstruct_url(request_rec *r) return url; } +/* Get the hostname of the current request. + * + * Parameters: + * request_rec *r The current request. + * + * Returns: + * The hostname of the current request. + */ +static const char *am_request_hostname(request_rec *r) +{ + const char *url; + apr_uri_t uri; + int ret; + + url = am_reconstruct_url(r); + + ret = apr_uri_parse(r->pool, url, &uri); + if (ret != APR_SUCCESS) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Failed to parse request URL: %s", url); + return NULL; + } + + if (uri.hostname == NULL) { + /* This shouldn't happen, since the request URL is built with a hostname, + * but log a message to make any debuggin around this code easier. + */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "No hostname in request URL: %s", url); + return NULL; + } + + return uri.hostname; +} + +/* Validate the redirect URL. + * + * Checks that the redirect URL is to a trusted domain & scheme. + * + * Parameters: + * request_rec *r The current request. + * const char *url The redirect URL to validate. + * + * Returns: + * OK if the URL is valid, HTTP_BAD_REQUEST if not. + */ +int am_validate_redirect_url(request_rec *r, const char *url) +{ + am_dir_cfg_rec *cfg = am_get_dir_cfg(r); + apr_uri_t uri; + int ret; + + ret = apr_uri_parse(r->pool, url, &uri); + if (ret != APR_SUCCESS) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Invalid redirect URL: %s", url); + return HTTP_BAD_REQUEST; + } + + /* Sanity check of the scheme of the domain. We only allow http and https. */ + if (uri.scheme) { + if (strcasecmp(uri.scheme, "http") + && strcasecmp(uri.scheme, "https")) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Only http or https scheme allowed in redirect URL: %s (%s)", + url, uri.scheme); + return HTTP_BAD_REQUEST; + } + } + + if (!uri.hostname) { + return OK; /* No hostname to check. */ + } + + for (int i = 0; cfg->redirect_domains[i] != NULL; i++) { + const char *redirect_domain = cfg->redirect_domains[i]; + if (!strcasecmp(redirect_domain, "[self]")) { + if (!strcasecmp(uri.hostname, am_request_hostname(r))) { + return OK; + } + } else if (apr_fnmatch(redirect_domain, uri.hostname, + APR_FNM_PERIOD | APR_FNM_CASE_BLIND) == + APR_SUCCESS) { + return OK; + } + } + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "Untrusted hostname (%s) in redirect URL: %s", + uri.hostname, url); + return HTTP_BAD_REQUEST; +} + /* This function builds an array of regexp backreferences * * Parameters: