Add MellonRedirectDomains option.

Limit the domains that we will redirect to after login / logout to a
set of trusted domains. By default we only allow redirects to the
current domain.

This change breaks backwards compatibility with any site that relies on
redirects to separate domains.

Fixes #35

README

@@ -531,6 +531,23 @@ MellonPostCount 100
         # sending an ECP PAOS <AuthnRequest> 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]
+
 </Location>
 
 

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);

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;
 }
 

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,

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: