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: remove webcrypto EdDSA key checks and properties #1093

Closed
wants to merge 1 commit into from
Closed

crypto: remove webcrypto EdDSA key checks and properties #1093

wants to merge 1 commit into from

Conversation

panva
Copy link
Contributor

@panva panva commented Aug 30, 2023

As per WICG/webcrypto-secure-curves#24 this removes the check for Ed25519 JWK alg during importKey and removes the alg for Ed25519 keys during JWK exportKey.

@github-actions
Copy link

github-actions bot commented Aug 30, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@panva
Copy link
Contributor Author

panva commented Aug 30, 2023

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Aug 30, 2023
@@ -2126,9 +2117,6 @@ private:
jwk.kty = kj::str("OKP");
jwk.crv = kj::str(getAlgorithmName() == "X25519"_kj ? "X25519"_kj : "Ed25519"_kj);
jwk.x = kj::encodeBase64Url(kj::arrayPtr(rawPublicKey, publicKeyLen));
if (getAlgorithmName() == "Ed25519"_kj) {
jwk.alg = kj::str("EdDSA");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a chance this could end up being a breaking change. Will need to determine if a compatibility flag is necessary on this particular bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, by all means please take this effort over, my hope here was merely to bring this to your attention ;)

@panva
Copy link
Contributor Author

panva commented Oct 25, 2023

I've deleted my fork so this PR automatically closed, i'm assuming you'll take this over @jasnell

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.

None yet

2 participants