deriveBits PBKDF2: algorithm.hash should allow a string value

[franky47]

Context

When using PBKDF2 to derive an array of bytes from a key, the WebCrypto documentation states you can do the following:

crypto.subtle.deriveBits(
  {
    name: 'PBKDF2',
    salt,
    iterations: 10000,
    hash: 'SHA-256'
  }, 
  key, 
  lengthInBits
)

Encountered issue

However, when calling this in node-webcrypto-ossl (which uses this library), I get the following error:

TypeError: Cannot read property 'toLowerCase' of undefined
      at Pbkdf2Provider.checkHashAlgorithm (node_modules/webcrypto-core/build/webcrypto-core.js:260:55)
      at Pbkdf2Provider.checkAlgorithmParams (node_modules/webcrypto-core/build/webcrypto-core.js:649:14)
      at Pbkdf2Provider.checkDeriveBits (node_modules/webcrypto-core/build/webcrypto-core.js:192:14)
      at Pbkdf2Provider.deriveBits (node_modules/webcrypto-core/build/webcrypto-core.js:187:30)
      at SubtleCrypto.deriveBits (node_modules/webcrypto-core/build/webcrypto-core.js:799:39)

Expected outcome

A string value should be allowed for the hash property of the algorithm.

Possible user remediation

For now, changing the call to use an object with a name property bypasses this issue:

crypto.subtle.deriveBits(
  {
    name: 'PBKDF2',
    salt,
    iterations: 10000,
    hash: { name: 'SHA-256' }
  }, 
  key, 
  lengthInBits
)

[microshine]

@franky47 What version of node-webcrypto-ossl do you use?

I can't reproduce the same error

import { Crypto } from "node-webcrypto-ossl";

async function main() {
  const crypto = new Crypto();
  console.log(crypto.getRandomValues(new Uint8Array(1)));

  const key = await crypto.subtle.importKey("raw", new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]), "pbkdf2", false, ["deriveBits"]);
  const bits = await crypto.subtle.deriveBits(
    {
      name: 'PBKDF2',
      salt: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8]),
      iterations: 10000,
      hash: 'sha-256',
    },
    key,
    128
  );
  console.log(bits);
}

main().catch(e => console.error(e));

Output

Uint8Array [ 255 ]
ArrayBuffer {
  [Uint8Contents]: <a4 e8 75 97 af f7 4e 9d 1b 62 be 7f b3 47 33 72>,
  byteLength: 16
}

Environment

└─┬ node-webcrypto-ossl@2.1.0
  ├── mkdirp@1.0.4
  ├── nan@2.14.1
  ├─┬ pvtsutils@1.0.10
  │ └── tslib@1.11.2 deduped
  ├── tslib@1.11.2
  └─┬ webcrypto-core@1.1.0

[franky47]

I have the same:

• node-webcrypto-ossl@2.1.0
• webcrypto-core@1.1.0

I was able to reproduce the error in the tests:

diff --git a/test/pbkdf.ts b/test/pbkdf.ts
index 235e397..cebb41e 100644
--- a/test/pbkdf.ts
+++ b/test/pbkdf.ts
@@ -54,6 +54,9 @@ context("HMAC", () => {
       provider.checkAlgorithmParams({ hash: { name: "SHA-256" }, salt: new Uint8Array(4), iterations: 1000 } as any);
     });

+    it("correct value (string hash)", () => {
+      provider.checkAlgorithmParams({ hash: "SHA-256", salt: new Uint8Array(4), iterations: 1000 } as any);
+    });
   });

   context("checkImportKey", () => {

[microshine]

I see.

SubtleCrypto prepares algorithm and coverts hash string value to Algorithm object.

https://github.com/PeculiarVentures/webcrypto-core/blob/master/src/subtle.ts#L94 https://github.com/PeculiarVentures/webcrypto-core/blob/master/src/subtle.ts#L193

If you want implement your own Crypto provider you can use @peculiar/webcrypto as an example.

[franky47]

I see, the issue is here: https://github.com/PeculiarVentures/webcrypto-core/blob/546711cbe652ca9fb84dead30eb6859891c7893f/src/subtle.ts#L11

This part of the test returns false in my case.

This is what the input & individual tests look like:

{
  data: {
    name: 'PBKDF2',
    salt: Uint8Array(32) [
      200,  53, 133,  62, 206, 213,  85,   6,
       43,  78,  67,  29, 111, 190, 220, 216,
      225,  34, 151, 136, 150, 164,  50, 104,
      214, 123,  41,  54, 195, 149, 143,  61
    ],
    iterations: 100000,
    hash: 'SHA-256'
  },
  'data instanceof Object': false,
  '"name" in data': true,
  '"hash" in data': true
}

[microshine]

Please update isHashedAlgorithm in node_modules. Does it fix problem? If so I'll publish a new version

function isHashedAlgorithm(data) {
  return data
    && typeof data === "object"
    && "name" in data
    && "hash" in data
    ? true
    : false;
}

[microshine]

@franky47 Is it possible to get a value you are passing to isHashedAlgorithm? I can't find a value which looks like a hashed algorithm but returns false for old implementation of isHAshedAlgorithm

[franky47]

This is what I did for my previous comment:

static isHashedAlgorithm(data) {
  console.dir({
    data,
    'data instanceof Object': data instanceof Object,
    '"name" in data': "name" in data,
    '"hash" in data': "hash" in data
  })
    return data instanceof Object
        && "name" in data
        && "hash" in data;
}
// prints:
{
  data: {
    name: 'PBKDF2',
    salt: Uint8Array(32) [
      200,  53, 133,  62, 206, 213,  85,   6,
       43,  78,  67,  29, 111, 190, 220, 216,
      225,  34, 151, 136, 150, 164,  50, 104,
      214, 123,  41,  54, 195, 149, 143,  61
    ],
    iterations: 100000,
    hash: 'SHA-256'
  },
  'data instanceof Object': false,
  '"name" in data': true,
  '"hash" in data': true
}

Changing isHashedAlgorithm to use typeof data === "object" works for me.

However I think checking if it's an object is not relevant, the only interesting part is that whatever is passed has a "name" and a "hash" property. Also, the in operator looks across the whole prototype chain, which could be an issue if an attacker managed to do that:

Object.prototype.name = 'none'
Object.prototype.hash = 'SHA-1' // or any weak hash algorithm

A safer call (that also works) would be:

    static isHashedAlgorithm(data) {
      return typeof data.hasOwnProperty === 'function' &&
        data.hasOwnProperty('name') &&
        data.hasOwnProperty('hash')
    }

[microshine]

@franky47 Thank you very much!

I published a new version webcrypto-core@1.1.1

It's base on typeof and in validations. For some cases, an algorithm can be an extended class with getters and setters and hasOwnProperty won't work

[franky47]

Looks good ! Thanks !