Skip to content

Commit b4b91e5

Browse files
craigjbassclaude
andcommitted
fix: signature-issue dialog and preserve policy across ACL re-key
Two bugs combined to wipe user policy on the v3 ACL migration: 1. NSXPC allowed-classes list for `signatureIssueDetected(_:)` was `[SignatureIssueNotification.self]` only. The notification's `init(coder:)` decodes `NSData` fields, which NSSecureCoding silently rejects when NSData is not in the allowed list. The entire call was dropped by NSXPC, so the suspect-signature dialog never fired and users had no way to recover quarantined `user_rules` and `user_allowlist` after a verification failure. Latent since the feature was introduced — masked because real-world signature failures were rare until the v3 re-key forced everyone into the suspect path at once. 2. ACL migration was lossy by design. Re-keying happened inside the first call to `PolicySigner.verify`, which invalidated every existing signature. With the dialog also broken (#1), the `user_rules` and `user_allowlist` quarantine was inaccessible, and `user_ancestor_allowlist`, `user_jail_rules`, `bundle_updater_signatures`, and `feature_flags` were silently discarded. Migration now runs explicitly inside `Database.init` after schema migrations: every table whose signature still verifies under the about-to-be-rotated key is captured, the key is rotated, and the captured content is re-signed with the new key in a single SQLite transaction. Nothing is lost across the upgrade. For users already on 5780b84: if no mutation has been made to the affected tables since the broken upgrade, the on-disk rows are still present and the now-working dialog will surface them on next launch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5780b84 commit b4b91e5

3 files changed

Lines changed: 160 additions & 51 deletions

File tree

clearancekit/App/XPCClient.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ final class XPCClient: NSObject, ObservableObject {
129129
ofReply: false
130130
)
131131
conn.exportedInterface?.setClasses(
132-
NSSet(array: [SignatureIssueNotification.self]) as! Set<AnyHashable>,
132+
NSSet(array: [SignatureIssueNotification.self, NSData.self]) as! Set<AnyHashable>,
133133
for: #selector(ClientProtocol.signatureIssueDetected(_:)),
134134
argumentIndex: 0,
135135
ofReply: false

opfilter/Database/Database.swift

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ final class Database {
5555
execute("PRAGMA foreign_keys=ON")
5656

5757
runMigrations()
58+
migrateAclIfNeeded()
5859
}
5960

6061
deinit {
@@ -565,6 +566,125 @@ final class Database {
565566
case suspect
566567
}
567568

569+
// MARK: - ACL migration
570+
571+
/// Captures every signed table whose existing signature verifies under the
572+
/// current (about-to-be-rotated) key, rotates the signing key so the next
573+
/// load creates a fresh one bound to opfilter's explicit executable path,
574+
/// then re-signs every captured table with the new key. Tables whose
575+
/// signatures don't verify under the old key are left alone — they'll
576+
/// enter the existing `.suspect` flow on next load.
577+
private func migrateAclIfNeeded() {
578+
guard PolicySigner.aclMigrationVersion() < PolicySigner.currentAclVersion() else { return }
579+
580+
let captured = captureAllVerifiableTableContent()
581+
582+
PolicySigner.rotateKey()
583+
584+
// Force creation of the new key now so re-sign uses it.
585+
_ = try? PolicySigner.loadOrCreateKey()
586+
587+
inTransaction {
588+
for (table, content) in captured {
589+
updateSignature(table: table, content: content)
590+
}
591+
}
592+
593+
PolicySigner.recordAclMigrationVersion()
594+
NSLog("Database: ACL migration v%d complete — re-signed %d table(s) with new key", PolicySigner.currentAclVersion(), captured.count)
595+
}
596+
597+
private func captureAllVerifiableTableContent() -> [(String, Data)] {
598+
var captured: [(String, Data)] = []
599+
for (table, content) in allTablesWithCanonicalContent() {
600+
let signature = readSignatureBlob(table: table)
601+
guard let sig = signature else { continue }
602+
// Existing signatures (pre-epoch migration) are over `content`
603+
// alone; the new format is `content || epoch.bigEndian`. At v3
604+
// migration time the epoch column was just added with default 0,
605+
// so the legacy format applies for every row we encounter here.
606+
if PolicySigner.canVerify(content, signature: sig) {
607+
captured.append((table, content))
608+
}
609+
}
610+
return captured
611+
}
612+
613+
private func readSignatureBlob(table: String) -> Data? {
614+
var signature: Data?
615+
query("SELECT signature FROM data_signatures WHERE table_name = ?", bindings: [.text(table)]) { stmt in
616+
guard let blobPtr = sqlite3_column_blob(stmt, 0) else { return }
617+
let blobLen = sqlite3_column_bytes(stmt, 0)
618+
signature = Data(bytes: blobPtr, count: Int(blobLen))
619+
}
620+
return signature
621+
}
622+
623+
private func allTablesWithCanonicalContent() -> [(String, Data)] {
624+
var result: [(String, Data)] = []
625+
626+
var rules: [FAARule] = []
627+
query("""
628+
SELECT id, protected_path_prefix,
629+
allowed_process_paths, allowed_signatures,
630+
allowed_ancestor_process_paths, allowed_ancestor_signatures,
631+
enforce_on_write_only, require_valid_signing,
632+
authorized_signatures, requires_authorization,
633+
authorization_session_duration
634+
FROM user_rules ORDER BY rowid
635+
""") { stmt in
636+
if let rule = ruleFromRow(stmt) { rules.append(rule) }
637+
}
638+
result.append(("user_rules", canonicalRulesJSON(rules)))
639+
640+
var allowlist: [AllowlistEntry] = []
641+
query("""
642+
SELECT id, signing_id, process_path, platform_binary, team_id
643+
FROM user_allowlist ORDER BY rowid
644+
""") { stmt in
645+
if let entry = allowlistEntryFromRow(stmt) { allowlist.append(entry) }
646+
}
647+
result.append(("user_allowlist", canonicalAllowlistJSON(allowlist)))
648+
649+
var ancestor: [AncestorAllowlistEntry] = []
650+
query("""
651+
SELECT id, signing_id, process_path, platform_binary, team_id
652+
FROM user_ancestor_allowlist ORDER BY rowid
653+
""") { stmt in
654+
if let entry = ancestorAllowlistEntryFromRow(stmt) { ancestor.append(entry) }
655+
}
656+
result.append(("user_ancestor_allowlist", canonicalAncestorAllowlistJSON(ancestor)))
657+
658+
var jail: [JailRule] = []
659+
query("""
660+
SELECT id, name, jailed_signature, allowed_path_prefixes
661+
FROM user_jail_rules ORDER BY rowid
662+
""") { stmt in
663+
if let rule = jailRuleFromRow(stmt) { jail.append(rule) }
664+
}
665+
result.append(("user_jail_rules", canonicalJailRulesJSON(jail)))
666+
667+
var flags: [FeatureFlag] = []
668+
query("SELECT id, name, enabled FROM feature_flags ORDER BY rowid") { stmt in
669+
if let flag = featureFlagFromRow(stmt) { flags.append(flag) }
670+
}
671+
result.append(("feature_flags", canonicalFeatureFlagsJSON(flags)))
672+
673+
var updaterSigs: [BundleUpdaterSignature] = []
674+
query("SELECT id, team_id, signing_id FROM bundle_updater_signatures ORDER BY rowid") { stmt in
675+
let uuidString = columnText(stmt, 0)
676+
guard let id = UUID(uuidString: uuidString) else { return }
677+
updaterSigs.append(BundleUpdaterSignature(
678+
id: id,
679+
teamID: columnText(stmt, 1),
680+
signingID: columnText(stmt, 2)
681+
))
682+
}
683+
result.append(("bundle_updater_signatures", canonicalBundleUpdaterSignaturesJSON(updaterSigs)))
684+
685+
return result
686+
}
687+
568688
private func updateSignature(table: String, content: Data) {
569689
let diskEpoch = readDiskEpoch(table: table) ?? 0
570690
let keychainEpoch = EpochRatchet.epoch(forTable: table) ?? 0

opfilter/Policy/PolicySigner.swift

Lines changed: 39 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,49 @@ enum PolicySigner {
9090
// MARK: - Key lifecycle
9191

9292
static func loadOrCreateKey() throws -> SecKey {
93-
rekeyForExplicitPathAclOnce()
9493
if let key = try? loadKey() { return key }
9594
return try createSoftwareKey()
9695
}
9796

97+
/// Verifies `signature` against `data` using the currently-loaded signing
98+
/// key and reports the result as a Bool, without throwing. Used by
99+
/// `migrateAclIfNeeded` to capture tables whose existing signature is
100+
/// valid under the old (about-to-be-rotated) key.
101+
static func canVerify(_ data: Data, signature: Data) -> Bool {
102+
(try? verify(data, signature: signature)) != nil
103+
}
104+
105+
/// Forces the next call to `loadOrCreateKey` to mint a fresh key by
106+
/// deleting any existing one. Used only by `migrateAclIfNeeded` after the
107+
/// caller has already captured everything they want to re-sign under the
108+
/// new key.
109+
static func rotateKey() {
110+
var query: [CFString: Any] = [
111+
kSecClass: kSecClassKey,
112+
kSecAttrApplicationTag: keyTag,
113+
kSecAttrKeyType: kSecAttrKeyTypeECSECPrimeRandom,
114+
kSecAttrKeyClass: kSecAttrKeyClassPrivate,
115+
]
116+
if let kc = systemKeychain { query[kSecUseKeychain] = kc }
117+
118+
let status = SecItemDelete(query as CFDictionary)
119+
switch status {
120+
case errSecSuccess:
121+
NSLog("PolicySigner: Rotated System Keychain key")
122+
case errSecItemNotFound:
123+
NSLog("PolicySigner: rotateKey — no existing key")
124+
default:
125+
NSLog("PolicySigner: SecItemDelete during rotateKey failed (%d)", status)
126+
}
127+
}
128+
129+
static func aclMigrationVersion() -> Int { readAclMigrationVersion() }
130+
static func currentAclVersion() -> Int { currentAclMigrationVersion }
131+
static func recordAclMigrationVersion() {
132+
for marker in legacyAclMarkers { try? FileManager.default.removeItem(at: marker) }
133+
writeAclMigrationVersion(currentAclMigrationVersion)
134+
}
135+
98136
private static func loadKey() throws -> SecKey {
99137
var query: [CFString: Any] = [
100138
kSecClass: kSecClassKey,
@@ -180,55 +218,6 @@ enum PolicySigner {
180218
return String(cString: buffer)
181219
}
182220

183-
// MARK: - One-time migration
184-
185-
/// Deletes any existing System Keychain key so that the next call to
186-
/// `loadOrCreateKey` rebuilds it with an explicit-executable-path ACL.
187-
/// Previous migration (v2) used `SecTrustedApplicationCreateFromPath(nil, …)`
188-
/// which observably produced an empty trusted-apps list when called from a
189-
/// system extension. v3 forces resolution against `_NSGetExecutablePath`.
190-
///
191-
/// The keychain version counter (`aclVersionService`) is the sole trust
192-
/// anchor — any state where the counter is below the current version is
193-
/// treated as needing migration. Because the counter is written with an
194-
/// opfilter-only ACL once v3 is active, an attacker cannot force a
195-
/// re-migration by tampering with on-disk markers; the legacy marker
196-
/// files written by earlier builds are ignored and cleaned up.
197-
///
198-
/// Existing signed tables become unverifiable because the signing key is
199-
/// fresh; PolicyRepository's load path quarantines or resets each table:
200-
/// - user_rules / user_allowlist → suspect dialog (Touch ID re-approve)
201-
/// - user_ancestor_allowlist / user_jail_rules / bundle_updater_signatures
202-
/// → silently discarded (existing behavior)
203-
/// - feature_flags → safe defaults (bundle protection ON)
204-
private static func rekeyForExplicitPathAclOnce() {
205-
if readAclMigrationVersion() >= currentAclMigrationVersion { return }
206-
207-
var query: [CFString: Any] = [
208-
kSecClass: kSecClassKey,
209-
kSecAttrApplicationTag: keyTag,
210-
kSecAttrKeyType: kSecAttrKeyTypeECSECPrimeRandom,
211-
kSecAttrKeyClass: kSecAttrKeyClassPrivate,
212-
]
213-
if let kc = systemKeychain { query[kSecUseKeychain] = kc }
214-
215-
let status = SecItemDelete(query as CFDictionary)
216-
switch status {
217-
case errSecSuccess:
218-
NSLog("PolicySigner: Deleted prior System Keychain key — re-keying with explicit-path ACL. User rules and allowlist will require Touch ID re-approval via the suspect-signature dialog; jail rules, ancestor allowlist, and bundle updater entries will be reset.")
219-
case errSecItemNotFound:
220-
NSLog("PolicySigner: No prior System Keychain key — fresh install, new key will be created with explicit-path ACL")
221-
default:
222-
NSLog("PolicySigner: Could not delete old key (%d) — it may need manual removal from Keychain Access", status)
223-
}
224-
225-
for marker in legacyAclMarkers {
226-
try? FileManager.default.removeItem(at: marker)
227-
}
228-
229-
writeAclMigrationVersion(currentAclMigrationVersion)
230-
}
231-
232221
// MARK: - ACL migration version counter
233222

234223
private static func readAclMigrationVersion() -> Int {

0 commit comments

Comments
 (0)