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.

Sign up for GitHub

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

Jump to bottom

crypto: re-add padding for AES-KW wrapped JWKs #46563

Merged
merged 1 commit into from
Feb 17, 2023
Merged

crypto: re-add padding for AES-KW wrapped JWKs #46563

merged 1 commit into from
Feb 17, 2023

Conversation

panva
Copy link
Member

@panva panva commented Feb 8, 2023 •
edited
Loading

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.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/49612/

@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
crypto: re-add padding for AES-KW wrapped JWKs
4a11ec6 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#46252
@panva panva added the backport-open-v18.x Indicate that the PR has an open backport. label Feb 17, 2023
@panva panva mentioned this pull request Feb 18, 2023
Closed
panva added a commit to panva/node that referenced this pull request Mar 5, 2023
@panva
crypto: re-add padding for AES-KW wrapped JWKs
0303a54 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#46252
panva added a commit to panva/node that referenced this pull request Mar 8, 2023
@panva
crypto: re-add padding for AES-KW wrapped JWKs
c12ff2c 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#46252
@RafaelGSS RafaelGSS mentioned this pull request Apr 3, 2023
Merged
panva added a commit to panva/node that referenced this pull request Apr 3, 2023
@panva
crypto: re-add padding for AES-KW wrapped JWKs
9d0fdf7 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
@panva panva mentioned this pull request Apr 3, 2023
Closed
panva added a commit to panva/node that referenced this pull request Apr 3, 2023
@panva
crypto: re-add padding for AES-KW wrapped JWKs
15c3756 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#47383
panva added a commit to panva/node that referenced this pull request Apr 18, 2023
@panva
crypto: re-add padding for AES-KW wrapped JWKs
203e269 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#47383
panva added a commit to panva/node that referenced this pull request Apr 18, 2023
@panva
crypto: re-add padding for AES-KW wrapped JWKs
b703389 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#46252
panva added a commit to panva/node that referenced this pull request May 15, 2023
@panva
crypto: re-add padding for AES-KW wrapped JWKs
588a166 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#47336
@panva panva mentioned this pull request May 15, 2023
Closed
danielleadams pushed a commit that referenced this pull request May 17, 2023
@panva @danielleadams
crypto: re-add padding for AES-KW wrapped JWKs
075d9ee 
PR-URL: #46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: #46252
@danielleadams danielleadams mentioned this pull request Jul 10, 2023
Merged
@panva panva removed the backport-open-v18.x Indicate that the PR has an open backport. label Jul 26, 2023
aduh95 pushed a commit to aduh95/node that referenced this pull request Feb 18, 2025
@panva @RafaelGSS
crypto: re-add padding for AES-KW wrapped JWKs
76e4d12 
PR-URL: nodejs#46563
Reviewed-By: James M Snell <jasnell@gmail.com>
Backport-PR-URL: nodejs#46252
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@panva @nodejs-github-bot @jasnell