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

Methods fail with Miniflare and Node.js 18.4+ #446

Closed
2 tasks done
ItalyPaleAle opened this issue Sep 3, 2022 · 6 comments
Closed
2 tasks done

Methods fail with Miniflare and Node.js 18.4+ #446

ItalyPaleAle opened this issue Sep 3, 2022 · 6 comments
Labels

Comments

@ItalyPaleAle
Copy link
Contributor

What happened?

Following the implementation of nodejs/node#43310 (an issue I'm sure you're very familiar with 😄) in 18.4, Node.js dropped support for the NODE-ED25519 identifier. However, that's still being used in Cloudflare Workers, which doesn't support the newer Ed25519 identifier.

The jose library correctly handles the CF Workers situation.

As you are likely familiar with, Miniflare is a "local emulator" for CF Workers, which uses Node.js and not V8.

Starting with Miniflare 2.7, the emulator automatically converts NODE-ED25519 keys to Ed25519 so it continues to work with Node.js 18.4+. cloudflare/miniflare#311

However, this causes a situation with jose so it doesn't work with Miniflare on Node.js 18.4+. jose has built-in detection for CF Workers, and it detects Miniflare as a CF Workers environment, and the EdDSA routines reject keys that have type Ed25519.

case isCloudflareWorkers() && 'EdDSA': {
if (!isAlgorithm(key.algorithm, 'NODE-ED25519')) throw unusable('NODE-ED25519')
break
}

This causes a runtime error CryptoKey does not support this operation, its algorithm.name must be NODE-ED25519

Version

v4.9.2

Runtime

Other (I will specify below)

Runtime Details

Miniflare 2.7.1 with Node.js 18.4+

Code to reproduce

// With Miniflare 2.7.0-2.7.1, code below fails with `CryptoKey does not support this operation, its algorithm.name must be NODE-ED25519` on Node.js 18.4+
// (On Node.js 18.3 will fail the validation because the JWT's payload is actually invalid - I removed PII)

import * as jose from 'jose'
;(async () => {
    const token =
        'eyJhbGciOiJFZERTQSIsImtpZCI6IjFobkZhY3A3bFJvRWgwcXI0MkZfcG1oeUo1QSJ9.eyJzY29wZSI6Im9wZW5pZCBvZmZsaW5lX2FjY2VzcyBwaC1hdXRoIiwic3ViIjoiMS5FUkszOGMyTnQ1TzZndXB2dG9ib3hBdXV2MDFBIiwiaXNzIjoiaHR0cDovLzEyNy4wLjAuMTo4Nzg3LyJ9.KSMCkroVwL7xi2N3_0ztLNJbVhIPN5JcYQTy9eGZ19XF_ByVrcDKjtlu2hVeHO5dgrbI9j28NCByzwZGsPQSCA'
    const pk = {
        alg: 'EdDSA',
        crv: 'Ed25519',
        kid: '1hnFacp7lRoEh0qr42F_pmhyJ5A',
        kty: 'OKP',
        use: 'sig',
        x: 'R0v6Jjr8AmHDNYJp-TVL7YNC1GrRjmeKIiPGKTukHzg',
    }

    const jwks = jose.createLocalJWKSet({keys: [pk]})
    const {payload} = await jose.jwtVerify(token, jwks, {
        algorithms: ['EdDSA'],
        clockTolerance: 300,
    })

    console.log(payload)
})()

Required

  • I have searched the issues tracker and discussions for similar topics and couldn't find anything related.
  • I agree to follow this project's Code of Conduct
@panva
Copy link
Owner

panva commented Sep 3, 2022

Hi @ItalyPaleAle,

I don't see a way to support both. If cloudflare supported the Ed25519 identifier, #385 would solve this issue.

As of right now, i'd rather support Workers than miniflare.

@panva
Copy link
Owner

panva commented Sep 3, 2022

cc @jasnell the NODE-ED25519 identifier was removed from Node, the Ed25519 identifier is needed for interoperability between node and workers.

@ItalyPaleAle
Copy link
Contributor Author

@panva yes I agree it’s a situation that requires trade-offs.

What would you think about accepting both Ed25519 and NODE-ED25519 In CF Worker environments (both actual and Miniflare)? Right now, it’s jose that is throwing the exception and not the platforms themselves. Of course, in CF Workers today there’d be a runtime exception some time later, but with the idea that eventually CF Workers will likely support Ed25519 too, the trade-off may be acceptable?

@mrbbot
Copy link

mrbbot commented Sep 3, 2022

Hey! 👋 This is a bug in Miniflare, which should be replicating the Workers environment. I'll try fix this there, and return the "correct" algorithm.name in CryptoKeys. 👍

mrbbot added a commit to cloudflare/miniflare that referenced this issue Sep 3, 2022
The `Ed25519` algorithm was leaking to user code via
`CryptoKey#algorithm.name`, but Cloudflare Workers only support the
`NODE-ED25519` algorithm. This change plugs the leak, by overriding
`algorithm.name`, and then using `Proxy`s before passing `CryptoKey`s
back to Node.

Closes panva/jose#446.
@panva
Copy link
Owner

panva commented Sep 3, 2022

This was resolved in cloudflare/miniflare@b3e187d

@panva panva closed this as not planned Won't fix, can't repro, duplicate, stale Sep 3, 2022
@ItalyPaleAle
Copy link
Contributor Author

Thanks @mrbbot for the fix and @panva for managing this issue! 🙏

@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2022
panva added a commit that referenced this issue Feb 7, 2023
…ions, get ready for future non-proprietary support

closes #446
closes #495
closes #497
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants