Fix Cross-Site Session Transfer vulnerability

mod_auth_mellon did not verify that the site the session was created
for was the same site as the site the user accessed. This allows an
attacker with access to one web site on a server to use the same
session to get access to a different site running on the same server.

This patch fixes this vulnerability by storing the cookie parameters
used when creating the session in the session, and verifying those
parameters when the session is loaded.

Thanks to François Kooman for reporting this vulnerability.

This vulnerability has been assigned CVE-2017-6807.

NEWS

@@ -1,6 +1,30 @@
 Version 0.13.1
 ---------------------------------------------------------------------------
 
+Security fix:
+
+Fix a cross-site session transfer vulnerability. mod_auth_mellon
+version 0.13.0 and older failed to validate that the session specified
+in the user's session cookie was created for the web site the user
+actually accesses.
+
+If two different web sites are hosted on the same web server, and both
+web sites use mod_auth_mellon for authentication, this vulnerability
+makes it possible for an attacker with access to one of the web sites
+to copy their session cookie to the other web site, and then use the
+same session to get access to the other web site.
+
+Thanks to François Kooman for reporting this vulnerability.
+
+This vulnerability has been assigned CVE-2017-6807.
+
+Note: The fix for this vunlerability makes mod_auth_mellon validate
+that the cookie parameters used when creating the session match the
+cookie parameters that should be used when accessing the current
+page. If you currently use mod_auth_mellon across multiple subdomains,
+you must make sure that you set the `MellonCookie`-option to the same
+value on all domains.
+
 Bug fixes:
 
 * Fix segmentation fault if a (trusted) identity provider returns a

auth_mellon.h

@@ -290,6 +290,7 @@ typedef struct am_cache_env_t {
 
 typedef struct am_cache_entry_t {
     char key[AM_CACHE_KEYSIZE];
+    am_cache_storage_t cookie_token;
     apr_time_t access;
     apr_time_t expires;
     int logged_in;
@@ -373,6 +374,7 @@ void *auth_mellon_server_config(apr_pool_t *p, server_rec *s);
 const char *am_cookie_get(request_rec *r);
 void am_cookie_set(request_rec *r, const char *id);
 void am_cookie_delete(request_rec *r);
+const char *am_cookie_token(request_rec *r);
 
 
 void am_cache_init(am_mod_cfg_rec *mod_cfg);
@@ -380,7 +382,9 @@ am_cache_entry_t *am_cache_lock(server_rec *s,
                                 am_cache_key_t type, const char *key);
 const char *am_cache_entry_get_string(am_cache_entry_t *e,
                                       am_cache_storage_t *slot);
-am_cache_entry_t *am_cache_new(server_rec *s, const char *key);
+am_cache_entry_t *am_cache_new(server_rec *s,
+                               const char *key,
+                               const char *cookie_token);
 void am_cache_unlock(server_rec *s, am_cache_entry_t *entry);
 
 void am_cache_update_expires(am_cache_entry_t *t, apr_time_t expires);

auth_mellon_cache.c

@@ -273,12 +273,15 @@ const char *am_cache_entry_get_string(am_cache_entry_t *e,
  * Parameters:
  *  server_rec *s        The current server.
  *  const char *key      The key of the session to allocate.
+ *  const char *cookie_token  The cookie token to tie the session to.
  *
  * Returns:
  *  The new session entry on success. NULL if key is a invalid session
  *  key.
  */
-am_cache_entry_t *am_cache_new(server_rec *s, const char *key)
+am_cache_entry_t *am_cache_new(server_rec *s,
+                               const char *key,
+                               const char *cookie_token)
 {
     am_cache_entry_t *t;
     am_mod_cfg_rec *mod_cfg;
@@ -374,6 +377,7 @@ am_cache_entry_t *am_cache_new(server_rec *s, const char *key)
     t->logged_in = 0;
     t->size = 0;
 
+    am_cache_storage_null(&t->cookie_token);
     am_cache_storage_null(&t->user);
     am_cache_storage_null(&t->lasso_identity);
     am_cache_storage_null(&t->lasso_session);
@@ -384,6 +388,18 @@ am_cache_entry_t *am_cache_new(server_rec *s, const char *key)
     t->pool[0] = '\0';
     t->pool_used = 1;
 
+    rv = am_cache_entry_store_string(t, &t->cookie_token, cookie_token);
+    if (rv != 0) {
+        /* For some strange reason our cookie token is too big to fit in the
+         * session. This should never happen outside of absurd configurations.
+         */
+        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+                     "Unable to store cookie token in new session.");
+        t->key[0] = '\0'; /* Mark the entry as free. */
+        apr_global_mutex_unlock(mod_cfg->lock);
+        return NULL;
+    }
+
     return t;
 }
 

auth_mellon_cookie.c

@@ -252,3 +252,31 @@ void am_cookie_delete(request_rec *r)
 
     apr_table_addn(r->err_headers_out, "Set-Cookie", cookie);
 }
+
+/* Get string that is used to tie a session to a specific cookie.
+ *
+ *  request_rec *r       The current request.
+ * Returns:
+ *  The cookie token, as a fixed length byte buffer.
+ */
+const char *am_cookie_token(request_rec *r)
+{
+    const char *cookie_name = am_cookie_name(r);
+    const char *cookie_domain = ap_get_server_name(r);
+    const char *cookie_path = "/";
+    am_dir_cfg_rec *cfg = am_get_dir_cfg(r);
+
+    if (cfg->cookie_domain) {
+        cookie_domain = cfg->cookie_domain;
+    }
+
+    if (cfg->cookie_path) {
+        cookie_path = cfg->cookie_path;
+    }
+
+    return apr_psprintf(r->pool, "Name='%s' Domain='%s' Path='%s'",
+                        cookie_name,
+                        cookie_domain,
+                        cookie_path
+                        );
+}

auth_mellon_session.c

@@ -25,6 +25,42 @@
 APLOG_USE_MODULE(auth_mellon);
 #endif
 
+/* Retrieve a session from the cache and validate its cookie settings
+ *
+ * Parameters:
+ *  request_rec *r       The request we received from the user.
+ *  am_cache_key_t type  AM_CACHE_SESSION or AM_CACHE_NAMEID
+ *  const char *key      The session key or user
+ *
+ * Returns:
+ *  The session associated, or NULL if unable to retrieve the given session.
+ */
+am_cache_entry_t *am_lock_and_validate(request_rec *r,
+                                       am_cache_key_t type,
+                                       const char *key)
+{
+    am_cache_entry_t *session = am_cache_lock(r->server, type, key);
+    if (session == NULL) {
+        return NULL;
+    }
+
+    const char *cookie_token_session = am_cache_entry_get_string(
+        session, &session->cookie_token);
+    const char *cookie_token_target = am_cookie_token(r);
+    if (strcmp(cookie_token_session, cookie_token_target)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                      "Session cookie parameter mismatch. "
+                      "Session created with {%s}, but current "
+                      "request has {%s}.",
+                      cookie_token_session,
+                      cookie_token_target);
+        am_cache_unlock(r->server, session);
+        return NULL;
+    }
+
+    return session;
+}
+
 /* This function gets the session associated with a user, using a cookie
  *
  * Parameters:
@@ -45,7 +81,7 @@ am_cache_entry_t *am_get_request_session(request_rec *r)
         return NULL;
     }
 
-    return am_cache_lock(r->server, AM_CACHE_SESSION, session_id);
+    return am_lock_and_validate(r, AM_CACHE_SESSION, session_id);
 }
 
 /* This function gets the session associated with a user, using a NameID
@@ -60,7 +96,7 @@ am_cache_entry_t *am_get_request_session(request_rec *r)
  */
 am_cache_entry_t *am_get_request_session_by_nameid(request_rec *r, char *nameid)
 {
-    return am_cache_lock(r->server, AM_CACHE_NAMEID, nameid);
+    return am_lock_and_validate(r, AM_CACHE_NAMEID, nameid);
 }
 
 /* This function creates a new session.
@@ -87,7 +123,8 @@ am_cache_entry_t *am_new_request_session(request_rec *r)
     /* Set session id. */
     am_cookie_set(r, session_id);
 
-    return am_cache_new(r->server, session_id);
+    const char *cookie_token = am_cookie_token(r);
+    return am_cache_new(r->server, session_id, cookie_token);
 }