Page MenuHomePhabricator

Insert CheckUser row events during certain 2FA actions
Open, MediumPublic

Description

It seems it could be beneficial having OATHAuth logging to CheckUser (if installed) during certain 2FA events:

Related Objects

Event Timeline

We don't log stuff like password change or email change to CU do we? I think 2FA should be at the same level of those. The only exception IMO is when someone else removes 2FA via the special page, where we add a log entry. I'll submit a patch for that in a few minutes.

I do think we should have MW debug logs for all the cases you mentioned with CU-like data if it's not already in place.

Change 763654 had a related patch set uploaded (by Legoktm; author: Legoktm):

[mediawiki/extensions/OATHAuth@master] Send log entries to CheckUser

https://gerrit.wikimedia.org/r/763654

We don't log stuff like password change or email change to CU do we? I think 2FA should be at the same level of those. The only exception IMO is when someone else removes 2FA via the special page, where we add a log entry. I'll submit a patch for that in a few minutes.

I do think we should have MW debug logs for all the cases you mentioned with CU-like data if it's not already in place.

I wonder if we should just be improving this sort of logging more generally. Maybe a bigger discussion to be had...

CUs/stews often look into account compromises, so I think it is somewhat reasonable to do so, but like Majavah said on the Gerrit patch, that is a shift from what we currently do.

If we are considering all auth-stuff, I think we should have a "account security events" log like other sites do, that track when you changed your password, email, etc. and shows it to you so you can audit your own account security. I think a framework like that belongs in core (something something E:AccountInfo!). And if we had that, I think it would be reasonable to allow trusted users including possibly CheckUsers to access it, but I don't think mixing it all in with CU makes sense.

Having looked at this my thoughts are:

  • There is concern about showing entries from the hidden log to users with checkuser rights
  • Having the IP, XFF and user agent associated with this log entry in Special:CheckUser is useful for compromise investigations. While the server logs can be inspected this requires someone with that right to do so which may take too long in a case of an urgent compromise check.

I agree that this would be good to have stored in the CheckUser table. One solution to the issue of showing the entries is by hiding the actiontext associated with the hidden log entry unless the user has the right to view the log. This can be done once migration in T324907 is stably set to read new as entries detailed in the task description would go into cu_log_event and store the associated logging table ID. CheckUser can then inspect the log entry to find out whether it can show it, displaying everything but the action text to the checkuser with some description about the entry being hidden because they need a right. The CheckUser can then pass the log ID that would be shown to a user with the right so they can see what the event was.

This method of hiding the actiontext for protected logs when the user does not have the right could be applied to many other actions, including the suppress log. The CheckUser would not be able to see what the log item was and only that they don't have the right to view it.

Change #763654 merged by jenkins-bot:

[mediawiki/extensions/OATHAuth@master] Send log entries to CheckUser

https://gerrit.wikimedia.org/r/763654

If I understand correctly, the only thing we are not logging listed in the description now is enrollment right ? Do we want to close this ticket, split that off, or are we good with the current state ?

If I understand correctly, the only thing we are not logging listed in the description now is enrollment right ? Do we want to close this ticket, split that off, or are we good with the current state ?

AFAIK we are currently missing:

  • Failed 2FA login attempts
  • Enrolment into 2FA

As to whether we want to split the ticket, I'm happy either way. I think we probably still want to log failed 2FA attempts to CheckUser, given that failed attempts to verify 2FA might indicate attempted account breaches where the password is known. Currently (AFAICS) the CheckUser data would only log a generic failed to login attempt, which wouldn't indicate the password used was correct (AFAIK).

Enrolment into 2FA seems less useful to log, as long as the enrolment was performed by the user themselves.

I think it is good to split out tickets whenever delivered functionality is spread over multiple MW releases (we probably don't do enough of that), so i made a separate ticket.

I agree enrollment is less useful indeed. However I think I have heard of attacks outside wikimedia where attackers enabled 2FA after takeover of an account. But i'm pretty sure other signals are more important and a better indication.

I agree enrollment is less useful indeed. However I think I have heard of attacks outside wikimedia where attackers enabled 2FA after takeover of an account. But i'm pretty sure other signals are more important and a better indication.

If you think enrolment should be logged, then if you could file a task for it? Then we can close this out (as you have suggested).