Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto.subtle.deriveKey different length arg in deno and browser #16180

Closed
petuhovskiy opened this issue Oct 6, 2022 · 1 comment · Fixed by #16201
Closed

crypto.subtle.deriveKey different length arg in deno and browser #16180

petuhovskiy opened this issue Oct 6, 2022 · 1 comment · Fixed by #16201
Labels
bug Something isn't working ext/crypto related to the ext/crypto crate

Comments

@petuhovskiy
Copy link

Describe the bug

We were playing with https://github.com/denodrivers/postgres and at some point realized that crypto SCRAM implementation works differently in Deno and other environments (browser, cloudflare workers, etc.).

After some debugging, we narrowed the scope to the deriveKeySignatures function, which uses SubtleCrypto with PBKDF2 and HMAC/SHA-256. It turned out that specifying an optional length parameter in a single place can fix the code to work without issues across all environments:

const key = await crypto.subtle.deriveKey(
  {
    hash: "SHA-256",
    iterations,
    name: "PBKDF2",
    salt,
  },
  pbkdf2_password,
  { name: "HMAC", hash: "SHA-256" }, // should be changed to `{ name: "HMAC", hash: "SHA-256", length: 256 }`
  false,
  ["sign"],
);

https://developer.mozilla.org/en-US/docs/Web/API/HmacKeyGenParams says that:

length Optional

    A Number — the length in bits of the key. If this is omitted, the length of the key is equal to the block size of the hash function you have chosen. Unless you have a good reason to use a different length, omit this property and use the default.

It looks like the issue is that Deno implementation for default length is wrong and differs from browser engines.

This issue is probably the same as #14938

Steps to Reproduce

Code sample based on https://github.com/denodrivers/postgres/blob/8a07131efa17f4a6bcab86fd81407f149de93449/connection/scram.ts#L93

Execute this code in deno and browser:

const text_encoder = new TextEncoder();

async function deriveKeySignatures(
  password,
  salt,
  iterations,
) {
  const pbkdf2_password = await crypto.subtle.importKey(
    "raw",
    text_encoder.encode(password),
    "PBKDF2",
    false,
    ["deriveBits", "deriveKey"],
  );
  
  const key = await crypto.subtle.deriveKey(
    {
      hash: "SHA-256",
      iterations,
      name: "PBKDF2",
      salt,
    },
    pbkdf2_password,
    { name: "HMAC", hash: "SHA-256" },
    false,
    ["sign"],
  );

  return new Uint8Array(
    await crypto.subtle.sign("HMAC", key, text_encoder.encode("Client Key")),
  );
}

const password = 'pencil';
const salt = new Uint8Array(16);
const iterations = 4096;

(async () => {
  const arr = await deriveKeySignatures(password, salt, iterations);
  console.log(`Output: \n${arr}`);
})();

Output in Deno:

% deno run abc.js
Output: 
179,101,253,54,56,183,193,229,31,232,75,181,36,12,242,54,206,53,167,9,215,163,190,68,194,32,159,180,206,100,226,35

Output in browser (Firefox 105.0.1, Chrome 105):

Output: 
114,244,238,221,188,128,172,235,219,75,104,188,200,132,225,54,16,132,146,123,247,172,246,49,151,171,157,81,145,58,13,179

Expected behavior

Code in Deno and browser bundle produces the same result.

Environment

  • OS: macOS 11.4
  • deno version: 1.26.0
@panva
Copy link
Contributor

panva commented Oct 7, 2022

@petuhovskiy this is a great find, it seems both deno and node make the same mistake of using the hash's output size instead of block size.

@dsherret dsherret added bug Something isn't working ext/crypto related to the ext/crypto crate labels Oct 9, 2022
littledivy pushed a commit that referenced this issue Oct 15, 2022
fixes #16180

`HMAC`'s `get key length` `op` uses the hash function's block size, not
output size.

refs
cloudflare/workerd#68 (comment)
bartlomieju pushed a commit that referenced this issue Oct 17, 2022
fixes #16180

`HMAC`'s `get key length` `op` uses the hash function's block size, not
output size.

refs
cloudflare/workerd#68 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ext/crypto related to the ext/crypto crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants