Do not strip www. from cookie domain by default because that leaks session cookies to subdomains

Fully agree.

This will need a change record if it is committed, because this behavior has been with us since #56357: Login issues with multiple sites in the same domain (session cookie collision) (commit b93ce19), more than 8 years, so many people probably assume it silently.

Seems good to me.

The change indeed works. However, I am under the impression that, to match exact request-host, the best course of action might be to not add a domain at all : the matching rules would be the same as with the exact request-host added, just with a shorter header.

In addition, it seems there is an issue with the comments in default.services.yml : the text says:

    # Drupal automatically generates a unique session cookie name based on the
    # full domain name used to access the site. This mechanism is sufficient
    # for most use-cases, including multi-site deployments. However, if it is
    # desired that a session can be reused across different subdomains, the
    # cookie domain needs to be set to the shared base domain. Doing so assures
    # that users remain logged in as they cross between various subdomains.
    # To maximize compatibility and normalize the behavior across user agents,
    # the cookie domain should start with a dot.
    #

However, when doing this, the cookie will be shared, but the session start and write_close will not be synchronized unless table sharing across instances (I hope no one still does this) or something similar is set up. I can't put my finger on it, but I'd bet there could be a security consequence in some scenarios, would it not ?

However, I am under the impression that, to match exact request-host, the best course of action might be to not add a domain at all : the matching rules would be the same as with the exact request-host added, just with a shorter header.

Except that this does not seem to work with IE, the blog post linked from the issue summary states that:

Update: this post was updated on April 9, 2014, to reflect that Internet Explorer misbehaves with domain-less cookies, as learned from this blog post [link offline]. Previously, I concluded that a www-prefix makes no difference, with this new knowledge, a www-prefix is safer.

@znerol : AIUI (maybe I'm wrong) this conclusion is about explicit domain-level cookie (.example.com) vs host-specific cookie (www.example.com), which is what this patch is about. But in retrospect, the EricLaw article appears to explain that this also applies to cookies without a domain, so it is probably better indeed to set the domain as per this patch.

@pwolanin 1 : a case like Domain/OG/GCC (if/when either gets ported to D8) can indeed make use of this info. However, the comment itself does not mention any such restriction and the original #56357: Login issues with multiple sites in the same domain (session cookie collision) issue was about multiple separate sites within a domain, and this is the case where I think there may be a security issue.

@pwolanin 2 : my bad. I had forgotten cookies were only sent once. Not a consideration, then.

Consider the following example:

1. example.com
2. www.example.com
3. blog.example.com
4. wiki.example.com

Site 1 through 3 is served from the same document root (Drupal), site 4 is a different application.

Case 1: Default behavior (no changes to sites/default/services.yml, no multisite)

HEAD:
If a session is opened in 1 or 2, then the session cookie domain is set to .example.com. On subsequent visits to any of the sites (including wiki), the session cookie will be sent.
If a session is opened in 3, then the session cookie domain is set to .blog.example.com. As a result the session cookie is only sent to 3, and never to 1, 2 and 4.

Patch:
If a session is opened in 1, then the session cookie domain is set to .example.com. On subsequent visits to any of the sites (including wiki), the session cookie will be sent.
If a session is opened in 2 or 3, then the session cookie domain is set to .www.example.com and .blog.example.com respectively. As a result the session cookie is only sent to the site which opened the session.

Note that this configure is subtly broken before and after the patch. Before, sessions opened in www.example.com and example.com would just work out of the box, but those opened on blog.example.com would be restricted to that subdomain. With the patch only those sessions opened on example.com would propagate to the subdomains, but not vice versa.

Hence the patch will only break installations accessible both with, and without the www subdomain. The blog subdomain case needs tweaking anyway, so it is irrelevant in this scenario. On the other hand, the patch enhances security on those sites which currently redirect to the www subdomain.

Case 2: Shared cookie domain (cookie_domain to sites/default/services.yml, no multisite)

If sessions should be shared between sites 1, 2 and 3, then the only way to do that is to set the cookie_domain to .example.com. As a consequence, the cookie will also be sent to 4. The patch does not introduce any behavioral change for those configurations.

Case 3: Multisite setup without shared cookie_domain, no shared db tables

This behaves exactly like case 1.

Case 4: Multisite setup with shared cookie_domain and shared session/user/... db tables

This behaves exactly like case 2.

Conclusion

The patch only changes behavior for sites which do not touch the cookie_domain container parameter. From those sites, only those expecting that sessions are shared between example.com and www.example.com are affected. More complex setups require changing the cookie_domain parameter anyway.

Disclaimer: This is how I expect it to work, I did not yet test through any of those scenarios.

Reading the CR, the following sentence does not make much sense to me:

Thus, a leading www. will be stripped, and users will not (by default) be logged in with the same session on http://www.example.com and http://example.com

After the patch a leading www. will not be stripped anymore.

Also there is no need to subclass anything to restore the old behavior. Just set the cookie_domain in services.yml and call it a day.

Maybe I wasn't clear : as per #14, I think that code-wise, the change is good, which #15 confirms, but our wording in default.services.yml could be improved about the implications of the cookie domain, even though the patch doesn't really change the situation. I mentioned this because this is something which is not touched often and this patch is one such occasion.

That would be fine too. I just expect it to remain untouched because no one will care enough, unlike a code issue.

Then RTBC++.

Nice discussion on issue. This looks very sensible indeed. Committed 20619e0 and pushed to 8.0.x. Thanks!

I'm not sure about the validity of backporting this.

Backport to D7.

Remove the www. stripping behavior to work all following example.

• example.com
• www.example.com
• blog.example.com
• wiki.example.com

Here is a test case for this patch that I am hoping someone else may have come across and solved:

1. A user logs into a site with a www. subdomain before this patch is applied. The user does not log out. (So a cookie is set without the www)

2. The patch is applied.

3. The user returns to the site and logs out. (The original cookie without the www subdomain is still set.)

4. The user attempts to log back in, but can not.

Unless that cookie is cleared by the user, or it expires, they will not be able to log in. I'm working with a site where it will be impractical to ask the users to clear their cookies. Has anyone found a workaround for a scenario like this?

@mchljams I guess we need to empty the session table to invalidate all the sessions.

Also the policy now is to create a new issue for Drupal 7 and move the patch there because having this issue still open make it feel that this issue has not been fixed for Drupal 8.

I think we are good to go here.

About concerns in #36, I think clearing the sessions from DB should help.

Not settings to RTBC, because not sure if we should add sessions clean up here

Clearing the DB won't work when sessions are not in the DB: there are multiple implementations of non-SQL sessions.

#29 has been working for us for over 4yrs. It's safe to roll this into a release before EOL (pretty sad that drupal dev process has always been so slow at patching things that they linger for decades in many cases). Sessions clearing can be added later.

Thanks for sharing your good results with the patch from #29 @crystaldawn. This issue has been added to #3179845: [meta] Priorities for 2020-12-02 bugfix release of Drupal 7.76 / 7.77 from which a new meta issue for February 2021 will be created, so hopefully the patch will be added soon.

Approved and RTBC + 1, but ensure that the change record has the snippet to set cookie_domain manually in settings.php.

@Fabianx I think we need a new Drupal 7 only change record for this.

Yup, we'll need a new CR just for the D7 change.

(In general I think we're following the pattern of separate issues for D7 backports but I'm not sure it's worth spinning up a new one here just for the patch in #29... I shall try to tidy up the tags etc.. when we're done and mark this as Fixed).

As per earlier comments I thought "it'd be good to add a unit test for this change".

However, the change is within bootstrap.inc's drupal_settings_initialize() which is around 80 lines long, includes the site's settings.php and manipulates a load of globals.

So not great for unit testing.

How about moving this code out to its own function?

Benefit: we can do unit testing.

Cost: an extra function call on pretty much every bootstrap.

Worth it?

(the interdiff looks a little weird 'cause we're moving the lines the original patch was changing)

RTBC on the helper function - and then this can be called from settings.php in the circumstances explained.

Draft CR: https://www.drupal.org/node/3250913

Thanks everyone!

Wow. Impressive. Only 6 yrs to make a 1 line change to a dev branch. That's some speedy turnaround right there.

CR needs to be published

@mcdruid nice improvement making it a helper function. Would it help to add an example in default.settings.php file?

There's already an example and some docs:

https://git.drupalcode.org/project/drupal/-/blob/7.82/sites/default/defa...

...might be an idea to link to that from the CR (or at least mention it)?

@andypost thanks for publishing the CR - I was thinking we'd publish them all with the release next week, but on commit makes more sense.

We experienced a lot of users unable to log in to our D7 websites using Chrome after the update. Adding the cookie domain to the settings.local.php file fixed my issue (replace with your domain):

$cookie_domain = '.example.com';

The change makes sense but I don't understand how it is causing issues.

@albertski on the sites that had a problem, do they use a www. subdomain?

Do you know what cookie domain the sites were using before the update here, and what that changed to (before you added the line to the settings.php)?

@mcdruid The site uses the www subdomain and all non-www calls redirect to the www site via .htaccess:

  # To redirect all users to access the site WITH the 'www.' prefix,
  # (http://example.com/foo will be redirected to http://www.example.com/foo)
  # uncomment the following:
  RewriteCond %{HTTP_HOST} .
  RewriteCond %{HTTP_HOST} !^www\. [NC]
RewriteCond %{REQUEST_URI} !^/[0-9]+\..+\.cpaneldcv$
RewriteCond %{REQUEST_URI} !^/\.well-known/acme-challenge/[0-9a-zA-Z_-]+$
RewriteCond %{REQUEST_URI} !^/\.well-known/pki-validation/[A-F0-9]{32}\.txt(?:\ Comodo\ DCV)?$
RewriteCond %{REQUEST_URI} !^/\.well-known/cpanel-dcv/[0-9a-zA-Z_-]+$
RewriteCond %{REQUEST_URI} !^/\.well-known/pki-validation/(?:\ Ballot169)?
  RewriteRule ^ http%{ENV:protossl}://www.%{HTTP_HOST}%{REQUEST_URI} [L,R=301]

I didn't have $cookie_domain set anywhere (commented out in settings.php) so not sure what the value was.

We had exactly the same problem reported at #2522002-56: Do not strip www. from cookie domain by default because that leaks session cookies to subdomains. We also have the same .htaccess setup, directing non-www traffic to www instead. Setting the cookie domain as suggested in #56 seems to have solved the problem, but it also logged out a number of users that were successfully logged in. I see that this issue was flagged all the way back in #2522002-36: Do not strip www. from cookie domain by default because that leaks session cookies to subdomains FOUR YEARS AGO. It certainly would have saved a lot of headaches to know we needed to set the domain cookie ahead of time! Maybe a note next time????

Here's a description of the behavior we saw, for anybody else having this problem:

Some of our users were able to log in after the 7.83 core update, but others were not. Affected users would log in be directed to their users/username page, where they'd see a 404. Navigating around the site, a logged-in user would not see editing links, admin menu, or other features they should have been able to see.

Clearing browser caches and switching browsers worked, but inconsistently. TFA stopped working almost entirely, turning into a loop of log in -> enter TFA code -> back to login screen -> back to TFA code.

None of this appeared in the server or watchdog logs. Logs reported that a session opened or not (resulting in a 404, I guess that's the new behavior in 7.83?), but either way, the user would not see the editing tools they should see as an authenticated user.

To solve this issue, set the cookie domain in settings.php and have users clear their browser caches, but be aware that it will log out anybody who is currently logged in.

@alrueden In comment #59 are you suggesting the change record (https://www.drupal.org/node/3250913) or that the release notes (https://www.drupal.org/project/drupal/releases/7.83) could be more explicit? Maybe it is still possible for either to be improved as some sites may approach a maintenance update more slowly.

@izmeez, yes, both!

For the change record, I'd like to see a message more like this:

All authenticated users will need to clear their browser cookies before logging in after this update. Site administrators should clear the session table after updating to ensure that all users have the correct cookie domain set. Note: sites that use .htaccess to redirect non-www to www are most likely to have authentication issues after this update.

If a user's cookies are not set correctly and need clearing, you may see:

• Users get a 404 page after logging in, when redirected to their user/username page
• Two-Factor Authentication redirects back to user login form, without errors
• Watchdog reports open sessions for users, but logged-in users are unable to edit the site or access admin areas
• Issues may persist across browsers
• Clearing Drupal caches does not solve the issue
• Clearing browser caches may solve the issue, or may not

To opt out of this change and retain the previous behavior, set the the $cookie_domain global variable in settings.php.

Is that even correct, though? I figured logging in was permanently broken in 7.83. What we saw went way beyond some sessions being disrupted. The change record implies that this is a temporary issue, but it went on for hours through cache clearings and changing browsers, until I finally set the cookie domain in settings.php.

For the release notes:

This update affects user logins and may cause issues including users not being able to log in. Sites that redirect non-www to www are most likely to have authentication problems. See change record for mitigation instructions.

@alrueden thanks for the suggestions.

However, I'd want to understand the issue better before adding all of those details to the CR.

For example, why would this be the case?

sites that use .htaccess to redirect non-www to www are most likely to have authentication issues

@mcdruid, honestly, I don't know why that's the case. But the three users complaining about major problems on this page (at #36, #56, and #59) all had that setup.

When I read the issue description, I thought this change wouldn't affect my site because we already force www through .htaccess -- nobody ever logs in on the bare domain, they just can't. And yet, this change caused massive problems and it sounds like nobody who worked on this issue knows why. So I would have appreciated that my setup be flagged as possibly problematic, given that somebody did report the issue way back in comment #36.

I think I'm a bit closer to understanding what's happening to some sites.

I've been able to reproduce being unable to login and create a new session after the change that was committed here.

The relevant code is:

https://git.drupalcode.org/project/drupal/-/blob/7.83/includes/bootstrap...

Steps to reproduce:

• Use a www subdomain, let's say www.example.com
• Do not set $cookie_domain in settings.php initially
• Checkout a version of D7 from before this change e.g. git checkout 67207cdd48
• Login as any user, and observe that your session cookie should have a domain attribute of .example.com
• The cookie will also have the following name (with an extra leading S if you're using https) which is based on the $base_url:
  

  php > print 'SESS' . substr(hash('sha256', 'www.example.com'), 0, 32);
  SESS80fc0fb9266db7b83f85850fa0e6548b
  

• Now update Drupal to 7.83, and at the same time empty the sessions table (or remove the row for the specific user you're testing with)
• Verify that you're no longer logged in, but your browser should still have the session cookie for .example.com
• Now try to login again. This is where you should see the log entry saying a new session has been opened, but your browser will most likely have failed to replace the old session cookie with a new one and you will not be logged in.

In Firefox, I got a message in the console saying something like:

Cookie SESS80fc0fb9266db7b83f85850fa0e6548b has been rejected because it is already expired

However, it seems that this may be misleading (see https://stackoverflow.com/questions/65038173/cannot-remove-a-cookie-fire... ). Note that D7's session cookies use an expiry date in 1978 (the explanation for which is a good Drupal Trivia question ;). edit: that was incorrect, I was mixing this up with the default HTTP Expires header.

In Chromium the new session cookie seemed to just fail silently.

I've not tracked down a detailed explanation in the RFCs etc.. but I suspect what's going on is that the new session cookie has the same name but a more specific domain attribute - it's now .www.example.com instead of .example.com and the browsers refuse to overwrite the existing cookie with the less specific domain attribute.

There is a section on potential problems like this titled "Weak Integrity" in https://www.rfc-editor.org/rfc/rfc6265#section-8.6 but it's actually describing the opposite situation if I understand correctly.

So in this specific case at least, clearing out the sessions table would do more harm than good (in the context of the previous discussion around #36 in this issue).

If we now set a $cookie_domain in settings.php it may resolve the problem, but not necessarily for the reason we'd expect.

  if ($cookie_domain) {
    // If the user specifies the cookie domain, also use it for session name.
    $session_name = $cookie_domain;
  }

So if we set $cookie_domain = '.example.com' then the $session_name will change and therefore so will the name of our session cookies; remember that is derived like this:

  $prefix = ini_get('session.cookie_secure') ? 'SSESS' : 'SESS';
  session_name($prefix . substr(hash('sha256', $session_name), 0, 32));

...where before we set $cookie_domain the $session_name variable was set to 'www.example.com' derived from $base_url.

It'd probably be better to set the $cookie_domain to '.www.example.com' (although the leading dot is supposed to be ignored by browsers these days) because that will also change the $session_name. The point of the change here was to be more specific and avoid some of the "Weak Integrity" issues with Drupal's session cookies being sent to other subdomains.

Either way, if $session_name changes then login starts to work again as our session cookie now has a different name and doesn't "clash" with the old one.

The above may not be exactly what's happened on every site that's had a problem, but perhaps it's a variation on this theme.

In this case, manually clearing the old cookie properly does make login work again. However, it's obviously not always practical for sites to instruct their users to do so.

I'll have a think about whether we can do anything in core to avoid sites hitting this issue when they update. We could make a change to the way that the session_name is set, but on the other hand that would invalidate session cookies for sites that would otherwise have not experienced any disruption.

We can also add some specific details to the CR / release notes.

I'll provide an update in the next day or so.

Can the problem scenario be detected (i.e., see if the weak cookie exists, even though it won't be valid for authentication), and the mitigation (e.g., "change to the way that the session_name is set") would be applied conditionally based on that so as not to affect "sites that would otherwise have not experienced any disruption"?

Interesting idea @mforbes but sounds like it could introduce unwanted complexity in code which runs on every bootstrap.

Here's a patch which simply uses $cookie_domain to derive the session_name regardless of whether it's set in settings.php or derived from $base_url - I think this should fix the issue, but it may have the downside of invalidating existing user sessions (and perhaps leaving old session cookies in people's browsers, but I'm not too concerned about that).

I'll do some manual testing of this (e.g. following steps in #'64). Can we write any tests within core for this?

We'll likely try to release a hotfix as soon as possible in order to avoid sites encountering login problems when they update to 7.83

NR for testbot

First pass manual testing with the steps from #64 - I confirmed that without the patch in #66 I was unable to login and was stuck with just the old session cookie.

With the patch, as expected, I got a new session cookie with a different name (and domain attribute) and was able to login successfully.

Applied hotfix patch in comment #66 to test site already updated to Drupal 7.83 without problems. Patch applied and login without errors. Will need to test on site that has not been previously updated to D7.83 core.

How about this?

Along similar lines to the suggestion in #65, but we don't need to check for actual cookies.

We know the problem happens when the host / base_url starts with www. so we can do a simple check for that, and only change the session_name if we have to.

This passed my first manual run through of the steps in #64

Patch in comment #70 applies equally well and does not cause problems with login. I do not think my testing is that helpful as it needs to be done on a site with www.domain.tld which will take further setup. Maybe someone else has a setup already to test this.

I discussed this with @Fabianx and we agreed we'll do a hotfix release of D7 with #70 (I may tweak the comments a little).

I've committed #70 with more detailed comments about why the workaround is there.

Marking as Fixed again.

Thanks everyone that reported problems and tested / reviewed fixes.

A hotfix release should follow fairly shortly.

https://www.drupal.org/project/drupal/releases/7.84 released.

Any sites that set $cookie_domain in settings.php (without the www) to resolve problems caused by this change could test removing that, as it's better to use the more specific domain attribute if possible.

I just noticed that Drupal 7.84 was released yesterday, with the only change from 7.83 being the additional patch from this issue:

+++ b/includes/bootstrap.inc
@@ -809,6 +809,16 @@ function drupal_settings_initialize() {
     if (!empty($_SERVER['HTTP_HOST'])) {
       $cookie_domain = _drupal_get_cookie_domain($_SERVER['HTTP_HOST']);
     }
+
+    // Drupal 7.83 included a security improvement whereby www. is no longer
+    // stripped from the cookie domain. However, this can cause problems with
+    // existing session cookies whereby some users are left unable to login. In
+    // order to avoid that, use the cookie domain (including leading dot) as the
+    // session name when a www. subdomain is in use.
+    // @see https://www.drupal.org/project/drupal/issues/2522002
+    if (strpos($session_name, 'www.') === 0) {
+      $session_name = $cookie_domain;
+    }

If anyone in the future comes across that code comment and follows the link to this issue, they will have to read through the comments to figure out why this was changed.

I guess that Comment #64 here has the most complete description of what is going on. It would probably focus my attention if my site had run into this problem. ;)

Since the code comment now has a link to this issue, can we add a section to the issue summary explaining the problem and the solution? Maybe include a reference to #64 for a more complete explanation. I would volunteer to update the issue summary, but I do not understand the issue well enough.

@mcdruid, thanks for all your work tracking down this issue, hopefully once and for all. On my sites, this version works perfectly without the cookie domain set in settings.php! It may still be worth noting that this update will log out any logged-in users, though.

It may still be worth noting that this update will log out any logged-in users, though.

But only in a specific condition, right? (I assume based on having received issue credit for suggesting such a thing -- thanks!)

The documentation ought to explain -- in terms of the htaccess/conf scenarios and so forth (redirecting from example.com to www.example.com or whatever the case may be?) -- some root causes that lead to the condition being true.

I just got around to updating my sites to 7.84, and have run into a $_SESSION collision problem with this minor code update.

We initially had temporary session issues with 7.83, but they were resolved as soon as users cleared their cookies and logged in again.

Unfortunately the additional fix for 7.84 has an unintended consequence for our series of D7 multi-sites. This is because all of our sites are working off of the "www." subdomain by default. Each subsite lives in its own subfolder path. For instance:

• www.example.com
• www.example.com/intranet
• www.example.com/site2

Each of these are separate Drupal 7 installs running off the same code base. They each have their own $base_urls, users, roles, sessions, etc.

We do not set a $cookie_domain for each site in settings.php.

In 7.83, the $session_name for each site was calculated on the $base_url because $cookie_domain wasn't set. This worked because each site has a separate $base_url.

With the 7.84 change, our $session_names are ALWAYS set to $cookie_domain in our instances because the newly calculated $cookie_domain always matches the added "www" conditional, which in turn overwrites the $session_name.

This unfortunately causes all of these 7.84 sites to use the same $session_name. Logging into one site, logs you out of the other subsite, because each site is trying to use the same $session_name and session ID. Users can only be logged into one site at a time.

We don't feel we should be setting a $cookie_domain for each subsite that includes the base_url or path (which would feasibly solve the $session_name issue), because URL paths shouldn't be included in that cookie setting from my understanding.

You have 2 options:
1. Set $cookie_domain for each site
2. Redevelop your subsites to use the correct method of using subdomains for separate sites under different databases.

You have a non-standard setup that doesnt use best practices when using "subsites" and it's not something that should be considered here. If I were in your shoes, I'd start re-evaluating why subsites are not using subdomains because I cant think of a single use case where that would be required or better than subdomains.

because URL paths shouldn't be included in that cookie setting from my understanding.

You are correct, paths usually shouldnt go in cookies but it doesnt sound like you understand why that is the case. Different sites are supposed to be on their own subdomain or domain, not specified in the URL. So you're right, but also wrong at the same time. I cant stress enough the idea of rearchitecting your platform to use subdomains, it really is your best option as well as best practice.

Hmm, sorry to hear about this unintended consequence of the workaround in 7.84 - thank you for the detailed explanation @andrew.green

We obviously don't want to make more changes that cause disruption for sites which have updated.

As mentioned in a previous comment, this code is not easy to test as it's within a fairly long function that reads from and sets globals.

Perhaps we could add some additional test coverage though.

In the meantime we can use drush to look at what's going on:

Drupal 7.83

$ drush --uri=www.example.com ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS80fc0fb9266db7b83f85850fa0e6548b
base_url: http://www.example.com
cookie_domain: .www.example.com

$ drush --uri=www.example.com/foo ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS8aaa63e4f8f04b640092a5014792cefc
base_url: http://www.example.com/foo
cookie_domain: .www.example.com

$ drush --uri=www.example.com/bar ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS3413f49e4dd6d895bd292ee1fb86147f
base_url: http://www.example.com/bar
cookie_domain: .www.example.com

Drupal 7.84

$ drush --uri=www.example.com ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS0e9f0e2600e4d8f26827597c5e324261
base_url: http://www.example.com
cookie_domain: .www.example.com

$ drush --uri=www.example.com/foo ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS0e9f0e2600e4d8f26827597c5e324261
base_url: http://www.example.com/foo
cookie_domain: .www.example.com

$ drush --uri=www.example.com/bar ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS0e9f0e2600e4d8f26827597c5e324261
base_url: http://www.example.com/bar
cookie_domain: .www.example.com

So yep, we can see the behaviour you described with the same session_name despite the different base_urls.

I think we could perhaps fix this with a simple change:

     if (strpos($session_name, 'www.') === 0) {
-      $session_name = $cookie_domain;
+      $session_name = '.' . $session_name;
     }

With that change made to 7.84 I think we maintain the same session_name for the base_url without the subdirectory (so hopefully no disruption to existing sites) but we get a different session_name for each of the subdir urls:

$ drush --uri=www.example.com ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS0e9f0e2600e4d8f26827597c5e324261
base_url: http://www.example.com
cookie_domain: .www.example.com

$ drush --uri=www.example.com/foo ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS0ddd0afc0bece848c840cdcd7b8ed9c9
base_url: http://www.example.com/foo
cookie_domain: .www.example.com

$ drush --uri=www.example.com/bar ev 'print "session_name: " . session_name() . PHP_EOL . "base_url: " . $GLOBALS["base_url"] . PHP_EOL . "cookie_domain: " . $GLOBALS["cookie_domain"] . PHP_EOL;'
session_name: SESS22f2cd08599536cb4363fcd09e29d264
base_url: http://www.example.com/bar
cookie_domain: .www.example.com

@andrew.green are you able to test this change to see if it resolves the problem for you?

@mcdruid It's possible this could fix it, but it would also be enabling bad practice of having the ability to set up subdomains incorrectly such as what @andrew.green is having to deal with right now. Bad practice usually results in pain points at some point in the future and that's what is going on here in this specific case which is different than the pain points we've dealt with in the issue where users did indeed have things setup correctly and with best practice. Using current code, he can fix this with the $cookie_domain that already exists. The site's he's dealing with use completely different DBs which means they really should be subdomains of their own.

@crystaldawn I do not understand why this is being identified as "bad practise" even though it is not a way we typically use there are some use cases where this approach may make sense. It may be off topic to this thread, but I would prefer to see a discussion on the pros and cons, still bearing in mind there may be other specific use cases.

It's bad practice because each domain should have it's own separate cookie. It's a security issue. We shouldnt be making patches to core code that enables bad practice that can lead to security problems. The problem Andrew is dealing with is a result of setting up multiple sites under a single cookie and as he mentioned, only 1 site can be signed in at a time. This is on purpose, it's a security feature. Thus, the correct "fix" is to set things up properly with subdomains. Just because something is "possible", doesnt mean it should be allowed or enabled ;)

Current code currently enforces this best practice softly because it's still possible to do what Andrew is doing if one modifies the $cookie_domain var in settings.php, but he states "We dont believe we should do that" which tells me that they dont understand the best practice of why subsites with different DBs should be their own subdomain (lack of understanding of security best practices is what I'm referring to here) and they need to be alerted to this bad practice setup and consider changing it if possible, otherwise, they should limp along with $cookie_domain for each site until they have a chance to re-architect their setup. It shouldnt involve changing any code in this issue is my point. Andrew needs to look into multisite setup where each site is it's own dir in /sites, such as /sites/site1.something.com /sites/something.com, /sites/site2.something.com, etc. There really isnt any need to have multiple code bases for multiple sites when Drupal has a facility to handle multisites natively. Each site gets it's own modules dir, theme dir, settings file, etc and still has the ability to use common files installed in /sites/all

But in order to use Drupal multisite, one first has to have setup their subsites correctly with subdomains. A step that Andrew has not yet completed. So for security best practice, that's why I am trying to steer away from modifying the code in this issue to support a setup similar to how Andrew has done things, because it breaks security best practices.

After thinking about things, the only use case where I could see this type of setup being needed is if the user doesnt have access to add subdomains to the parent domain. In which case, that's not a Drupal problem. It's an administrative issue on their end. There may be very valid reasons for the domain administrator for doing this and one of them may very well be to block the ability to add sites other than the main site. IDK if thats the case here, but it's a possibility. It's the only thing I could think of as a "use case" where this oddball setup would be needed. It could also just be lack of knowledge about Drupal as well. Maybe Andrew isnt even aware of Drupal's built in Multisite capabilities or "inherited" such a platform (I see "inheriting" used a lot in these types of odd requests/reports).

I agree that multiple sites on the same subdomain has security implications (cookie theft, as discussed), but there is definitely precedent for supporting it, down to the fact that the parsing of the $sites array elements actually works for this use case. If we were to stop supporting it, tons of documentation would be invalidated, or would at least need to disclose that it's unsupported:

• https://www.drupal.org/docs/7/multisite-drupal/multi-site-in-subdirectories
• https://www.drupal.org/docs/7/multisite-drupal/multisite-mapping-with-dr...
• https://docs.acquia.com/cloud-platform/develop/drupal/multisite/#creatin...
• https://drupal.stackexchange.com/questions/80145/sites-php-setup
• etc.

That's correct, it's possible, but not recommended. To do this type of setup, you dont need a code change here since $cookie_domain handles this type of setup already. The issue is that Andrew believes that "they shouldnt need to do that", but that's actually incorrect. They should need to do that for every single site especially since it's not a recommended setup (sort of like an "Are you sure?" question). These setups have always assumed that $cookie_domain would be filled in with the proper values AFAIK and is mentioned in the docs here: https://www.drupal.org/docs/7/modules/domain-access/configuring-settingsphp No precedents would be broken nor fixed by including something that would allow Andrew's setup by default in this issue item. I believe it would instead enable bad practice site configurations that would eventually lead to serious security problems and that's not a very value added gain in my opinion. If you have a site like this, you should already be aware of the security implications and know how to set up the base url, cookie domain, etc and that doesnt appear to be the case here.

Interesting discussion, but from my point of view this is a straightforward regression.

When D7 derives the $base_url directories are taken into account:

https://git.drupalcode.org/project/drupal/-/blob/7.84/includes/bootstrap...

Most of this code is over 10 years old, and the behaviour change around the session_name when there are one or more directories in the base_url was not intentional.

In the situation described by @andrew.green in #80 it's not clear to me how setting a $cookie_domain in each site's settings.php is a viable workaround; it sounds like each site needs to use the same cookie domain and therefore the session_name is how the different sites' session cookies are differentiated.

As the name suggests, the $cookie_domain corresponds to the domain attribute on session cookies and doesn't affect the path attribute. There's effectively no support for setting the path attribute on session cookies in D7 (apart from a hack to support the samesite attribute in older PHP versions).

There are certainly downsides to this arrangement - not least that all session cookies the browser has for the shared cookie domain will be sent to all sites. However, it's been supported in Drupal for a long time, and it was not the intention of the changes made in this issue to remove that support.

You could argue that Drupal should set a path attribute for its session cookies based on any directories that are present in the base_url. That's not in scope here though.

I think this patch should retain the workaround in 7.84 and also restore the support for different base_urls leading to different session_names being generated.

Would be good to have some testing of this before we consider whether to do a hotfix to the hotfix.

Back to NR for that, but I wouldn't really expect tests to fail as there's unfortunately little if any test coverage for this functionality (as discussed previously).

This feels pretty hacky as we're mucking around with globals and superglobals, but here's a unit test which reproduces what we looked at with drush in #82.

Locally the test only patch fails and the one with the fix passes.

Let's see if the testbot agrees; it may not as IIRC it always runs the test Drupal instance with a base_url that includes a directory etc..

I initially started off storing the original values of the (super)globals we're changing and restoring them at the end, but I'm not certain that's necessary. We can go back to that if leaving the test values in place cause problems for other tests.

The interdiff with 88 would be the test_only patch.

Well that went better than expected :)

Here are a handful more test scenarios with different base_url set ups.

Patch in #91 applies without difficulty and works with simple site with www.domain.tld and also simple site with othersub.domain.tld without problems.

Thank you for everyone's work on this.

I can also confirm that #91 solves my particular session issue across all of my sites.

I apologize in that I didn't want to start a discussion on multi-site setup. I will note that we are using the documentation found here: https://www.drupal.org/docs/7/multisite-drupal/multi-site-in-subdirectories. We have one primary D7 install, separate /sites folders, .htaccess for the redirects, and then defined subsites in sites.php. When we launched these particular instances in 2012, we used this subdirectory method so that we only had to use 1 shared SSL cert when we initially deployed the family of sites (before free SSL certs were ubiquitous). It also allows us to keep everything under the www banner for consistency and SEO.

Thanks for confirming that the fix works; I'm going to call that RTBC.

Looks like I missed out one test scenario I'd meant to include, which I've added in this patch. No other changes.

I'm not going to rush out a hotfix release for this right now.

I plan to review it with @Fabianx next week and we'll go from there.

As I said in #76,

Since the code comment now has a link to this issue, can we add a section to the issue summary explaining the problem and the solution? Maybe include a reference to #64 for a more complete explanation. I would volunteer to update the issue summary, but I do not understand the issue well enough.

Alternatively, add a change record and update the code comment to refer to that instead of to this issue.

Sorry, I did not mean to change the issue status.

Thanks for the reminder @benjifisher - I've added a section to the start of the IS which hopefully explains what the 7.84 hotfix was all about.

If we release another hotfix, I'll add a note to the IS about that too.

Committed #94, and will do another D7 release shortly (having discussed this with Fabianx).

Upgrading from 7.84 to 7.85 will only be necessary in the situation described by @andrew.green in #93 (and #80) whereby multiple sites share the same domain but have different base_urls.

A note should be added to the IS once the release is done.

https://www.drupal.org/project/drupal/releases/7.85

I added some detailed notes about this new fix in the release notes, and have added some more info to the IS here.

Fingers crossed for no more hotfixes (to the hotfixes)!

FYI, this patch causes a major conflict for any module running the Drupal Authcache module with personalization (p13n).

Prior to this patch, the aucp13n cookie is set at the domain name without www. Once the www is re-added, the site attempts to continuously reload the page, spinning endlessly. Also pages are never fully cached. The only way to fix the issue is to delete the aucp13n cookie, and allow the module to create a new one.

Is there an issue in the authcache module's queue?

I plan on adding one, we just discovered the issue. Wanted to make sure it was known on here asap since those upgrading to 7.85 are more likely to look here first than realize it is a conflict with Authcache.

@ron_s Just wondering about status of adding issue to authcache and adding link here for others to follow. Thanks.

Hi, sorry, i'm trying to follow this thread, has this patch been applied to the core drupal?

i'm trying to update to drupal 7.90 from 7.80 without all our users loosing their sessions.
why have we added a dot prefix to the session name where there want one before?

@keenly yes this change was added in Drupal 7.83 (Change Record).

Unfortunately the change caused problems on some sites that left users unable to login (see #64).

A hotfix for that was released as Drupal 7.84 but that in turn caused problems in some very specific cases (see #80) so another hotfix - Drupal 7.85 - was released. It was this one that added the dot prefix in the generation of the session name.

If you want to upgrade past these releases (e.g. 7.80 to 7.90) without any users losing their existing sessions one way to do that might be to set your own $cookie_domain in settings.php

If your site is www.example.com setting your cookie domain to exactly that (without a leading dot) should result in the session_name not changing, for example.

Whether you would need to do this or not depends on the domain(s) / base_urls that your site(s) use. Testing any changes thoroughly before releasing to production is highly recommended as always.

The tests using drush in #82 might be useful to check what happens with different values for $cookie_domain across different D7 releases.

There was a tradeoff in making these changes between improving security vs. the inconvenience of possibly losing existing sessions. The hotfixes addressed specific problems that were otherwise hard for affected sites to workaround without e.g. instructing all users to manually clear their cookies.

Thank you for the catchup,

My site is just simply using base_url = 'https://www.example.com'.

am i correct, the purpose of adding the dot in front of the $session_name is to force all users to re-login after the cookie domain had changed to include 'www'?

so if i set $cookie_domain to 'www.example.com', then i might face the same login issue that started this thread?