Refactor: Testable Keychain and Identity Manager (#584)

* Make static functions instance functions to be testable

* Injectable KeychainManager + Mock + updated tests

* Remove `pendingActions` from identity manager (dead code)

* Remove `getHandshakeState` from identity manager (dead code)

* Remove `getAllSocialIdentities` from identity manager (dead code)

* Remove `getCryptographicIdentity` from identity manager (dead code)

* Remove `resolveIdentity` from identity manager (dead code)

* Identity Manager: minor clean up

* Put Identity Manager behind a protocol

* Remove Keychain and Identity Manager singletons

* Tests: include MockKeychain/MockIdentityManager in project; init identityManager in CommandProcessorTests

---------

Co-authored-by: jack <jackjackbits@users.noreply.github.com>
This commit is contained in:
Islam
2025-09-12 14:37:34 +02:00
committed by GitHub
co-authored by jack
parent bb3d99bdca
commit 920dc31795
22 changed files with 445 additions and 234 deletions
+8 -5
View File
@@ -88,7 +88,8 @@ final class BLEService: NSObject {
var myPeerID: String = ""
var myNickname: String = "anon"
private let noiseService = NoiseEncryptionService()
private let noiseService: NoiseEncryptionService
private let identityManager: SecureIdentityStateManagerProtocol
private var myPeerIDData: Data = Data()
// MARK: - Advertising Privacy
@@ -325,7 +326,9 @@ final class BLEService: NSObject {
}
}
override init() {
init(keychain: KeychainManagerProtocol, identityManager: SecureIdentityStateManagerProtocol) {
noiseService = NoiseEncryptionService(keychain: keychain)
self.identityManager = identityManager
super.init()
// Derive stable peer ID from Noise static public key fingerprint (first 8 bytes 16 hex chars)
@@ -1548,7 +1551,7 @@ final class BLEService: NSObject {
// Derive fingerprint from Noise public key
let hash = SHA256.hash(data: announcement.noisePublicKey)
let fingerprint = hash.map { String(format: "%02x", $0) }.joined()
SecureIdentityStateManager.shared.upsertCryptographicIdentity(
identityManager.upsertCryptographicIdentity(
fingerprint: fingerprint,
noisePublicKey: announcement.noisePublicKey,
signingPublicKey: announcement.signingPublicKey,
@@ -1630,13 +1633,13 @@ final class BLEService: NSObject {
// Fallback: verify signature using persisted signing key for this peerID's fingerprint prefix
if let signature = packet.signature, let packetData = packet.toBinaryDataForSigning() {
// Find candidate identities by peerID prefix (16 hex)
let candidates = SecureIdentityStateManager.shared.getCryptoIdentitiesByPeerIDPrefix(peerID)
let candidates = identityManager.getCryptoIdentitiesByPeerIDPrefix(peerID)
for candidate in candidates {
if let signingKey = candidate.signingPublicKey,
noiseService.verifySignature(signature, for: packetData, publicKey: signingKey) {
accepted = true
// Prefer persisted social petname or claimed nickname
if let social = SecureIdentityStateManager.shared.getSocialIdentity(for: candidate.fingerprint) {
if let social = identityManager.getSocialIdentity(for: candidate.fingerprint) {
senderNickname = social.localPetname ?? social.claimedNickname
} else {
senderNickname = "anon" + String(peerID.prefix(4))
+14 -12
View File
@@ -20,10 +20,12 @@ enum CommandResult {
final class CommandProcessor {
weak var chatViewModel: ChatViewModel?
weak var meshService: Transport?
private let identityManager: SecureIdentityStateManagerProtocol
init(chatViewModel: ChatViewModel? = nil, meshService: Transport? = nil) {
init(chatViewModel: ChatViewModel? = nil, meshService: Transport? = nil, identityManager: SecureIdentityStateManagerProtocol) {
self.chatViewModel = chatViewModel
self.meshService = meshService
self.identityManager = identityManager
}
/// Process a command string
@@ -189,7 +191,7 @@ final class CommandProcessor {
}
// Geohash blocked names (prefer visible display names; fallback to #suffix)
let geoBlocked = Array(SecureIdentityStateManager.shared.getBlockedNostrPubkeys())
let geoBlocked = Array(identityManager.getBlockedNostrPubkeys())
var geoNames: [String] = []
if let vm = chatViewModel {
let visible = vm.visibleGeohashPeople()
@@ -213,14 +215,14 @@ final class CommandProcessor {
if let peerID = chatViewModel?.getPeerIDForNickname(nickname),
let fingerprint = meshService?.getFingerprint(for: peerID) {
if SecureIdentityStateManager.shared.isBlocked(fingerprint: fingerprint) {
if identityManager.isBlocked(fingerprint: fingerprint) {
return .success(message: "\(nickname) is already blocked")
}
// Block the user (mesh/noise identity)
if var identity = SecureIdentityStateManager.shared.getSocialIdentity(for: fingerprint) {
if var identity = identityManager.getSocialIdentity(for: fingerprint) {
identity.isBlocked = true
identity.isFavorite = false
SecureIdentityStateManager.shared.updateSocialIdentity(identity)
identityManager.updateSocialIdentity(identity)
} else {
let blockedIdentity = SocialIdentity(
fingerprint: fingerprint,
@@ -231,16 +233,16 @@ final class CommandProcessor {
isBlocked: true,
notes: nil
)
SecureIdentityStateManager.shared.updateSocialIdentity(blockedIdentity)
identityManager.updateSocialIdentity(blockedIdentity)
}
return .success(message: "blocked \(nickname). you will no longer receive messages from them")
}
// Mesh lookup failed; try geohash (Nostr) participant by display name
if let pub = chatViewModel?.nostrPubkeyForDisplayName(nickname) {
if SecureIdentityStateManager.shared.isNostrBlocked(pubkeyHexLowercased: pub) {
if identityManager.isNostrBlocked(pubkeyHexLowercased: pub) {
return .success(message: "\(nickname) is already blocked")
}
SecureIdentityStateManager.shared.setNostrBlocked(pub, isBlocked: true)
identityManager.setNostrBlocked(pub, isBlocked: true)
return .success(message: "blocked \(nickname) in geohash chats")
}
@@ -257,18 +259,18 @@ final class CommandProcessor {
if let peerID = chatViewModel?.getPeerIDForNickname(nickname),
let fingerprint = meshService?.getFingerprint(for: peerID) {
if !SecureIdentityStateManager.shared.isBlocked(fingerprint: fingerprint) {
if !identityManager.isBlocked(fingerprint: fingerprint) {
return .success(message: "\(nickname) is not blocked")
}
SecureIdentityStateManager.shared.setBlocked(fingerprint, isBlocked: false)
identityManager.setBlocked(fingerprint, isBlocked: false)
return .success(message: "unblocked \(nickname)")
}
// Try geohash unblock
if let pub = chatViewModel?.nostrPubkeyForDisplayName(nickname) {
if !SecureIdentityStateManager.shared.isNostrBlocked(pubkeyHexLowercased: pub) {
if !identityManager.isNostrBlocked(pubkeyHexLowercased: pub) {
return .success(message: "\(nickname) is not blocked")
}
SecureIdentityStateManager.shared.setNostrBlocked(pub, isBlocked: false)
identityManager.setNostrBlocked(pub, isBlocked: false)
return .success(message: "unblocked \(nickname) in geohash chats")
}
return .error(message: "cannot unblock \(nickname): not found")
+14 -7
View File
@@ -9,16 +9,23 @@
import Foundation
import Security
final class KeychainManager {
static let shared = KeychainManager()
protocol KeychainManagerProtocol {
func saveIdentityKey(_ keyData: Data, forKey key: String) -> Bool
func getIdentityKey(forKey key: String) -> Data?
func deleteIdentityKey(forKey key: String) -> Bool
func deleteAllKeychainData() -> Bool
func secureClear(_ data: inout Data)
func secureClear(_ string: inout String)
func verifyIdentityKeyExists() -> Bool
}
final class KeychainManager: KeychainManagerProtocol {
// Use consistent service name for all keychain items
private let service = "chat.bitchat"
private let appGroup = "group.chat.bitchat"
private init() {}
private func isSandboxed() -> Bool {
#if os(macOS)
// More robust sandbox detection using multiple methods
@@ -310,7 +317,7 @@ final class KeychainManager {
// MARK: - Security Utilities
/// Securely clear sensitive data from memory
static func secureClear(_ data: inout Data) {
func secureClear(_ data: inout Data) {
_ = data.withUnsafeMutableBytes { bytes in
// Use volatile memset to prevent compiler optimization
memset_s(bytes.baseAddress, bytes.count, 0, bytes.count)
@@ -319,7 +326,7 @@ final class KeychainManager {
}
/// Securely clear sensitive string from memory
static func secureClear(_ string: inout String) {
func secureClear(_ string: inout String) {
// Convert to mutable data and clear
if var data = string.data(using: .utf8) {
secureClear(&data)
+11 -8
View File
@@ -155,6 +155,7 @@ final class NoiseEncryptionService {
// Security components
private let rateLimiter = NoiseRateLimiter()
private let keychain: KeychainManagerProtocol
// Session maintenance
private var rekeyTimer: Timer?
@@ -181,12 +182,14 @@ final class NoiseEncryptionService {
}
}
init() {
init(keychain: KeychainManagerProtocol) {
self.keychain = keychain
// Load or create static identity key (ONLY from keychain)
let loadedKey: Curve25519.KeyAgreement.PrivateKey
// Try to load from keychain
if let identityData = KeychainManager.shared.getIdentityKey(forKey: "noiseStaticKey"),
if let identityData = keychain.getIdentityKey(forKey: "noiseStaticKey"),
let key = try? Curve25519.KeyAgreement.PrivateKey(rawRepresentation: identityData) {
loadedKey = key
SecureLogger.logKeyOperation(.load, keyType: "noiseStaticKey", success: true)
@@ -197,7 +200,7 @@ final class NoiseEncryptionService {
let keyData = loadedKey.rawRepresentation
// Save to keychain
let saved = KeychainManager.shared.saveIdentityKey(keyData, forKey: "noiseStaticKey")
let saved = keychain.saveIdentityKey(keyData, forKey: "noiseStaticKey")
SecureLogger.logKeyOperation(.create, keyType: "noiseStaticKey", success: saved)
}
@@ -209,7 +212,7 @@ final class NoiseEncryptionService {
let loadedSigningKey: Curve25519.Signing.PrivateKey
// Try to load from keychain
if let signingData = KeychainManager.shared.getIdentityKey(forKey: "ed25519SigningKey"),
if let signingData = keychain.getIdentityKey(forKey: "ed25519SigningKey"),
let key = try? Curve25519.Signing.PrivateKey(rawRepresentation: signingData) {
loadedSigningKey = key
SecureLogger.logKeyOperation(.load, keyType: "ed25519SigningKey", success: true)
@@ -220,7 +223,7 @@ final class NoiseEncryptionService {
let keyData = loadedSigningKey.rawRepresentation
// Save to keychain
let saved = KeychainManager.shared.saveIdentityKey(keyData, forKey: "ed25519SigningKey")
let saved = keychain.saveIdentityKey(keyData, forKey: "ed25519SigningKey")
SecureLogger.logKeyOperation(.create, keyType: "ed25519SigningKey", success: saved)
}
@@ -229,7 +232,7 @@ final class NoiseEncryptionService {
self.signingPublicKey = signingKey.publicKey
// Initialize session manager
self.sessionManager = NoiseSessionManager(localStaticKey: staticIdentityKey)
self.sessionManager = NoiseSessionManager(localStaticKey: staticIdentityKey, keychain: keychain)
// Set up session callbacks
sessionManager.onSessionEstablished = { [weak self] peerID, remoteStaticKey in
@@ -266,8 +269,8 @@ final class NoiseEncryptionService {
/// Clear persistent identity (for panic mode)
func clearPersistentIdentity() {
// Clear from keychain
let deletedStatic = KeychainManager.shared.deleteIdentityKey(forKey: "noiseStaticKey")
let deletedSigning = KeychainManager.shared.deleteIdentityKey(forKey: "ed25519SigningKey")
let deletedStatic = keychain.deleteIdentityKey(forKey: "noiseStaticKey")
let deletedSigning = keychain.deleteIdentityKey(forKey: "ed25519SigningKey")
SecureLogger.logKeyOperation(.delete, keyType: "identity keys", success: deletedStatic && deletedSigning)
SecureLogger.warning("Panic mode activated - identity cleared", category: .security)
// Stop rekey timer
+15 -4
View File
@@ -21,10 +21,15 @@ final class NostrTransport: Transport {
private var readQueue: [QueuedRead] = []
private var isSendingReadAcks = false
private let readAckInterval: TimeInterval = TransportConfig.nostrReadAckInterval
private let keychain: KeychainManagerProtocol
var myPeerID: String { senderPeerID }
var myNickname: String { "" }
func setNickname(_ nickname: String) { /* not used for Nostr */ }
init(keychain: KeychainManagerProtocol) {
self.keychain = keychain
}
func startServices() { /* no-op */ }
func stopServices() { /* no-op */ }
@@ -38,11 +43,17 @@ final class NostrTransport: Transport {
func getFingerprint(for peerID: String) -> String? { nil }
func getNoiseSessionState(for peerID: String) -> LazyHandshakeState { .none }
func triggerHandshake(with peerID: String) { /* no-op */ }
// Nostr does not use Noise sessions here; return a cached placeholder to avoid reallocation
private static var cachedNoiseService: NoiseEncryptionService = {
NoiseEncryptionService()
}()
func getNoiseService() -> NoiseEncryptionService { Self.cachedNoiseService }
private static var cachedNoiseService: NoiseEncryptionService?
func getNoiseService() -> NoiseEncryptionService {
if let noiseService = Self.cachedNoiseService {
return noiseService
}
let noiseService = NoiseEncryptionService(keychain: keychain)
Self.cachedNoiseService = noiseService
return noiseService
}
// Public broadcast not supported over Nostr here
func sendMessage(_ content: String, mentions: [String]) { /* no-op */ }
+7 -5
View File
@@ -27,14 +27,16 @@ final class UnifiedPeerService: ObservableObject, TransportPeerEventsDelegate {
private var peerIndex: [String: BitchatPeer] = [:]
private var fingerprintCache: [String: String] = [:] // peerID -> fingerprint
private let meshService: Transport
private let identityManager: SecureIdentityStateManagerProtocol
weak var messageRouter: MessageRouter?
private let favoritesService = FavoritesPersistenceService.shared
private var cancellables = Set<AnyCancellable>()
// MARK: - Initialization
init(meshService: Transport) {
init(meshService: Transport, identityManager: SecureIdentityStateManagerProtocol) {
self.meshService = meshService
self.identityManager = identityManager
// Subscribe to changes from both services
setupSubscriptions()
@@ -174,7 +176,7 @@ final class UnifiedPeerService: ObservableObject, TransportPeerEventsDelegate {
// Determine reachability based on lastSeen and identity trust
let now = Date()
let fingerprint = peerInfo.noisePublicKey?.sha256Fingerprint()
let isVerified = fingerprint.map { SecureIdentityStateManager.shared.isVerified(fingerprint: $0) } ?? false
let isVerified = fingerprint.map { identityManager.isVerified(fingerprint: $0) } ?? false
let isFav = peerInfo.noisePublicKey.flatMap { favorites[$0]?.isFavorite } ?? false
let retention: TimeInterval = (isVerified || isFav) ? TransportConfig.bleReachabilityRetentionVerifiedSeconds : TransportConfig.bleReachabilityRetentionUnverifiedSeconds
// A peer is reachable if we recently saw them AND we are attached to the mesh
@@ -247,7 +249,7 @@ final class UnifiedPeerService: ObservableObject, TransportPeerEventsDelegate {
guard let fingerprint = getFingerprint(for: peerID) else { return false }
// Check SecureIdentityStateManager for block status
if let identity = SecureIdentityStateManager.shared.getSocialIdentity(for: fingerprint) {
if let identity = identityManager.getSocialIdentity(for: fingerprint) {
return identity.isBlocked
}
@@ -324,7 +326,7 @@ final class UnifiedPeerService: ObservableObject, TransportPeerEventsDelegate {
guard let fingerprint = getFingerprint(for: peerID) else { return }
// Get or create social identity
var identity = SecureIdentityStateManager.shared.getSocialIdentity(for: fingerprint)
var identity = identityManager.getSocialIdentity(for: fingerprint)
?? SocialIdentity(
fingerprint: fingerprint,
localPetname: nil,
@@ -347,7 +349,7 @@ final class UnifiedPeerService: ObservableObject, TransportPeerEventsDelegate {
}
}
SecureIdentityStateManager.shared.updateSocialIdentity(identity)
identityManager.updateSocialIdentity(identity)
}
/// Get fingerprint for peer ID