Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-21 11:35:34 +01:00
Code Issues Projects Releases Wiki Activity

[PM-4531] Do not override webauthn on excluded domains (#6790)

Browse Source 
* [PM-4531] feat: bypass fido2 if origin present in neverDomains

* [PM-4531] feat: bypass fido2 during asserts as well

* [PM-4531] fix: crashes when using `localhost`

* [PM-4531] fix: add missing check

* [PM-4531] fix: broken TLD logic

* [PM-4531] feat: only allow localhost
This commit is contained in:
Andreas Coroiu 2023-11-03 18:27:32 +01:00 committed by GitHub
parent 665aa2fc0d
commit 62e1e165c4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 80 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
apps/browser/src/background/main.background.ts
View File

@ -598,6 +598,7 @@ export default class MainBackground {
this.fido2AuthenticatorService,
this.configService,
this.authService,
this.stateService,
this.logService
);

30
libs/common/src/vault/services/fido2/domain-utils.spec.ts
View File

@ -23,6 +23,36 @@ describe("validateRpId", () => {
expect(isValidRpId(rpId, origin)).toBe(false);
});
it("should not be valid when rpId and origin are both different TLD", () => {
const rpId = "bitwarden";
const origin = "localhost";
expect(isValidRpId(rpId, origin)).toBe(false);
});
// Only allow localhost for rpId, need to properly investigate the implications of
// adding support for ip-addresses and other TLDs
it("should not be valid when rpId and origin are both the same TLD", () => {
const rpId = "bitwarden";
const origin = "bitwarden";
expect(isValidRpId(rpId, origin)).toBe(false);
});
it("should not be valid when rpId and origin are ip-addresses", () => {
const rpId = "127.0.0.1";
const origin = "127.0.0.1";
expect(isValidRpId(rpId, origin)).toBe(false);
});
it("should be valid when domains of rpId and origin are localhost", () => {
const rpId = "localhost";
const origin = "https://localhost:8080";
expect(isValidRpId(rpId, origin)).toBe(true);
});
it("should be valid when domains of rpId and origin are the same", () => {
const rpId = "bitwarden.com";
const origin = "https://bitwarden.com";

8
libs/common/src/vault/services/fido2/domain-utils.ts
View File

@ -5,7 +5,11 @@ export function isValidRpId(rpId: string, origin: string) {
const parsedRpId = parse(rpId, { allowPrivateDomains: true });
return (
parsedOrigin.domain === parsedRpId.domain &&
parsedOrigin.subdomain.endsWith(parsedRpId.subdomain)
(parsedOrigin.domain == null &&
parsedOrigin.hostname == parsedRpId.hostname &&
parsedOrigin.hostname == "localhost") ||
(parsedOrigin.domain != null &&
parsedOrigin.domain == parsedRpId.domain &&
parsedOrigin.subdomain.endsWith(parsedRpId.subdomain))
);
}

32
libs/common/src/vault/services/fido2/fido2-client.service.spec.ts
View File

@ -3,6 +3,7 @@ import { mock, MockProxy } from "jest-mock-extended";
import { AuthService } from "../../../auth/abstractions/auth.service";
import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
import { ConfigServiceAbstraction } from "../../../platform/abstractions/config/config.service.abstraction";
import { StateService } from "../../../platform/abstractions/state.service";
import { Utils } from "../../../platform/misc/utils";
import {
Fido2AuthenticatorError,
@ -27,6 +28,7 @@ describe("FidoAuthenticatorService", () => {
let authenticator!: MockProxy<Fido2AuthenticatorService>;
let configService!: MockProxy<ConfigServiceAbstraction>;
let authService!: MockProxy<AuthService>;
let stateService!: MockProxy<StateService>;
let client!: Fido2ClientService;
let tab!: chrome.tabs.Tab;
@ -34,8 +36,9 @@ describe("FidoAuthenticatorService", () => {
authenticator = mock<Fido2AuthenticatorService>();
configService = mock<ConfigServiceAbstraction>();
authService = mock<AuthService>();
stateService = mock<StateService>();
client = new Fido2ClientService(authenticator, configService, authService);
client = new Fido2ClientService(authenticator, configService, authService, stateService);
configService.getFeatureFlag.mockResolvedValue(true);
tab = { id: 123, windowId: 456 } as chrome.tabs.Tab;
});
@ -97,7 +100,7 @@ describe("FidoAuthenticatorService", () => {
it("should throw error if rp.id is not valid for this origin", async () => {
const params = createParams({
origin: "https://passwordless.dev",
rp: { id: "bitwarden.com", name: "Bitwraden" },
rp: { id: "bitwarden.com", name: "Bitwarden" },
});
const result = async () => await client.createCredential(params, tab);
@ -107,10 +110,22 @@ describe("FidoAuthenticatorService", () => {
await rejects.toBeInstanceOf(DOMException);
});
it("should fallback if origin hostname is found in neverDomains", async () => {
const params = createParams({
origin: "https://bitwarden.com",
rp: { id: "bitwarden.com", name: "Bitwarden" },
});
stateService.getNeverDomains.mockResolvedValue({ "bitwarden.com": null });
const result = async () => await client.createCredential(params, tab);
await expect(result).rejects.toThrow(FallbackRequestedError);
});
it("should throw error if origin is not an https domain", async () => {
const params = createParams({
origin: "http://passwordless.dev",
rp: { id: "bitwarden.com", name: "Bitwraden" },
rp: { id: "bitwarden.com", name: "Bitwarden" },
});
const result = async () => await client.createCredential(params, tab);
@ -295,6 +310,17 @@ describe("FidoAuthenticatorService", () => {
await rejects.toBeInstanceOf(DOMException);
});
it("should fallback if origin hostname is found in neverDomains", async () => {
const params = createParams({
origin: "https://bitwarden.com",
});
stateService.getNeverDomains.mockResolvedValue({ "bitwarden.com": null });
const result = async () => await client.assertCredential(params, tab);
await expect(result).rejects.toThrow(FallbackRequestedError);
});
it("should throw error if origin is not an http domain", async () => {
const params = createParams({
origin: "http://passwordless.dev",

20
libs/common/src/vault/services/fido2/fido2-client.service.ts
View File

@ -5,6 +5,7 @@ import { AuthenticationStatus } from "../../../auth/enums/authentication-status"
import { FeatureFlag } from "../../../enums/feature-flag.enum";
import { ConfigServiceAbstraction } from "../../../platform/abstractions/config/config.service.abstraction";
import { LogService } from "../../../platform/abstractions/log.service";
import { StateService } from "../../../platform/abstractions/state.service";
import { Utils } from "../../../platform/misc/utils";
import {
Fido2AuthenticatorError,
@ -40,6 +41,7 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
private authenticator: Fido2AuthenticatorService,
private configService: ConfigServiceAbstraction,
private authService: AuthService,
private stateService: StateService,
private logService?: LogService
) {}
@ -84,6 +86,12 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
const parsedOrigin = parse(params.origin, { allowPrivateDomains: true });
params.rp.id = params.rp.id ?? parsedOrigin.hostname;
const neverDomains = await this.stateService.getNeverDomains();
if (neverDomains != null && parsedOrigin.hostname in neverDomains) {
this.logService?.warning(`[Fido2Client] Excluded domain`);
throw new FallbackRequestedError();
}
if (parsedOrigin.hostname == undefined || !params.origin.startsWith("https://")) {
this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`);
throw new DOMException("'origin' is not a valid https origin", "SecurityError");
@ -211,15 +219,15 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
throw new FallbackRequestedError();
}
const { domain: effectiveDomain } = parse(params.origin, { allowPrivateDomains: true });
if (effectiveDomain == undefined) {
this.logService?.warning(`[Fido2Client] Invalid origin: ${params.origin}`);
throw new DOMException("'origin' is not a valid domain", "SecurityError");
}
const parsedOrigin = parse(params.origin, { allowPrivateDomains: true });
params.rpId = params.rpId ?? parsedOrigin.hostname;
const neverDomains = await this.stateService.getNeverDomains();
if (neverDomains != null && parsedOrigin.hostname in neverDomains) {
this.logService?.warning(`[Fido2Client] Excluded domain`);
throw new FallbackRequestedError();
}
if (parsedOrigin.hostname == undefined || !params.origin.startsWith("https://")) {
this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`);
throw new DOMException("'origin' is not a valid https origin", "SecurityError");