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

Allow specifying usages when importing public ECDH key, deriveBits/deriveKey refactor #415

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

fhanau
Copy link
Collaborator

@fhanau fhanau commented Mar 2, 2023

  • Allow specifying deriveBits and deriveKey usages when importing public ECDH keys. This behavior is already supported when importing raw ECDH keys and is more intuitive even though the usages are not required for public keys, see comments.
  • Create CryptoKeyUsageSet::derivationKeyMask() to include both derive operations
  • Deletes two superfluous assertions in importEllipticRaw – the validation right afterwards is more thorough and provides better error handling.

Please let me know if you find anything unclear

@fhanau
Copy link
Collaborator Author

fhanau commented Mar 2, 2023

Opened an upstream PR with the same branch name.

@fhanau fhanau merged commit 1b5057f into main Mar 3, 2023
@panva
Copy link
Contributor

panva commented Mar 6, 2023

Please let me know if you find anything unclear

All import key formats that result (or may result) in a public CryptoKey are clear that that the usages array must be empty for the ECDH algorithm public keys.

This change makes something non-standard work on workers and not be portable to other runtimes. For a portable standard API that's very unfortunate.

@jasnell
Copy link
Member

jasnell commented Mar 6, 2023

Ah, right, I'd forgotten that detail. This is definitely something we should reconsider. /cc @fhanau @dom96

@fhanau
Copy link
Collaborator Author

fhanau commented Mar 6, 2023

Please let me know if you find anything unclear

All import key formats that result (or may result) in a public CryptoKey are clear that that the usages array must be empty for the ECDH algorithm.

This change makes something non-standard work on workers and not be portable to other runtimes. For a portable standard API that's very unfortunate.

You're right, I missed that aspect of the specification. I'll work on fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants