Page MenuHomePhabricator

Enable Echo on loginwiki
Closed, ResolvedPublic0 Estimated Story Points

Description

Enable Echo on loginwiki in order to facilitate the use of the LoginNotify extension. See previous discussions at T156421 and T154064.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

The Collaboration team is okay with this, but I'd like to do a quick check with Security due to loginwiki's special security role.

@dpatrick @Reedy @Bawolff Do any of you have concerns about this?

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

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

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.)

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 :)

kaldari triaged this task as Medium priority.Feb 9 2017, 6:03 PM
kaldari set the point value for this task to 2.
kaldari moved this task from Needs Discussion to Up Next on the Community-Tech board.

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.

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.

loginwiki and votewiki are declined for security reasons.

kaldari set Security to Software security bug.
kaldari changed the visibility from "Public (No Login Required)" to "Custom Policy".

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.

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 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.

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.)

Change 336548 merged by jenkins-bot:
Enable Echo on loginwiki

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

@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

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

Change 337450 merged by jenkins-bot:
Enable Echo on loginwiki

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

kaldari claimed this task.

Echo is now live on loginwiki!

kaldari changed the visibility from "Custom Policy" to "Public (No Login Required)".

@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?

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).

kaldari shifted this object from the S1 Public space to the Restricted Space space.Feb 15 2017, 7:04 PM
kaldari shifted this object from the Restricted Space space to the S1 Public space.
Jdforrester-WMF changed the point value for this task from 2 to 0.Feb 28 2017, 12:58 AM