Security issue: configuring session.cookie_secure with Drupal 6.9: patch included, review and advice needed

Update: (10 Mar 2009) See secure pages hijack prevention module. Read on for my original comments.

MOTIVATION AND GOAL:

It is important to insure that passwords are always transmitted over secure connections lest they be intercepted by third parties and put to bad use. The same can be said of the Drupal session ID (see http://drupal.org/node/101499), as any authenticated session that can be used to inject content that is sent back to other site visitors as vaild javascript code could result in attackers inserting malware and other undesirable attacks into the Drupal-managed website.

Setting session.cookie_secure in php ini prevents cookies set via https from being transmitted over http connections and is an effective way to protect the Drupal session ID. Drupal was patched in 5.x to handle this setting at the session level (see http://drupal.org/node/170310), but other site-wide configuration is needed to make Drupal work correctly when this PHP option is set.

RECOMMENDATIONS:

• Modify core modules/user to provide a secure login preference (patch included below)
• Insure that https sessions do not redirect to http (help needed; discussion below)
• Deprecate Secure Login and Secure Pages modules in favor of above changes (explanation below)
• Strongly recommend the use of session.cookie_secure in Drupal installation documentation

WORKAROUNDS:

• Use Secure Pages or modrewrite to send ALL requests to https, abandoning http connections entirely
• Insure that the login form only appears on secure pages (have not tried this, but apparently other Drupal users are using this technique successfully)

The techniques in this post allow http connections to be used by anonymous users, but logged-in users remain in secure sessions.

MODIFICATIONS TO modules/user FOR SECURE LOGIN:

The first issue that must be addressed is insuring that the login form always posts over an https connection. This can only be done if the web server is configured to accept secure connections, so an admin setting is needed.

In Drupal 5.x, it appears that the Secure Login module provided this functionality. There is no Drupal 6.x version of this library, and while it appears that it would not be hard to update it to work again, I feel that this security issue is important enough to justify an obvious setting in a core module, so that new Drupal users do not need to search extensively to figure out how to keep login passwords from being transmitted in the clear.

A patch to affect this change is included below, at the end of this post.

PREVENTING https REDIRECTS TO http:

Most Drupal links are relative, so serving pages over https will naturally tend to keep the user inside a secure session. However, code paths the route through the function drupal_goto() can send the user back to an http connection, even after a POST to an https page. If session.cookie_secure is set, this will log the user out, which is the correct and expected result; however, the Drupal login function uses this redirect mechanism immediately after logging the user in, so it is clearly necessary it prevent this behavior.

I tried to effect this change by modifying the url() function in includes/common.inc, and also attempted to modify the Secure Pages module to do the same thing. Unfortunately, neither of these techniques worked, as Drupal firmly wants to redirect to the path specified in the global $base_url.

The technique that worked was to put the following snippit in my sites/mydomain.com/settings.php file:

$base_url = 'http://mydomain.com';  // NO trailing slash!
if (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') {
        $base_url = preg_replace('/http[s]?:\/\//i', 'https://', $base_url);
}

RESULT

I am not sure if this is the best technique, but it does work very well (better than other things I tried) and achieves the desired result: authenticated users stay in https, unauthenticated users stay in http (presuming that settings.php is configured as described above), moving from https to http logs you out (presuming that session.cookie_secure is set), and passwords are not sent in the clear (provided the patch below is applied).

DEPRECATION OF SECURE LOGIN AND SECURE PAGES MODULES:

The Secure Login module, if updated to Drupal 6, would be a fine solution on a technical level and is in my opinion acceptable. However, I think that extremely central security measures (preventing clear-text transmission of admin passwords) should be present in Drupal core in a very easy-to-find location. I would therefore recommend deprecating Secure Login in favor to the two-line modification to modules/user that appears below.

The Secure Pages module is security theater, and should be deprecated immediately. Secure Pages allows an administrator to protect pages on their website that may include sensitive information; it contains a feature that will even redirect back to http connections for pages that are not secure. Unfortunately, this redirect back to http keeps the user logged in, and exposes the user's session to observation. Any third party who wished to see the content protected by Secure Pages could instead pick up the session cookie from the HTTP headers, and fetch the secure pages directly. If the modifications made in this post are made, then Secure Pages is not necessary, as all pages returned by authenticated users will always be sent via https. Therefore, the Secure Pages module should be deprecated.

DOCUMENTATION:

The Drupal installation documentation is great. It would be a good idea to include instructions in this document on how to secure a Drupal site using session.cookie_secure so that new Drupal users can easily insure that their admin passwords and secure sessions remain protected in https connections if they have configured their web servers for SSL, as everyone should.

FURTHER WORK:

The recommendations in this post should be reviewed and ideally, the module/user patch should be added to Drupal Core.

It would also be useful if the log out function could be modified to redirect back to a non-secure connection after the user has logged out.

PATCH FOR SECURE LOGIN:

This patch changes the core 'user' modules in the following two ways:

• A secure login preference is added to the User settings admin page
• If this setting is on, the user_login_block will always use an https connection to submit user passwords

I can explain the if requested, but it is only two lines long, so I believe it to be self explanatory. I did not write test cases for this patch. I spent about four hours on this patch, including research time (to find the best solution) and testing time inspecting generated html and observing connections with wireshark to insure that the password is not transmitted in the clear by any downstream redirects after this change is made (it is not). I also spent a couple of hours insuring that the https connection would be maintained, trying different techniques as described above. This post documenting the work also took a couple of hours.

Status: "It works on my machine."

? module-user-secure-login-2009-21-2.patch
Index: modules/user/user.admin.inc
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.admin.inc,v
retrieving revision 1.39
diff -u -p -r1.39 user.admin.inc
--- modules/user/user.admin.inc	18 Feb 2009 15:19:57 -0000	1.39
+++ modules/user/user.admin.inc	22 Feb 2009 04:00:39 -0000
@@ -234,6 +234,9 @@ function user_admin_account_validate($fo
  * @see system_settings_form()
  */
 function user_admin_settings() {
+  // User login settings.
+  $form['login_settings'] = array('#type' => 'fieldset', '#title' => t('User login settings'));
+  $form['login_settings']['secure_login'] = array('#type' => 'radios', '#title' => t('Secure login'), '#default_value' => variable_get('secure_login', 0), '#options' => array(t('Login form may post using http.'), t('Login form always posts with https.')));
   // User registration settings.
   $form['registration'] = array('#type' => 'fieldset', '#title' => t('User registration settings'));
   $form['registration']['user_register'] = array('#type' => 'radios', '#title' => t('Public registrations'), '#default_value' => variable_get('user_register', 1), '#options' => array(t('Only site administrators can create new user accounts.'), t('Visitors can create accounts and no administrator approval is required.'), t('Visitors can create accounts but administrator approval is required.')));
Index: modules/user/user.module
===================================================================
RCS file: /cvs/drupal/drupal/modules/user/user.module,v
retrieving revision 1.965
diff -u -p -r1.965 user.module
--- modules/user/user.module	19 Feb 2009 12:09:30 -0000	1.965
+++ modules/user/user.module	22 Feb 2009 04:00:41 -0000
@@ -858,8 +858,9 @@ function user_user_categories($edit, $ac
 }
 
 function user_login_block() {
+  global $base_url;
   $form = array(
-    '#action' => url($_GET['q'], array('query' => drupal_get_destination())),
+    '#action' => url($_GET['q'], (variable_get('secure_login', 0) ? array('query' => drupal_get_destination(), 'absolute' => TRUE, 'base_url' => preg_replace('/http[s]?:\/\//i', 'https://', $base_url, 1)) : array('query' => drupal_get_destination()))),
     '#id' => 'user-login-form',
     '#validate' => user_login_default_validators(),
     '#submit' => array('user_login_submit'),