Page MenuHomePhabricator

Cookies should not be forwarded to different domains
Open, LowPublic

Description

Whenever a RESTBase module sets mediawiki_auth_filter.js as a route filter, all subrequests it makes with the forward_headers options on get the client cookies copied on them. This is a problem for Reading Lists which (in the case of the GET /lists/{id}/entries/ endpoint) makes requests to both the local API (for getting the entries) and remote APIs, to hydrate page summaries; the local request needs the cookies, the remote requests should not have them.

This is (at least with the current code) a very fringe problem as the RESTBase URLs do not have forward_headers enabled, and cross-domain MW API requests only happen when the summary request triggers a siteinfo request (which only happens once per domain due to caching). So cookies only get forwarded to the wrong place when a Reading Lists summary hydration is the first time the server encounters the given domain (ie. nearly never). Nevertheless, it feels wrong.

Event Timeline

Tgr created this task.Nov 2 2017, 1:13 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 2 2017, 1:13 AM
mobrovac triaged this task as Low priority.Nov 2 2017, 9:41 AM
mobrovac added a project: Services (later).
mobrovac added a subscriber: mobrovac.

RESTBase's mediawiki auth filter is meant to be used only on routes that both need auth(n|z) and whose sub-requests need cookies to complete the action. In your case, that can be achieved by creating an internal end point (in the /sys/ hierarchy) that calls the MW action API for manipulating lists and then declare the auth filter on that route only.

In practice, though, this is not a concern for our environment since (i) we control which domains are to be reached; and (ii) we don't allow RESTBase to make requests outside of the production environment.

That said, we may want to revisit the way the filter works at some point and try to cover such cases.

In your case, that can be achieved by creating an internal end point (in the /sys/ hierarchy) that calls the MW action API for manipulating lists and then declare the auth filter on that route only.

Thanks, that sounds like a good way to handle it.

In your case, that can be achieved by creating an internal end point (in the /sys/ hierarchy) that calls the MW action API for manipulating lists and then declare the auth filter on that route only.

Thanks, that sounds like a good way to handle it.

I actually think this will not work. The auth filter stores the headers that need to be forwarded in the per-request context, so when we're obtaining the list the headers will be stored in the context, and then for all subsequent requests originated by this global external requests the cookies will be in the context. Since we allow the headers to be forwarded to any WMF domain, when requesting summaries cross-domain this will now start forwarding the cookies to all the domains.

For headers other then a cookie is actually a good thing - we're forwarding x-client-ip, user-agent and x-forwarded-for by default (although it might be arguable that x-client-ip is private piece of info and should not be forwarded, since we only allow forwarding to WMF domains it's fine.

If we want special treatment for the cookie header, we need a special same-domain-only cookie parameter, and change the filter to store the root request domain alongside with the cookies in the request context and then check that each of the individual subsequent request domains match it.