Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2025-01-02 18:17:46 +01:00
Code Issues Projects Releases Wiki Activity

Vault/pm-4185/checksum uris (#6485)

Browse Source 
* Validate checksum on decrypt of URI

* Add uri checksum to domain during encryption

* Move hash to stateless encrypt service

* Add checksum field to all the other models necessary for syncing with server

* Remove old test in favor of `describe` block

* PM-4185 Added a boolean to control checksum validation

* PM-4185 Fi unit tests

* [PM-4810][PM-4825][PM-4880] Fix encrypted import and add null check (#6935)

* PM-4810 Bumped up version

* PM-4880 Add null check

* PM-4825 Fix encrypted export

* PM-5462 Fix item saving with blank URI (#7640)

* PM-4185 Add back uriChecksum setting

---------

Co-authored-by: Carlos Gonçalves <cgoncalves@bitwarden.com>
Co-authored-by: Matt Bishop <mbishop@bitwarden.com>
Co-authored-by: gbubemismith <gsmithwalter@gmail.com>
Co-authored-by: bnagawiecki <107435978+bnagawiecki@users.noreply.github.com>
This commit is contained in:
Matt Gibson 2024-01-24 12:22:58 -05:00 committed by GitHub
parent 622791307a
commit af0d2f515d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 192 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
libs/common/src/models/export/login-uri.export.ts
View File

@ -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;
}

6
libs/common/src/platform/abstractions/encrypt.service.ts
View File

@ -18,4 +18,10 @@ export abstract class EncryptService {
items: Decryptable<T>[],
key: SymmetricCryptoKey,
) => Promise<T[]>;
/**
* 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<string>;
}

5
libs/common/src/platform/services/cryptography/encrypt.service.implementation.ts
View File

@ -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<string> {
const hashArray = await this.cryptoFunctionService.hash(value, algorithm);
return Utils.fromBufferToB64(hashArray);
}
private async aesEncrypt(data: Uint8Array, key: SymmetricCryptoKey): Promise<EncryptedObject> {
const obj = new EncryptedObject();
obj.key = key;

2
libs/common/src/vault/models/api/login-uri.api.ts
View File

@ -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;
}

2
libs/common/src/vault/models/data/login-uri.data.ts
View File

@ -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;
}
}

12
libs/common/src/vault/models/domain/cipher.spec.ts
View File

@ -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: [
{

4
libs/common/src/vault/models/domain/cipher.ts
View File

@ -125,10 +125,12 @@ export class Cipher extends Domain implements Decryptable<CipherView> {
// 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<CipherView> {
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<CipherView> {
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);

42
libs/common/src/vault/models/domain/login-uri.spec.ts
View File

@ -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<EncryptService>;
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<LoginUri>);
expect(actual).toEqual({
uri: "myUri_fromJSON",
uriChecksum: "myUriChecksum_fromJSON",
match: UriMatchType.Domain,
});
expect(actual).toBeInstanceOf(LoginUri);
});

18
libs/common/src/vault/models/domain/login-uri.ts
View File

@ -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,
});
}
}

79
libs/common/src/vault/models/domain/login.spec.ts
View File

@ -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<LoginUri>();
describe("decrypt", () => {
let loginUri: MockProxy<LoginUri>;
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",

15
libs/common/src/vault/models/domain/login.ts
View File

@ -50,7 +50,11 @@ export class Login extends Domain {
}
}
async decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise<LoginView> {
async decrypt(
orgId: string,
bypassValidation: boolean,
encKey?: SymmetricCryptoKey,
): Promise<LoginView> {
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);
}
}
}

1
libs/common/src/vault/models/request/cipher.request.ts
View File

@ -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;

44
libs/common/src/vault/services/cipher.service.spec.ts
View File

@ -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<EncArrayBuffer>();
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", () => {

5
libs/common/src/vault/services/cipher.service.ts
View File

@ -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);
}
}