WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 6 months ago

#25287 closed defect (bug) (wontfix)

3.6 introduced a cookie with a non-"wordpress_" prefix. Some reverse proxy setups affected.

Reported by: markjaquith Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Autosave Keywords: reporter-feedback
Focuses: Cc:

Description

A common thing to do in Varnish caching/LB layers is to drop cookies that don't match a whitelist when forwarding requests on to application servers. We should avoid new cookies that aren't prefixed with "wordpress_" so that those rules don't have to be updated as WordPress adds new cookies. Instead, a generic rule that looks for "wordpress_" can stay in place (in addition to ones related to comments and other long-established WordPress cookies).

WordPress 3.6 introduced wp-saving-post-{$post->ID}. We should change that to wordpress_saving_post_{$post->ID} (at the very least).

This issue was reported to me by Joshua Strebel at Page.ly.

Attachments (2)

25287.diff (1.6 KB) - added by simonwheatley 6 months ago.
Add cookie path and domain to 'wp-saving-post-' + post_id
25287.2.diff (1.8 KB) - added by nacin 6 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 nacin7 months ago

Also, it seems like this cookie should ideally be blog_id-safe. Or a hash of site_url (we likely do this elsewhere? there aren't many site-specific cookies).

comment:2 johnbillion7 months ago

We should also set the path and domain with (SITE)?COOKIEPATH and COOKIE_DOMAIN.

comment:3 nacin7 months ago

So, I'm actually not sure how big of a problem this is.

Other cookies in WordPress include:

  • comment_author_*
  • wp-settings*
  • wp-postpass*

While wordpress_ is used for authentication cookies by default, we do use wp- for post passwords and user settings, and then we also use comment_* for people leaving comments.

It seems that blocking 'wp-' is very important and was already an existing thing.

Additionally, johnbillion points out in IRC that we don't have any site-specific cookies ($blog_id-specific, that is). And we probably don't need one here, either, given how it is set and removed quickly.

I think all we need to do is set this cookie up with the right path and domain.

comment:4 follow-up: johnbillion6 months ago

What this needs:

  • Modify the server-side setcookie() so it also includes COOKIEPATH and COOKIE_DOMAIN.
  • Ensure that the path and domain arguments used when the cookie set client-side match these. This is in the wpCookies JS class.

We won't switch to wordpress- as a prefix because this cookies doesn't need to be whitelisted to pass through a varnish proxy.

simonwheatley6 months ago

Add cookie path and domain to 'wp-saving-post-' + post_id

comment:5 simonwheatley6 months ago

Untested patch, uploading so it doesn't get lost as I get kicked out of the venue. :)

nacin6 months ago

comment:6 in reply to: ↑ 4 azaozz6 months ago

Replying to johnbillion:

Modify the server-side setcookie() so it also includes COOKIEPATH and COOKIE_DOMAIN.
Ensure that the path and domain arguments used when the cookie set client-side match these. This is in the wpCookies JS class.

When setting a cookie, if the domain part is not set, the cookie is set only for the current page. In this case it is set only for [blog-domain]/wp-admin/post.php. Also if the expire time/date is not set, it is a "session cookie" lasting until the browser is closed (but persists if the browser crashes). Don't think we need COOKIEPATH and COOKIE_DOMAIN, this cookie is not used on any other page in the admin.

Last edited 6 months ago by azaozz (previous) (diff)

comment:7 nacin6 months ago

  • Component changed from General to Autosave
  • Keywords reporter-feedback added

I am inclined to agree with azaozz that setting the domain and path is not necessary here; and thus I'm not sure what's left. Need feedback from strebel and/or markjaquith for whether we should do anything here at all.

comment:8 markjaquith6 months ago

Considering that there are other "wp-"-prefixed cookies that have to be whitelisted am okay with WONTFIX here, and a recommendation that Varnish configs be set to pass wp-* cookies.

comment:9 nacin6 months ago

  • Milestone 3.6.2 deleted
  • Resolution set to wontfix
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.