fix: harden identity restore against review findings

Verify Nostr keychain writes with read-back (and roll back on failure),
always reopen BLE/Nostr after a failed restore suspend, and reject
unauthenticated PBKDF iteration counts before deriving.
This commit is contained in:
Amirhossein Rezaei 2026-07-27 16:57:12 +03:30
parent 71d2ff0b41
commit ab7ec2be89
5 changed files with 186 additions and 31 deletions

View File

@ -73,18 +73,24 @@ final class NostrIdentityBridge {
/// Replace the primary Nostr identity and device seed with restored material.
/// Clears derived-identity caches so geohash keys recompute from the new seed.
///
/// `keychain.save` is fire-and-forget, so this method always read-backs both
/// values and throws if either write did not stick. On failure it attempts
/// to restore the previous primary/seed snapshot so Noise and Nostr cannot
/// diverge after a half-applied restore.
@discardableResult
func installRestoredIdentity(privateKey: Data, deviceSeed: Data) throws -> NostrIdentity {
guard deviceSeed.count == 32 else {
throw NSError(
domain: "NostrIdentityBridge",
code: -2,
userInfo: [NSLocalizedDescriptionKey: "Device seed must be 32 bytes"]
)
throw IdentityBackupError.invalidKeyMaterial
}
let identity = try NostrIdentity(privateKeyData: privateKey)
let encoded = try JSONEncoder().encode(identity)
// Snapshot whatever is currently durable so a failed write after wipe
// can be rolled back instead of leaving an empty Nostr identity.
let previousIdentity = keychain.load(key: currentIdentityKey, service: keychainService)
let previousSeed = keychain.load(key: deviceSeedKey, service: keychainService)
// Drop prior associations/caches first so a partial failure cannot leave
// a mix of old seed + new primary (or vice versa).
clearAllAssociations()
@ -101,6 +107,33 @@ final class NostrIdentityBridge {
service: keychainService,
accessible: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
)
let storedIdentity = keychain.load(key: currentIdentityKey, service: keychainService)
let storedSeed = keychain.load(key: deviceSeedKey, service: keychainService)
guard storedIdentity == encoded, storedSeed == deviceSeed else {
// Best-effort rollback to the pre-restore snapshot.
if let previousIdentity {
keychain.save(
key: currentIdentityKey,
data: previousIdentity,
service: keychainService,
accessible: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
)
}
if let previousSeed {
keychain.save(
key: deviceSeedKey,
data: previousSeed,
service: keychainService,
accessible: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly
)
deviceSeedCache = previousSeed
} else {
deviceSeedCache = nil
}
throw IdentityBackupError.persistenceFailed
}
deviceSeedCache = deviceSeed
return identity
}

View File

@ -947,7 +947,7 @@ final class BLEService: NSObject {
}
guard keychain.getIdentityKey(forKey: "noiseStaticKey") == noiseStaticPrivateKey,
keychain.getIdentityKey(forKey: "ed25519SigningKey") == ed25519SigningPrivateKey else {
throw IdentityBackupError.encodingFailed
throw IdentityBackupError.persistenceFailed
}
localIdentityState.setNickname(currentNickname)

View File

@ -82,6 +82,7 @@ enum IdentityBackupError: Error, Equatable, LocalizedError {
case decryptionFailed
case unsupportedVersion
case encodingFailed
case persistenceFailed
var errorDescription: String? {
switch self {
@ -127,6 +128,12 @@ enum IdentityBackupError: Error, Equatable, LocalizedError {
defaultValue: "failed to build the encrypted backup",
comment: "Error when encrypting or encoding the backup envelope fails"
)
case .persistenceFailed:
return String(
localized: "identity_backup.error.persistence_failed",
defaultValue: "could not save the restored identity to the keychain",
comment: "Error when keychain write/read-back of restored identity keys fails"
)
}
}
}
@ -259,6 +266,12 @@ enum IdentityBackupService {
UInt32(bigEndian: $0.load(as: UInt32.self))
}
offset += 4
// Envelope fields before the AEAD tag are unauthenticated. Bound the
// iteration count before PBKDF2 so a crafted backup cannot force
// billions of derivation rounds on the main actor (DoS).
guard iterations == pbkdf2Iterations else {
throw IdentityBackupError.invalidPayload
}
let salt = envelope.subdata(in: offset..<(offset + saltByteCount))
offset += saltByteCount

View File

@ -1824,37 +1824,61 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage
bleService.suspendForPanicReset()
try idBridge.installRestoredIdentity(
privateKey: material.nostrPrivateKey,
deviceSeed: material.nostrDeviceSeed
)
// Always reopen admission + Nostr after a suspend, even when install
// throws otherwise a failed restore leaves BLE/internet dead until
// relaunch.
do {
try idBridge.installRestoredIdentity(
privateKey: material.nostrPrivateKey,
deviceSeed: material.nostrDeviceSeed
)
try bleService.installRestoredNoiseIdentity(
noiseStaticPrivateKey: material.noiseStaticPrivateKey,
ed25519SigningPrivateKey: material.ed25519SigningPrivateKey,
currentNickname: nickname,
restartServices: false
)
try bleService.installRestoredNoiseIdentity(
noiseStaticPrivateKey: material.noiseStaticPrivateKey,
ed25519SigningPrivateKey: material.ed25519SigningPrivateKey,
currentNickname: nickname,
restartServices: false
)
// Nostr transport caches the previous sender peer ID; point it at the
// restored mesh identity.
messageRouter.updateNostrSenderPeerID(meshService.myPeerID)
// Nostr transport caches the previous sender peer ID; point it at the
// restored mesh identity.
messageRouter.updateNostrSenderPeerID(meshService.myPeerID)
bleService.completePanicReset(restartServices: restartServices)
resumeTransportsAfterIdentityRestore(
bleService: bleService,
restartServices: restartServices
)
if restartServices {
if !TestEnvironment.isRunningTests {
nostrRelayManager = NostrRelayManager.shared
setupNostrMessageHandling()
}
panicNetworkLifecycle.restart()
SecureLogger.info(
"Identity restored from backup; fingerprint=\(fingerprint.prefix(16))",
category: .security
)
return fingerprint
} catch {
resumeTransportsAfterIdentityRestore(
bleService: bleService,
restartServices: restartServices
)
SecureLogger.error(
"Identity restore failed after transport suspend; services reopened: \(error)",
category: .security
)
throw error
}
}
SecureLogger.info(
"Identity restored from backup; fingerprint=\(fingerprint.prefix(16))",
category: .security
)
return fingerprint
@MainActor
private func resumeTransportsAfterIdentityRestore(
bleService: BLEService,
restartServices: Bool
) {
bleService.completePanicReset(restartServices: restartServices)
guard restartServices else { return }
if !TestEnvironment.isRunningTests {
nostrRelayManager = NostrRelayManager.shared
setupNostrMessageHandling()
}
panicNetworkLifecycle.restart()
}
/// BCH-01-013: Clear iOS app switcher snapshots during panic mode

View File

@ -136,6 +136,47 @@ struct IdentityBackupServiceTests {
let restored = try IdentityBackupService.decrypt(token, passphrase: passphrase)
#expect(restored == material)
}
@Test func rejectsUnauthenticatedPBKDFIterationCount() throws {
let material = try sampleMaterial()
let passphrase = "twelve-chars-min"
let uri = try IdentityBackupService.encrypt(material, passphrase: passphrase)
let token = try #require(URL(string: uri)?.pathComponents.last)
let envelope = try #require(Base64URLCoding.decode(token))
// Flip the big-endian iteration field (bytes after magic+version+kdf)
// to UInt32.max without touching the AEAD ciphertext the point is
// that we must reject before running PBKDF2.
var poisoned = envelope
let iterationsOffset = 4 /* BCID */ + 1 /* version */ + 1 /* kdf */
poisoned.replaceSubrange(
iterationsOffset..<(iterationsOffset + 4),
with: Data([0xFF, 0xFF, 0xFF, 0xFF])
)
let poisonedURI =
"bitchat://identity-backup/v1/\(Base64URLCoding.encode(poisoned))"
#expect(throws: IdentityBackupError.invalidPayload) {
_ = try IdentityBackupService.decrypt(poisonedURI, passphrase: passphrase)
}
}
@Test func nostrInstallFailsWhenKeychainWriteDoesNotStick() throws {
let keychain = DropWritesKeychain()
let bridge = NostrIdentityBridge(keychain: keychain)
let nostr = try NostrIdentity.generate()
var seed = Data(count: 32)
seed.withUnsafeMutableBytes { ptr in
_ = SecRandomCopyBytes(kSecRandomDefault, 32, ptr.baseAddress!)
}
#expect(throws: IdentityBackupError.persistenceFailed) {
_ = try bridge.installRestoredIdentity(
privateKey: nostr.privateKey,
deviceSeed: seed
)
}
}
}
struct IdentityBackupInstallTests {
@ -163,3 +204,47 @@ struct IdentityBackupInstallTests {
#expect(restored.getSigningPublicKeyData() == signingPub)
}
}
/// Keychain that accepts writes but never retains them models a failed
/// `SecItemAdd` after `clearAllAssociations` so restore must throw instead of
/// reporting success with an empty Nostr identity.
private final class DropWritesKeychain: KeychainManagerProtocol {
private var serviceStorage: [String: [String: Data]] = [:]
func saveIdentityKey(_ keyData: Data, forKey key: String) -> Bool { true }
func getIdentityKey(forKey key: String) -> Data? { nil }
func deleteIdentityKey(forKey key: String) -> Bool { true }
func deleteAllKeychainData() -> Bool {
serviceStorage.removeAll()
return true
}
func secureClear(_ data: inout Data) { data = Data() }
func secureClear(_ string: inout String) { string = "" }
func verifyIdentityKeyExists() -> Bool { false }
func getIdentityKeyWithResult(forKey key: String) -> KeychainReadResult { .itemNotFound }
func saveIdentityKeyWithResult(_ keyData: Data, forKey key: String) -> KeychainSaveResult { .success }
func save(key: String, data: Data, service: String, accessible: CFString?) {
// Pretend to succeed; do not retain read-back must fail.
_ = (key, data, service, accessible)
}
func load(key: String, service: String) -> Data? {
serviceStorage[service]?[key]
}
func loadWithResult(key: String, service: String) -> KeychainReadResult {
if let data = serviceStorage[service]?[key] {
return .success(data)
}
return .itemNotFound
}
func delete(key: String, service: String) {
serviceStorage[service]?.removeValue(forKey: key)
}
func deleteAll(service: String) {
serviceStorage.removeValue(forKey: service)
}
}