Argon2 KDF Support

Quexten · January 21, 2023, 7:19am

Pull request for mobile argon2id now lives here:

github.com/bitwarden/mobile

Argon2id KDF

bitwarden:master ← bitwarden:argon2ios

opened 10:26PM - 19 Jan 23 UTC

kspearrin

+113 -33

## Type of change - [ ] Bug fix - [x] New feature development - [ ] Tech debt… (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ## Objective ## Code changes * **file.ext:** Description of what was changed and why ## Screenshots ## Before you submit - Please check for formatting errors (`dotnet format --verify-no-changes`) (required) - Please add **unit tests** where it makes sense to do so (encouraged but not required) - If this change requires a **documentation update** - notify the documentation team - If this change has particular **deployment requirements** - notify the DevOps team

trparky · January 24, 2023, 4:12pm

Any idea when this will go live?

Quexten · January 24, 2023, 5:15pm

Considering the Bitwarden team is actively involved in all 3 PRs and mobile and server changes are pretty much finished, I don’t think it should take too much longer. Do keep in mind Bitwarden still needs to do QA on the changes and they have a 5 week release cycle.

cksapp · January 24, 2023, 5:23pm

Still fairly quick comparatively for any corporate software honestly IMO.
Really glad the team is so quick and ready to accept such changes and work to get them implemented.

trparky · January 24, 2023, 5:26pm

Considering the Bitwarden team is actively involved in all 3 PRs

PR? What’s that mean?

Still fairly quick comparatively for any corporate software honestly IMO.
Really glad the team is so quick and ready to accept such changes and work to get them implemented.

That’s open source for you.

Quexten · January 24, 2023, 5:34pm

PR = Pull request, I.e someone writes code and suggests to (in this case) Bitwarden: “Hey, there is this code I wrote, would you like to include (pull) it?”

trparky · January 24, 2023, 5:40pm

Oh, ok. So, considering that the core teams are involved in this, you think it won’t be long. Good.

grb · January 24, 2023, 10:41pm

@Quexten

It seems that the password-protected JSON exports are hardcoded to use PBKDF2:

github.com

bitwarden/clients/blob/feb10128f0963bf6a9dabf23d1a01d8ae8ea117c/libs/common/src/services/export.service.ts#L62


      
          async getPasswordProtectedExport(password: string, organizationId?: string): Promise<string> {
            const clearText = organizationId
              ? await this.getOrganizationExport(organizationId, "json")
              : await this.getExport("json");
          
          
  const salt = Utils.fromBufferToB64(await this.cryptoFunctionService.randomBytes(16));
            const kdfIterations = DEFAULT_KDF_ITERATIONS;
            const key = await this.cryptoService.makePinKey(
              password,
              salt,
              KdfType.PBKDF2_SHA256,
              kdfIterations
            );
          
          
  const encKeyValidation = await this.cryptoService.encrypt(Utils.newGuid(), key);
            const encText = await this.cryptoService.encrypt(clearText, key);
          
          
  const jsonDoc: any = {
              encrypted: true,
              passwordProtected: true,
              salt: salt,

Perhaps this requires a separate PR, but it would be nice if when a user sets the hash function option to Argon2 for their vault, this setting would also be carried over to exports (and Sends).

Quexten · January 24, 2023, 10:45pm

I already have a PR for this open

github.com/bitwarden/clients

[PS-2264] Fix encrypted export ignoring user's KDF settings

bitwarden:master ← quexten:fix/encrypted-export-kdf

opened 12:58PM - 15 Jan 23 UTC

quexten

+18 -11

## Type of change ``` - [x] Bug fix - [ ] New feature development - [ …] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ``` ## Objective The encrypted export format contains fields for the KDF type and iteration count, but it was only statically set to PBKDF2 and 100.000 iterations. As a user I would expect it to follow my vault's KDF settings, should I choose to configure them with higher iteration counts/other kdf functions. This pull request changes the export and import to remove the hardcording, such that they work with different iteration counts and different KDF types. ## Code changes We just inject the stateservice into the export service to get the KDF type and iterations, and write them into the exported json/use them to encrypt.

By the way, Sends (which I don’t really use) also have 100K fixed pbkdf2. But they don’t even store the kdf / iterations in the database, so changing it would require another database migration / backend change which I didn’t really feel like taking on considering how low the risk for a send is anyways.

Quexten · January 25, 2023, 2:04pm

Mobile and Server support has been merged to master , only clients (Web/Desktop/Browser/CLI) is still in-progress.

deviator · January 26, 2023, 3:44pm

Good stuff! I have one comment regarding the Argon2id default parametes as I noticed that you have chosen extremely conservative default values:

export const DEFAULT_ARGON2_MEMORY = 19;
export const DEFAULT_ARGON2_PARALLELISM = 1;
export const DEFAULT_ARGON2_ITERATIONS = 2;

It seems that the default values are based on the OWASP recommendation which I believe applies to server side operations where you may have a significant number of hashing/key derivation operations at the same time. This is not the case with Bitwarden as the key derivation is done on the client side. KeePass, for example, recommends following method to determine the memory parameter:

Find out the RAM size of each of your devices on which you want to open your database file. Let M be the minimum of these sizes. Set the memory parameter to min(M /2, 1 GB) (i.e. use the half of M , if it is less than 1 GB, otherwise use 1 GB).

Quexten · January 26, 2023, 3:45pm

I agree, they are based on OWASP. The defaults should run on all devices, so 1GiB is too large as a default. But I have suggested higher defaults on the configuration pull requests, let’s see what the bitwarden team thinks on this.

grb · January 26, 2023, 4:08pm

OWASP recommendations are apparently cribbed from Steve Thomas, who explains the rationale for his recommendations in the Info section at the bottom of his Minimum Password Settings page. The settings are designed to throttle an attacker’s hash speeds to 10kH/s/GPU. Of note is that this is the recommendation when using passwords for authentication purposes only. As I’ve noted in another post, when the password is used for encryption, hash rates should ideally be much slower (<1kH/s/GPU).

Quexten · January 26, 2023, 4:09pm

Thanks for finding this! I’ll note this on the GitHub thread aswell.

tomtom · January 26, 2023, 4:14pm

Thanks for the hard work you‘re doing. Really appreciate it.

Have a nice day

Quexten · January 28, 2023, 2:40am

Updated defaults now to Iterations = 3, Memory = 64MiB, Parallelism = 4 from RFC9106’s recommendation on “safe defaults”. If you have significantly worse system specs, then you can go lower. If you have higher specs on your devices you can go higher.

grb · January 28, 2023, 3:19am

The recommended defaults in RFC9106 also specify “128-bit salt, and 256-bit tag size” — are these specs also implemented in your PR?

Quexten · January 28, 2023, 3:33am

The salt is the email, so we don’t directly influence how many bit’s it has. Since it has to be at least 8 bytes, I had to update it to be the SHA256 hash of the email such that even very short emails will work correctly. So it is 32 Byte i.e 256 bit.

Tag size = hash size. This is 32 Byte (or 256), same as for PBKDF2.

Quexten · January 30, 2023, 4:36pm

The last 2 Pull-requests have now been merged!

mkucharski · February 13, 2023, 8:09pm

I saw comments about that WASM support is only affecting old browsers, in this thread and other KDF related threads.

Please be aware, there are modern web browser setups where WASM is disabled by default. Reliance on WASM is not that an optimal solution and I imagine it will break some clients.