Make WordPress Core

Opened 12 years ago

Closed 12 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's profile markjaquith Owned by: nacin's profile 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 12 years ago.
22705.2.diff (577 bytes) - added by nacin 12 years ago.

Download all attachments as: .zip

Change History (12)

@markjaquith
12 years ago

#1 follow-up: @nacin
12 years ago

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

@nacin
12 years ago

#2 in reply to: ↑ 1 ; follow-up: @nacin
12 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
12 years ago

  • Keywords has-patch needs-testing added

#4 @ryan
12 years ago

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

#5 in reply to: ↑ 2 @evansolomon
12 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
12 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
12 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
12 years ago

I'm +1 on .2.diff.

#9 @markjaquith
12 years ago

  • Keywords commit added

Ready to go in.

#10 @nacin
12 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.