From f73ca9532c6f7857821b1a5ddf6f06104fdf3c7b Mon Sep 17 00:00:00 2001 From: Andrew Heard Date: Thu, 19 Oct 2023 17:28:38 -0400 Subject: [PATCH] [App Check] Add failure reasons for App Attest error cases (#11956) --- .../AppAttestProvider/FIRAppAttestProvider.m | 56 ++++++++++++------- .../Core/Errors/FIRAppCheckErrorUtil.h | 12 ++++ .../Core/Errors/FIRAppCheckErrorUtil.m | 39 +++++++++++++ .../FIRAppAttestProviderTests.m | 14 ++++- 4 files changed, 97 insertions(+), 24 deletions(-) diff --git a/FirebaseAppCheck/Sources/AppAttestProvider/FIRAppAttestProvider.m b/FirebaseAppCheck/Sources/AppAttestProvider/FIRAppAttestProvider.m index b59e8101009..d2afd4df14b 100644 --- a/FirebaseAppCheck/Sources/AppAttestProvider/FIRAppAttestProvider.m +++ b/FirebaseAppCheck/Sources/AppAttestProvider/FIRAppAttestProvider.m @@ -280,16 +280,21 @@ - (void)getTokenWithCompletion:(void (^)(FIRAppCheckToken *_Nullable, NSError *_ do:^NSData *_Nullable { return [FIRAppCheckCryptoUtils sha256HashFromData:challenge]; }] - .thenOn( - self.queue, - ^FBLPromise *(NSData *challengeHash) { - return [FBLPromise onQueue:self.queue - wrapObjectOrErrorCompletion:^(FBLPromiseObjectOrErrorCompletion _Nonnull handler) { - [self.appAttestService attestKey:keyID - clientDataHash:challengeHash - completionHandler:handler]; - }]; - }) + .thenOn(self.queue, + ^FBLPromise *(NSData *challengeHash) { + return [FBLPromise onQueue:self.queue + wrapObjectOrErrorCompletion:^( + FBLPromiseObjectOrErrorCompletion _Nonnull handler) { + [self.appAttestService attestKey:keyID + clientDataHash:challengeHash + completionHandler:handler]; + }] + .recoverOn(self.queue, ^id(NSError *error) { + return [FIRAppCheckErrorUtil appAttestAttestKeyFailedWithError:error + keyId:keyID + clientDataHash:challengeHash]; + }); + }) .thenOn(self.queue, ^FBLPromise *(NSData *attestation) { FIRAppAttestKeyAttestationResult *result = [[FIRAppAttestKeyAttestationResult alloc] initWithKeyID:keyID @@ -391,17 +396,22 @@ - (void)getTokenWithCompletion:(void (^)(FIRAppCheckToken *_Nullable, NSError *_ // 1.2. Get the statement SHA256 hash. return [FIRAppCheckCryptoUtils sha256HashFromData:[statementForAssertion copy]]; }] - .thenOn( - self.queue, - ^FBLPromise *(NSData *statementHash) { - // 2. Generate App Attest assertion. - return [FBLPromise onQueue:self.queue - wrapObjectOrErrorCompletion:^(FBLPromiseObjectOrErrorCompletion _Nonnull handler) { - [self.appAttestService generateAssertion:keyID - clientDataHash:statementHash - completionHandler:handler]; - }]; - }) + .thenOn(self.queue, + ^FBLPromise *(NSData *statementHash) { + return [FBLPromise onQueue:self.queue + wrapObjectOrErrorCompletion:^( + FBLPromiseObjectOrErrorCompletion _Nonnull handler) { + [self.appAttestService generateAssertion:keyID + clientDataHash:statementHash + completionHandler:handler]; + }] + .recoverOn(self.queue, ^id(NSError *error) { + return [FIRAppCheckErrorUtil + appAttestGenerateAssertionFailedWithError:error + keyId:keyID + clientDataHash:statementHash]; + }); + }) // 3. Compose the result object. .thenOn(self.queue, ^FIRAppAttestAssertionData *(NSData *assertion) { return [[FIRAppAttestAssertionData alloc] initWithChallenge:challenge @@ -479,6 +489,10 @@ - (void)getTokenWithCompletion:(void (^)(FIRAppCheckToken *_Nullable, NSError *_ wrapObjectOrErrorCompletion:^(FBLPromiseObjectOrErrorCompletion _Nonnull handler) { [self.appAttestService generateKeyWithCompletionHandler:handler]; }] + .recoverOn(self.queue, + ^id(NSError *error) { + return [FIRAppCheckErrorUtil appAttestGenerateKeyFailedWithError:error]; + }) .thenOn(self.queue, ^FBLPromise *(NSString *keyID) { return [self.keyIDStorage setAppAttestKeyID:keyID]; }); diff --git a/FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.h b/FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.h index 2b6b74c1ec5..00cf891c036 100644 --- a/FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.h +++ b/FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.h @@ -51,6 +51,18 @@ void FIRAppCheckSetErrorToPointer(NSError *error, NSError **pointer); + (NSError *)appAttestKeyIDNotFound; +// MARK: - App Attest Errors + ++ (NSError *)appAttestGenerateKeyFailedWithError:(NSError *)error; + ++ (NSError *)appAttestAttestKeyFailedWithError:(NSError *)error + keyId:(NSString *)keyId + clientDataHash:(NSData *)clientDataHash; + ++ (NSError *)appAttestGenerateAssertionFailedWithError:(NSError *)error + keyId:(NSString *)keyId + clientDataHash:(NSData *)clientDataHash; + @end NS_ASSUME_NONNULL_END diff --git a/FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.m b/FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.m index f071287eebe..0b85171df6d 100644 --- a/FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.m +++ b/FirebaseAppCheck/Sources/Core/Errors/FIRAppCheckErrorUtil.m @@ -118,6 +118,45 @@ + (NSError *)errorWithFailureReason:(NSString *)failureReason { underlyingError:nil]; } +#pragma mark - App Attest + ++ (NSError *)appAttestGenerateKeyFailedWithError:(NSError *)error { + NSString *failureReason = @"Failed to generate a new cryptographic key for use with the App " + @"Attest service (`generateKeyWithCompletionHandler:`)."; + // TODO(#11967): Add a new error code for this case (e.g., FIRAppCheckAppAttestGenerateKeyFailed). + return [self appCheckErrorWithCode:FIRAppCheckErrorCodeUnknown + failureReason:failureReason + underlyingError:error]; +} + ++ (NSError *)appAttestAttestKeyFailedWithError:(NSError *)error + keyId:(NSString *)keyId + clientDataHash:(NSData *)clientDataHash { + NSString *failureReason = + [NSString stringWithFormat:@"Failed to attest the validity of the generated cryptographic " + @"key (`attestKey:clientDataHash:completionHandler:`); " + @"keyId.length = %lu, clientDataHash.length = %lu", + keyId.length, clientDataHash.length]; + // TODO(#11967): Add a new error code for this case (e.g., FIRAppCheckAppAttestAttestKeyFailed). + return [self appCheckErrorWithCode:FIRAppCheckErrorCodeUnknown + failureReason:failureReason + underlyingError:error]; +} + ++ (NSError *)appAttestGenerateAssertionFailedWithError:(NSError *)error + keyId:(NSString *)keyId + clientDataHash:(NSData *)clientDataHash { + NSString *failureReason = [NSString + stringWithFormat:@"Failed to create a block of data that demonstrates the legitimacy of the " + @"app instance (`generateAssertion:clientDataHash:completionHandler:`); " + @"keyId.length = %lu, clientDataHash.length = %lu.", + keyId.length, clientDataHash.length]; + // TODO(#11967): Add error code for this case (e.g., FIRAppCheckAppAttestGenerateAssertionFailed). + return [self appCheckErrorWithCode:FIRAppCheckErrorCodeUnknown + failureReason:failureReason + underlyingError:error]; +} + #pragma mark - Helpers + (NSError *)unknownErrorWithError:(NSError *)error { diff --git a/FirebaseAppCheck/Tests/Unit/AppAttestProvider/FIRAppAttestProviderTests.m b/FirebaseAppCheck/Tests/Unit/AppAttestProvider/FIRAppAttestProviderTests.m index d20fb1d179b..0ff369ec25a 100644 --- a/FirebaseAppCheck/Tests/Unit/AppAttestProvider/FIRAppAttestProviderTests.m +++ b/FirebaseAppCheck/Tests/Unit/AppAttestProvider/FIRAppAttestProviderTests.m @@ -393,6 +393,10 @@ - (void)testGetToken_WhenUnregisteredKeyAndKeyAttestationError { NSError *attestationError = [NSError errorWithDomain:@"testGetTokenWhenKeyAttestationError" code:0 userInfo:nil]; + NSError *expectedError = + [FIRAppCheckErrorUtil appAttestAttestKeyFailedWithError:attestationError + keyId:existingKeyID + clientDataHash:self.randomChallengeHash]; id attestCompletionArg = [OCMArg invokeBlockWithArgs:[NSNull null], attestationError, nil]; OCMExpect([self.mockAppAttestService attestKey:existingKeyID clientDataHash:self.randomChallengeHash @@ -411,7 +415,7 @@ - (void)testGetToken_WhenUnregisteredKeyAndKeyAttestationError { [completionExpectation fulfill]; XCTAssertNil(token); - XCTAssertEqualObjects(error, attestationError); + XCTAssertEqualObjects(error, expectedError); }]; [self waitForExpectations:@[ self.fakeBackoffWrapper.backoffExpectation, completionExpectation ] @@ -422,7 +426,7 @@ - (void)testGetToken_WhenUnregisteredKeyAndKeyAttestationError { [self verifyAllMocks]; // 9. Verify backoff error. - XCTAssertEqualObjects(self.fakeBackoffWrapper.operationError, attestationError); + XCTAssertEqualObjects(self.fakeBackoffWrapper.operationError, expectedError); } - (void)testGetToken_WhenUnregisteredKeyAndKeyAttestationExchangeError { @@ -671,6 +675,10 @@ - (void)testGetToken_WhenKeyRegisteredAndGenerateAssertionError { [NSError errorWithDomain:@"testGetToken_WhenKeyRegisteredAndGenerateAssertionError" code:0 userInfo:nil]; + NSError *expectedError = + [FIRAppCheckErrorUtil appAttestGenerateAssertionFailedWithError:generateAssertionError + keyId:existingKeyID + clientDataHash:self.randomChallengeHash]; id completionBlockArg = [OCMArg invokeBlockWithArgs:[NSNull null], generateAssertionError, nil]; OCMExpect([self.mockAppAttestService generateAssertion:existingKeyID @@ -690,7 +698,7 @@ - (void)testGetToken_WhenKeyRegisteredAndGenerateAssertionError { [completionExpectation fulfill]; XCTAssertNil(token); - XCTAssertEqualObjects(error, generateAssertionError); + XCTAssertEqualObjects(error, expectedError); }]; [self waitForExpectations:@[ completionExpectation ] timeout:0.5];