[PM-6413] Add http loophole for localhost (#9236)

* [PM-6413] feat: add http loophole for localhost

Fixes #6882

* feat: add sanity check

* feat: change fido2 filters to allow scripts on localhost

* [PM-6413] fix: injection tests
This commit is contained in:
Andreas Coroiu 2024-06-25 11:06:04 +02:00 committed by GitHub
parent 591f44438a
commit dce5c0f184
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 79 additions and 10 deletions

View File

@ -32,7 +32,7 @@ const contentScriptDetails = {
...sharedScriptInjectionDetails, ...sharedScriptInjectionDetails,
}; };
const sharedRegistrationOptions = { const sharedRegistrationOptions = {
matches: ["https://*/*"], matches: ["https://*/*", "http://localhost/*"],
excludeMatches: ["https://*/*.xml*"], excludeMatches: ["https://*/*.xml*"],
allFrames: true, allFrames: true,
...sharedExecuteScriptOptions, ...sharedExecuteScriptOptions,

View File

@ -33,7 +33,7 @@ export class Fido2Background implements Fido2BackgroundInterface {
runAt: "document_start", runAt: "document_start",
}; };
private readonly sharedRegistrationOptions: SharedFido2ScriptRegistrationOptions = { private readonly sharedRegistrationOptions: SharedFido2ScriptRegistrationOptions = {
matches: ["https://*/*"], matches: ["https://*/*", "http://localhost/*"],
excludeMatches: ["https://*/*.xml*"], excludeMatches: ["https://*/*.xml*"],
allFrames: true, allFrames: true,
...this.sharedInjectionDetails, ...this.sharedInjectionDetails,

View File

@ -17,7 +17,9 @@ import { MessageWithMetadata, Messenger } from "./messaging/messenger";
(function (globalContext) { (function (globalContext) {
const shouldExecuteContentScript = const shouldExecuteContentScript =
globalContext.document.contentType === "text/html" && globalContext.document.contentType === "text/html" &&
globalContext.document.location.protocol === "https:"; (globalContext.document.location.protocol === "https:" ||
(globalContext.document.location.protocol === "http:" &&
globalContext.document.location.hostname === "localhost"));
if (!shouldExecuteContentScript) { if (!shouldExecuteContentScript) {
return; return;

View File

@ -8,7 +8,9 @@ import { Messenger } from "./messaging/messenger";
(function (globalContext) { (function (globalContext) {
const shouldExecuteContentScript = const shouldExecuteContentScript =
globalContext.document.contentType === "text/html" && globalContext.document.contentType === "text/html" &&
globalContext.document.location.protocol === "https:"; (globalContext.document.location.protocol === "https:" ||
(globalContext.document.location.protocol === "http:" &&
globalContext.document.location.hostname === "localhost"));
if (!shouldExecuteContentScript) { if (!shouldExecuteContentScript) {
return; return;

View File

@ -16,8 +16,9 @@ const mockGlobalThisDocument = {
contentType: "text/html", contentType: "text/html",
location: { location: {
...originalGlobalThis.document.location, ...originalGlobalThis.document.location,
href: "https://localhost", href: "https://bitwarden.com",
origin: "https://localhost", origin: "https://bitwarden.com",
hostname: "bitwarden.com",
protocol: "https:", protocol: "https:",
}, },
}; };
@ -166,8 +167,8 @@ describe("Fido2 page script with native WebAuthn support", () => {
...mockGlobalThisDocument, ...mockGlobalThisDocument,
location: { location: {
...mockGlobalThisDocument.location, ...mockGlobalThisDocument.location,
href: "http://localhost", href: "http://bitwarden.com",
origin: "http://localhost", origin: "http://bitwarden.com",
protocol: "http:", protocol: "http:",
}, },
})); }));

View File

@ -19,6 +19,7 @@ import {
} from "../../abstractions/fido2/fido2-client.service.abstraction"; } from "../../abstractions/fido2/fido2-client.service.abstraction";
import { Utils } from "../../misc/utils"; import { Utils } from "../../misc/utils";
import * as DomainUtils from "./domain-utils";
import { Fido2AuthenticatorService } from "./fido2-authenticator.service"; import { Fido2AuthenticatorService } from "./fido2-authenticator.service";
import { Fido2ClientService } from "./fido2-client.service"; import { Fido2ClientService } from "./fido2-client.service";
import { Fido2Utils } from "./fido2-utils"; import { Fido2Utils } from "./fido2-utils";
@ -36,6 +37,7 @@ describe("FidoAuthenticatorService", () => {
let domainSettingsService: MockProxy<DomainSettingsService>; let domainSettingsService: MockProxy<DomainSettingsService>;
let client!: Fido2ClientService; let client!: Fido2ClientService;
let tab!: chrome.tabs.Tab; let tab!: chrome.tabs.Tab;
let isValidRpId!: jest.SpyInstance;
beforeEach(async () => { beforeEach(async () => {
authenticator = mock<Fido2AuthenticatorService>(); authenticator = mock<Fido2AuthenticatorService>();
@ -44,6 +46,8 @@ describe("FidoAuthenticatorService", () => {
vaultSettingsService = mock<VaultSettingsService>(); vaultSettingsService = mock<VaultSettingsService>();
domainSettingsService = mock<DomainSettingsService>(); domainSettingsService = mock<DomainSettingsService>();
isValidRpId = jest.spyOn(DomainUtils, "isValidRpId");
client = new Fido2ClientService( client = new Fido2ClientService(
authenticator, authenticator,
configService, configService,
@ -58,6 +62,10 @@ describe("FidoAuthenticatorService", () => {
tab = { id: 123, windowId: 456 } as chrome.tabs.Tab; tab = { id: 123, windowId: 456 } as chrome.tabs.Tab;
}); });
afterEach(() => {
isValidRpId.mockRestore();
});
describe("createCredential", () => { describe("createCredential", () => {
describe("input parameters validation", () => { describe("input parameters validation", () => {
// Spec: If sameOriginWithAncestors is false, return a "NotAllowedError" DOMException. // Spec: If sameOriginWithAncestors is false, return a "NotAllowedError" DOMException.
@ -113,6 +121,7 @@ describe("FidoAuthenticatorService", () => {
}); });
// Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm. // Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm.
// This is actually checked by `isValidRpId` function, but we'll test it here as well
it("should throw error if rp.id is not valid for this origin", async () => { it("should throw error if rp.id is not valid for this origin", async () => {
const params = createParams({ const params = createParams({
origin: "https://passwordless.dev", origin: "https://passwordless.dev",
@ -126,6 +135,20 @@ describe("FidoAuthenticatorService", () => {
await rejects.toBeInstanceOf(DOMException); await rejects.toBeInstanceOf(DOMException);
}); });
// Sanity check to make sure that we use `isValidRpId` to validate the rp.id
it("should throw if isValidRpId returns false", async () => {
const params = createParams();
authenticator.makeCredential.mockResolvedValue(createAuthenticatorMakeResult());
// `params` actually has a valid rp.id, but we're mocking the function to return false
isValidRpId.mockReturnValue(false);
const result = async () => await client.createCredential(params, tab);
const rejects = expect(result).rejects;
await rejects.toMatchObject({ name: "SecurityError" });
await rejects.toBeInstanceOf(DOMException);
});
it("should fallback if origin hostname is found in neverDomains", async () => { it("should fallback if origin hostname is found in neverDomains", async () => {
const params = createParams({ const params = createParams({
origin: "https://bitwarden.com", origin: "https://bitwarden.com",
@ -151,6 +174,16 @@ describe("FidoAuthenticatorService", () => {
await rejects.toBeInstanceOf(DOMException); await rejects.toBeInstanceOf(DOMException);
}); });
it("should not throw error if localhost is http", async () => {
const params = createParams({
origin: "http://localhost",
rp: { id: undefined, name: "localhost" },
});
authenticator.makeCredential.mockResolvedValue(createAuthenticatorMakeResult());
await client.createCredential(params, tab);
});
// Spec: If credTypesAndPubKeyAlgs is empty, return a DOMException whose name is "NotSupportedError", and terminate this algorithm. // Spec: If credTypesAndPubKeyAlgs is empty, return a DOMException whose name is "NotSupportedError", and terminate this algorithm.
it("should throw error if no support key algorithms were found", async () => { it("should throw error if no support key algorithms were found", async () => {
const params = createParams({ const params = createParams({
@ -360,6 +393,7 @@ describe("FidoAuthenticatorService", () => {
}); });
// Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm. // Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm.
// This is actually checked by `isValidRpId` function, but we'll test it here as well
it("should throw error if rp.id is not valid for this origin", async () => { it("should throw error if rp.id is not valid for this origin", async () => {
const params = createParams({ const params = createParams({
origin: "https://passwordless.dev", origin: "https://passwordless.dev",
@ -373,6 +407,20 @@ describe("FidoAuthenticatorService", () => {
await rejects.toBeInstanceOf(DOMException); await rejects.toBeInstanceOf(DOMException);
}); });
// Sanity check to make sure that we use `isValidRpId` to validate the rp.id
it("should throw if isValidRpId returns false", async () => {
const params = createParams();
authenticator.getAssertion.mockResolvedValue(createAuthenticatorAssertResult());
// `params` actually has a valid rp.id, but we're mocking the function to return false
isValidRpId.mockReturnValue(false);
const result = async () => await client.assertCredential(params, tab);
const rejects = expect(result).rejects;
await rejects.toMatchObject({ name: "SecurityError" });
await rejects.toBeInstanceOf(DOMException);
});
it("should fallback if origin hostname is found in neverDomains", async () => { it("should fallback if origin hostname is found in neverDomains", async () => {
const params = createParams({ const params = createParams({
origin: "https://bitwarden.com", origin: "https://bitwarden.com",
@ -506,6 +554,16 @@ describe("FidoAuthenticatorService", () => {
expect.anything(), expect.anything(),
); );
}); });
it("should not throw error if localhost is http", async () => {
const params = createParams({
origin: "http://localhost",
});
params.rpId = undefined;
authenticator.getAssertion.mockResolvedValue(createAuthenticatorAssertResult());
await client.assertCredential(params, tab);
});
}); });
describe("assert discoverable credential", () => { describe("assert discoverable credential", () => {

View File

@ -103,7 +103,10 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
} }
params.rp.id = params.rp.id ?? parsedOrigin.hostname; params.rp.id = params.rp.id ?? parsedOrigin.hostname;
if (parsedOrigin.hostname == undefined || !params.origin.startsWith("https://")) { if (
parsedOrigin.hostname == undefined ||
(!params.origin.startsWith("https://") && parsedOrigin.hostname !== "localhost")
) {
this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`); this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`);
throw new DOMException("'origin' is not a valid https origin", "SecurityError"); throw new DOMException("'origin' is not a valid https origin", "SecurityError");
} }
@ -238,7 +241,10 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
params.rpId = params.rpId ?? parsedOrigin.hostname; params.rpId = params.rpId ?? parsedOrigin.hostname;
if (parsedOrigin.hostname == undefined || !params.origin.startsWith("https://")) { if (
parsedOrigin.hostname == undefined ||
(!params.origin.startsWith("https://") && parsedOrigin.hostname !== "localhost")
) {
this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`); this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`);
throw new DOMException("'origin' is not a valid https origin", "SecurityError"); throw new DOMException("'origin' is not a valid https origin", "SecurityError");
} }