Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2025-01-31 22:51:28 +01:00
Code Issues Projects Releases Wiki Activity

[EC-598] feat: add support for user verifiction using MP during attestation

Browse Source
This commit is contained in:
Andreas Coroiu 2023-04-20 15:43:49 +02:00
parent 9b2f8b9462
commit 757050430d
No known key found for this signature in database
GPG Key ID: E70B5FFC81DFEC1A
6 changed files with 152 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
apps/browser/src/fido2/content/page-script.ts
View File

@ -65,6 +65,7 @@ navigator.credentials.get = async (
abortController?: AbortController
): Promise<Credential> => {
console.log("navigator.credentials.get()", options);
try {
const response = await messenger.request(
{

28
apps/browser/src/fido2/popup/fido2/fido2.component.ts
View File

@ -11,10 +11,11 @@ import {
takeUntil,
} from "rxjs";
import { Fido2KeyView } from "@bitwarden/common/fido2/models/view/fido2-key.view";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { PasswordRepromptService } from "@bitwarden/common/vault/abstractions/password-reprompt.service";
import { CipherType } from "@bitwarden/common/vault/enums/cipher-type";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { Fido2KeyView } from "@bitwarden/common/fido2/models/view/fido2-key.view";
import { BrowserApi } from "../../../browser/browserApi";
import {
@ -35,7 +36,11 @@ export class Fido2Component implements OnInit, OnDestroy {
protected ciphers?: CipherView[] = [];
protected loading = false;
constructor(private activatedRoute: ActivatedRoute, private cipherService: CipherService) {}
constructor(
private activatedRoute: ActivatedRoute,
private cipherService: CipherService,
private passwordRepromptService: PasswordRepromptService
) {}
ngOnInit(): void {
const sessionId$ = this.activatedRoute.queryParamMap.pipe(
@ -118,10 +123,16 @@ export class Fido2Component implements OnInit, OnDestroy {
type: "PickCredentialResponse",
});
} else if (data?.type === "ConfirmNewNonDiscoverableCredentialRequest") {
let userVerified = false;
if (data.userVerification) {
userVerified = await this.passwordRepromptService.showPasswordPrompt();
}
this.send({
sessionId: this.sessionId,
cipherId: cipher.id,
type: "ConfirmNewNonDiscoverableCredentialResponse",
userVerified,
});
}
@ -136,10 +147,21 @@ export class Fido2Component implements OnInit, OnDestroy {
this.loading = true;
}
confirmNew() {
async confirmNew() {
const data = this.data$.value;
if (data.type !== "ConfirmNewCredentialRequest") {
return;
}
let userVerified = false;
if (data.userVerification) {
userVerified = await this.passwordRepromptService.showPasswordPrompt();
}
this.send({
sessionId: this.sessionId,
type: "ConfirmNewCredentialResponse",
userVerified,
});
this.loading = true;
}

21
apps/browser/src/services/fido2/browser-fido2-user-interface.service.ts
View File

@ -62,18 +62,22 @@ export type BrowserFido2Message = { sessionId: string } & (
type: "ConfirmNewCredentialRequest";
credentialName: string;
userName: string;
userVerification: boolean;
}
| {
type: "ConfirmNewCredentialResponse";
userVerified: boolean;
}
| {
type: "ConfirmNewNonDiscoverableCredentialRequest";
credentialName: string;
userName: string;
userVerification: boolean;
}
| {
type: "ConfirmNewNonDiscoverableCredentialResponse";
cipherId: string;
userVerified: boolean;
}
| {
type: "InformExcludedCredentialRequest";
@ -201,35 +205,42 @@ export class BrowserFido2UserInterfaceSession implements Fido2UserInterfaceSessi
return response.cipherId;
}
async confirmNewCredential({ credentialName, userName }: NewCredentialParams): Promise<boolean> {
async confirmNewCredential({
credentialName,
userName,
userVerification,
}: NewCredentialParams): Promise<{ confirmed: boolean; userVerified: boolean }> {
const data: BrowserFido2Message = {
type: "ConfirmNewCredentialRequest",
sessionId: this.sessionId,
credentialName,
userName,
userVerification,
};
await this.send(data);
await this.receive("ConfirmNewCredentialResponse");
const response = await this.receive("ConfirmNewCredentialResponse");
return true;
return { confirmed: true, userVerified: response.userVerified };
}
async confirmNewNonDiscoverableCredential({
credentialName,
userName,
}: NewCredentialParams): Promise<string> {
userVerification,
}: NewCredentialParams): Promise<{ cipherId: string; userVerified: boolean }> {
const data: BrowserFido2Message = {
type: "ConfirmNewNonDiscoverableCredentialRequest",
sessionId: this.sessionId,
credentialName,
userName,
userVerification,
};
await this.send(data);
const response = await this.receive("ConfirmNewNonDiscoverableCredentialResponse");
return response.cipherId;
return { cipherId: response.cipherId, userVerified: response.userVerified };
}
async informExcludedCredential(existingCipherIds: string[]): Promise<void> {

5
libs/common/src/fido2/abstractions/fido2-user-interface.service.abstraction.ts
View File

@ -1,6 +1,7 @@
export interface NewCredentialParams {
credentialName: string;
userName: string;
userVerification: boolean;
}
export abstract class Fido2UserInterfaceService {
@ -16,11 +17,11 @@ export abstract class Fido2UserInterfaceSession {
confirmNewCredential: (
params: NewCredentialParams,
abortController?: AbortController
) => Promise<boolean>;
) => Promise<{ confirmed: boolean; userVerified: boolean }>;
confirmNewNonDiscoverableCredential: (
params: NewCredentialParams,
abortController?: AbortController
) => Promise<string | undefined>;
) => Promise<{ cipherId: string; userVerified: boolean }>;
informExcludedCredential: (
existingCipherIds: string[],
abortController?: AbortController

135
libs/common/src/fido2/services/fido2-authenticator.service.spec.ts
View File

@ -71,8 +71,9 @@ describe("FidoAuthenticatorService", () => {
/**
* Spec: If requireUserVerification is true and the authenticator cannot perform user verification, return an error code equivalent to "ConstraintError" and terminate the operation.
* Deviation: User verification is checked before checking for excluded credentials
* */
it("should throw error if requireUserVerification is set to true", async () => {
**/
/** TODO: This test should only be activated if we disable support for user verification */
it.skip("should throw error if requireUserVerification is set to true", async () => {
const params = await createParams({ requireUserVerification: true });
const result = async () => await authenticator.makeCredential(params);
@ -81,7 +82,10 @@ describe("FidoAuthenticatorService", () => {
});
it("should not request confirmation from user", async () => {
userInterfaceSession.confirmNewCredential.mockResolvedValue(true);
userInterfaceSession.confirmNewCredential.mockResolvedValue({
confirmed: true,
userVerified: false,
});
const invalidParams = await createInvalidParams();
for (const p of Object.values(invalidParams)) {
@ -258,30 +262,41 @@ describe("FidoAuthenticatorService", () => {
/**
* Spec: Collect an authorization gesture confirming user consent for creating a new credential. The prompt for the authorization gesture is shown by the authenticator if it has its own output capability. The prompt SHOULD display rpEntity.id, rpEntity.name, userEntity.name and userEntity.displayName, if possible.
* If requireUserVerification is true, the authorization gesture MUST include user verification.
* Deviation: Only `rpEntity.name` and `userEntity.name` is shown.
* */
it("should request confirmation from user", async () => {
userInterfaceSession.confirmNewCredential.mockResolvedValue(true);
cipherService.encrypt.mockResolvedValue({} as unknown as Cipher);
cipherService.createWithServer.mockImplementation(async (cipher) => {
cipher.id = Utils.newGuid();
return cipher;
for (const userVerification of [true, false]) {
it(`should request confirmation from user when user verification is ${userVerification}`, async () => {
params.requireUserVerification = userVerification;
userInterfaceSession.confirmNewCredential.mockResolvedValue({
confirmed: true,
userVerified: userVerification,
});
cipherService.encrypt.mockResolvedValue({} as unknown as Cipher);
cipherService.createWithServer.mockImplementation(async (cipher) => {
cipher.id = Utils.newGuid();
return cipher;
});
await authenticator.makeCredential(params, new AbortController());
expect(userInterfaceSession.confirmNewCredential).toHaveBeenCalledWith(
{
credentialName: params.rpEntity.name,
userName: params.userEntity.displayName,
userVerification,
} as NewCredentialParams,
expect.anything()
);
});
await authenticator.makeCredential(params, new AbortController());
expect(userInterfaceSession.confirmNewCredential).toHaveBeenCalledWith(
{
credentialName: params.rpEntity.name,
userName: params.userEntity.displayName,
} as NewCredentialParams,
expect.anything()
);
});
}
it("should save credential to vault if request confirmed by user", async () => {
const encryptedCipher = {};
userInterfaceSession.confirmNewCredential.mockResolvedValue(true);
userInterfaceSession.confirmNewCredential.mockResolvedValue({
confirmed: true,
userVerified: false,
});
cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher);
cipherService.createWithServer.mockImplementation(async (cipher) => {
cipher.id = Utils.newGuid();
@ -314,7 +329,10 @@ describe("FidoAuthenticatorService", () => {
/** Spec: If the user does not consent or if user verification fails, return an error code equivalent to "NotAllowedError" and terminate the operation. */
it("should throw error if user denies creation request", async () => {
userInterfaceSession.confirmNewCredential.mockResolvedValue(false);
userInterfaceSession.confirmNewCredential.mockResolvedValue({
confirmed: false,
userVerified: false,
});
const result = async () => await authenticator.makeCredential(params);
@ -324,7 +342,10 @@ describe("FidoAuthenticatorService", () => {
/** Spec: If any error occurred while creating the new credential object, return an error code equivalent to "UnknownError" and terminate the operation. */
it("should throw unkown error if creation fails", async () => {
const encryptedCipher = {};
userInterfaceSession.confirmNewCredential.mockResolvedValue(true);
userInterfaceSession.confirmNewCredential.mockResolvedValue({
confirmed: true,
userVerified: false,
});
cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher);
cipherService.createWithServer.mockRejectedValue(new Error("Internal error"));
@ -351,27 +372,33 @@ describe("FidoAuthenticatorService", () => {
* Spec: Collect an authorization gesture confirming user consent for creating a new credential. The prompt for the authorization gesture is shown by the authenticator if it has its own output capability. The prompt SHOULD display rpEntity.id, rpEntity.name, userEntity.name and userEntity.displayName, if possible.
* Deviation: Only `rpEntity.name` and `userEntity.name` is shown.
* */
it("should request confirmation from user", async () => {
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue(
existingCipher.id
);
for (const userVerification of [true, false]) {
it(`should request confirmation from user when user verification is ${userVerification}`, async () => {
params.requireUserVerification = userVerification;
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({
cipherId: existingCipher.id,
userVerified: userVerification,
});
await authenticator.makeCredential(params, new AbortController());
await authenticator.makeCredential(params, new AbortController());
expect(userInterfaceSession.confirmNewNonDiscoverableCredential).toHaveBeenCalledWith(
{
credentialName: params.rpEntity.name,
userName: params.userEntity.displayName,
} as NewCredentialParams,
expect.anything()
);
});
expect(userInterfaceSession.confirmNewNonDiscoverableCredential).toHaveBeenCalledWith(
{
credentialName: params.rpEntity.name,
userName: params.userEntity.displayName,
userVerification,
} as NewCredentialParams,
expect.anything()
);
});
}
it("should save credential to vault if request confirmed by user", async () => {
const encryptedCipher = Symbol();
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue(
existingCipher.id
);
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({
cipherId: existingCipher.id,
userVerified: false,
});
cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher);
await authenticator.makeCredential(params);
@ -402,7 +429,10 @@ describe("FidoAuthenticatorService", () => {
/** Spec: If the user does not consent or if user verification fails, return an error code equivalent to "NotAllowedError" and terminate the operation. */
it("should throw error if user denies creation request", async () => {
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue(undefined);
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({
cipherId: undefined,
userVerified: false,
});
const params = await createParams();
const result = async () => await authenticator.makeCredential(params);
@ -413,9 +443,10 @@ describe("FidoAuthenticatorService", () => {
/** Spec: If any error occurred while creating the new credential object, return an error code equivalent to "UnknownError" and terminate the operation. */
it("should throw unkown error if creation fails", async () => {
const encryptedCipher = Symbol();
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue(
existingCipher.id
);
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({
cipherId: existingCipher.id,
userVerified: false,
});
cipherService.encrypt.mockResolvedValue(encryptedCipher as unknown as Cipher);
cipherService.updateWithServer.mockRejectedValue(new Error("Internal error"));
@ -444,8 +475,14 @@ describe("FidoAuthenticatorService", () => {
beforeEach(async () => {
const cipher = createCipherView({ id: cipherId, type: CipherType.Login });
params = await createParams({ requireResidentKey });
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue(cipherId);
userInterfaceSession.confirmNewCredential.mockResolvedValue(true);
userInterfaceSession.confirmNewNonDiscoverableCredential.mockResolvedValue({
cipherId,
userVerified: false,
});
userInterfaceSession.confirmNewCredential.mockResolvedValue({
confirmed: true,
userVerified: false,
});
cipherService.get.mockImplementation(async (cipherId) =>
cipherId === cipher.id ? ({ decrypt: () => cipher } as any) : undefined
);
@ -573,8 +610,12 @@ describe("FidoAuthenticatorService", () => {
await expect(result).rejects.toThrowError(Fido2AutenticatorErrorCode.Unknown);
});
/** Deviation: User verification is checked before checking for credentials */
it("should throw error if requireUserVerification is set to true", async () => {
/**
* Spec: If requireUserVerification is true and the authenticator cannot perform user verification, return an error code equivalent to "ConstraintError" and terminate the operation.
* Deviation: User verification is checked before checking for excluded credentials
**/
/** TODO: This test should only be activated if we disable support for user verification */
it.skip("should throw error if requireUserVerification is set to true", async () => {
const params = await createParams({ requireUserVerification: true });
const result = async () => await authenticator.getAssertion(params);

31
libs/common/src/fido2/services/fido2-authenticator.service.ts
View File

@ -62,10 +62,6 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown);
}
if (params.requireUserVerification) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Constraint);
}
const existingCipherIds = await this.findExcludedCredentials(
params.excludeCredentialDescriptorList
);
@ -78,16 +74,24 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
let cipher: CipherView;
let fido2Key: Fido2KeyView;
let keyPair: CryptoKeyPair;
let userVerified = false;
if (params.requireResidentKey) {
const userVerification = await userInterfaceSession.confirmNewCredential(
const response = await userInterfaceSession.confirmNewCredential(
{
credentialName: params.rpEntity.name,
userName: params.userEntity.displayName,
userVerification: params.requireUserVerification,
},
abortController
);
const userConfirmation = response.confirmed;
userVerified = response.userVerified;
if (!userVerification) {
if (!userConfirmation) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed);
}
if (params.requireUserVerification && !userVerified) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed);
}
@ -105,18 +109,25 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown);
}
} else {
const cipherId = await userInterfaceSession.confirmNewNonDiscoverableCredential(
const response = await userInterfaceSession.confirmNewNonDiscoverableCredential(
{
credentialName: params.rpEntity.name,
userName: params.userEntity.displayName,
userVerification: params.requireUserVerification,
},
abortController
);
const cipherId = response.cipherId;
userVerified = response.userVerified;
if (cipherId === undefined) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed);
}
if (params.requireUserVerification && !userVerified) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.NotAllowed);
}
try {
keyPair = await createKeyPair();
@ -138,7 +149,7 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
credentialId: Utils.guidToRawFormat(credentialId),
counter: fido2Key.counter,
userPresence: true,
userVerification: false,
userVerification: userVerified,
keyPair,
});
const attestationObject = new Uint8Array(
@ -174,10 +185,6 @@ export class Fido2AuthenticatorService implements Fido2AuthenticatorServiceAbstr
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Unknown);
}
if (params.requireUserVerification) {
throw new Fido2AutenticatorError(Fido2AutenticatorErrorCode.Constraint);
}
let cipherOptions: CipherView[];
// eslint-disable-next-line no-empty