Skip to content

Commit

Permalink
[PM-3565] Enforce higher minimum KDF (#6440)
Browse files Browse the repository at this point in the history
Changes minimum iterations for PBKDF2 to 600 000. Also converts the constants into ranges to ensure there is only a single place for all checks.
  • Loading branch information
Hinton authored Dec 5, 2023
1 parent 5686048 commit 7bbdee9
Show file tree
Hide file tree
Showing 10 changed files with 150 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ export class ChangeKdfConfirmationComponent {
const masterKey = await this.cryptoService.getOrDeriveMasterKey(masterPassword);
request.masterPasswordHash = await this.cryptoService.hashMasterKey(masterPassword, masterKey);
const email = await this.stateService.getEmail();

// Ensure the KDF config is valid.
this.cryptoService.validateKdfConfig(this.kdf, this.kdfConfig);

const newMasterKey = await this.cryptoService.makeMasterKey(
masterPassword,
email,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ <h1>{{ "encKeySettings" | i18n }}</h1>
<input
id="kdfMemory"
type="number"
min="16"
max="1024"
[min]="ARGON2_MEMORY.min"
[max]="ARGON2_MEMORY.max"
name="Memory"
class="form-control mb-3"
[(ngModel)]="kdfConfig.memory"
Expand All @@ -57,8 +57,8 @@ <h1>{{ "encKeySettings" | i18n }}</h1>
<input
id="kdfIterations"
type="number"
min="100000"
max="2000000"
[min]="PBKDF2_ITERATIONS.min"
[max]="PBKDF2_ITERATIONS.max"
name="KdfIterations"
class="form-control"
[(ngModel)]="kdfConfig.iterations"
Expand All @@ -70,8 +70,8 @@ <h1>{{ "encKeySettings" | i18n }}</h1>
<input
id="iterations"
type="number"
min="2"
max="10"
[min]="ARGON2_ITERATIONS.min"
[max]="ARGON2_ITERATIONS.max"
name="Iterations"
class="form-control mb-3"
[(ngModel)]="kdfConfig.iterations"
Expand All @@ -81,8 +81,8 @@ <h1>{{ "encKeySettings" | i18n }}</h1>
<input
id="kdfParallelism"
type="number"
min="1"
max="16"
[min]="ARGON2_PARALLELISM.min"
[max]="ARGON2_PARALLELISM.max"
name="Parallelism"
class="form-control"
[(ngModel)]="kdfConfig.parallelism"
Expand All @@ -94,7 +94,7 @@ <h1>{{ "encKeySettings" | i18n }}</h1>
<div class="col-12">
<ng-container *ngIf="kdf == kdfType.PBKDF2_SHA256">
<p class="small form-text text-muted">
{{ "kdfIterationsDesc" | i18n: (recommendedPbkdf2Iterations | number) }}
{{ "kdfIterationsDesc" | i18n: (PBKDF2_ITERATIONS.defaultValue | number) }}
</p>
<bit-callout type="warning">
{{ "kdfIterationsWarning" | i18n: (100000 | number) }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { KdfConfig } from "@bitwarden/common/auth/models/domain/kdf-config";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import {
DEFAULT_KDF_CONFIG,
DEFAULT_PBKDF2_ITERATIONS,
DEFAULT_ARGON2_ITERATIONS,
DEFAULT_ARGON2_MEMORY,
DEFAULT_ARGON2_PARALLELISM,
PBKDF2_ITERATIONS,
ARGON2_ITERATIONS,
ARGON2_MEMORY,
ARGON2_PARALLELISM,
KdfType,
} from "@bitwarden/common/platform/enums";
import { DialogService } from "@bitwarden/components";
Expand All @@ -23,7 +23,12 @@ export class ChangeKdfComponent implements OnInit {
kdfConfig: KdfConfig = DEFAULT_KDF_CONFIG;
kdfType = KdfType;
kdfOptions: any[] = [];
recommendedPbkdf2Iterations = DEFAULT_PBKDF2_ITERATIONS;

// Default values for template
protected PBKDF2_ITERATIONS = PBKDF2_ITERATIONS;
protected ARGON2_ITERATIONS = ARGON2_ITERATIONS;
protected ARGON2_MEMORY = ARGON2_MEMORY;
protected ARGON2_PARALLELISM = ARGON2_PARALLELISM;

constructor(
private stateService: StateService,
Expand All @@ -42,12 +47,12 @@ export class ChangeKdfComponent implements OnInit {

async onChangeKdf(newValue: KdfType) {
if (newValue === KdfType.PBKDF2_SHA256) {
this.kdfConfig = new KdfConfig(DEFAULT_PBKDF2_ITERATIONS);
this.kdfConfig = new KdfConfig(PBKDF2_ITERATIONS.defaultValue);
} else if (newValue === KdfType.Argon2id) {
this.kdfConfig = new KdfConfig(
DEFAULT_ARGON2_ITERATIONS,
DEFAULT_ARGON2_MEMORY,
DEFAULT_ARGON2_PARALLELISM,
ARGON2_ITERATIONS.defaultValue,
ARGON2_MEMORY.defaultValue,
ARGON2_PARALLELISM.defaultValue,
);
} else {
throw new Error("Unknown KDF type.");
Expand Down
6 changes: 4 additions & 2 deletions apps/web/src/app/vault/individual-vault/vault.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service"
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { DEFAULT_PBKDF2_ITERATIONS, KdfType } from "@bitwarden/common/platform/enums";
import { KdfType, PBKDF2_ITERATIONS } from "@bitwarden/common/platform/enums";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
Expand Down Expand Up @@ -967,7 +967,9 @@ export class VaultComponent implements OnInit, OnDestroy {
async isLowKdfIteration() {
const kdfType = await this.stateService.getKdfType();
const kdfOptions = await this.stateService.getKdfConfig();
return kdfType === KdfType.PBKDF2_SHA256 && kdfOptions.iterations < DEFAULT_PBKDF2_ITERATIONS;
return (
kdfType === KdfType.PBKDF2_SHA256 && kdfOptions.iterations < PBKDF2_ITERATIONS.defaultValue
);
}

protected async repromptCipher(ciphers: CipherView[]) {
Expand Down
8 changes: 8 additions & 0 deletions libs/common/src/platform/abstractions/crypto.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,14 @@ export abstract class CryptoService {
privateKey: EncString;
}>;

/**
* Validate that the KDF config follows the requirements for the given KDF type.
*
* @remarks
* Should always be called before updating a users KDF config.
*/
validateKdfConfig: (kdf: KdfType, kdfConfig: KdfConfig) => void;

/**
* @deprecated Left for migration purposes. Use decryptUserKeyWithPin instead.
*/
Expand Down
11 changes: 6 additions & 5 deletions libs/common/src/platform/enums/kdf-type.enum.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { KdfConfig } from "../../auth/models/domain/kdf-config";
import { RangeWithDefault } from "../misc/range-with-default";

export enum KdfType {
PBKDF2_SHA256 = 0,
Argon2id = 1,
}

export const DEFAULT_ARGON2_MEMORY = 64;
export const DEFAULT_ARGON2_PARALLELISM = 4;
export const DEFAULT_ARGON2_ITERATIONS = 3;
export const ARGON2_MEMORY = new RangeWithDefault(16, 1024, 64);
export const ARGON2_PARALLELISM = new RangeWithDefault(1, 16, 4);
export const ARGON2_ITERATIONS = new RangeWithDefault(2, 10, 3);

export const DEFAULT_KDF_TYPE = KdfType.PBKDF2_SHA256;
export const DEFAULT_PBKDF2_ITERATIONS = 600000;
export const DEFAULT_KDF_CONFIG = new KdfConfig(DEFAULT_PBKDF2_ITERATIONS);
export const PBKDF2_ITERATIONS = new RangeWithDefault(600_000, 2_000_000, 600_000);
export const DEFAULT_KDF_CONFIG = new KdfConfig(PBKDF2_ITERATIONS.defaultValue);
26 changes: 26 additions & 0 deletions libs/common/src/platform/misc/range-with-default.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { RangeWithDefault } from "./range-with-default";

describe("RangeWithDefault", () => {
describe("constructor", () => {
it("should throw an error when min is greater than max", () => {
expect(() => new RangeWithDefault(10, 5, 0)).toThrowError("10 is greater than 5.");
});

it("should throw an error when default value is not in range", () => {
expect(() => new RangeWithDefault(0, 10, 20)).toThrowError("Default value is not in range.");
});
});

describe("inRange", () => {
it("should return true when in range", () => {
const range = new RangeWithDefault(0, 10, 5);
expect(range.inRange(5)).toBe(true);
});

it("should return false when not in range", () => {
const range = new RangeWithDefault(5, 10, 7);
expect(range.inRange(1)).toBe(false);
expect(range.inRange(20)).toBe(false);
});
});
});
24 changes: 24 additions & 0 deletions libs/common/src/platform/misc/range-with-default.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* A range with a default value.
*
* Enforces constraints to ensure min > default > max.
*/
export class RangeWithDefault {
constructor(
readonly min: number,
readonly max: number,
readonly defaultValue: number,
) {
if (min > max) {
throw new Error(`${min} is greater than ${max}.`);
}

if (this.inRange(defaultValue) === false) {
throw new Error("Default value is not in range.");
}
}

inRange(value: number): boolean {
return value >= this.min && value <= this.max;
}
}
69 changes: 52 additions & 17 deletions libs/common/src/platform/services/crypto.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ import {
KeySuffixOptions,
HashPurpose,
KdfType,
DEFAULT_ARGON2_ITERATIONS,
DEFAULT_ARGON2_MEMORY,
DEFAULT_ARGON2_PARALLELISM,
ARGON2_ITERATIONS,
ARGON2_MEMORY,
ARGON2_PARALLELISM,
EncryptionType,
PBKDF2_ITERATIONS,
} from "../enums";
import { sequentialize } from "../misc/sequentialize";
import { EFFLongWordList } from "../misc/wordlist";
Expand Down Expand Up @@ -175,6 +176,12 @@ export class CryptoService implements CryptoServiceAbstraction {
));
}

/**
* Derive a master key from a password and email.
*
* @remarks
* Does not validate the kdf config to ensure it satisfies the minimum requirements for the given kdf type.
*/
async makeMasterKey(
password: string,
email: string,
Expand Down Expand Up @@ -841,6 +848,43 @@ export class CryptoService implements CryptoServiceAbstraction {
return null;
}

/**
* Validate that the KDF config follows the requirements for the given KDF type.
*
* @remarks
* Should always be called before updating a users KDF config.
*/
validateKdfConfig(kdf: KdfType, kdfConfig: KdfConfig): void {
switch (kdf) {
case KdfType.PBKDF2_SHA256:
if (!PBKDF2_ITERATIONS.inRange(kdfConfig.iterations)) {
throw new Error(
`PBKDF2 iterations must be between ${PBKDF2_ITERATIONS.min} and ${PBKDF2_ITERATIONS.max}`,
);
}
break;
case KdfType.Argon2id:
if (!ARGON2_ITERATIONS.inRange(kdfConfig.iterations)) {
throw new Error(
`Argon2 iterations must be between ${ARGON2_ITERATIONS.min} and ${ARGON2_ITERATIONS.max}`,
);
}

if (!ARGON2_MEMORY.inRange(kdfConfig.memory)) {
throw new Error(
`Argon2 memory must be between ${ARGON2_MEMORY.min}mb and ${ARGON2_MEMORY.max}mb`,
);
}

if (!ARGON2_PARALLELISM.inRange(kdfConfig.parallelism)) {
throw new Error(
`Argon2 parallelism must be between ${ARGON2_PARALLELISM.min} and ${ARGON2_PARALLELISM.max}.`,
);
}
break;
}
}

protected async clearAllStoredUserKeys(userId?: string): Promise<void> {
await this.stateService.setUserKeyAutoUnlock(null, { userId: userId });
await this.stateService.setPinKeyEncryptedUserKeyEphemeral(null, { userId: userId });
Expand Down Expand Up @@ -900,30 +944,21 @@ export class CryptoService implements CryptoServiceAbstraction {
let key: Uint8Array = null;
if (kdf == null || kdf === KdfType.PBKDF2_SHA256) {
if (kdfConfig.iterations == null) {
kdfConfig.iterations = 5000;
} else if (kdfConfig.iterations < 5000) {
throw new Error("PBKDF2 iteration minimum is 5000.");
kdfConfig.iterations = PBKDF2_ITERATIONS.defaultValue;
}

key = await this.cryptoFunctionService.pbkdf2(password, salt, "sha256", kdfConfig.iterations);
} else if (kdf == KdfType.Argon2id) {
if (kdfConfig.iterations == null) {
kdfConfig.iterations = DEFAULT_ARGON2_ITERATIONS;
} else if (kdfConfig.iterations < 2) {
throw new Error("Argon2 iteration minimum is 2.");
kdfConfig.iterations = ARGON2_ITERATIONS.defaultValue;
}

if (kdfConfig.memory == null) {
kdfConfig.memory = DEFAULT_ARGON2_MEMORY;
} else if (kdfConfig.memory < 16) {
throw new Error("Argon2 memory minimum is 16 MB");
} else if (kdfConfig.memory > 1024) {
throw new Error("Argon2 memory maximum is 1024 MB");
kdfConfig.memory = ARGON2_MEMORY.defaultValue;
}

if (kdfConfig.parallelism == null) {
kdfConfig.parallelism = DEFAULT_ARGON2_PARALLELISM;
} else if (kdfConfig.parallelism < 1) {
throw new Error("Argon2 parallelism minimum is 1.");
kdfConfig.parallelism = ARGON2_PARALLELISM.defaultValue;
}

const saltHash = await this.cryptoFunctionService.hash(salt, "sha256");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { KdfConfig } from "@bitwarden/common/auth/models/domain/kdf-config";
import { CipherWithIdExport } from "@bitwarden/common/models/export/cipher-with-ids.export";
import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service";
import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service";
import { KdfType, DEFAULT_PBKDF2_ITERATIONS } from "@bitwarden/common/platform/enums";
import { KdfType, PBKDF2_ITERATIONS } from "@bitwarden/common/platform/enums";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { EncryptedString, EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { StateService } from "@bitwarden/common/platform/services/state.service";
Expand Down Expand Up @@ -159,7 +159,7 @@ describe("VaultExportService", () => {
folderService.getAllDecryptedFromState.mockResolvedValue(UserFolderViews);
folderService.getAllFromState.mockResolvedValue(UserFolders);
stateService.getKdfType.mockResolvedValue(KdfType.PBKDF2_SHA256);
stateService.getKdfConfig.mockResolvedValue(new KdfConfig(DEFAULT_PBKDF2_ITERATIONS));
stateService.getKdfConfig.mockResolvedValue(new KdfConfig(PBKDF2_ITERATIONS.defaultValue));
cryptoService.encrypt.mockResolvedValue(new EncString("encrypted"));

exportService = new VaultExportService(
Expand Down Expand Up @@ -240,7 +240,7 @@ describe("VaultExportService", () => {
});

it("specifies kdfIterations", () => {
expect(exportObject.kdfIterations).toEqual(DEFAULT_PBKDF2_ITERATIONS);
expect(exportObject.kdfIterations).toEqual(PBKDF2_ITERATIONS.defaultValue);
});

it("has kdfType", () => {
Expand Down

0 comments on commit 7bbdee9

Please sign in to comment.