1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-20 10:13:31 +00:00

[PM-25250] Prevent configuration and access of self hosted urls over http (#17095)

* feat: ban urls not using https

* feat: add exception for dev env

* feat: block fetching of insecure URLs

* feat: add exception for dev env

* feat: block notifications from using insecure URL

* fix: bug where submission was possible regardless of error

* feat: add exception for dev env

* fix: missing constructor param
This commit is contained in:
Andreas Coroiu
2025-10-31 08:12:44 +01:00
committed by GitHub
parent 2dd314e992
commit 48fb8b2bfe
11 changed files with 106 additions and 11 deletions

View File

@@ -18,6 +18,8 @@ import {
NotificationResponse,
} from "../../models/response/notification.response";
import { EnvironmentService } from "../../platform/abstractions/environment.service";
import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service";
import { InsecureUrlNotAllowedError } from "../../services/api-errors";
import { AnonymousHubService as AnonymousHubServiceAbstraction } from "../abstractions/anonymous-hub.service";
export class AnonymousHubService implements AnonymousHubServiceAbstraction {
@@ -27,10 +29,14 @@ export class AnonymousHubService implements AnonymousHubServiceAbstraction {
constructor(
private environmentService: EnvironmentService,
private authRequestService: AuthRequestServiceAbstraction,
private platformUtilsService: PlatformUtilsService,
) {}
async createHubConnection(token: string) {
this.url = (await firstValueFrom(this.environmentService.environment$)).getNotificationsUrl();
if (!this.url.startsWith("https://") && !this.platformUtilsService.isDev()) {
throw new InsecureUrlNotAllowedError();
}
this.anonHubConnection = new HubConnectionBuilder()
.withUrl(this.url + "/anonymous-hub?Token=" + token, {

View File

@@ -10,8 +10,10 @@ import { Observable, Subscription } from "rxjs";
import { ApiService } from "../../../abstractions/api.service";
import { NotificationResponse } from "../../../models/response/notification.response";
import { InsecureUrlNotAllowedError } from "../../../services/api-errors";
import { UserId } from "../../../types/guid";
import { LogService } from "../../abstractions/log.service";
import { PlatformUtilsService } from "../../abstractions/platform-utils.service";
// 2 Minutes
const MIN_RECONNECT_TIME = 2 * 60 * 1000;
@@ -69,12 +71,17 @@ export class SignalRConnectionService {
constructor(
private readonly apiService: ApiService,
private readonly logService: LogService,
private readonly platformUtilsService: PlatformUtilsService,
private readonly hubConnectionBuilderFactory: () => HubConnectionBuilder = () =>
new HubConnectionBuilder(),
private readonly timeoutManager: TimeoutManager = globalThis,
) {}
connect$(userId: UserId, notificationsUrl: string) {
if (!notificationsUrl.startsWith("https://") && !this.platformUtilsService.isDev()) {
throw new InsecureUrlNotAllowedError();
}
return new Observable<SignalRNotification>((subsciber) => {
const connection = this.hubConnectionBuilderFactory()
.withUrl(notificationsUrl + "/hub", {

View File

@@ -0,0 +1,9 @@
export class InsecureUrlNotAllowedError extends Error {
constructor(url?: string) {
if (url === undefined) {
super("Insecure URL not allowed. All URLs must use HTTPS.");
} else {
super(`Insecure URL not allowed: ${url}. All URLs must use HTTPS.`);
}
}
}

View File

@@ -20,6 +20,7 @@ import { Environment, EnvironmentService } from "../platform/abstractions/enviro
import { LogService } from "../platform/abstractions/log.service";
import { PlatformUtilsService } from "../platform/abstractions/platform-utils.service";
import { InsecureUrlNotAllowedError } from "./api-errors";
import { ApiService, HttpOperations } from "./api.service";
describe("ApiService", () => {
@@ -411,4 +412,39 @@ describe("ApiService", () => {
).rejects.toMatchObject(error);
},
);
it("throws error when trying to fetch an insecure URL", async () => {
environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue(
of({
getApiUrl: () => "http://example.com",
} satisfies Partial<Environment> as Environment),
);
httpOperations.createRequest.mockImplementation((url, request) => {
return {
url: url,
cache: request.cache,
credentials: request.credentials,
method: request.method,
mode: request.mode,
signal: request.signal ?? undefined,
headers: new Headers(request.headers),
} satisfies Partial<Request> as unknown as Request;
});
const nativeFetch = jest.fn<Promise<Response>, [request: Request]>();
nativeFetch.mockImplementation((request) => {
return Promise.resolve({
ok: true,
status: 204,
headers: new Headers(),
} satisfies Partial<Response> as Response);
});
sut.nativeFetch = nativeFetch;
await expect(
async () => await sut.send("GET", "/something", null, true, true, null),
).rejects.toThrow(InsecureUrlNotAllowedError);
expect(nativeFetch).not.toHaveBeenCalled();
});
});

View File

@@ -117,6 +117,8 @@ import { AttachmentResponse } from "../vault/models/response/attachment.response
import { CipherResponse } from "../vault/models/response/cipher.response";
import { OptionalCipherResponse } from "../vault/models/response/optional-cipher.response";
import { InsecureUrlNotAllowedError } from "./api-errors";
export type HttpOperations = {
createRequest: (url: string, request: RequestInit) => Request;
};
@@ -1310,6 +1312,10 @@ export class ApiService implements ApiServiceAbstraction {
}
async fetch(request: Request): Promise<Response> {
if (!request.url.startsWith("https://") && !this.platformUtilsService.isDev()) {
throw new InsecureUrlNotAllowedError();
}
if (request.method === "GET") {
request.headers.set("Cache-Control", "no-store");
request.headers.set("Pragma", "no-cache");