diff --git a/libs/common/src/models/export/login-uri.export.ts b/libs/common/src/models/export/login-uri.export.ts index 9c8a96e99d..8a21c3ff97 100644 --- a/libs/common/src/models/export/login-uri.export.ts +++ b/libs/common/src/models/export/login-uri.export.ts @@ -19,11 +19,13 @@ export class LoginUriExport { static toDomain(req: LoginUriExport, domain = new LoginUriDomain()) { domain.uri = req.uri != null ? new EncString(req.uri) : null; + domain.uriChecksum = req.uriChecksum != null ? new EncString(req.uriChecksum) : null; domain.match = req.match; return domain; } uri: string; + uriChecksum: string | undefined; match: UriMatchType = null; constructor(o?: LoginUriView | LoginUriDomain) { @@ -35,6 +37,7 @@ export class LoginUriExport { this.uri = o.uri; } else { this.uri = o.uri?.encryptedString; + this.uriChecksum = o.uriChecksum?.encryptedString; } this.match = o.match; } diff --git a/libs/common/src/platform/abstractions/encrypt.service.ts b/libs/common/src/platform/abstractions/encrypt.service.ts index 09f6b8a9b7..e76dff59d3 100644 --- a/libs/common/src/platform/abstractions/encrypt.service.ts +++ b/libs/common/src/platform/abstractions/encrypt.service.ts @@ -18,4 +18,10 @@ export abstract class EncryptService { items: Decryptable[], key: SymmetricCryptoKey, ) => Promise; + /** + * Generates a base64-encoded hash of the given value + * @param value The value to hash + * @param algorithm The hashing algorithm to use + */ + hash: (value: string | Uint8Array, algorithm: "sha1" | "sha256" | "sha512") => Promise; } diff --git a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts index eecf4e39c6..b61f44fa3c 100644 --- a/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts +++ b/libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts @@ -158,6 +158,11 @@ export class EncryptServiceImplementation implements EncryptService { return await Promise.all(items.map((item) => item.decrypt(key))); } + async hash(value: string | Uint8Array, algorithm: "sha1" | "sha256" | "sha512"): Promise { + const hashArray = await this.cryptoFunctionService.hash(value, algorithm); + return Utils.fromBufferToB64(hashArray); + } + private async aesEncrypt(data: Uint8Array, key: SymmetricCryptoKey): Promise { const obj = new EncryptedObject(); obj.key = key; diff --git a/libs/common/src/vault/models/api/login-uri.api.ts b/libs/common/src/vault/models/api/login-uri.api.ts index f4e18bebd8..ace8a31700 100644 --- a/libs/common/src/vault/models/api/login-uri.api.ts +++ b/libs/common/src/vault/models/api/login-uri.api.ts @@ -3,6 +3,7 @@ import { UriMatchType } from "../../enums"; export class LoginUriApi extends BaseResponse { uri: string; + uriChecksum: string; match: UriMatchType = null; constructor(data: any = null) { @@ -11,6 +12,7 @@ export class LoginUriApi extends BaseResponse { return; } this.uri = this.getResponseProperty("Uri"); + this.uriChecksum = this.getResponseProperty("UriChecksum"); const match = this.getResponseProperty("Match"); this.match = match != null ? match : null; } diff --git a/libs/common/src/vault/models/data/login-uri.data.ts b/libs/common/src/vault/models/data/login-uri.data.ts index 344e6c377b..973470ffc7 100644 --- a/libs/common/src/vault/models/data/login-uri.data.ts +++ b/libs/common/src/vault/models/data/login-uri.data.ts @@ -3,6 +3,7 @@ import { LoginUriApi } from "../api/login-uri.api"; export class LoginUriData { uri: string; + uriChecksum: string; match: UriMatchType = null; constructor(data?: LoginUriApi) { @@ -10,6 +11,7 @@ export class LoginUriData { return; } this.uri = data.uri; + this.uriChecksum = data.uriChecksum; this.match = data.match; } } diff --git a/libs/common/src/vault/models/domain/cipher.spec.ts b/libs/common/src/vault/models/domain/cipher.spec.ts index 6b3c35efd4..856d6d3887 100644 --- a/libs/common/src/vault/models/domain/cipher.spec.ts +++ b/libs/common/src/vault/models/domain/cipher.spec.ts @@ -75,7 +75,9 @@ describe("Cipher DTO", () => { reprompt: CipherRepromptType.None, key: "EncryptedString", login: { - uris: [{ uri: "EncryptedString", match: UriMatchType.Domain }], + uris: [ + { uri: "EncryptedString", uriChecksum: "EncryptedString", match: UriMatchType.Domain }, + ], username: "EncryptedString", password: "EncryptedString", passwordRevisionDate: "2022-01-31T12:00:00.000Z", @@ -148,7 +150,13 @@ describe("Cipher DTO", () => { username: { encryptedString: "EncryptedString", encryptionType: 0 }, password: { encryptedString: "EncryptedString", encryptionType: 0 }, totp: { encryptedString: "EncryptedString", encryptionType: 0 }, - uris: [{ match: 0, uri: { encryptedString: "EncryptedString", encryptionType: 0 } }], + uris: [ + { + match: 0, + uri: { encryptedString: "EncryptedString", encryptionType: 0 }, + uriChecksum: { encryptedString: "EncryptedString", encryptionType: 0 }, + }, + ], }, attachments: [ { diff --git a/libs/common/src/vault/models/domain/cipher.ts b/libs/common/src/vault/models/domain/cipher.ts index 229405487b..475c933752 100644 --- a/libs/common/src/vault/models/domain/cipher.ts +++ b/libs/common/src/vault/models/domain/cipher.ts @@ -125,10 +125,12 @@ export class Cipher extends Domain implements Decryptable { // We will refactor the EncString.decrypt() in https://bitwarden.atlassian.net/browse/PM-3762 to remove the dependency on the organizationId. async decrypt(encKey: SymmetricCryptoKey): Promise { const model = new CipherView(this); + let bypassValidation = true; if (this.key != null) { const encryptService = Utils.getContainerService().getEncryptService(); encKey = new SymmetricCryptoKey(await encryptService.decryptToBytes(this.key, encKey)); + bypassValidation = false; } await this.decryptObj( @@ -143,7 +145,7 @@ export class Cipher extends Domain implements Decryptable { switch (this.type) { case CipherType.Login: - model.login = await this.login.decrypt(this.organizationId, encKey); + model.login = await this.login.decrypt(this.organizationId, bypassValidation, encKey); break; case CipherType.SecureNote: model.secureNote = await this.secureNote.decrypt(this.organizationId, encKey); diff --git a/libs/common/src/vault/models/domain/login-uri.spec.ts b/libs/common/src/vault/models/domain/login-uri.spec.ts index 8f0657e37e..1c432cf8d7 100644 --- a/libs/common/src/vault/models/domain/login-uri.spec.ts +++ b/libs/common/src/vault/models/domain/login-uri.spec.ts @@ -1,6 +1,8 @@ +import { MockProxy, mock } from "jest-mock-extended"; import { Jsonify } from "type-fest"; import { mockEnc, mockFromJson } from "../../../../spec"; +import { EncryptService } from "../../../platform/abstractions/encrypt.service"; import { EncString } from "../../../platform/models/domain/enc-string"; import { UriMatchType } from "../../enums"; import { LoginUriData } from "../data/login-uri.data"; @@ -13,6 +15,7 @@ describe("LoginUri", () => { beforeEach(() => { data = { uri: "encUri", + uriChecksum: "encUriChecksum", match: UriMatchType.Domain, }; }); @@ -24,6 +27,7 @@ describe("LoginUri", () => { expect(loginUri).toEqual({ match: null, uri: null, + uriChecksum: null, }); }); @@ -33,6 +37,7 @@ describe("LoginUri", () => { expect(loginUri).toEqual({ match: 0, uri: { encryptedString: "encUri", encryptionType: 0 }, + uriChecksum: { encryptedString: "encUriChecksum", encryptionType: 0 }, }); }); @@ -58,16 +63,53 @@ describe("LoginUri", () => { }); }); + describe("validateChecksum", () => { + let encryptService: MockProxy; + + beforeEach(() => { + encryptService = mock(); + global.bitwardenContainerService = { + getEncryptService: () => encryptService, + getCryptoService: () => null, + }; + }); + + it("returns true if checksums match", async () => { + const loginUri = new LoginUri(); + loginUri.uriChecksum = mockEnc("checksum"); + encryptService.hash.mockResolvedValue("checksum"); + + const actual = await loginUri.validateChecksum("uri", null, null); + + expect(actual).toBe(true); + expect(encryptService.hash).toHaveBeenCalledWith("uri", "sha256"); + }); + + it("returns false if checksums don't match", async () => { + const loginUri = new LoginUri(); + loginUri.uriChecksum = mockEnc("checksum"); + encryptService.hash.mockResolvedValue("incorrect checksum"); + + const actual = await loginUri.validateChecksum("uri", null, null); + + expect(actual).toBe(false); + }); + }); + describe("fromJSON", () => { it("initializes nested objects", () => { jest.spyOn(EncString, "fromJSON").mockImplementation(mockFromJson); const actual = LoginUri.fromJSON({ uri: "myUri", + uriChecksum: "myUriChecksum", + match: UriMatchType.Domain, } as Jsonify); expect(actual).toEqual({ uri: "myUri_fromJSON", + uriChecksum: "myUriChecksum_fromJSON", + match: UriMatchType.Domain, }); expect(actual).toBeInstanceOf(LoginUri); }); diff --git a/libs/common/src/vault/models/domain/login-uri.ts b/libs/common/src/vault/models/domain/login-uri.ts index eee20cb15b..dcdd1de294 100644 --- a/libs/common/src/vault/models/domain/login-uri.ts +++ b/libs/common/src/vault/models/domain/login-uri.ts @@ -1,5 +1,6 @@ import { Jsonify } from "type-fest"; +import { Utils } from "../../../platform/misc/utils"; import Domain from "../../../platform/models/domain/domain-base"; import { EncString } from "../../../platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; @@ -9,6 +10,7 @@ import { LoginUriView } from "../view/login-uri.view"; export class LoginUri extends Domain { uri: EncString; + uriChecksum: EncString | undefined; match: UriMatchType; constructor(obj?: LoginUriData) { @@ -23,6 +25,7 @@ export class LoginUri extends Domain { obj, { uri: null, + uriChecksum: null, }, [], ); @@ -39,6 +42,18 @@ export class LoginUri extends Domain { ); } + async validateChecksum(clearTextUri: string, orgId: string, encKey: SymmetricCryptoKey) { + if (this.uriChecksum == null) { + return false; + } + + const cryptoService = Utils.getContainerService().getEncryptService(); + const localChecksum = await cryptoService.hash(clearTextUri, "sha256"); + + const remoteChecksum = await this.uriChecksum.decrypt(orgId, encKey); + return remoteChecksum === localChecksum; + } + toLoginUriData(): LoginUriData { const u = new LoginUriData(); this.buildDataModel( @@ -46,6 +61,7 @@ export class LoginUri extends Domain { u, { uri: null, + uriChecksum: null, match: null, }, ["match"], @@ -59,8 +75,10 @@ export class LoginUri extends Domain { } const uri = EncString.fromJSON(obj.uri); + const uriChecksum = EncString.fromJSON(obj.uriChecksum); return Object.assign(new LoginUri(), obj, { uri, + uriChecksum, }); } } diff --git a/libs/common/src/vault/models/domain/login.spec.ts b/libs/common/src/vault/models/domain/login.spec.ts index 9c9e005ced..232ca035aa 100644 --- a/libs/common/src/vault/models/domain/login.spec.ts +++ b/libs/common/src/vault/models/domain/login.spec.ts @@ -1,4 +1,4 @@ -import { mock } from "jest-mock-extended"; +import { MockProxy, mock } from "jest-mock-extended"; import { mockEnc, mockFromJson } from "../../../../spec"; import { EncryptedString, EncString } from "../../../platform/models/domain/enc-string"; @@ -30,7 +30,7 @@ describe("Login DTO", () => { it("Convert from full LoginData", () => { const fido2CredentialData = initializeFido2Credential(new Fido2CredentialData()); const data: LoginData = { - uris: [{ uri: "uri", match: UriMatchType.Domain }], + uris: [{ uri: "uri", uriChecksum: "checksum", match: UriMatchType.Domain }], username: "username", password: "password", passwordRevisionDate: "2022-01-31T12:00:00.000Z", @@ -46,7 +46,13 @@ describe("Login DTO", () => { username: { encryptedString: "username", encryptionType: 0 }, password: { encryptedString: "password", encryptionType: 0 }, totp: { encryptedString: "123", encryptionType: 0 }, - uris: [{ match: 0, uri: { encryptedString: "uri", encryptionType: 0 } }], + uris: [ + { + match: 0, + uri: { encryptedString: "uri", encryptionType: 0 }, + uriChecksum: { encryptedString: "checksum", encryptionType: 0 }, + }, + ], fido2Credentials: [encryptFido2Credential(fido2CredentialData)], }); }); @@ -57,48 +63,67 @@ describe("Login DTO", () => { expect(login).toEqual({}); }); - it("Decrypts correctly", async () => { - const loginUri = mock(); + describe("decrypt", () => { + let loginUri: MockProxy; const loginUriView = new LoginUriView(); - loginUriView.uri = "decrypted uri"; - loginUri.decrypt.mockResolvedValue(loginUriView); - - const login = new Login(); const decryptedFido2Credential = Symbol(); - login.uris = [loginUri]; - login.username = mockEnc("encrypted username"); - login.password = mockEnc("encrypted password"); - login.passwordRevisionDate = new Date("2022-01-31T12:00:00.000Z"); - login.totp = mockEnc("encrypted totp"); - login.autofillOnPageLoad = true; - login.fido2Credentials = [ - { decrypt: jest.fn().mockReturnValue(decryptedFido2Credential) } as any, - ]; - - const loginView = await login.decrypt(null); - expect(loginView).toEqual({ + const login = Object.assign(new Login(), { + username: mockEnc("encrypted username"), + password: mockEnc("encrypted password"), + passwordRevisionDate: new Date("2022-01-31T12:00:00.000Z"), + totp: mockEnc("encrypted totp"), + autofillOnPageLoad: true, + fido2Credentials: [{ decrypt: jest.fn().mockReturnValue(decryptedFido2Credential) } as any], + }); + const expectedView = { username: "encrypted username", password: "encrypted password", passwordRevisionDate: new Date("2022-01-31T12:00:00.000Z"), totp: "encrypted totp", uris: [ { - match: null, + match: null as UriMatchType, _uri: "decrypted uri", - _domain: null, - _hostname: null, - _host: null, - _canLaunch: null, + _domain: null as string, + _hostname: null as string, + _host: null as string, + _canLaunch: null as boolean, }, ], autofillOnPageLoad: true, fido2Credentials: [decryptedFido2Credential], + }; + + beforeEach(() => { + loginUri = mock(); + loginUriView.uri = "decrypted uri"; + }); + + it("should decrypt to a view", async () => { + loginUri.decrypt.mockResolvedValue(loginUriView); + loginUri.validateChecksum.mockResolvedValue(true); + login.uris = [loginUri]; + + const loginView = await login.decrypt(null, true); + expect(loginView).toEqual(expectedView); + }); + + it("should ignore uris that fail checksum", async () => { + loginUri.decrypt.mockResolvedValue(loginUriView); + loginUri.validateChecksum + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true); + login.uris = [loginUri, loginUri, loginUri]; + + const loginView = await login.decrypt(null, false); + expect(loginView).toEqual(expectedView); }); }); it("Converts from LoginData and back", () => { const data: LoginData = { - uris: [{ uri: "uri", match: UriMatchType.Domain }], + uris: [{ uri: "uri", uriChecksum: "checksum", match: UriMatchType.Domain }], username: "username", password: "password", passwordRevisionDate: "2022-01-31T12:00:00.000Z", diff --git a/libs/common/src/vault/models/domain/login.ts b/libs/common/src/vault/models/domain/login.ts index 7a80adcd61..92e6f69ec8 100644 --- a/libs/common/src/vault/models/domain/login.ts +++ b/libs/common/src/vault/models/domain/login.ts @@ -50,7 +50,11 @@ export class Login extends Domain { } } - async decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise { + async decrypt( + orgId: string, + bypassValidation: boolean, + encKey?: SymmetricCryptoKey, + ): Promise { const view = await this.decryptObj( new LoginView(this), { @@ -66,7 +70,14 @@ export class Login extends Domain { view.uris = []; for (let i = 0; i < this.uris.length; i++) { const uri = await this.uris[i].decrypt(orgId, encKey); - view.uris.push(uri); + // URIs are shared remotely after decryption + // we need to validate that the string hasn't been changed by a compromised server + // This validation is tied to the existence of cypher.key for backwards compatibility + // So we bypass the validation if there's no cipher.key or procceed with the validation and + // Skip the value if it's been tampered with. + if (bypassValidation || (await this.uris[i].validateChecksum(uri.uri, orgId, encKey))) { + view.uris.push(uri); + } } } diff --git a/libs/common/src/vault/models/request/cipher.request.ts b/libs/common/src/vault/models/request/cipher.request.ts index 68e0c6330d..52a55b6c3e 100644 --- a/libs/common/src/vault/models/request/cipher.request.ts +++ b/libs/common/src/vault/models/request/cipher.request.ts @@ -51,6 +51,7 @@ export class CipherRequest { const uri = new LoginUriApi(); uri.uri = u.uri != null ? u.uri.encryptedString : null; uri.match = u.match != null ? u.match : null; + uri.uriChecksum = u.uriChecksum != null ? u.uriChecksum.encryptedString : null; return uri; }) ?? []; this.login.username = cipher.login.username ? cipher.login.username.encryptedString : null; diff --git a/libs/common/src/vault/services/cipher.service.spec.ts b/libs/common/src/vault/services/cipher.service.spec.ts index 21d59a232c..b347c79fda 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -1,4 +1,4 @@ -import { mock, mockReset } from "jest-mock-extended"; +import { mock } from "jest-mock-extended"; import { of } from "rxjs"; import { makeStaticByteArray } from "../../../spec/utils"; @@ -25,10 +25,14 @@ import { CipherCreateRequest } from "../models/request/cipher-create.request"; import { CipherPartialRequest } from "../models/request/cipher-partial.request"; import { CipherRequest } from "../models/request/cipher.request"; import { CipherView } from "../models/view/cipher.view"; +import { LoginUriView } from "../models/view/login-uri.view"; import { CipherService } from "./cipher.service"; const ENCRYPTED_TEXT = "This data has been encrypted"; +function encryptText(clearText: string | Uint8Array) { + return Promise.resolve(new EncString(`${clearText} has been encrypted`)); +} const ENCRYPTED_BYTES = mock(); const cipherData: CipherData = { @@ -48,7 +52,7 @@ const cipherData: CipherData = { key: "EncKey", reprompt: CipherRepromptType.None, login: { - uris: [{ uri: "EncryptedString", match: UriMatchType.Domain }], + uris: [{ uri: "EncryptedString", uriChecksum: "EncryptedString", match: UriMatchType.Domain }], username: "EncryptedString", password: "EncryptedString", passwordRevisionDate: "2022-01-31T12:00:00.000Z", @@ -105,16 +109,6 @@ describe("Cipher Service", () => { let cipherObj: Cipher; beforeEach(() => { - mockReset(apiService); - mockReset(cryptoService); - mockReset(stateService); - mockReset(settingsService); - mockReset(cipherFileUploadService); - mockReset(i18nService); - mockReset(searchService); - mockReset(encryptService); - mockReset(configService); - encryptService.encryptToBytes.mockReturnValue(Promise.resolve(ENCRYPTED_BYTES)); encryptService.encrypt.mockReturnValue(Promise.resolve(new EncString(ENCRYPTED_TEXT))); @@ -134,6 +128,11 @@ describe("Cipher Service", () => { cipherObj = new Cipher(cipherData); }); + + afterEach(() => { + jest.resetAllMocks(); + }); + describe("saveAttachmentRawWithServer()", () => { it("should upload encrypted file contents with save attachments", async () => { const fileName = "filename"; @@ -252,7 +251,26 @@ describe("Cipher Service", () => { cryptoService.makeCipherKey.mockReturnValue( Promise.resolve(new SymmetricCryptoKey(makeStaticByteArray(64)) as CipherKey), ); - cryptoService.encrypt.mockReturnValue(Promise.resolve(new EncString(ENCRYPTED_TEXT))); + cryptoService.encrypt.mockImplementation(encryptText); + }); + + describe("login encryption", () => { + it("should add a uri hash to login uris", async () => { + encryptService.hash.mockImplementation((value) => Promise.resolve(`${value} hash`)); + cipherView.login.uris = [ + { uri: "uri", match: UriMatchType.RegularExpression } as LoginUriView, + ]; + + const domain = await cipherService.encrypt(cipherView); + + expect(domain.login.uris).toEqual([ + { + uri: new EncString("uri has been encrypted"), + uriChecksum: new EncString("uri hash has been encrypted"), + match: UriMatchType.RegularExpression, + }, + ]); + }); }); describe("cipher.key", () => { diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 799a4e234d..8cafde5705 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -50,7 +50,7 @@ import { CipherView } from "../models/view/cipher.view"; import { FieldView } from "../models/view/field.view"; import { PasswordHistoryView } from "../models/view/password-history.view"; -const CIPHER_KEY_ENC_MIN_SERVER_VER = new SemVer("2023.9.1"); +const CIPHER_KEY_ENC_MIN_SERVER_VER = new SemVer("2023.12.0"); export class CipherService implements CipherServiceAbstraction { private sortedCiphersCache: SortedCiphersCache = new SortedCiphersCache( @@ -1127,6 +1127,7 @@ export class CipherService implements CipherServiceAbstraction { if (model.login.uris != null) { cipher.login.uris = []; + model.login.uris = model.login.uris.filter((u) => u.uri != null); for (let i = 0; i < model.login.uris.length; i++) { const loginUri = new LoginUri(); loginUri.match = model.login.uris[i].match; @@ -1138,6 +1139,8 @@ export class CipherService implements CipherServiceAbstraction { }, key, ); + const uriHash = await this.encryptService.hash(model.login.uris[i].uri, "sha256"); + loginUri.uriChecksum = await this.cryptoService.encrypt(uriHash, key); cipher.login.uris.push(loginUri); } }