Description
Details
Subject | Repo | Branch | Lines +/- | |
---|---|---|---|---|
Enable Echo on loginwiki | operations/mediawiki-config | master | +0 -1 | |
Enable Echo on loginwiki | operations/mediawiki-config | master | +0 -1 |
Related Objects
- Mentioned In
- T140167: Security Review of LoginNotify extension
T154064: Investigation: What would be the best way to support loginwiki from LoginNotify - Mentioned Here
- rOMWC3ceb0cddaf61: Remove loginwiki from echowiki dblist
T97760: Enable Echo on all Wikimedia wikis
T154064: Investigation: What would be the best way to support loginwiki from LoginNotify
T156421: Disable login on loginwiki
Event Timeline
I don't see any issues straight off... Probably wants a little deeper digging first though..
'loginwiki' => 'Wikimedia Login Wiki',
I presume some override of the sitename is going to be made (to what exactly, I'm not sure)? As notifications from "Wikimedia Login Wiki" is going to be both weird and confusing... Or, how exactly would it be used in the notifications from LoginNotify etc when enabled? Sorry if that has been discussed already on the other tickets
how exactly would it be used in the notifications from LoginNotify etc when enabled?
"notification-loginnotify-login-fail-new-emailbatch": "There {{PLURAL:$2|has been a failed attempt|have been $2 failed attempts}} to login to your account '$1' on {{SITENAME}} from a computer you haven't used before.", "notification-loginnotify-login-fail-known-emailbatch": "There {{PLURAL:$2|has been a failed attempt|have been $2 failed attempts}} to login to your account, '$1' on {{SITENAME}}.", "notification-loginnotify-login-success-emailbatch": "Someone has successfully logged into your account '$1' on {{SITENAME}} from a computer which you have not edited from recently."
"Wikimedia Login Wiki" is fine with me. Really it doesn't matter that much, as we aren't expecting the user to actually visit the site in question. We just want them to know that someone is trying to log in as them. There is very little chance that a hacker will be trying to use loginwiki anyway. Enabling Echo there is just to close the 1 obscure hole in LoginNotify's ability to detect login attempts.
Aha, ok. I wasn't sure if you were going to be making all of the login notify messages come from loginwiki or similar :)
Change 336548 had a related patch set uploaded (by Kaldari):
Enable Echo on loginwiki
Hey @Reedy, can you +1 the config change so that I have sign off from security (otherwise they won't let me merge it). I talked to Matt and he says it's ready to go. (All the backend requirements are in place.)
Security meeting has been moved to tonight... I'll discuss it with Darian and Brian quickly to check they have no obvious objections. Then I'll give a +1/-2 as appropriate after :)
I have no objections for what its worth, although I'm not sure what the details are behind it not being enabled in the first place.
I have no objections for what its worth, although I'm not sure what the details are behind it not being enabled in the first place.
No idea. Presumably the assumption was that no one actually used loginwiki (besides the CentralAuth extension), so there was no reason to have Echo there.
Still need a +1 from someone on Security (@Reedy, @dpatrick, @Bawolff): https://gerrit.wikimedia.org/r/#/c/336548/
Since we're about to discuss security issues and attack scenarios, restricting visibility to Security (and existing subscribers).
Reading through T97760, it looks like the rationale for not enabling Echo on loginwiki was simply that it wasn't needed there and having fewer extensions meant a smaller attack surface. Now, however, we have a security trade-off. If we don't have Echo on loginwiki, hackers could presumably use that wiki to avoid detection by the LoginNotify Extension.
It was removed in rOMWC3ceb0cddaf61: Remove loginwiki from echowiki dblist a long time ago.
I know of no specific reason for it not to be on loginwiki, which is why I have no objection here, but I do think it should be considered by Security (as it is being).
My understanding matches what was said at T97760 (general "lower attack surface" goal).
You can just make another wiki send the email, via the job queue.
True, but we'll need to have Echo turned on anyway in order to be able to generate the notification.
I think it's fine to enable it tbh; there is a specific use case (and as such, reason) to do so.
It was about reducing attack surfaces, and generally just disabling stuff that had no benefit to be enabled on the wiki
CR +1 on the patch on gerrit
You don't have to; you can put all Echo-specific code in the job class (which will then run on another wiki). It's unnecessary complexity though (unless Security objects to installing Echo).
Then again, maybe it makes sense for language reasons? (We might prefer sending login warnings in your home wiki language over the language of whatever wiki the attacker happened to log on at.)
@Mattflaschen-WMF: When I tried turning on Echo on loginwiki I got the following error:
A database query error has occurred. Did you forget to run your application's database schema updater after upgrading? Query: SELECT * FROM `echo_event` INNER JOIN `echo_notification` ON ((notification_event=event_id)) INNER JOIN `echo_target_page` ON ((etp_event=event_id)) WHERE event_deleted = '0' AND notification_user = '995' AND notification_read_timestamp IS NULL AND etp_page = '1' Function: EchoEventMapper::fetchUnreadByUserAndPage Error: 1146 Table 'loginwiki.echo_target_page' doesn't exist (10.64.16.18)
echo_target_page doesn't seem to exist on any of the wikis though. Is this a shared table?
wmgEchoCluster should be extension1 for loginwiki
Tables presumably still need creating?
Change 337450 had a related patch set uploaded (by Kaldari):
Enable Echo on loginwiki
Sorry, I expected (wrongly) that if echo_event was there, the others were too. Looks like they got out of sync (probably because it was disabled).