[21:01:07] #startmeeting RFC meeting [21:01:15] Meeting started Wed Apr 11 21:01:07 2018 UTC and is due to finish in 60 minutes. The chair is kchapman. Information about MeetBot at http://wiki.debian.org/MeetBot. [21:01:15] Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. [21:01:15] The meeting name has been set to 'rfc_meeting' [21:01:28] #topic T40417 MediaWiki's anonymous edit token leaves wiki installations (incl. Wikipedia) open to mass anonymous spam we can't block [21:01:29] T40417: MediaWiki's anonymous edit token leaves wiki installations (incl. Wikipedia) open to mass anonymous spam we can't block - https://phabricator.wikimedia.org/T40417 [21:01:43] who's here for the RFC meeting? [21:01:50] <_joe_> o/ [21:02:10] hello [21:02:15] Woohoo [21:02:25] \o [21:02:37] o/ [21:02:39] there's a meeting? [21:03:11] do you want to take it away Krinkle? [21:03:25] Sure [21:03:30] * bawolff also watching [21:03:38] So last week I've updated the task description [21:03:56] hopefully it now represents the original problem in a concise and understandable manner. [21:04:30] I've also tried to extract the various requirements (must do) we found along the way, and the obstacles/restrictions found (eg. can't do's) [21:05:33] In short: The current edit token for logged-out users is predictable. Which means an attacker could set up a phishing page that, if you were to visit it while logged-out, would edit Wikipedia invisibly on your behalf. [21:06:05] # Problem summary: The current edit token for logged-out users is predictable. Which means an attacker could set up a phishing page that, if you were to visit it while logged-out, would edit Wikipedia invisibly on your behalf. [21:06:09] #info Problem summary: The current edit token for logged-out users is predictable. Which means an attacker could set up a phishing page that, if you were to visit it while logged-out, would edit Wikipedia invisibly on your behalf. [21:06:10] (sorry) [21:10:20] I assume nobody has questions about this so far? [21:10:26] So far so good [21:10:28] <_joe_> Krinkle: what is the reason the token is predictable by a third party? [21:10:50] It's a constant value [21:10:59] It's \+ [21:10:59] Hehe, there is a reason the current token is predictable, but it's not because we want it to be predictable for third parties. [21:11:03] That's it, just a backslash and a plus [21:11:17] Which brings me to the restrictions we uncovered. Also known as: Why can't we just use normal tokens? [21:11:20] <_joe_> oh so it's just /that/ [21:11:34] It's constant to allow non-session-bearing no cookies users to edit [21:11:39] For logged-in users it's a hash of a secret value followed by +\ [21:12:01] ... which said secret value is generated on the fly and persisted in session storage [21:12:03] (the + and \ are there to deliberately cause token failures in clients that misencode things) [21:12:05] The \ and + are to check for some common form encoding bugs [21:12:06] <_joe_> Krinkle: ok so the the restriction cited there is the real reason of the problem [21:12:08] :) [21:12:11] .. and we ensure we can find it again by setting a session cookie [21:12:45] (Sorry, it's +\ per the task) [21:13:05] Anyway, so the above is what we currently do for logged-in users (and anons after they've edited) [21:13:32] Or at least after they preview. You don't have to actually save the page [21:13:39] Right [21:14:11] So you didn't write out any solutions in the task description, but some were proposed in the discussion on the task over time, right? [21:14:23] Have you wrapped you head around what they are? [21:14:31] The reason we don't use normal session-based edit tokens for anons is because the edit form itself is retreived over GET. In order to put a session-based edit token in there, MW would need to start a session (with storage in backend and with session cookie returned) in response to that GET request. [21:15:04] <_joe_> so it's a scalability problem with the session storage backend? [21:15:09] And the core assumption we made so far is that we dont' want to start sessions there because the edit page gets lots of non-human traffic that we don't want to start sessions for. [21:15:11] Indeed. [21:15:30] Very specifically, I want to point out that we do not care about whether anons bypass page view cache if they see the edit page once. [21:15:32] Not the session storage backend so much as the fact that everyone with a session bypasses Varnish for page views, I thought [21:15:36] Or at least, I don't care. [21:15:37] <_joe_> ok, so that's something that needs to be evaluated [21:15:56] Oh? [21:16:02] But maybe I'm wrong. I suppose that could be a secondary reason for not wanting to start too many sessions. [21:16:22] But it seems like a micro optimisation to start on preview and save, but not want to session on edit view. [21:16:27] I mean if we can avoid it (and I think we can), then it's better to not have more people bypass Varnish than already do [21:16:29] <_joe_> no I mean, that's clearly an issue potentially, but I'd like to quantify it [21:16:35] I believe historically there were concerns that that could take down the site, but I think that was more from the 2003 era [21:16:55] Yeah I'd love more up to date quantification of this. Last I heard it was still a concern tho [21:17:03] <_joe_> we could set a short-lived session maybe [21:17:21] So, one of the proposals I heard for this last week was interesting [21:17:25] Right, so there's three concerns: 1) session storage scaling, 2) app server capacity for users who don't preview/save staying in a session 3) performance for end-users after visiting an edit page without doing anything. [21:17:31] I think 3 is not worth talking about today. [21:17:32] There's also a lot of people who just click the edit button out of curiosity. If we started a session then and there, they would be bypassing varnish cache for the rest of it, and thus get a slower browsing experiance [21:17:35] When an anon visits the edit page, set a cookie with a token in it [21:17:44] <_joe_> Without better data, I don't know what to say about 1 or 2 [21:17:49] Then when they submit an edit, verify they send back that cookie [21:17:59] BUT don't create a session and don't make that cookie bypass Varnish [21:18:25] This was the token verifiable via public key? [21:18:31] we don't want to do writes on GET requests in MW, but if the session storage is separated out and scales appropriately and/or is m-dc-ready, then that becomes a non-issue [21:18:36] Instead only the GET for the edit form and the POST with the edit need to bypass Varnish, and they already do [21:18:36] you just need to take the random ID from the cookie, HMAC it, and put it into the form [21:19:08] bawolff: yes, but that's a problem for all editors and logged-in users, I don't think that should be a blocker given it already affects all contributors, and there are separate roadmaps and RFCs about that (e.g. page composition etc.) [21:19:20] Oh right, it needs to be in a form, you can't just check that they have the cookie because that doesn't prevent CSRF [21:19:39] Before we explore solutions, I think it's worth taking a minute to revisit that _joe_ said about the underlying assumptions. [21:19:46] right, CSRF protection comes from verifying that the cookie matches the form submission [21:20:06] <_joe_> RoanKattouw: yeah if we could avoid caching the edit page, we could do CSRF and wouldn't that be enough to avoid the current issue? [21:20:21] I also think its kind of cool that you can edit right now without cookies enabled if you're not logged in [21:20:36] the form token is fetched with a GET that can't be done cross-origin [21:20:45] We already don't cache the edit page [21:20:50] _joe_: No, the edit page already bypasses varnish (although I think currently it can combine similar requests during a short time window). [21:20:54] But that's a smal difference [21:21:06] Anyway, to Krinkle's point about underlying assumptions: I think it's enough to assume that we don't want to create all these sessions, even if we don't agree about why [21:21:51] <_joe_> RoanKattouw: we need to keep state in some way if we want to be able to verify something server-side [21:22:00] Not necessarily [21:22:07] As Tim said, you can use an HMAC [21:22:11] <_joe_> yes [21:22:21] OK. I'm willing to skip over that. I was going to do that originally, but found interesting that _joe_ finds it possible that maybe session storage and/or app capacity is not an issue. [21:22:23] hmac(id|ua) doesn't help because the remote site has that information [21:22:25] Generate a random value, give that as a cookie to the anon, then HMAC that random value and put that as the token in hte form [21:22:33] it has to be hmac(random cookie) [21:22:51] I meant hmac(ip|ua) doesn't help [21:22:55] <_joe_> TimStarling: when do you assign the cookie? [21:23:10] <_joe_> on any pageview? [21:23:18] * Krinkle doesn't know which proposal is being described at the moment. [21:23:21] On an action=edit GET request I think [21:23:28] on edit, the edit page would deliver a Set-Cookie header and also provide the token in the body of the same request [21:23:33] (or a token request through api.php) [21:23:40] I think we can do hmac(ip, $wgSecretKey) [21:23:43] <_joe_> TimStarling: ok, that makes sense [21:23:44] since the anon has no state [21:23:45] Krinkle: We are discussing the hmac-of-random-value-in-cookie proposal [21:24:03] so we only have to verify its from the right IP [21:24:12] bawolff: IP has previously been rejected. Too variable. [21:24:24] <_joe_> I was about to say that [21:24:30] <_joe_> mobile is a curse :) [21:25:18] <_joe_> Krinkle: back to the scalability issues: I'm saying that without data about pageviews and behaviours we're doing wishful thinking. Of course if we can get away with more caching, it's better [21:25:54] Would it make sense to do targeted testing of session enabling for anons to check the actual numbers? [21:25:55] * DanielK_WMDE remeinds everyone to make copious use of #info. makes it much easier to read the log later. [21:26:18] <_joe_> brion: I think analytics can help getting some numbers [21:26:22] So the idea is to generate a random crypt that isn't stored anywhere (and no session), and send it in a cookie to the user. This cookie would not cause cache miss. But given that POST requests do bypass cache (naturally) the server will have its crypt value there available to verify the hmac. [21:26:44] <_joe_> yes [21:27:10] And users would get this cookie on first view? [21:27:18] bawolff: of the edit page, yes. [21:27:59] #info So the idea is to generate a random crypt that isn't stored anywhere (and no session), and send it in a cookie to the user. This cookie would not cause cache miss. But given that POST requests do bypass cache (naturally) the server will have its crypt value there available to verify the hmac. [21:28:32] <_joe_> so AIUI, that HMAC would be checked only if a session is not started yet [21:28:43] It could have standard expiry, or shorter if we want (at the expense of increasing likelihood of token failure the longer the user takes to draft their edit). I suppose it would make sense to use the same expiry here as we normally do for csrf tokens. [21:28:45] <_joe_> if there is a session, that takes precedence, right? [21:28:55] And upon submission, we convert to a session like now, and can clear the cookie. [21:29:48] _joe_: Yeah, it should take precedence [21:29:51] _joe_: Yes. More precisely, you'd be validating an edit token in both cases, but how you determine whether the token is correct differs (HMAC of the cookie in the no-session case, value from session storage in the session case) [21:30:07] #info If there is a session and an anon-csrf-crypt cookie, session should take precedence. [21:30:40] <_joe_> Krinkle: yes. Introducing a new cookie is not ideal for edge caching (we need to check if we need to clean it up, I don't recall which limitations are there), but it's surely better than bypassing the cache much more than it's necessary [21:30:40] #info What about editing without cookies. [21:31:09] <_joe_> bawolff: if you're not willing to accept cookies, you will not be able to establish a session either [21:31:10] although tbh, editing without cookies is something we'll probably just have to give up on [21:31:22] _joe_: last I checked, arbitrary cookies don't matter to varnish. We only have a blacklist of cookies we use for bypassing cache (e.g. session cookies) and the rest is stripped on GET and preserved otherwise. [21:31:27] <_joe_> it creates all the sort of breakages with our current assumptions [21:31:46] At the moment you don't need a session to edit [21:31:49] <_joe_> Krinkle: I'm 99.9% sure as well, but there are edge cases (analytics and all) [21:32:19] <_joe_> bawolff: once you edit, if you refuse to set up an anon session, you probably don't even see your edit [21:32:20] As I understand it, sessions are required to login, and sessions are required to bypass cache as anon editor (so that you get talk page notifications etc.). But without session/cookies, they're edit will still work. [21:32:45] _joe_: actually, they probably will, but maybe after 1 refresh, we do purge Varnish post-edit. [21:32:48] <_joe_> yes, they work, but it's already a miserable user experience [21:33:12] <_joe_> also, "anons" expose their IPs, which makes them actually more vulnerable to state actors, but I digress [21:33:12] Perhaps we should gather data on it. [21:33:30] <_joe_> a really privacy conscious user edits wikis logged in :P [21:33:30] #info Maybe gather stats on non-cookie anon edits. [21:33:36] The only thing is that we'd have to tell Varnish to disable request coalescing for non-session GET requests to ?action=edit (and other forms with tokens), right? [21:33:56] RoanKattouw: Indeed. Probably by changing it from maxage=0 to private, if not already. [21:34:07] Or vary: everything [21:34:11] :P [21:34:11] Right, I was hoping there was something in Cache-Control that allowed us to d othat [21:34:29] I don't think vary is a good idea, there must be an easier way to tell varnish not to cache/share something? [21:34:37] CC: private should work right? [21:34:39] Or will it always do that if all input seems identical between requests? [21:34:39] <_joe_> RoanKattouw: I thought we said we already bypass the cache for the edit page? [21:34:49] _joe_: Yes but Krinkle said that coalescing can still happen [21:34:51] What about - user goes to edit page, user does something else that starts a session, user saves page [21:34:51] <_joe_> Krinkle: vary is a very bad idea, yes [21:34:59] RoanKattouw: Ignore me if that is just me thinking that, I haven't confirmed. [21:35:12] OK [21:35:18] <_joe_> it's better to not cache that page completely, imho [21:35:19] I kind of think that on the initial save, it should work with both session or cookie thing, in the event of request interleving [21:35:22] I just assumed given that to varnish two edit page requests for the same page may seem identical, it might re-use it. [21:35:25] bawolff: If they started a session by logging in, then that already breaks (and is arguably a feature) [21:35:41] hmm, that's true [21:35:46] But they could open two anon edit tabs, then save one then save the other, and that would break [21:35:51] and session failure is not a big deal [21:35:55] <_joe_> Krinkle: if you set the cache headers correctly, the cache will be bypassed [21:36:05] <_joe_> RoanKattouw: yes [21:36:49] <_joe_> RoanKattouw: wait, if your two tabs share cookies, as they do, once you save one tab you have a valid session [21:36:58] Yes but the token is encoded in the form HTML [21:37:03] And that one won't be valid anymore [21:37:14] <_joe_> so your POST request will include the session cookie, which has precedence, right? [21:37:18] Because submitting the first edit creates a session [21:37:26] Yes, and that precedence is exactly why the token will be wrong [21:37:41] The token is encoded into the HTML of the edit form, and you requested that HTML before you had a session [21:37:45] <_joe_> ok I assumed we'd ignore the token [21:37:55] <_joe_> if the request includes a session cookie [21:38:02] <_joe_> a vaid one [21:38:06] No, we can't, for CSRF reasons [21:38:07] <_joe_> *valid [21:39:11] <_joe_> right, I see your point [21:39:18] 3rd party web sites can use AJAX to send POST requests to us, and those requests will have that session cookie [21:39:42] So in order to prove that that's not what happened, you need to send along a token that (if we did our jobs right) can't be obtained using cross-domain stuff [21:40:11] <_joe_> yeah I didn't realize we didn't store any token in the session that can be on the page [21:40:29] For api.php we make you make a separate request to api.php to get the token, but for traditional HTML forms we put the token in the form as an tag [21:40:58] confirmed, we use 'max-age=0, no-cache, no-store, must-revalidate' from app servers on edit page for anons [21:41:07] an anecdote from the time when the mobile team tried to give people visiting S:MobileOptions sessions to protect from CSRF: we had at least 1/3 token failures [21:41:11] Which should not be re-used by varnish no matter how similar the requests are [21:41:43] it doesn't say private though.. [21:42:05] Semantically it probably should say private once we have tokens in there [21:42:13] no-store is stricter than private, right? [21:42:21] but yeah, couldn't hurt to add it. [21:42:34] Is it? idk how no-store interacts with coalescing [21:42:36] It comes from an else branch in OutputPage that says "private" [21:42:40] Yeah [21:42:42] OK. [21:43:04] But thinking about the timeline of coalescing, there might also have to be rules in VCL to prevent it from happening [21:43:10] Or prevent it from trying, at least [21:43:18] <_joe_> Krinkle: indeed, from what I see varnish makes the page uncacheable to clients too [21:43:22] #info <‎bawolff‎>‎ editing without cookies is something we'll probably just have to give up on [21:44:09] So regarding session precedence, I think there's two things going on here but maybe they're the same [21:44:19] #info confirmed, we use 'max-age=0, no-cache, no-store, must-revalidate' from app servers on edit page for anons [21:45:05] 1) mw session (post-submit) vs new anon-crypto cookie, which should be checked for on submission; 2) for cross-origin abuse cases, we must ignore the cookie. [21:45:30] What do you mean by #2? [21:45:34] the former was intended to be about genuine contexts, I assumed there that of the two cookies, the session should be preferred. [21:45:43] Given the other may be left-over in race conditions. [21:45:48] And seems more secure anyway? [21:45:52] But might be a weak argument. [21:46:49] I can't come up with reasons why it'd be more secure, but I feel better about preferring the session in cases of ambiguity [21:46:56] #2 is what you described with regards to cross-origin, where (naturally) for security reasons we don't authenticate based on cookie but also require a token in the form itself, which is what csrf is all about. [21:47:29] Oh, I see what you're saying [21:47:44] And browser-side, we don't support cross-origin edits via the API, even if you obtain it first, because there's no authenticity on that. [21:47:57] Right now one of the ways that we make sure tokens don't leak over JSONP is that we unauthenticate the user when JSONP is used (and pretend they're an anon) [21:48:02] That's going to stop being bulletproof [21:48:24] There's still separate logic saying we won't give tokens over JSONP, but the unauthentication is meant to be a backstop [21:48:37] We'd have to add something to that backstop that also forgets the random cookie [21:49:19] "we don't support cross-origin edits via the API" that's not technically true, you can POST whatever you want to whereever you want, you just can't read the response [21:49:30] Let's wrap up. Do we think there may be concerns in these details where there might not be a viable outcome? If not, I propose we figure that out in Code review later. [21:49:44] I think this is all very surmountable [21:50:14] There are other little things I can think of, like making sure that token-getting functions aren't called too often so that we don't set random cookies when not needed, but as you say that can all be dealt with in code review [21:50:47] However your point about having to ignore the cookie for JSONP is a very good one, I hadn't thought of that [21:51:03] RE: cross-origin edits, we support it if you login first, via the API (I think) and technically one could also submit edits then. For anon editing, it would only work if they obtain a csrf token first, which requires POST and won't work browser-side (opaque response), they could do it server-side and transfer to the client, but then the crypto cookie will be missing. [21:51:14] + we should also prevent ourselves from setting cookies in JSONP mode [21:51:32] #info < RoanKattouw > we should also prevent ourselves from setting cookies in [API] JSONP mode [21:51:41] waaaait [21:51:57] Hmm you can't set a cookie for another domain so I think it's fine [21:52:20] And tokens expire [21:52:43] * DanielK_WMDE is waving the 7 minute flag [21:52:45] hmm [21:52:57] Never mind, I was concerned about someone replaying a secret cookie + token pair from one machine to another, but I think that probably can't be done? [21:53:08] You'd have to do it in (near) real time because tokens expire [21:53:11] Indeed, because the cookie still has to match as well. [21:53:27] And you'd have to somehow inject that cookie into the client you're attacking, which I don't think you can do [21:53:37] We don't act on cookies along as authentication, but we also don't ignore them. [21:53:41] alone* [21:53:49] If you found an XSS somewhere in the site you could [21:53:55] but that's a bigger issue than a csrf [21:53:57] Sure but then we already lose [21:54:15] Krinkle: Yeah it's the cookie+token pair that matters, but as a CSRF attacker you have control over the token [21:54:34] Right, but csrf basics are: 1) authenticated session (cookie-based), 2) form token (not obtaintable cross-domain) [21:54:54] you can also set cookies for other subdomains, so there could be situations where you have an xss on a less valuable subdomain that you can leverage to set it on the more valuable subdomain [21:55:12] Yes, but the big difference here is that a cookie+token pair that is valid for person A is also valid for person B [21:55:33] Let's set the bar at our existing csrf practices :) [21:55:45] Right, by that standard none of this matters [21:55:57] Because our existing practices are that anons are completely unprotected [21:56:01] Without browser intervention, those practices seem the best we can do with the tools available. [21:56:21] If someone could do what bawolff describes, all that would give them is the ability to CSRF an anon the same way they can already do today (but with more effort) [21:56:30] yeah, cookie+token pair being valid for person a & person b is true of the existing session based system [21:56:38] and would give logged-out users the same level of csrf verification as for logged-in users, only difference being where teh secret key is stored (client-sdie instead of server-side) [21:56:41] Not really, because there it's tied to identity [21:57:12] * kchapman just a few minutes left. [21:57:17] Under the existing system for logged-in users, if I can trick you into making an edit with my token and my session cookie, it gets attributed to me, so there's no point in me trying to do that [21:57:32] Under the proposed system for anons, if I pulled that off, the edit would be attributed to your IP instead of mine [21:57:36] Hm.. good point. [21:57:39] There was some past discussion on cookie over different subdomain setting on T113790#1676633 (sorry, that's a secret bug) [21:57:53] In any case I think that's probably a minor point [21:57:57] I suppose checkuser could still expose it was not your IP but that's pretty thin indeed. [21:58:07] And we seem to have been OK with leaving anons completely unprotected for over a decade [21:58:10] This would be fixed by Tim's IP name proposal based on sessions. [21:58:13] (maybe) [21:58:23] Then it woudl be attributed to your IP :) [21:58:30] Krinkle do you have what you need to take next steps with this RFC? [21:58:49] https://phabricator.wikimedia.org/T172477 [21:58:50] RoanKattouw: ^ [21:58:54] kchapman: yes, thank you :) [21:59:19] #info This would be fixed by Tim's IP name proposal based on sessions. [21:59:28] I wanted to say a big thank you to Krinkle for working on this. It makes me really happy this issue is being worked on :) [21:59:29] #link https://phabricator.wikimedia.org/T172477 [21:59:38] Feel free to continue in #mediawiki-core for anyone having questions or follow-up. [22:01:13] thanks all last chance to add anything to the minutes before I close the meeting [22:03:17] #endmeeting [22:03:18] Meeting ended Wed Apr 11 22:03:17 2018 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) [22:03:18] Minutes: https://tools.wmflabs.org/meetbot/wikimedia-office/2018/wikimedia-office.2018-04-11-21.01.html [22:03:18] Minutes (text): https://tools.wmflabs.org/meetbot/wikimedia-office/2018/wikimedia-office.2018-04-11-21.01.txt [22:03:18] Minutes (wiki): https://tools.wmflabs.org/meetbot/wikimedia-office/2018/wikimedia-office.2018-04-11-21.01.wiki [22:03:18] Log: https://tools.wmflabs.org/meetbot/wikimedia-office/2018/wikimedia-office.2018-04-11-21.01.log.html