WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#22705 closed defect (bug) (fixed)

Admin cookies set to wrong path for main blog in a WP-in-subdir-sites-on-root install that uses subdomains

Reported by: markjaquith Owned by: nacin
Milestone: 3.5 Priority: high
Severity: blocker Version:
Component: Administration Keywords: has-patch needs-testing commit
Focuses: Cc:

Description

  1. Install WordPress in /wordpress/
  2. Move 'home' to /
  3. Convert it to Multisite, using subdomains.
  4. Complete the .htaccess/wp-config.php steps
  5. Go to /wordpress/wp-login.php
  6. Try to log in.

Failure. It'll give you wp-admin cookies for /wp-admin/ but redirect you to /wordpress/wp-admin/. You'll get kicked back to wp-login.php

Attachments (2)

22705.diff (633 bytes) - added by markjaquith 5 years ago.
22705.2.diff (577 bytes) - added by nacin 5 years ago.

Download all attachments as: .zip

Change History (12)

@markjaquith
5 years ago

#1 follow-up: @nacin
5 years ago

This breaks the network admin, looks like, which operates from the root.

@nacin
5 years ago

#2 in reply to: ↑ 1 ; follow-up: @nacin
5 years ago

Replying to nacin:

This breaks the network admin, looks like, which operates from the root.

If you have home != siteurl and update to WordPress, we designed it (in part at my urging to keep the feature to rewrites only) to keep the existing /wordpress/wp-admin/ URLs for the main site. But, this paradigm breaks the entire network admin, which forces itself to be $current_site->domain + $current_site->path.

I am not sure if these two things are reconcilable. We'd basically need to force a new login for the network admin.

The only alternative would be to relax the cookie security for these networks, down to what subdirectory installs already do. It isn't ideal, but it seems to work and is the least-impact change. (It also would only affect new networks.)

#3 @nacin
5 years ago

  • Keywords has-patch needs-testing added

#4 @ryan
5 years ago

That seems sanest at this point and shouldn't impede future improvements. Looks good.

#5 in reply to: ↑ 2 @evansolomon
5 years ago

Replying to nacin:

We'd basically need to force a new login for the network admin.

The only alternative would be to relax the cookie security for these networks, down to what subdirectory installs already do. It isn't ideal, but it seems to work and is the least-impact change. (It also would only affect new networks.)

I agree with this. More generous cookie paths seem like a much better solution than multiple logins. I also don't think there's an actual security concern, other than the security-by-obscurity of a slightly different path.

Tested .2.diff and it works for me.

#6 follow-up: @nacin
5 years ago

The security concern is that we like to keep the admin cookies limited to wp-admin only. This means a vulnerability via the front-end of the site wouldn't necessarily result in any serious compromise.

But, we already relax those rules for subdirectory installs, so we're going to need to do it for this specific case of subdomain installs as well, for now. When we bring multiple-domain support into core, with that we'll need to do cross-site logins, which would mean we can again go back to having properly sequestered admin cookies for all types of sites.

#7 in reply to: ↑ 6 @evansolomon
5 years ago

Replying to nacin:

we already relax those rules for subdirectory installs

This is what I was referring to. Maybe a better way to say it would have been that there are no new or unknown security concerns that we haven't already made a choice to accept in similar situations.

#8 @markjaquith
5 years ago

I'm +1 on .2.diff.

#9 @markjaquith
5 years ago

  • Keywords commit added

Ready to go in.

#10 @nacin
5 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 23005:

Multisite in a subdirectory: For subdomain installs, use a root admin cookie path, not a wp-admin specific one.

This is necessary because, like subdirectory installs, we will have wp-admin accessed at different levels.

fixes #22705.

Note: See TracTickets for help on using tickets.