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

Support for Ed(X)25519 #4263

Open
olegbespalov opened this issue Oct 30, 2024 · 5 comments · May be fixed by #4556
Open

Support for Ed(X)25519 #4263

olegbespalov opened this issue Oct 30, 2024 · 5 comments · May be fixed by #4556

Comments

@olegbespalov
Copy link
Contributor

olegbespalov commented Oct 30, 2024

What?

Even though original WebCrypto API doesn't include support of the Curve25519, but many of webcrypto implementation does w3c/webcrypto#362

Even WebPlatfrom test suit tests this support, which led us to patch https://github.com/grafana/k6/blob/master/internal/js/modules/k6/webcrypto/tests/wpt-patches/WebCryptoAPI__generateKey__failures.js.patch

Draft can be found https://wicg.github.io/webcrypto-secure-curves/

From the Golang's perspective, support also looks good since we have packages:

Why?

Seems like support of this Curve25519 is demanded, so having it in k6 makes k6's implementation more attractive and useful for customers.

@McMastS
Copy link
Contributor

McMastS commented Feb 15, 2025

Hey @olegbespalov! I'd like to take this issue on, I'll hopefully have a draft PR up by the end of the weekend.

@olegbespalov
Copy link
Contributor Author

olegbespalov commented Feb 15, 2025

Hey @McMastS !

Sure, go ahead, thanks for doing that!

Just in case, I've updated the description to reflect the actual path (we've got back-merged extensions code to k6).

Don't hesitate to ask if you need any help or want to discuss something!

P.S. I've assigned you to the issue for now just to highlight that the issue has activity.

Cheers!

@olegbespalov olegbespalov assigned McMastS and unassigned codebien Feb 15, 2025
@McMastS
Copy link
Contributor

McMastS commented Feb 15, 2025

Hey @olegbespalov! I've uncommented the X25519 and Ed25519 lines in that test but I can't actually get the go test to fail locally.

My allTestVectors now looks like this in internal/js/modules/k6/webcrypto/tests/wpt/WebCryptoAPI/generateKey/successes.js:

    var allTestVectors = [ // Parameters that should work for generateKey
        {name: "AES-CTR",  resultType: CryptoKey, usages: ["encrypt", "decrypt", "wrapKey", "unwrapKey"], mandatoryUsages: []},
        {name: "AES-CBC",  resultType: CryptoKey, usages: ["encrypt", "decrypt", "wrapKey", "unwrapKey"], mandatoryUsages: []},
        {name: "AES-GCM",  resultType: CryptoKey, usages: ["encrypt", "decrypt", "wrapKey", "unwrapKey"], mandatoryUsages: []},
        // {name: "AES-KW",   resultType: CryptoKey, usages: ["wrapKey", "unwrapKey"], mandatoryUsages: []},
        {name: "HMAC",     resultType: CryptoKey, usages: ["sign", "verify"], mandatoryUsages: []},
        {name: "RSASSA-PKCS1-v1_5", resultType: "CryptoKeyPair", usages: ["sign", "verify"], mandatoryUsages: ["sign"]},
        {name: "RSA-PSS",  resultType: "CryptoKeyPair", usages: ["sign", "verify"], mandatoryUsages: ["sign"]},
        {name: "RSA-OAEP", resultType: "CryptoKeyPair", usages: ["encrypt", "decrypt", "wrapKey", "unwrapKey"], mandatoryUsages: ["decrypt", "unwrapKey"]},
        {name: "ECDSA",    resultType: "CryptoKeyPair", usages: ["sign", "verify"], mandatoryUsages: ["sign"]},
        {name: "ECDH",     resultType: "CryptoKeyPair", usages: ["deriveKey", "deriveBits"], mandatoryUsages: ["deriveKey", "deriveBits"]},
        {name: "Ed25519",  resultType: "CryptoKeyPair", usages: ["sign", "verify"], mandatoryUsages: ["sign", "wrapKey"]},
        // {name: "Ed448",    resultType: "CryptoKeyPair", usages: ["sign", "verify"], mandatoryUsages: ["sign"]},
        {name: "X25519",   resultType: "CryptoKeyPair", usages: ["deriveKey", "deriveBits"], mandatoryUsages: ["deriveKey", "deriveBits"]},
        // {name: "X448",     resultType: "CryptoKeyPair", usages: ["deriveKey", "deriveBits"], mandatoryUsages: ["deriveKey", "deriveBits"]},
    ];

The go test passes in about 20 seconds so it is running something, and if I throw an error on line 64 of that same file then the test "passes" in less than a second so it's definitely getting cut short. Is there any Sobek setup or something I'm missing to get these tests to propagate the JS error to the Go test?

@olegbespalov
Copy link
Contributor Author

olegbespalov commented Feb 17, 2025

@McMastS you're absolutely right 🤔 If I uncomment these lines locally, the tests currently pass without any failures.

Image

But the simple script like:

import { crypto } from "k6/experimental/webcrypto";

export default async function () {
  const key = await crypto.subtle.generateKey(
    {
      name: "Ed25519"
    },
    true,
    ["sign", "verify"]
  );

  console.log(JSON.stringify(key));
}

Fails with the:

ERRO[0000] Uncaught (in promise) NotSupportedError: algorithm "ED25519" doesn't support (in implementation) operation "generateKey"  executor=per-vu-iterations scenario=default

Image

It's probably some degradation (maybe in the tests happened), but I need to check deeper, why it so, and I've opened a separated issue for a better visibility of the problem.

In theory, the work on this issue still could be continued since the key generation test case is simple enough, but doesn't actually check if the functionality there. But there are tests in internal/js/modules/k6/webcrypto/tests/wpt/WebCryptoAPI/sign_verify with entrypoint eddsa_curve25519.https.any.js that could help validate implementation.

@McMastS
Copy link
Contributor

McMastS commented Feb 18, 2025

Thanks for looking into it! Good to know I wasn't going crazy hahaha. I'll keep working on it without those tests and verify with manual Javascript scripts for now.

@McMastS McMastS linked a pull request Feb 18, 2025 that will close this issue
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants