Skip to content

Commit f5f0ba3

Browse files
authored
Add additional reglock tests, fix a registration edge case.
1 parent e79c878 commit f5f0ba3

File tree

2 files changed

+699
-3
lines changed

2 files changed

+699
-3
lines changed

Signal/Registration/RegistrationCoordinatorImpl.swift

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,7 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
693693
// be valid, or may not correspond with the current e164.
694694
var svr2AuthCredentialCandidates: [SVR2AuthCredential]?
695695
var svrAuthCredential: SVRAuthCredential?
696+
696697
// If we had SVR backups before registration even began.
697698
var didHaveSVRBackupsPriorToReg = false
698699

@@ -859,6 +860,11 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
859860
/// PIN guesses or decide to give up before exhausting them.
860861
var hasGivenUpTryingToRestoreWithSVR = false
861862

863+
/// Have we restored the pin form SVR already? This serves as a hint to the registration
864+
/// flow that it doesn't need to fetch from SVR in the case of an error and can move on
865+
/// to alternate registration paths (e.g. falling back to session based registration)
866+
var hasRestoredFromSVR = false
867+
862868
/// Root key entered or generated during registration. This value should be persisted at
863869
/// the end of registration
864870
var accountEntropyPool: SignalServiceKit.AccountEntropyPool?
@@ -996,6 +1002,7 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
9961002
case hasSkippedPinEntry
9971003
// Legacy naming
9981004
case hasGivenUpTryingToRestoreWithSVR = "hasGivenUpTryingToRestoreWithKBS"
1005+
case hasRestoredFromSVR
9991006
case sessionState
10001007
case accountIdentity
10011008
case didRefreshOneTimePreKeys
@@ -1654,13 +1661,29 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
16541661
// (If we wiped it and our SVR server guesses were consumed by the reglock-challenger,
16551662
// we'd be outta luck and have no way to recover).
16561663
//
1657-
// However, because the master key can be much more fluid in an AEP world, there is a
1664+
// Because the master key can be much more fluid in an AEP world, there is a
16581665
// much more common case that the SVR master key is wrong, but we can still fetch a
16591666
// valid master key from SVR. To that point, don't clear out the SVR auth credentials here.
16601667
// Instead, clear out just the piece of information we now know to be invalid to inform
16611668
// the state machine to bypass any RRP attempts and fall back to fetching from SVR (or
16621669
// restoring to starting a session from scratch)
1663-
inMemoryState.regRecoveryPw = nil
1670+
//
1671+
// However, we should only attempt to restore from SVR once. If we successfully restore
1672+
// from SVR, and still encounter this error, we
1673+
// (a) have already restored so don't need the svrCredentials any longer, and
1674+
// (b) should revert back to session based registration.
1675+
if persistedState.hasRestoredFromSVR {
1676+
db.write { tx in
1677+
// We do want to clear out any credentials permanently; we know we
1678+
// have to use the session path so credentials aren't helpful.
1679+
if let svr2Credential = inMemoryState.svrAuthCredential {
1680+
deps.svrAuthCredentialStore.deleteInvalidCredentials([svr2Credential], tx)
1681+
}
1682+
}
1683+
wipeInMemoryStateToPreventSVRPathAttempts()
1684+
} else {
1685+
inMemoryState.regRecoveryPw = nil
1686+
}
16641687

16651688
// Instead of moving directly to starting a session, like we do in the .reglockFailed case above,
16661689
// let the state machine determine next steps. It may be the user had a bad
@@ -1769,6 +1792,7 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
17691792
self.db.write {
17701793
self.updatePersistedState($0) { state in
17711794
state.recoveredSVRMasterKey = masterKey
1795+
state.hasRestoredFromSVR = true
17721796
}
17731797
self.updateMasterKeyAndLocalState(masterKey: masterKey, tx: $0)
17741798
}
@@ -2928,6 +2952,7 @@ public class RegistrationCoordinatorImpl: RegistrationCoordinator {
29282952
self.updateMasterKeyAndLocalState(masterKey: masterKey, tx: tx)
29292953
self.updatePersistedState(tx) {
29302954
$0.recoveredSVRMasterKey = masterKey
2955+
$0.hasRestoredFromSVR = true
29312956
}
29322957
self.updatePersistedSessionState(session: session, tx) {
29332958
// Now we have the state we need to get past reglock.

0 commit comments

Comments
 (0)