Skip to content

crypto: use WebIDL converters in WebCryptoAPI#46067

Merged
nodejs-github-bot merged 27 commits intonodejs:mainfrom
panva:webidl
Jan 17, 2023
Merged

crypto: use WebIDL converters in WebCryptoAPI#46067
nodejs-github-bot merged 27 commits intonodejs:mainfrom
panva:webidl

Conversation

@panva
Copy link
Member

@panva panva commented Jan 2, 2023

It is currently the case that what works in Node.js webcrypto will most likely work in every browser's webcrypto as well.

This PR makes the other direction work as well.

Our built in validators are just too strict and intervene in cases when WebCryptoAPI through use of WebIDL should successfully coerse a value.


This is a semver-major PRs that contain breaking changes and should be released in the next major version. change because:

  • it changes some of the error codes
  • no longer accepts string values for BufferSource inputs and options

Example errors generated through the webcrypto specific webidl
await crypto.subtle.digest('sha-256', []);

TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument is not instance of ArrayBuffer, Buffer, TypedArray, or DataView.
    at codedTypeError (node:internal/crypto/webidl:43:15)
    at makeException (node:internal/crypto/webidl:52:10)
    at converters.BufferSource (node:internal/crypto/webidl:216:11)
    at SubtleCrypto.digest (node:internal/crypto/webcrypto:79:28)
  code: 'ERR_INVALID_ARG_TYPE'
await crypto.subtle.generateKey({ name: 'HMAC' }, true, ['sign', 'verify']);

TypeError: Failed to normalize algorithm: passed algorithm can not be converted to 'HmacKeyGenParams' because 'hash' is required in 'HmacKeyGenParams'.
    at codedTypeError (node:internal/crypto/webidl:43:15)
    at makeException (node:internal/crypto/webidl:52:10)
    at Object.HmacKeyGenParams (node:internal/crypto/webidl:321:15)
    at normalizeAlgorithm (node:internal/crypto/util:316:61)
    at SubtleCrypto.generateKey (node:internal/crypto/webcrypto:116:15)
  code: 'ERR_MISSING_OPTION'
await crypto.subtle.generateKey({name:'HMAC', hash: 'SHA-256', length: {}}, false, ['sign', 'verify']);

TypeError: Failed to normalize algorithm: 'length' of 'HmacKeyGenParams' (passed algorithm) is not a finite number.
    at codedTypeError (node:internal/crypto/webidl:43:15)
    at makeException (node:internal/crypto/webidl:52:10)
    at Object.unsigned long (node:internal/crypto/webidl:118:15)
    at converter (node:internal/crypto/webidl:526:36)
    at Object.HmacKeyGenParams (node:internal/crypto/webidl:318:32)
    at normalizeAlgorithm (node:internal/crypto/util:316:61)
    at SubtleCrypto.generateKey (node:internal/crypto/webcrypto:116:15)
  code: 'ERR_INVALID_ARG_TYPE'

I would also love to hide all stack traces from all functions in lib/internal/crypto/webidl.js and also ones stemming from the normalizeAlgorithm function in lib/internal/crypto/util.js.

I've tried my best using the utils exported from lib/internal/errors.js but it lead nowhere, all it did was rename the functions in the trace but they remained visible.

If you have an idea how to do this please reach out to me on the OpenJS Foundation slack. Probably not required for the PR to land though.


notable-change PRs with changes that should be highlighted in changelogs. summary

WebCryptoAPI functions' arguments are now coersed and validated as per their WebIDL definitions like in other Web Crypto API implementations. This further improves interoperability with other implementations of Web Crypto API.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. review wanted PRs that need reviews. semver-major PRs that contain breaking changes and should be released in the next major version. webcrypto

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants