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: re-add padding for AES-KW wrapped JWKs #46563

Merged
merged 1 commit into from
Feb 17, 2023

Conversation

panva
Copy link
Member

@panva panva commented Feb 8, 2023

This re-introduces auto-padding of JWK exported keys that are to be wrapped with AES-KW if they don't meet the pre-conditions for AES-KW (length % 8 === 0). This is optional as per NOTE in step 13 of https://w3c.github.io/webcrypto/#SubtleCrypto-method-wrapKey.

The key wrapping operations for some algorithms place constraints on the payload size. For example AES-KW requires the payload to be a multiple of 8 bytes in length and RSA-OAEP places a restriction on the length. For key formats that offer flexibility in serialization of a given key (for example JWK), implementations may choose to adapt the serialization to the constraints of the wrapping algorithm. This is why JSON.stringify is not normatively required, as otherwise it would prohibit implementations from introducing added padding.

This is being re-added because I have unfortunately removed it in #46067 due to not having any effect on test or WPT outcome. Now with enabled test vectors for it in test/parallel/test-webcrypto-wrap-unwrap.js.

When this PR lands I will add it to the v18.x backport of #46067 (#46252).

@panva panva added crypto Issues and PRs related to the crypto subsystem. dont-land-on-v14.x request-ci Add this label to start a Jenkins CI on a PR. webcrypto dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. labels Feb 8, 2023
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Feb 8, 2023
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2023

Failed to start CI
- Validating Jenkins credentials
✖  Jenkins credentials invalid
https://github.com/nodejs/node/actions/runs/4123171581

@panva panva removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Feb 8, 2023
@panva panva added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Feb 12, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 16, 2023
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 17, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 17, 2023
@nodejs-github-bot nodejs-github-bot merged commit 4c1a277 into nodejs:main Feb 17, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 4c1a277

@panva panva deleted the webcrypto-jwk-aes-kw branch February 17, 2023 13:12
panva added a commit to panva/node that referenced this pull request Feb 17, 2023
@panva panva added the backport-open-v18.x Indicate that the PR has an open backport. label Feb 17, 2023
panva added a commit to panva/node that referenced this pull request Mar 5, 2023
panva added a commit to panva/node that referenced this pull request Mar 8, 2023
panva added a commit to panva/node that referenced this pull request Apr 3, 2023
panva added a commit to panva/node that referenced this pull request Apr 3, 2023
panva added a commit to panva/node that referenced this pull request Apr 18, 2023
panva added a commit to panva/node that referenced this pull request Apr 18, 2023
panva added a commit to panva/node that referenced this pull request May 15, 2023
danielleadams pushed a commit that referenced this pull request May 17, 2023
PR-URL: #46563
Reviewed-By: James M Snell <[email protected]>
Backport-PR-URL: #46252
@panva panva removed the backport-open-v18.x Indicate that the PR has an open backport. label Jul 26, 2023
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. crypto Issues and PRs related to the crypto subsystem. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. needs-ci PRs that need a full CI run. webcrypto
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants