Why is this bug marked as private? I'm not seeing anything obviously sensitive here.

I'm not entirely sure whether this was on purpose.

The fact that tokens change based on the time they're generated is necessary behavior of the time-limiting: the timestamp must be included (in "plain" text) in the token so it can be checked, and it must be included in the hash so it can't be faked.

We should implement a way to bypass the minification cache.

Or the tokens could just be fetched from the API when they're needed instead of being served with every request.

In T84960#936308, @Anomie wrote:

Why is this bug marked as private? I'm not seeing anything obviously sensitive here.

I filed it under security because the varying tokens, while sensible in general, wasn't necessarily applied on purpose for API requests from the web interface. There also seems potential abuse against the minification cache. Or at the very least a performance/scale hazard for sites without LRU in their cache system. MediaWiki's the default DB cache, for instance, would get blown up by this (my localhost install did).

In T84960#936308, @Anomie wrote:

Or the tokens could just be fetched from the API when they're needed instead of being served with every request.

That would require fair amount of token fetching from the client-side for AJAX actions originating from core modules (e.g. watch), as well as gadgets and extensions.

I'd like to better understand why tokens are time-limited. I assume it has to do with reducing impact of compromise. However does that apply to client-side tokens? The client is able to fetch tokens whenever using its session cookie (which doesn't mutate on every request). Sessions do expire, however. And having tokens last longer than the session seems odd. Perhaps we can associate them with a session and only give clients new tokens once per session? If I recall correctly, that was the previous behaviour as well for CSRF tokens used in HTML (e.g. for EditPage). Or was there a problem with that?

I have no idea what distinction you're making between "client-side" tokens and other tokens. All tokens are sent from the server to the client via a channel that CSRF attackers shouldn't have access to, and then sent from the client back to the server to detect CSRF.

The point isn't to make the tokens last longer than the session. That won't happen anyway, if the session is lost then all the tokens go away too just as they have always done. The point is to make it possible for tokens to expire before the session does to reduce the window of vulnerability if an attacker somehow manages to compromise a single token, with a predictable lifetime after generation for each token. You won't get a 5-minute token and be able to extend it indefinitely by re-requesting it, or get a 5-minute token but then requesting it again after 4 minutes gives you the same token that will expire in only 1 minute, or get a 5-minute token but then it expires early because some other code requests another instance of the same token, and you won't be able to flood the session with hundreds of tokens waiting to expire. The constantly-changing nature of the tokens also protects against attacks such as BREACH which depend on multiple requests to reveal individual bits or bytes of a constant token.

Now, if the caching situation really can't be worked around by requesting the tokens from the API as needed, we might be able to make User::getEditTokenAtTimestamp() public and use that to generate a token with a constant distant-past timestamp for your JS. Uses of the token that don't specify a maximum age will accept such a token as long as the session is valid, while uses that do will still treat them as expired. But I'd wait for Chris to look that plan over before spending a lot of time on it.

In T84960#950866, @Anomie wrote:

The point isn't to make the tokens last longer than the session. [...] You won't get a 5-minute token and be able to extend it indefinitely by re-requesting it, or get a 5-minute token but then requesting it again after 4 minutes gives you the same token that will expire in only 1 minute, or [..]

I understand that. My point is that it seems useless to embed tokens in the html that client-side JavaScript (from core, extensions or gadgets) cannot reliably use because by design it will always expire after a few minutes. Similarly it seems useless that VisualEditor and other uses of tokens almost always have to make two or three round trips just to make a write action. Right now its three. 1) Failed write action for token expired, 2) get new token, 3) Re-try write again. Not only does it make two more http requests then desired, it also transmits a potentially large amount of data to the server multiple times. We could cut this back to 2 requests by making the client-side aware of how long the tokens are valid for. While that is better, it is imho not good enough.

Two facts:

• In the past we made the decision to embed the user.tokens module in the page because we saw performance problems with the client-side having to request a token for write actions by JavaScript (for e.g. watch star click, or VE edit save).
• In the past we made the observation that saving content with VisualEditor is a performance bottle neck, that we worked hard to cut down. Due to this token expiration problem, we basically nullified all that effort by having to submit everything twice for a lot of users.

As such, I would recommend strongly that we figure out a feasible way to allow MediaWiki frontend to perform write actions with the API without needing to fetch a token over a separate HTTP request. The client's cookie is already authority enough to request as many tokens as it wants during the session, so in theory it seems really stupid that we need a token at all when making a write action. Clearly whatever origin can make or forge one request, can also forge the other. Raising the bar might make sense to just make it a tiny bit harder, but I'm questioning whether that's worth it.

From this perspective, this change was a significant regression as both of these two expectations are no longer met by the system.

The naive approach of not making the tokens change every request is understandably not acceptable from a security perspective because of "BREACH"-type reflection attacks. The only option I left then is to continue issuing new tokens each page view but have those tokens (that are embedded in the page) be valid for the entire duration of a page view (or otherwise, at least for an hour or so).

In T84960#1284083, @Krinkle wrote:

I understand that. My point is that it seems useless to embed tokens in the html that client-side JavaScript (from core, extensions or gadgets) cannot reliably use because by design it will always expire after a few minutes.

Except that's not true. I see nothing in MediaWiki core or WMF-deployed extensions that uses the $maxage parameter for User::matchEditToken() except the ApiCheckTokens module. Maybe Restbase is doing it, but nothing currently in PHP is considering tokens expired any sooner than the end of the session.

In T84960#1284121, @Anomie wrote:

In T84960#1284083, @Krinkle wrote:

I understand that. My point is that it seems useless to embed tokens in the html that client-side JavaScript (from core, extensions or gadgets) cannot reliably use because by design it will always expire after a few minutes.

Except that's not true. I see nothing in MediaWiki core or WMF-deployed extensions that uses the $maxage parameter for User::matchEditToken() except the ApiCheckTokens module. Maybe Restbase is doing it, but nothing currently in PHP is considering tokens expired any sooner than the end of the session.

I'm guessing VE is submitting to restbase, and restbase is using ApiCheckTokens to check the token. I've lost track of who did that integration (Roan? Gwicke?), but I'm guessing that call needs to fix the maxtokenage param?

Here's a graph showing a successful drop in RL-minification cache lookups after deploying I6016e4b01e44.

Labels added to match relevant events from the Server Admin Log.

Source: https://graphite.wikimedia.org/render?from=-4weeks&until=now&width=900&target=MediaWiki.rl-minify-js-cache-misses.count