From 7c3a73d1a7e54810ff82c0142280aeda720c8645 Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Wed, 20 May 2026 01:28:44 -0500 Subject: [PATCH] Adjust SVR2PinHash protocol --- Signal.xcodeproj/project.pbxproj | 4 + SignalServiceKit/Environment/AppSetup.swift | 1 + .../SVR2/SVR2PinHash.swift | 78 ++++++++++++ .../SecureValueRecovery/SVR2/SVR2Shims.swift | 114 ------------------ .../SVR2/SecureValueRecovery2Impl.swift | 67 +++------- .../SVR2/SVR2ConcurrencyTests.swift | 4 +- .../SVR2/SecureValueRecovery2Tests.swift | 18 ++- 7 files changed, 107 insertions(+), 179 deletions(-) create mode 100644 SignalServiceKit/SecureValueRecovery/SVR2/SVR2PinHash.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 7a5e18f2a5..344f803424 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -783,6 +783,7 @@ 509DC8DA2BCED88600375E86 /* RemoteMegaphoneFetcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = D98DD85D28EE53B00089333E /* RemoteMegaphoneFetcher.swift */; }; 50A156C72FA11AA8008FE086 /* Fingerprint.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50A156C62FA11AA8008FE086 /* Fingerprint.swift */; }; 50A1CE3A2A00931900730C40 /* DebugLogger+MainApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50A1CE392A00931900730C40 /* DebugLogger+MainApp.swift */; }; + 50A26F1A2FB6991F000A2D8B /* SVR2PinHash.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50A26F192FB6991F000A2D8B /* SVR2PinHash.swift */; }; 50A40ED32B88005A0060C5A5 /* DisplayName.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50A40ED22B88005A0060C5A5 /* DisplayName.swift */; }; 50A4AC622C111FAE00D89C8E /* CallLinkAuthCredential.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50A4AC612C111FAE00D89C8E /* CallLinkAuthCredential.swift */; }; 50A5AA992A7449A100CF2ECC /* DecryptedIncomingEnvelope.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50A5AA982A7449A100CF2ECC /* DecryptedIncomingEnvelope.swift */; }; @@ -5054,6 +5055,7 @@ 50A156C62FA11AA8008FE086 /* Fingerprint.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Fingerprint.swift; sourceTree = ""; }; 50A1CE372A00894C00730C40 /* DebugLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DebugLogger.swift; sourceTree = ""; }; 50A1CE392A00931900730C40 /* DebugLogger+MainApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DebugLogger+MainApp.swift"; sourceTree = ""; }; + 50A26F192FB6991F000A2D8B /* SVR2PinHash.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SVR2PinHash.swift; sourceTree = ""; }; 50A40ED22B88005A0060C5A5 /* DisplayName.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DisplayName.swift; sourceTree = ""; }; 50A4AC612C111FAE00D89C8E /* CallLinkAuthCredential.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CallLinkAuthCredential.swift; sourceTree = ""; }; 50A5AA982A7449A100CF2ECC /* DecryptedIncomingEnvelope.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DecryptedIncomingEnvelope.swift; sourceTree = ""; }; @@ -10697,6 +10699,7 @@ children = ( 662C440A2A156DF7001F83E2 /* SecureValueRecovery2Impl.swift */, 66C2B13C2A0E9116008DDE72 /* SVR2AuthCredential.swift */, + 50A26F192FB6991F000A2D8B /* SVR2PinHash.swift */, 669947B92A20129000E4DC0C /* SVR2Shims.swift */, 66C2B1552A1400E8008DDE72 /* SVR2WebsocketConfigurator.swift */, ); @@ -19903,6 +19906,7 @@ D945319E2CE53CEB004DAB30 /* SubscriptionRedemptionNecessityChecker.swift in Sources */, 662C44092A1567E4001F83E2 /* svr2.pb.swift in Sources */, 66C2B13D2A0E9116008DDE72 /* SVR2AuthCredential.swift in Sources */, + 50A26F1A2FB6991F000A2D8B /* SVR2PinHash.swift in Sources */, 669947BA2A20129000E4DC0C /* SVR2Shims.swift in Sources */, 66C2B1562A1400E8008DDE72 /* SVR2WebsocketConfigurator.swift in Sources */, 66C2B1382A0DB6A9008DDE72 /* SVRAuthCredential.swift in Sources */, diff --git a/SignalServiceKit/Environment/AppSetup.swift b/SignalServiceKit/Environment/AppSetup.swift index 8276e6279b..60e7ae0378 100644 --- a/SignalServiceKit/Environment/AppSetup.swift +++ b/SignalServiceKit/Environment/AppSetup.swift @@ -446,6 +446,7 @@ extension AppSetup.GlobalsContinuation { credentialStorage: svrCredentialStorage, db: db, accountKeyStore: accountKeyStore, + pinHasher: LibSignalPinHasher(), storageServiceManager: storageServiceManager, svrLocalStorage: svrLocalStorage, tsAccountManager: tsAccountManager, diff --git a/SignalServiceKit/SecureValueRecovery/SVR2/SVR2PinHash.swift b/SignalServiceKit/SecureValueRecovery/SVR2/SVR2PinHash.swift new file mode 100644 index 0000000000..ef1f3e6ecd --- /dev/null +++ b/SignalServiceKit/SecureValueRecovery/SVR2/SVR2PinHash.swift @@ -0,0 +1,78 @@ +// +// Copyright 2026 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only +// + +import CryptoKit +import Foundation +import LibSignalClient + +protocol SVR2PinHasher { + func hashPin( + normalizedPin: String, + username: String, + mrEnclave: MrEnclave, + ) throws -> SVR2PinHash +} + +struct LibSignalPinHasher: SVR2PinHasher { + func hashPin(normalizedPin: String, username: String, mrEnclave: MrEnclave) throws -> SVR2PinHash { + return try SVR2PinHash.derive(normalizedPin: normalizedPin, username: username, mrEnclave: mrEnclave) + } +} + +#if TESTABLE_BUILD + +/// A "mock" PIN hasher that works even for invalid MrEnclave values. +struct MockPinHasher: SVR2PinHasher { + func hashPin(normalizedPin: String, username: String, mrEnclave: MrEnclave) throws -> SVR2PinHash { + // A fake hashing function that considers the same inputs as the real one. + let result = try hkdf(outputLength: 64, inputKeyMaterial: Data(normalizedPin.utf8), salt: Data(username.utf8), info: mrEnclave.dataValue) + return SVR2PinHash(accessKey: result.prefix(32), encryptionKey: result.suffix(32)) + } +} + +#endif + +struct SVR2PinHash { + // The thing we use as the "pin" in SVR2 backup/restore requests. + var accessKey: Data + + var encryptionKey: Data + + // The data we put into SVR2 backups, encrypted with the PIN. + func encryptMasterKey(_ masterKey: Data) throws -> Data { + let (iv, cipherText) = try Sha256HmacSiv.encrypt(data: masterKey, key: encryptionKey) + if iv.count != 16 || cipherText.count != 32 { + throw SVR.SVRError.assertion + } + let encryptedMasterKey = iv + cipherText + return encryptedMasterKey + } + + func decryptMasterKey(_ encryptedMasterKey: Data) throws -> Data { + guard encryptedMasterKey.count == 48 else { throw SVR.SVRError.assertion } + + let startIndex: Int = encryptedMasterKey.startIndex + let ivRange = startIndex...(startIndex + 15) + let cipherRange = (startIndex + 16)...(startIndex + 47) + let masterKey = try Sha256HmacSiv.decrypt( + iv: encryptedMasterKey[ivRange], + cipherText: encryptedMasterKey[cipherRange], + key: encryptionKey, + ) + + guard masterKey.count == MasterKey.Constants.byteLength else { throw SVR.SVRError.assertion } + + return masterKey + } + + static func derive( + normalizedPin: String, + username: String, + mrEnclave: MrEnclave, + ) throws -> Self { + let pinHash = try LibSignalClient.PinHash(normalizedPin: Data(normalizedPin.utf8), username: username, mrenclave: mrEnclave.dataValue) + return Self(accessKey: pinHash.accessKey, encryptionKey: pinHash.encryptionKey) + } +} diff --git a/SignalServiceKit/SecureValueRecovery/SVR2/SVR2Shims.swift b/SignalServiceKit/SecureValueRecovery/SVR2/SVR2Shims.swift index c7f085f03e..957dfd2994 100644 --- a/SignalServiceKit/SecureValueRecovery/SVR2/SVR2Shims.swift +++ b/SignalServiceKit/SecureValueRecovery/SVR2/SVR2Shims.swift @@ -57,81 +57,6 @@ public class _SVR2_OWS2FAManagerWrapper: SVR2.Shims.OWS2FAManager { } } -// NOTE: The below aren't shims/wrappers in the normal sense; they -// wrap libsignal stuff that we will _always_ need to wrap. - -protocol SVR2PinHash { - // The thing we use as the "pin" in SVR2 backup/restore requests. - var accessKey: Data { get } - - // The data we put into SVR2 backups, encrypted with the PIN. - func encryptMasterKey(_ masterKey: Data) throws -> Data - - func decryptMasterKey(_ encryptedMasterKey: Data) throws -> Data -} - -protocol SVR2ClientWrapper { - - func hashPin( - connection: SgxWebsocketConnection, - utf8NormalizedPin: Data, - username: String, - ) throws -> SVR2PinHash -} - -class SVR2ClientWrapperImpl: SVR2ClientWrapper { - - init() {} - - private class SVR2PinHashImpl: SVR2PinHash { - - private let pinHash: PinHash - - init(_ pinHash: PinHash) { - self.pinHash = pinHash - } - - var accessKey: Data { pinHash.accessKey } - private var encryptionKey: Data { pinHash.encryptionKey } - - func encryptMasterKey(_ masterKey: Data) throws -> Data { - let (iv, cipherText) = try Sha256HmacSiv.encrypt(data: masterKey, key: encryptionKey) - if iv.count != 16 || cipherText.count != 32 { - throw SVR.SVRError.assertion - } - let encryptedMasterKey = iv + cipherText - return encryptedMasterKey - } - - func decryptMasterKey(_ encryptedMasterKey: Data) throws -> Data { - guard encryptedMasterKey.count == 48 else { throw SVR.SVRError.assertion } - - let startIndex: Int = encryptedMasterKey.startIndex - let ivRange = startIndex...(startIndex + 15) - let cipherRange = (startIndex + 16)...(startIndex + 47) - let masterKey = try Sha256HmacSiv.decrypt( - iv: encryptedMasterKey[ivRange], - cipherText: encryptedMasterKey[cipherRange], - key: encryptionKey, - ) - - guard masterKey.count == MasterKey.Constants.byteLength else { throw SVR.SVRError.assertion } - - return masterKey - } - } - - func hashPin( - connection: SgxWebsocketConnection, - utf8NormalizedPin: Data, - username: String, - ) throws -> SVR2PinHash { - let pinHash = try LibSignalClient.PinHash(normalizedPin: utf8NormalizedPin, username: username, mrenclave: connection.mrEnclave.dataValue) - return SVR2PinHashImpl(pinHash) - } - -} - #if TESTABLE_BUILD extension SVR2 { @@ -149,43 +74,4 @@ class _SVR2_AppContextMock: _SVR2_AppContextShim { var isNSE: Bool { false } } -class MockSVR2ClientWrapper: SVR2ClientWrapper { - - init() {} - - class MockSVR2PinHash: SVR2PinHash { - - init(utf8NormalizedPin: Data) { - self.accessKey = utf8NormalizedPin - } - - var accessKey: Data - - var didEncryptMasterKey: (_ masterKey: Data) throws -> Data = { return $0 } - - func encryptMasterKey(_ masterKey: Data) throws -> Data { - return try didEncryptMasterKey(masterKey) - } - - var didDecryptMasterKey: (_ encryptedMasterKey: Data) throws -> Data = { return $0 } - - func decryptMasterKey(_ encryptedMasterKey: Data) throws -> Data { - return try didDecryptMasterKey(encryptedMasterKey) - } - } - - var didHashPin: ((_ utf8NormalizedPin: Data, _ username: String) throws -> MockSVR2PinHash) = { utf8NormalizedPin, _ in - return MockSVR2PinHash(utf8NormalizedPin: utf8NormalizedPin) - } - - func hashPin( - connection: SgxWebsocketConnection, - utf8NormalizedPin: Data, - username: String, - ) throws -> SVR2PinHash { - return try didHashPin(utf8NormalizedPin, username) - } - -} - #endif diff --git a/SignalServiceKit/SecureValueRecovery/SVR2/SecureValueRecovery2Impl.swift b/SignalServiceKit/SecureValueRecovery/SVR2/SecureValueRecovery2Impl.swift index 6b23092d1d..621bcca761 100644 --- a/SignalServiceKit/SecureValueRecovery/SVR2/SecureValueRecovery2Impl.swift +++ b/SignalServiceKit/SecureValueRecovery/SVR2/SecureValueRecovery2Impl.swift @@ -12,7 +12,7 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { private let appContext: SVR2.Shims.AppContext private let appReadiness: AppReadiness private let appVersion: AppVersion - private let clientWrapper: SVR2ClientWrapper + private let pinHasher: any SVR2PinHasher private let connectionFactory: SgxWebsocketConnectionFactory private let credentialStorage: SVRAuthCredentialStorage private let db: any DB @@ -23,46 +23,15 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { private let tsConstants: TSConstantsProtocol private let twoFAManager: SVR2.Shims.OWS2FAManager - public convenience init( - appContext: SVR2.Shims.AppContext, - appReadiness: AppReadiness, - appVersion: AppVersion, - connectionFactory: SgxWebsocketConnectionFactory, - credentialStorage: SVRAuthCredentialStorage, - db: any DB, - accountKeyStore: AccountKeyStore, - storageServiceManager: StorageServiceManager, - svrLocalStorage: SVRLocalStorageInternal, - tsAccountManager: TSAccountManager, - tsConstants: TSConstantsProtocol, - twoFAManager: SVR2.Shims.OWS2FAManager, - ) { - self.init( - appContext: appContext, - appReadiness: appReadiness, - appVersion: appVersion, - clientWrapper: SVR2ClientWrapperImpl(), - connectionFactory: connectionFactory, - credentialStorage: credentialStorage, - db: db, - accountKeyStore: accountKeyStore, - storageServiceManager: storageServiceManager, - svrLocalStorage: svrLocalStorage, - tsAccountManager: tsAccountManager, - tsConstants: tsConstants, - twoFAManager: twoFAManager, - ) - } - init( appContext: SVR2.Shims.AppContext, appReadiness: AppReadiness, appVersion: AppVersion, - clientWrapper: SVR2ClientWrapper, connectionFactory: SgxWebsocketConnectionFactory, credentialStorage: SVRAuthCredentialStorage, db: any DB, accountKeyStore: AccountKeyStore, + pinHasher: any SVR2PinHasher, storageServiceManager: StorageServiceManager, svrLocalStorage: SVRLocalStorageInternal, tsAccountManager: TSAccountManager, @@ -72,12 +41,12 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { self.appContext = appContext self.appReadiness = appReadiness self.appVersion = appVersion - self.clientWrapper = clientWrapper self.connectionFactory = connectionFactory self.credentialStorage = credentialStorage self.db = db self.accountKeyStore = accountKeyStore self.localStorage = svrLocalStorage + self.pinHasher = pinHasher self.storageServiceManager = storageServiceManager self.tsAccountManager = tsAccountManager self.tsConstants = tsConstants @@ -471,10 +440,7 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { let pinHash: SVR2PinHash let encryptedMasterKey: Data do { - pinHash = try connection.hashPin( - pin: pin, - wrapper: clientWrapper, - ) + pinHash = try hashPin(pin, forConnection: connection.connection) encryptedMasterKey = try pinHash.encryptMasterKey(masterKey) } catch { return .localEncryptionError @@ -723,7 +689,7 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { ) async -> RestoreResult { let pinHash: SVR2PinHash do { - pinHash = try connection.hashPin(pin: pin, wrapper: clientWrapper) + pinHash = try hashPin(pin, forConnection: connection.connection) } catch { return .decryptionError } @@ -1024,7 +990,7 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { /// we keep track of how many requests are going out, decrement when they finish, /// and close the connection when there are none left. private class WebsocketConnection { - private let connection: SgxWebsocketConnection + let connection: SgxWebsocketConnection let mrEnclave: MrEnclave init(connection: SgxWebsocketConnection) { @@ -1050,18 +1016,17 @@ public class SecureValueRecovery2Impl: SecureValueRecovery { func disconnect(isNormalClosure: Bool) { connection.disconnect(code: isNormalClosure ? .normalClosure : nil) } + } - func hashPin( - pin: String, - wrapper: SVR2ClientWrapper, - ) throws -> SVR2PinHash { - let utf8NormalizedPin = Data(SVRUtil.normalizePin(pin).utf8) - return try wrapper.hashPin( - connection: connection, - utf8NormalizedPin: utf8NormalizedPin, - username: connection.auth.username, - ) - } + func hashPin( + _ pin: String, + forConnection connection: SgxWebsocketConnection, + ) throws -> SVR2PinHash { + return try pinHasher.hashPin( + normalizedPin: SVRUtil.normalizePin(pin), + username: connection.auth.username, + mrEnclave: connection.mrEnclave, + ) } private struct CachedWebsocketConnection { diff --git a/SignalServiceKit/tests/SecureValueRecovery/SVR2/SVR2ConcurrencyTests.swift b/SignalServiceKit/tests/SecureValueRecovery/SVR2/SVR2ConcurrencyTests.swift index 4d33460535..318495eecb 100644 --- a/SignalServiceKit/tests/SecureValueRecovery/SVR2/SVR2ConcurrencyTests.swift +++ b/SignalServiceKit/tests/SecureValueRecovery/SVR2/SVR2ConcurrencyTests.swift @@ -27,8 +27,6 @@ struct SVR2ConcurrencyTests { mockConnection.mockAuth = RemoteAttestation.Auth(username: "username", password: "password") mockConnectionFactory = MockSgxWebsocketConnectionFactory() - let mockClientWrapper = MockSVR2ClientWrapper() - let accountKeyStore = AccountKeyStore( backupSettingsStore: BackupSettingsStore(), ) @@ -38,11 +36,11 @@ struct SVR2ConcurrencyTests { appContext: SVR2.Mocks.AppContext(), appReadiness: AppReadinessMock(), appVersion: MockAppVerion(), - clientWrapper: mockClientWrapper, connectionFactory: mockConnectionFactory, credentialStorage: credentialStorage, db: db, accountKeyStore: accountKeyStore, + pinHasher: MockPinHasher(), storageServiceManager: FakeStorageServiceManager(), svrLocalStorage: localStorage, tsAccountManager: MockTSAccountManager(), diff --git a/SignalServiceKit/tests/SecureValueRecovery/SVR2/SecureValueRecovery2Tests.swift b/SignalServiceKit/tests/SecureValueRecovery/SVR2/SecureValueRecovery2Tests.swift index 426c3eb108..3abdfafa4c 100644 --- a/SignalServiceKit/tests/SecureValueRecovery/SVR2/SecureValueRecovery2Tests.swift +++ b/SignalServiceKit/tests/SecureValueRecovery/SVR2/SecureValueRecovery2Tests.swift @@ -45,11 +45,11 @@ class SecureValueRecovery2Tests: XCTestCase { appContext: SVR2.Mocks.AppContext(), appReadiness: AppReadinessMock(), appVersion: MockAppVerion(), - clientWrapper: MockSVR2ClientWrapper(), connectionFactory: mockConnectionFactory, credentialStorage: credentialStorage, db: db, accountKeyStore: accountKeyStore, + pinHasher: MockPinHasher(), storageServiceManager: FakeStorageServiceManager(), svrLocalStorage: localStorage, tsAccountManager: mockTSAccountManager, @@ -106,9 +106,8 @@ class SecureValueRecovery2Tests: XCTestCase { case 0: // First it should issue a backup to the new enclave. XCTAssert(request.hasBackup) - // Test mock encruption just passes along the unmodified master key and pin. - XCTAssertEqual(request.backup.data, masterKey.rawData) - XCTAssertEqual(request.backup.pin, pin.data(using: .utf8)) + XCTAssertEqual(request.backup.data.count, 48) + XCTAssertEqual(request.backup.pin.count, 32) var backupResponse = SVR2Proto_BackupResponse() backupResponse.status = .ok @@ -116,8 +115,7 @@ class SecureValueRecovery2Tests: XCTestCase { case 1: // Then an expose XCTAssert(request.hasExpose) - // Test mock encruption just passes along the unmodified master key. - XCTAssertEqual(request.expose.data, masterKey.rawData) + XCTAssertEqual(request.expose.data.count, 48) var exposeResponse = SVR2Proto_ExposeResponse() exposeResponse.status = .ok @@ -211,9 +209,8 @@ class SecureValueRecovery2Tests: XCTestCase { case 0: // First it should issue a backup to the new enclave. XCTAssert(request.hasBackup) - // Test mock encruption just passes along the unmodified master key and pin. - XCTAssertEqual(request.backup.data, masterKey.rawData) - XCTAssertEqual(request.backup.pin, pin.data(using: .utf8)) + XCTAssertEqual(request.backup.data.count, 48) + XCTAssertEqual(request.backup.pin.count, 32) var backupResponse = SVR2Proto_BackupResponse() backupResponse.status = .ok @@ -221,8 +218,7 @@ class SecureValueRecovery2Tests: XCTestCase { case 1: // Then an expose XCTAssert(request.hasExpose) - // Test mock encruption just passes along the unmodified master key. - XCTAssertEqual(request.expose.data, masterKey.rawData) + XCTAssertEqual(request.expose.data.count, 48) var exposeResponse = SVR2Proto_ExposeResponse() exposeResponse.status = .ok