wintercg / proposal-common-minimum-api

https://common-min-api.proposal.wintercg.org/
Other
216 stars 15 forks source link

Include a subset of Web Crypto API Secure Curves #24

Closed panva closed 7 months ago

panva commented 1 year ago

I'd like to propose that Ed25519 EdDSA (proposed in Secure Curves in the Web Cryptography API) be part of the Minimum Common Web Platform API proposal.

Depending on feasibility possibly also X25519 key agreement.

jasnell commented 1 year ago

Currently we don't like specific algorithm sets but we likely should. I'd certainly be +1 on this.

guest271314 commented 7 months ago

I've run into this issue recently building Signed Web Bundles for Isolated Web Apps. I don't think Deno and Bun implementation is the same as Node.js. Even though Deno supports the algorithm. See https://github.com/GoogleChromeLabs/webbundle-plugins/issues/11#issuecomment-1847301072.

Some Deno/Node compatibility code https://github.com/denoland/deno/blob/ca64771257d23ceee97e882965269702c359f6aa/cli/tests/node_compat/test/parallel/test-webcrypto-sign-verify.js#L115-L133.

// Test Sign/Verify Ed25519
{
  async function test(data) {
    const ec = new TextEncoder();
    const { publicKey, privateKey } = await subtle.generateKey({
      name: 'Ed25519',
    }, true, ['sign', 'verify']);

    const signature = await subtle.sign({
      name: 'Ed25519',
    }, privateKey, ec.encode(data));

    assert(await subtle.verify({
      name: 'Ed25519',
    }, publicKey, signature, ec.encode(data)));
  }

  test('hello world').then(common.mustCall());
}

Some Node.js-specific code https://www.npmjs.com/package/wbn-sign?activeTab=code at node-crypto-signing-strategy.js

import crypto from 'crypto';
import { checkIsValidEd25519Key } from '../utils/utils.js';
// Class to be used when signing with parsed `crypto.KeyObject` private key
// provided directly in the constructor.
export class NodeCryptoSigningStrategy {
    privateKey;
    constructor(privateKey) {
        this.privateKey = privateKey;
        checkIsValidEd25519Key('private', privateKey);
    }
    async sign(data) {
        return crypto.sign(/*algorithm=*/ undefined, data, this.privateKey);
    }
    async getPublicKey() {
        return crypto.createPublicKey(this.privateKey);
    }
}

which throws "Unsupported algorithm" in Deno and Bun.

panva commented 7 months ago

Deno and Bun both support the Ed25519 Algorithm Identifier and its operations in Web Crypto API, same as Node.js

panva commented 7 months ago

Also I don't think this issue is relevant anymore, any new runtime striving for parity is bound to implement it already.

guest271314 commented 7 months ago

If you notice the signatures of Node.js and Deno code are not the same.

crypto from var crypto = require('crypto') is not the same as crypto.webcrypto. Developers that write and publish code exclusively with Node.js code tend to use Node.js-specific implementations.

Thanks for bringing this up.

panva commented 7 months ago

If you notice the signatures of Node.js and Deno code are not the same.

That is just not true, a simple demonstration shows that the three mentioned runtimes (Node.js, Deno, and Bun) all implement the Ed25519 Web Crypto API algorithm and end up with the same deterministic signature.

$ cat some.mjs 
const { subtle } = globalThis.crypto

const privateKey = await subtle.importKey('jwk', {
  crv: 'Ed25519',
  d: 'V0q2XioKqnAmNdVaBhBUq633t5kQxPRBjus4uXAAU7s',
  x: 'qDeZ0OXyLv-PUWqAL3C0DyhDAJ-qG-p5eH1J49DV6ow',
  kty: 'OKP'
}, 'Ed25519', false, ['sign'])

const signature = await subtle.sign('Ed25519', privateKey, new Uint8Array(0))

console.log(new Uint8Array(signature))
$ deno run some.mjs
Uint8Array(64) [
   97, 156, 185, 235, 170,   5, 219, 149, 197, 208, 105,
    7,  55, 221,  35,  71,  39,  78,  81, 119, 234, 142,
  126, 237, 129,  38, 149, 101, 157, 212, 170, 221, 233,
   39, 123,  49,  92, 178,   5,  87,  50, 100, 105,  33,
  102,  54, 150,  42, 198,  47, 147,   2,  48,  51,  14,
  196, 201,  60, 139,  85,  65, 166, 115,   9
]
$ bun run some.mjs 
Uint8Array(64) [ 97, 156, 185, 235, 170, 5, 219, 149, 197, 208, 105, 7, 55, 221, 35, 71, 39, 78, 81, 119, 234, 142, 126, 237, 129, 38, 149, 101, 157, 212, 170, 221, 233, 39, 123, 49, 92, 178, 5, 87, 50, 100, 105, 33, 102, 54, 150, 42, 198, 47, 147, 2, 48, 51, 14, 196, 201, 60, 139, 85, 65, 166, 115, 9 ]
$ node --no-warnings some.mjs 
Uint8Array(64) [
   97, 156, 185, 235, 170,   5, 219, 149, 197, 208, 105,
    7,  55, 221,  35,  71,  39,  78,  81, 119, 234, 142,
  126, 237, 129,  38, 149, 101, 157, 212, 170, 221, 233,
   39, 123,  49,  92, 178,   5,  87,  50, 100, 105,  33,
  102,  54, 150,  42, 198,  47, 147,   2,  48,  51,  14,
  196, 201,  60, 139,  85,  65, 166, 115,   9
]
guest271314 commented 7 months ago

Cf.

const signature = await subtle.sign('Ed25519', privateKey, new Uint8Array(0))
return crypto.sign(/*algorithm=*/ undefined, data, this.privateKey);

The data and keys are in different parameter locations.

If you run the code I posted from the GoogleChromeLabs repository deno and bun will throw. Now, is that because the developer is using the Node.js crypto object itself, rather than crypto.webcrypto, perhaps.

panva commented 7 months ago

So two different APIs (Web Cryptography API !== node:crypto) have different parameter signatures. I fail to see a problem. It is irrelevant since only one of those APIs is part of the WinterCG minimal API.

Your claims wrt. the produced signatures in different runtimes using WebCryptoAPI are not true.

guest271314 commented 7 months ago

So two different APIs (Web Cryptography API !== node:crypto) have different parameter signatures. I fail to see a problem.

Because you haven't run the code and encountered those errors. I don't have a cryptography background. I read the code and figured out on my own that the repositories are using node:crypto and not Web Cryptography API. I didn't know that node had its own parent object that webcrypto is defined in, and that is what the developers were using, not webcrypto. I just observed the errors in the wild. That's when I noticed the signatures of the parameters are not the same. So this is just feedback.

It is true that Node.js signature is different from Web Cryptography API signature.

I think that needs to be made clear.