From 39d8f6a40523afa688df4f0f6f8e7ddb8090e55f Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Tue, 27 Oct 2026 12:33:05 -0700 Subject: [PATCH] fix(ios): keep rejected enrollment journals until replaced A rejected journal was deleted before its replacement was prepared, so a failed challenge fetch or restart lost the rejection count and reset the backoff. Keep the rejected journal until the fresh request overwrites it. A 400 or 401 can also reject a request that committed before its response was lost, once its attestation certificate expires. The replacement then conflicts with the live installation, consumes its challenge, and replays as 404, which replaced again at once with the count reset. Count every replacement and apply the doubling wait to a 404 that follows one, so each such cycle mints a bounded number of App Attest keys. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Tom Brow --- .../BuzzDevPushEnrollmentDriver.swift | 71 ++++++-------- .../BuzzDevPushEnrollmentDriverTests.swift | 93 +++++++++++++++---- 2 files changed, 104 insertions(+), 60 deletions(-) diff --git a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzDevPushEnrollmentDriver.swift b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzDevPushEnrollmentDriver.swift index 4a4e82dfe3..cc7eb8dc91 100644 --- a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzDevPushEnrollmentDriver.swift +++ b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzDevPushEnrollmentDriver.swift @@ -404,13 +404,15 @@ public final class BuzzDevPushEnrollmentDriver { deviceToken: Data, relayURL: URL ) async throws -> BuzzPushEndpointGrantRecord { - try await enroll(deviceToken: deviceToken, relayURL: relayURL, rejectedReplacements: 0) + try await enroll(deviceToken: deviceToken, relayURL: relayURL, replacing: nil) } + /// `rejected` is a journal the gateway terminally rejected. It stays durable, + /// with its rejection count, until a fresh request overwrites it. private func enroll( deviceToken: Data, relayURL: URL, - rejectedReplacements: Int + replacing rejected: BuzzPushPendingEnrollmentRecord? ) async throws -> BuzzPushEndpointGrantRecord { precondition(!deviceToken.isEmpty, "The APNs device token must not be empty") let relayOrigin = try Self.relayOrigin(relayURL) @@ -424,10 +426,11 @@ public final class BuzzDevPushEnrollmentDriver { let storedForOrigin = storedRecords.first { $0.gatewayOrigin == gatewayOrigin && $0.relayOrigin == relayOrigin.text } - let pendingEnrollment = try store.pendingEnrollment( + let journal = try store.pendingEnrollment( gatewayOrigin: gatewayOrigin, relayOrigin: relayOrigin.text ) + let pendingEnrollment = journal == rejected ? nil : journal if let pending = pendingEnrollment, pending.relayPubkey != relayPubkey || pending.endpointHash != endpointHash || pending.expiresAt <= nowSeconds @@ -435,14 +438,13 @@ public final class BuzzDevPushEnrollmentDriver { // Finish an unexpired request using its exact journaled endpoint before // replacing it. A token/key change does not make a response-lost request // safe to discard, nor does it require revoking sibling installations. - var replacements = 0 if pending.expiresAt > nowSeconds { - if case .replaced(let count) = try await resumeEnrollment( + if case .rejected = try await resumeEnrollment( pending, storedRecords: storedRecords, relayMetadataPubkey: nil ) { - replacements = count + return try await enroll(deviceToken: deviceToken, relayURL: relayURL, replacing: pending) } } else { try store.removePendingEnrollment( @@ -450,11 +452,7 @@ public final class BuzzDevPushEnrollmentDriver { relayOrigin: relayOrigin.text ) } - return try await enroll( - deviceToken: deviceToken, - relayURL: relayURL, - rejectedReplacements: replacements - ) + return try await enroll(deviceToken: deviceToken, relayURL: relayURL, replacing: nil) } let newestSharedGrant = storedRecords.filter { $0.gatewayOrigin == gatewayOrigin && $0.relayPubkey == relayPubkey @@ -561,9 +559,10 @@ public final class BuzzDevPushEnrollmentDriver { keyId: attestation.keyId, attestation: attestation.attestation, preparedAt: nowSeconds, - rejectedReplacements: rejectedReplacements + rejectedReplacements: rejected.map { ($0.rejectedReplacements ?? 0) + 1 } ) // The exact signed request is durable before the first network attempt. + // Saving it is also what replaces a rejected journal. try store.savePendingEnrollment(pending) } @@ -575,12 +574,8 @@ public final class BuzzDevPushEnrollmentDriver { ) { case .completed(let record): return record - case .replaced(let count): - return try await enroll( - deviceToken: deviceToken, - relayURL: relayURL, - rejectedReplacements: count - ) + case .rejected: + return try await enroll(deviceToken: deviceToken, relayURL: relayURL, replacing: pending) } } return try await completeEnrollment( @@ -592,8 +587,8 @@ public final class BuzzDevPushEnrollmentDriver { private enum Resumption { case completed(BuzzPushEndpointGrantRecord) - /// The journal was discarded; a fresh request carries this rejection count. - case replaced(rejectedReplacements: Int) + /// The gateway terminally rejected the journal, which may now be replaced. + case rejected } private func resumeEnrollment( @@ -615,37 +610,31 @@ public final class BuzzDevPushEnrollmentDriver { // Only a terminal response permits replacing the old journal with a fresh // challenge and key. A fresh request's failure propagates instead of // recursively retrying forever. - try store.removePendingEnrollment( - gatewayOrigin: gatewayOrigin, - relayOrigin: pending.relayOrigin - ) - return .replaced( - rejectedReplacements: actual == 404 ? 0 : (pending.rejectedReplacements ?? 0) + 1 - ) + return .rejected } } /// 404 means the exact replay found no committed installation and its - /// challenge expired. The gateway returns 400 and 401 before looking up a - /// committed installation, so the same bytes can never recover one; those - /// journals are replaced only once old enough, with the wait doubling for - /// each consecutive replacement, because every replacement generates and - /// attests a new App Attest key. + /// challenge is gone. The gateway returns 400 and 401 before looking up a + /// committed installation, so the same bytes can never recover one, even + /// when an earlier attempt committed and its response was lost. Every + /// replacement generates and attests a new App Attest key, so only a first + /// 404 replaces at once. Otherwise the journal must be old enough, with the + /// wait doubling for each consecutive replacement. That bounds new keys when + /// a replacement itself fails, for example with 409 against a live + /// installation for the same token, whose next replay then returns 404. private func replacesRejectedJournal( _ pending: BuzzPushPendingEnrollmentRecord, _ status: Int ) -> Bool { - switch status { - case 404: + guard [400, 401, 404].contains(status) else { return false } + let replacements = pending.rejectedReplacements ?? 0 + guard status != 404 || replacements > 0, let preparedAt = pending.preparedAt else { return true - case 400, 401: - guard let preparedAt = pending.preparedAt else { return true } - let nowSeconds = Int64(now().timeIntervalSince1970) - let doublings = min(pending.rejectedReplacements ?? 0, 20) - return nowSeconds - preparedAt >= Self.rejectedEnrollmentReplacementSeconds << doublings - default: - return false } + let nowSeconds = Int64(now().timeIntervalSince1970) + let wait = Self.rejectedEnrollmentReplacementSeconds << min(replacements, 20) + return nowSeconds - preparedAt >= wait } private func completeEnrollment( diff --git a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzDevPushEnrollmentDriverTests.swift b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzDevPushEnrollmentDriverTests.swift index 3c97a64b08..f7fa488e70 100644 --- a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzDevPushEnrollmentDriverTests.swift +++ b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzDevPushEnrollmentDriverTests.swift @@ -395,15 +395,21 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { func testRejectedEnrollmentReplayIsReplacedOnlyOnTerminalStatus() async throws { // A journaled request the gateway rejects must not be replayed for its whole // lifetime, nor replaced (with a fresh App Attest key) on every retry; a - // transient failure must keep the journal. + // transient failure must keep the journal. A fresh 409 models a request that + // committed before its response was lost and was rejected once its + // attestation certificate expired: the replacement conflicts with the live + // installation and its consumed challenge then returns 404. let interval = BuzzDevPushEnrollmentDriver.rejectedEnrollmentReplacementSeconds let cases: [(replay: Int, fresh: Int?, elapsed: Int64, legacy: Bool)] = [ (400, 201, interval, false), (401, 201, interval, false), (404, 201, 0, false), (401, 201, 0, true), (401, nil, interval - 1, false), (401, 401, interval, false), - (503, nil, interval, false), + (401, 409, interval, false), (503, nil, interval, false), ] for (replayStatus, freshStatus, elapsed, legacy) in cases { - let label = "replay \(replayStatus) after \(elapsed)s, legacy \(legacy)" + let fresh = freshStatus.map(String.init) ?? "none" + let label = "replay \(replayStatus), fresh \(fresh) after \(elapsed)s, legacy \(legacy)" + // Status for a later exact replay of a fresh request. + let freshReplayStatus = freshStatus == 409 ? 404 : freshStatus let store = MemoryGrantStore() let appAttest = RecordingAppAttest() var clock = Self.now @@ -414,8 +420,10 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { lifetimeSeconds: 30 * 86_400 ) var challengeCount = 0 + var failChallenge = false var replayRequests = 0 var freshRequests = 0 + var sentChallenges = Set() URLProtocolStub.handler = { request in switch (request.httpMethod, request.url?.absoluteString) { case ("GET", "https://relay.example/"): @@ -423,9 +431,16 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { "push": ["keys": [["pubkey": Self.relayPubkey, "current": true]]] ]) case ("POST", "http://push.example/v1/installations/challenges"): + if failChallenge { + failChallenge = false + throw URLError(.notConnectedToInternet) + } challengeCount += 1 return Self.response(request, status: 200, json: [ - "challenge_id": challengeCount == 1 ? Self.firstChallengeId : Self.secondChallengeId, + "challenge_id": [Self.firstChallengeId, Self.secondChallengeId].indices + .contains(challengeCount - 1) + ? [Self.firstChallengeId, Self.secondChallengeId][challengeCount - 1] + : UUID().uuidString.lowercased(), "challenge": Self.challenge, "expires_at": clock + 300, ]) @@ -437,7 +452,12 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { return Self.response(request, status: replayStatus, json: ["error": "rejected"]) } freshRequests += 1 - let status = try XCTUnwrap(freshStatus, "\(label) must keep the journal") + let challengeId = try XCTUnwrap(body["challenge_id"] as? String) + let isReplay = !sentChallenges.insert(challengeId).inserted + let status = try XCTUnwrap( + isReplay ? freshReplayStatus : freshStatus, + "\(label) must keep the journal" + ) guard status == 201 else { return Self.response(request, status: status, json: ["error": "rejected"]) } @@ -469,7 +489,7 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { clock += elapsed // Two retries: the second must not replace a journal again. - for _ in 0..<2 { + for attempt in 0..<2 { do { let grant = try await driver.enroll(deviceToken: token, relayURL: Self.relayURL) XCTAssertEqual(freshStatus, 201, label) @@ -479,7 +499,11 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { route: "v1/installations", _, let actual, _ ) { // The failure stays visible and leaves a durable journal to retry. - XCTAssertEqual(actual, freshStatus ?? replayStatus, label) + XCTAssertEqual( + actual, + (attempt == 0 ? freshStatus : freshReplayStatus) ?? replayStatus, + label + ) XCTAssertEqual( store.pending.first?.challengeId, freshStatus == nil ? Self.firstChallengeId : Self.secondChallengeId, @@ -490,20 +514,51 @@ final class BuzzDevPushEnrollmentDriverTests: XCTestCase { XCTAssertEqual(freshRequests, freshStatus == nil ? 0 : freshStatus == 201 ? 1 : 2, label) XCTAssertEqual(appAttest.preparedAttestations.count, freshStatus == nil ? 1 : 2, label) - guard freshStatus == 401 else { continue } + guard let freshStatus, let freshReplayStatus, freshStatus != 201 else { continue } // Each consecutive replacement doubles the wait before the next one. - XCTAssertEqual(store.pending.first?.rejectedReplacements, 1) - for (advance, keys) in [(interval, 2), (interval, 3)] { - clock += advance - do { - _ = try await driver.enroll(deviceToken: token, relayURL: Self.relayURL) - XCTFail("Expected the persistent rejection") - } catch BuzzDevPushEnrollmentError.unexpectedStatus( - route: "v1/installations", _, actual: 401, _ - ) {} - XCTAssertEqual(appAttest.preparedAttestations.count, keys) + XCTAssertEqual(store.pending.first?.rejectedReplacements, 1, label) + clock += interval + await Self.assertEnrollmentStatus(driver, token, freshReplayStatus, label) + XCTAssertEqual(appAttest.preparedAttestations.count, 2, label) + + // A replacement that fails before its request is durable keeps the + // rejected journal and its count, even across a driver restart. + clock += interval + failChallenge = true + do { + _ = try await driver.enroll(deviceToken: token, relayURL: Self.relayURL) + XCTFail("Expected the simulated challenge failure: \(label)") + } catch { + XCTAssertEqual((error as NSError).domain, NSURLErrorDomain, label) } - XCTAssertEqual(store.pending.first?.rejectedReplacements, 2) + XCTAssertEqual(store.pending.first?.rejectedReplacements, 1, label) + let restarted = try makeDriver( + store: store, + appAttest: appAttest, + now: { Date(timeIntervalSince1970: TimeInterval(clock)) }, + lifetimeSeconds: 30 * 86_400 + ) + await Self.assertEnrollmentStatus(restarted, token, freshStatus, label) + XCTAssertEqual(appAttest.preparedAttestations.count, 3, label) + XCTAssertEqual(store.pending.first?.rejectedReplacements, 2, label) + } + } + + private static func assertEnrollmentStatus( + _ driver: BuzzDevPushEnrollmentDriver, + _ token: Data, + _ status: Int, + _ label: String + ) async { + do { + _ = try await driver.enroll(deviceToken: token, relayURL: relayURL) + XCTFail("Expected gateway status \(status): \(label)") + } catch BuzzDevPushEnrollmentError.unexpectedStatus( + route: "v1/installations", _, let actual, _ + ) { + XCTAssertEqual(actual, status, label) + } catch { + XCTFail("Unexpected error \(error): \(label)") } }