From 177ad68c27ff606048b0ce1533c8bc1710185cd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 5 Aug 2026 19:31:51 +0200 Subject: [PATCH 1/2] fix(auth): compare App Attest key ids as bytes Apple's `DCAppAttestService.generateKey` returns the key id in standard base64. The verifier re-encoded the credential id from authData as base64url and compared the strings, so the padding alone made every first-time iOS attestation fail with KEY_ID_MISMATCH. Under enforce mode that surfaced as "Your device can't be verified" on every iOS sign-in. Compare the decoded bytes instead. Node's base64 decoder accepts both alphabets, so either wire form verifies. The credential-id check now runs before the nonce hash, which is both cheaper and reachable in tests. The client stored the key id as soon as `attestKeyAsync` resolved, which says nothing about the server persisting it, so a refused device then asserted against an unknown key forever. Clear the stored id when the server refuses admission, in `postAuth`, where every native auth POST already routes through. --- apps/mobile/src/lib/auth/admission.test.ts | 23 +++++++++- apps/mobile/src/lib/auth/admission.ts | 15 +++++++ apps/mobile/src/lib/auth/auth-fetch.ts | 9 +++- .../lib/auth/native-admission-apple.test.ts | 42 +++++++++++++++++- .../src/lib/auth/native-admission-apple.ts | 43 +++++++++++-------- 5 files changed, 110 insertions(+), 22 deletions(-) diff --git a/apps/mobile/src/lib/auth/admission.test.ts b/apps/mobile/src/lib/auth/admission.test.ts index 03cf062cb1..3558f3beea 100644 --- a/apps/mobile/src/lib/auth/admission.test.ts +++ b/apps/mobile/src/lib/auth/admission.test.ts @@ -5,7 +5,11 @@ import { CryptoDigestAlgorithm, CryptoEncoding, digestStringAsync } from 'expo-c import * as SecureStore from 'expo-secure-store'; import { Platform } from 'react-native'; -import { ADMISSION_CHALLENGE_FAILED, hasAttestationCapability } from './admission'; +import { + ADMISSION_CHALLENGE_FAILED, + clearAttestKeyOnRefusal, + hasAttestationCapability, +} from './admission'; import { ATTEST_KEY_ID_KEY } from '@/lib/storage-keys'; /** @@ -236,4 +240,21 @@ describe('getAdmission', () => { it('exports ADMISSION_CHALLENGE_FAILED as a constant for caller catch blocks', () => { expect(ADMISSION_CHALLENGE_FAILED).toBe('admission_challenge_failed'); }); + + describe('clearAttestKeyOnRefusal', () => { + it('drops the stored key id so the next attempt re-attests', async () => { + // The server refuses an assertion for a key it never persisted. Without + // the clear, the device asserts against that key forever. + await clearAttestKeyOnRefusal('ADMISSION_REQUIRED'); + + expect(SecureStore.deleteItemAsync).toHaveBeenCalledWith(ATTEST_KEY_ID_KEY); + }); + + it('keeps the key id for any other error code', async () => { + await clearAttestKeyOnRefusal('INVALID_CODE'); + await clearAttestKeyOnRefusal(undefined); + + expect(SecureStore.deleteItemAsync).not.toHaveBeenCalled(); + }); + }); }); diff --git a/apps/mobile/src/lib/auth/admission.ts b/apps/mobile/src/lib/auth/admission.ts index 51c47086f6..e4077596b1 100644 --- a/apps/mobile/src/lib/auth/admission.ts +++ b/apps/mobile/src/lib/auth/admission.ts @@ -144,6 +144,21 @@ async function getAndroidAdmission(challenge: string): Promise }; } +/** + * Drop the stored key id when the server refuses admission. + * + * The client stores the key id as soon as `attestKeyAsync` resolves, which says + * nothing about whether the server accepted and persisted the key. When it did + * not, every later sign-in asserts against a key the server has never seen and + * is refused forever. Clearing the id makes the next attempt re-attest, which + * Apple still has to sign, so this weakens nothing. + */ +export async function clearAttestKeyOnRefusal(errorCode: string | undefined): Promise { + if (errorCode === 'ADMISSION_REQUIRED' && Platform.OS === 'ios') { + await SecureStore.deleteItemAsync(ATTEST_KEY_ID_KEY); + } +} + /** * Request a server admission challenge and produce a platform-specific * attestation or assertion. diff --git a/apps/mobile/src/lib/auth/auth-fetch.ts b/apps/mobile/src/lib/auth/auth-fetch.ts index b2d5d5c408..97d14bd3ab 100644 --- a/apps/mobile/src/lib/auth/auth-fetch.ts +++ b/apps/mobile/src/lib/auth/auth-fetch.ts @@ -1,9 +1,14 @@ import { API_BASE_URL } from '@/lib/config'; +import { clearAttestKeyOnRefusal } from '@/lib/auth/admission'; import { parseAuthErrorCode } from '@/lib/auth/native-auth-contract'; /** * Minimal fetch helper for auth endpoints. Returns success with parsed body * or failure with an optional error code. + * + * Every native auth POST routes through here, so this is also where a refused + * admission drops the stored App Attest key id. Putting it here rather than at + * each caller means a new sign-in path cannot forget it. */ export async function postAuth( path: string, @@ -24,7 +29,9 @@ export async function postAuth( } if (!response.ok) { - return { ok: false, errorCode: parseAuthErrorCode(json) }; + const errorCode = parseAuthErrorCode(json); + await clearAttestKeyOnRefusal(errorCode); + return { ok: false, errorCode }; } return { ok: true, data: json }; diff --git a/apps/web/src/lib/auth/native-admission-apple.test.ts b/apps/web/src/lib/auth/native-admission-apple.test.ts index 7f789d1d00..b1d131877a 100644 --- a/apps/web/src/lib/auth/native-admission-apple.test.ts +++ b/apps/web/src/lib/auth/native-admission-apple.test.ts @@ -159,8 +159,11 @@ describe('verifyAppleAttestation certificate chain', () => { const teamId = 'WRPHYY66V6'; const bundleId = 'com.reelreel.app.dev'; const rpIdHash = createHash('sha256').update(`${teamId}.${bundleId}`).digest(); - const credentialId = Buffer.from('0123456789abcdef'); - const keyId = credentialId.toString('base64url'); + // Apple's keyId is the SHA-256 of the public key in standard base64, so 32 + // bytes and a `=` pad. Build the fixture the way the device does; base64url + // here would hide the padding mismatch that broke every real attestation. + const credentialId = createHash('sha256').update('device-key').digest(); + const keyId = credentialId.toString('base64'); const challenge = 'c2VydmVyLWNoYWxsZW5nZQ'; // arbitrary base64url bytes function attestationFor(x5c: Buffer[]): string { @@ -203,6 +206,41 @@ describe('verifyAppleAttestation certificate chain', () => { expect(result).toEqual({ ok: false, error: 'CERT_CHAIN_INVALID' }); }); + test('accepts the standard-base64 keyId the device sends', async () => { + // Regression: comparing `credentialId.toString('base64url')` against + // Apple's padded standard-base64 keyId never matched, so every first-time + // attestation was refused with ADMISSION_REQUIRED. Reaching NONCE_MISMATCH + // proves the credential-ID check passed. + expect(keyId).toMatch(/=$/); + const result = await verifyAppleAttestation( + attestationFor([REAL_LEAF_DER, REAL_INTERMEDIATE_DER]), + challenge, + keyId, + bundleId + ); + expect(result).toEqual({ ok: false, error: 'NONCE_MISMATCH' }); + }); + + test('accepts the same key id in base64url form', async () => { + const result = await verifyAppleAttestation( + attestationFor([REAL_LEAF_DER, REAL_INTERMEDIATE_DER]), + challenge, + credentialId.toString('base64url'), + bundleId + ); + expect(result).toEqual({ ok: false, error: 'NONCE_MISMATCH' }); + }); + + test('rejects a key id that is not the credential id', async () => { + const result = await verifyAppleAttestation( + attestationFor([REAL_LEAF_DER, REAL_INTERMEDIATE_DER]), + challenge, + createHash('sha256').update('other-key').digest('base64'), + bundleId + ); + expect(result).toEqual({ ok: false, error: 'KEY_ID_MISMATCH' }); + }); + test('rejects a chain whose signatures do not verify', async () => { // The real leaf is signed by the intermediate, not by the root. const result = await verifyAppleAttestation( diff --git a/apps/web/src/lib/auth/native-admission-apple.ts b/apps/web/src/lib/auth/native-admission-apple.ts index fdf4d577ea..cae7fd1321 100644 --- a/apps/web/src/lib/auth/native-admission-apple.ts +++ b/apps/web/src/lib/auth/native-admission-apple.ts @@ -181,6 +181,31 @@ export async function verifyAppleAttestation( return { ok: false, error: 'RP_ID_MISMATCH' }; } + // Extract credential ID from authData + const flagsByte = authData[32]; + if (flagsByte === undefined) return { ok: false, error: 'INVALID_ATTEST_FORMAT' }; + const flags = flagsByte; + if (!(flags & 0x40)) return { ok: false, error: 'INVALID_ATTEST_FORMAT' }; // AT flag + + let pos = 37; // rpIdHash(32) + flags(1) + signCount(4) + pos += 16; // aaguid + const credIdLen = authData.readUInt16BE(pos); + pos += 2; + const credentialId = authData.subarray(pos, pos + credIdLen); + pos += credIdLen; + + // Verify keyId matches credential ID. Runs before the nonce hash because it + // is a byte comparison on data already parsed. + // + // `DCAppAttestService.generateKey` hands back standard base64 (padded, `+` + // and `/`), which the client forwards verbatim. Compare the decoded bytes, + // never the re-encoded string: base64url of the same 32 bytes drops the + // padding, so a string comparison against Apple's keyId never matches. + // Node's base64 decoder accepts both alphabets, so either form works here. + if (!credentialId.equals(Buffer.from(expectedKeyId, 'base64'))) { + return { ok: false, error: 'KEY_ID_MISMATCH' }; + } + // Nonce check. Apple's nonce is SHA256(authData || clientDataHash). // `@expo/app-integrity` computes clientDataHash as SHA256 over the UTF-8 // bytes of the challenge string it was handed, so hash the same bytes here. @@ -200,24 +225,6 @@ export async function verifyAppleAttestation( return { ok: false, error: 'NONCE_MISMATCH' }; } - // Extract credential ID from authData - const flagsByte = authData[32]; - if (flagsByte === undefined) return { ok: false, error: 'INVALID_ATTEST_FORMAT' }; - const flags = flagsByte; - if (!(flags & 0x40)) return { ok: false, error: 'INVALID_ATTEST_FORMAT' }; // AT flag - - let pos = 37; // rpIdHash(32) + flags(1) + signCount(4) - pos += 16; // aaguid - const credIdLen = authData.readUInt16BE(pos); - pos += 2; - const credentialId = authData.subarray(pos, pos + credIdLen); - pos += credIdLen; - - // Verify keyId matches credential ID - if (credentialId.toString('base64url') !== expectedKeyId) { - return { ok: false, error: 'KEY_ID_MISMATCH' }; - } - // Extract COSE public key and export it as SPKI DER const coseKeyBuf = authData.subarray(pos); let publicKeySpkiBase64: string; From b880c0e6993a37a1091ba71de79b7e289050e486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Wed, 5 Aug 2026 19:41:24 +0200 Subject: [PATCH 2/2] fix(auth): reject authData too short to parse The credential-ID parse reads a uint16 at offset 53, so it needs 55 bytes, but the only length guard allowed 37. Moving that parse ahead of the nonce check made offsets 37 to 54 reachable with crafted authData, where readUInt16BE throws a RangeError and the route reports a 500 instead of refusing admission. Raise the floor to 55, the shortest authData that can hold the credential-ID length prefix. A real attestation always exceeds it. --- .../src/lib/auth/native-admission-apple.test.ts | 17 +++++++++++++++++ apps/web/src/lib/auth/native-admission-apple.ts | 6 +++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/apps/web/src/lib/auth/native-admission-apple.test.ts b/apps/web/src/lib/auth/native-admission-apple.test.ts index b1d131877a..6b1e820d54 100644 --- a/apps/web/src/lib/auth/native-admission-apple.test.ts +++ b/apps/web/src/lib/auth/native-admission-apple.test.ts @@ -241,6 +241,23 @@ describe('verifyAppleAttestation certificate chain', () => { expect(result).toEqual({ ok: false, error: 'KEY_ID_MISMATCH' }); }); + test('rejects authData too short to hold the credential-ID length', async () => { + // 37 to 54 bytes cleared the old floor but cannot be parsed: readUInt16BE(53) + // throws a RangeError, which the route surfaces as a 500 instead of a + // refusal. Malformed authData must keep failing closed. + const full = buildAuthData({ rpIdHash, credentialId, coseKey: buildCoseKey() }); + + for (const length of [37, 48, 54]) { + const result = await verifyAppleAttestation( + buildAttestation([REAL_LEAF_DER, REAL_INTERMEDIATE_DER], full.subarray(0, length)), + challenge, + keyId, + bundleId + ); + expect(result).toEqual({ ok: false, error: 'INVALID_ATTEST_FORMAT' }); + } + }); + test('rejects a chain whose signatures do not verify', async () => { // The real leaf is signed by the intermediate, not by the root. const result = await verifyAppleAttestation( diff --git a/apps/web/src/lib/auth/native-admission-apple.ts b/apps/web/src/lib/auth/native-admission-apple.ts index cae7fd1321..f314e5126b 100644 --- a/apps/web/src/lib/auth/native-admission-apple.ts +++ b/apps/web/src/lib/auth/native-admission-apple.ts @@ -123,8 +123,12 @@ export async function verifyAppleAttestation( if (attestMap.get('fmt') !== 'apple-appattest') return { ok: false, error: 'INVALID_ATTEST_FORMAT' }; + // Shortest possible attested authData: rpIdHash(32) + flags(1) + signCount(4) + // + aaguid(16) + credIdLen(2). Anything shorter cannot hold the credential-ID + // length prefix, and `readUInt16BE` would throw a RangeError instead of + // failing closed. A real attestation always exceeds this. const authData = asBuffer(attestMap.get('authData')); - if (!authData || authData.length < 37) return { ok: false, error: 'INVALID_ATTEST_FORMAT' }; + if (!authData || authData.length < 55) return { ok: false, error: 'INVALID_ATTEST_FORMAT' }; const attStmt = attestMap.get('attStmt'); if (!(attStmt instanceof Map)) return { ok: false, error: 'INVALID_ATTEST_FORMAT' };