Page MenuHomePhabricator

CentralAuth: Locked accounts can keep editing after lock is made
Closed, ResolvedPublicSecurity

Description

Summary

A globally locked account was observed continuing to edit and hold an active session for nearly an hour after the lock.

Technical notes

  • Observed on LivMaddieFan26M3: account locked at 07:34, edit at 07:47, "IP change within the same session" entries in Logstash as late as 08:36:27.
  • One possibility is that after CentralAuthUser::adminLock(), a cache miss on the local wiki repopulates memcached with a pre-lock row read from a lagged replica of the central DB, leaving gu_locked=0 and the old gu_auth_token cached until the CentralAuthUser TTL expires. If that's the case, then both CentralAuthSessionProvider::provideSessionInfo() and PermissionsHookHandler::onGetUserPermissionsErrorsExpensive() would consult that stale cache. This needs verification
  • Possibly impacted by the CentralAuthUser cache version bump in T423687, which is in wmf.26
  • Similar issue observed in T380260

Acceptance criteria

  • root cause of post-lock session persistence is identified and documented
  • a locked user cannot perform any non-read action within one request cycle of the lock

Details

Risk Rating
Medium
Author Affiliation
WMF Product

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Zabe added subscribers: Bewfip, Bugreporter, Stang.
Zabe subscribed.

During the day, I've been looking at https://pl.wikipedia.org/wiki/Specjalna:Wkład/M_Z_Wojalski?uselng=en – user who's been locked today UTC morning. Throughout the day, the page didn't display a notice about the account being locked (only global block was shown). Only a while ago I noticed that the lock notice is there.

I suspect that this issue was related to me bumping CentralAuthUser cache version in patch 1277468, which was deployed as part of wmf.26.

CentralAuthUser uses global cache (not per-wiki), which means that typically all wikis use the same shared cache. And writing to it or invalidating it from Meta-Wiki has therefore effect on all wikis in CentralAuth cluster.

However, what IMO happened today was likely:

  • An account is created at a few wikis.
    • There exist two versions of the CentralAuthUser objects in cache (13 and 14)
    • One of the versions in stored under the original key (keygroup centralauth-user)
    • The other is stored under an artificial key: as de-conflicted in WANObjectCache (keygroup WANCache-key-variant)
  • The account is locked.
    • The cache for CentralAuthUser is purged.
    • Only entry belonging to keygroup centralauth-user is actually purged.
    • The derived key remains – codesearching suggests that only getWithSetCallback respects versioned keys, and delete does not affect the derived key (missing checkKeys in getWithSetCallback?)

As a result, only one of the cache versions – 13 or 14 was purged, so the other one still contained the original unlocked user object, which allowed the user to edit on that wiki (or actually, that group of wikis).

We've also changed the cache key in T419336… (ironically, trying to fix a bug that allowed locks to be bypassed).

From reading the docs, bumping the cache version should not cause this problem (although I didn't test it, but we bumped the version at least 13 times before and did not notice problems like this), but changing the cache key probably did.

From reading the docs, bumping the cache version should not cause this problem (although I didn't test it, but we bumped the version at least 13 times before and did not notice problems like this), but changing the cache key probably did.

Oh, it makes sense then – I originally missed the minAsOf behavior which should propagate purges to variant keys, even if there's no explicit purge issued to them.

I suppose we can make this public and resolve?

sbassett triaged this task as Medium priority.
sbassett changed Author Affiliation from N/A to WMF Product.
sbassett added a project: SecTeam-Processed.
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett changed the edit policy from "Custom Policy" to "All Users".
sbassett changed Risk Rating from N/A to Medium.