From 89bfdbf03e879fc8ad495648961050155761fc17 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 27 Apr 2023 14:51:36 +0200 Subject: [PATCH] [EC-598] feat: add logging to fido2 authenticator --- .../browser/src/background/main.background.ts | 13 ++-- .../services/fido2-authenticator.service.ts | 63 +++++++++++++++++-- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 395f2867ef8..5efe5ea4bda 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -45,6 +45,11 @@ import { TwoFactorService } from "@bitwarden/common/auth/services/two-factor.ser import { UserVerificationApiService } from "@bitwarden/common/auth/services/user-verification/user-verification-api.service"; import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service"; import { StateFactory } from "@bitwarden/common/factories/stateFactory"; +import { Fido2AuthenticatorService as Fido2AuthenticatorServiceAbstraction } from "@bitwarden/common/fido2/abstractions/fido2-authenticator.service.abstraction"; +import { Fido2ClientService as Fido2ClientServiceAbstraction } from "@bitwarden/common/fido2/abstractions/fido2-client.service.abstraction"; +import { Fido2UserInterfaceService as Fido2UserInterfaceServiceAbstraction } from "@bitwarden/common/fido2/abstractions/fido2-user-interface.service.abstraction"; +import { Fido2AuthenticatorService } from "@bitwarden/common/fido2/services/fido2-authenticator.service"; +import { Fido2ClientService } from "@bitwarden/common/fido2/services/fido2-client.service"; import { GlobalState } from "@bitwarden/common/models/domain/global-state"; import { AvatarUpdateService } from "@bitwarden/common/services/account/avatar-update.service"; import { ApiService } from "@bitwarden/common/services/api.service"; @@ -89,11 +94,6 @@ import { CipherFileUploadService } from "@bitwarden/common/vault/services/file-u import { FolderApiService } from "@bitwarden/common/vault/services/folder/folder-api.service"; import { SyncNotifierService } from "@bitwarden/common/vault/services/sync/sync-notifier.service"; import { SyncService } from "@bitwarden/common/vault/services/sync/sync.service"; -import { Fido2AuthenticatorService as Fido2AuthenticatorServiceAbstraction } from "@bitwarden/common/fido2/abstractions/fido2-authenticator.service.abstraction"; -import { Fido2ClientService as Fido2ClientServiceAbstraction } from "@bitwarden/common/fido2/abstractions/fido2-client.service.abstraction"; -import { Fido2UserInterfaceService as Fido2UserInterfaceServiceAbstraction } from "@bitwarden/common/fido2/abstractions/fido2-user-interface.service.abstraction"; -import { Fido2AuthenticatorService } from "@bitwarden/common/fido2/services/fido2-authenticator.service"; -import { Fido2ClientService } from "@bitwarden/common/fido2/services/fido2-client.service"; import { BrowserOrganizationService } from "../admin-console/services/browser-organization.service"; import { BrowserPolicyService } from "../admin-console/services/browser-policy.service"; @@ -504,7 +504,8 @@ export default class MainBackground { this.fido2UserInterfaceService = new BrowserFido2UserInterfaceService(this.popupUtilsService); this.fido2AuthenticatorService = new Fido2AuthenticatorService( this.cipherService, - this.fido2UserInterfaceService + this.fido2UserInterfaceService, + this.logService ); this.fido2ClientService = new Fido2ClientService(this.fido2AuthenticatorService); diff --git a/libs/common/src/fido2/services/fido2-authenticator.service.ts b/libs/common/src/fido2/services/fido2-authenticator.service.ts index b5b071caf7b..d5a0a349493 100644 --- a/libs/common/src/fido2/services/fido2-authenticator.service.ts +++ b/libs/common/src/fido2/services/fido2-authenticator.service.ts @@ -1,5 +1,6 @@ import { CBOR } from "cbor-redux"; +import { LogService } from "../../abstractions/log.service"; import { Utils } from "../../misc/utils"; import { CipherService } from "../../vault/abstractions/cipher.service"; import { CipherType } from "../../vault/enums/cipher-type"; @@ -35,7 +36,8 @@ const KeyUsages: KeyUsage[] = ["sign"]; export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstraction { constructor( private cipherService: CipherService, - private userInterface: Fido2UserInterfaceService + private userInterface: Fido2UserInterfaceService, + private logService?: LogService ) {} async makeCredential( params: Fido2AuthenticatorMakeCredentialsParams, @@ -45,6 +47,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr try { if (params.credTypesAndPubKeyAlgs.every((p) => p.alg !== Fido2AlgorithmIdentifier.ES256)) { + const requestedAlgorithms = params.credTypesAndPubKeyAlgs.map((p) => p.alg).join(", "); + this.logService?.warning( + `[Fido2Authenticator] No compatible algorithms found, RP requested: ${requestedAlgorithms}` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotSupported); } @@ -52,6 +58,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr params.requireResidentKey != undefined && typeof params.requireResidentKey !== "boolean" ) { + this.logService?.error( + `[Fido2Authenticator] Invalid 'requireResidentKey' value: ${String( + params.requireResidentKey + )}` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); } @@ -59,6 +70,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr params.requireUserVerification != undefined && typeof params.requireUserVerification !== "boolean" ) { + this.logService?.error( + `[Fido2Authenticator] Invalid 'requireUserVerification' value: ${String( + params.requireUserVerification + )}` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); } @@ -66,8 +82,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr params.excludeCredentialDescriptorList ); if (existingCipherIds.length > 0) { + this.logService?.info( + `[Fido2Authenticator] Aborting due to excluded credential found in vault.` + ); await userInterfaceSession.informExcludedCredential(existingCipherIds, abortController); - throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); } @@ -88,10 +106,16 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr userVerified = response.userVerified; if (!userConfirmation) { + this.logService?.warning( + `[Fido2Authenticator] Aborting because user confirmation was not recieved.` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); } if (params.requireUserVerification && !userVerified) { + this.logService?.warning( + `[Fido2Authenticator] Aborting because user verification was not successful.` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); } @@ -105,7 +129,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr const encrypted = await this.cipherService.encrypt(cipher); await this.cipherService.createWithServer(encrypted); // encrypted.id is assigned inside here cipher.id = encrypted.id; - } catch { + } catch (error) { + this.logService?.error( + `[Fido2Authenticator] Aborting because of unknown error when creating discoverable credential: ${error}` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); } } else { @@ -121,10 +148,16 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr userVerified = response.userVerified; if (cipherId === undefined) { + this.logService?.warning( + `[Fido2Authenticator] Aborting because user confirmation was not recieved.` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); } if (params.requireUserVerification && !userVerified) { + this.logService?.warning( + `[Fido2Authenticator] Aborting because user verification was unsuccessful.` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); } @@ -136,7 +169,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr cipher.login.fido2Key = fido2Key = await createKeyView(params, keyPair.privateKey); const reencrypted = await this.cipherService.encrypt(cipher); await this.cipherService.updateWithServer(reencrypted); - } catch { + } catch (error) { + this.logService?.error( + `[Fido2Authenticator] Aborting because of unknown error when creating non-discoverable credential: ${error}` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); } } @@ -182,6 +218,11 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr params.requireUserVerification != undefined && typeof params.requireUserVerification !== "boolean" ) { + this.logService?.error( + `[Fido2Authenticator] Invalid 'requireUserVerification' value: ${String( + params.requireUserVerification + )}` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); } @@ -198,6 +239,9 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr } if (cipherOptions.length === 0) { + this.logService?.info( + `[Fido2Authenticator] Aborting because no matching credentials were found in the vault.` + ); await userInterfaceSession.informCredentialNotFound(); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); } @@ -211,10 +255,16 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr const selectedCipher = cipherOptions.find((c) => c.id === selectedCipherId); if (selectedCipher === undefined) { + this.logService?.error( + `[Fido2Authenticator] Aborting because the selected credential could not be found.` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); } if (params.requireUserVerification && !userVerified) { + this.logService?.warning( + `[Fido2Authenticator] Aborting because user verification was unsuccessful.` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed); } @@ -259,7 +309,10 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr }, signature, }; - } catch { + } catch (error) { + this.logService?.error( + `[Fido2Authenticator] Aborting because of unknown error when asserting credential: ${error}` + ); throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown); } } finally {