dhensby / node-http-message-signatures

A node package for signing and verifying HTTP messages as per RFC 9421 - HTTP Message Signatures specification
ISC License
13 stars 8 forks source link

Support client side (react-script) #159

Open 0x0ece opened 10 months ago

0x0ece commented 10 months ago

Would you consider support for client side?

Right now, if I try to use the lib in a react app, I get this errors:

Module not found: Error: Can't resolve 'crypto' in '/path/to/node_modules/.pnpm/http-message-signatures@1.0.1/node_modules/http-message-signatures/lib/algorithm'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }'
    - install 'crypto-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "crypto": false }
ERROR in ./node_modules/.pnpm/http-message-signatures@1.0.1/node_modules/http-message-signatures/lib/algorithm/index.js 8:20-40
Module not found: Error: Can't resolve 'constants' in '/path/to/.pnpm/http-message-signatures@1.0.1/node_modules/http-message-signatures/lib/algorithm'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "constants": require.resolve("constants-browserify") }'
    - install 'constants-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "constants": false }

Unfortunately react-script is a little bit of its own beast... it uses webpack 5 but has a very complex internal config, that makes it practically impossible to override it. So, the solution "add resolve.fallback" is not straightforward in a real/new react application (and most workaround you can find on google just work for a limited amount of time, then people stop maintaining them).

In summary, the solution would be to make it possible to just link the relevant parts of the library without direct dependency on node's crypto and constants.

For additional context, I'm signing requests on the client side (react app) using two types of signers:

  1. ed25519 using https://github.com/paulmillr/noble-curves
  2. webauthn, ed25519 or es256
dhensby commented 10 months ago

I would be prepared to support client-side, yes - but I don't have a real-world use-case for it, so I wouldn't be putting it through any battle hardening, which can be fairly important for reliable libraries.

As I understand it, the 2 areas that would need to be addressed would be:

  1. Decoupling crypto engine so that there is no reliance on node's crypto package - that is kind of done, in that a consumer can provide any signing/verifying function they like (see recent update to README).
  2. Remove explicit dependency on Buffer - either by relying on Uint8Array internally instead, or allowing strings to be passed around as well as buffers.

In my opinion, there is little value in using this package on the client side for signing because the secret signing key will have to be available to the browser - possibly verifying is also not very useful because if the client is communicating with a compromised server (that has been able to break SSL, etc, then they can just turn off the message signing and even just change the secret, etc).

But, I can see from #156 that there is value in being able to use it in stripped down JS environments that may not have access to NodeJS packages, but they do have access to the web APIs.

dhensby commented 10 months ago

In summary, the solution would be to make it possible to just link the relevant parts of the library without direct dependency on node's crypto and constants.

Right - I think I see the problem. We have the algorithm/index.ts file which imports crypto (node library) and that gets exported so that consumers have access to the "built-in" crypto helpers, which means that (without tree-shaking, I suppose) it'll break for any non-nodejs environment.

0x0ece commented 10 months ago

Yes, you got all the points.

As for the keys, we use webauthn so the key is actually stored in a separated device, whether it's a hardware security key or a mobile phone (passkey). Webauthn requires user interaction for every signature, therefore for better ux we: 1) generate an ephemeral ed25519 key in the browser, 2) issue a signed-with-webauthn api request to register this session cred, 3) use this cred to sign all following api requests. The server only ever stores public keys.

The custom signer is great, and I don't see many issues managing keys and signatures. The big advantage for me personally is to use your lib for all the serialization part, as the standard is pretty complex :)

dhensby commented 10 months ago

Would you be prepared to provide a PR for a side-by-side node/browser implementation/build?

0x0ece commented 10 months ago

I gave it a try but I got stuck :/

First a good news, changing the types from Buffer -> Uint8Array is kind of smooth, at least algorithm/index.ts seems to remain valid because Buffer implements Uint8Array. (assuming algorithm will only be used/available for node).

It remains to replace Buffer.from in various places, essentially handling conversions from string and/or base64. For string, for example, the common way is new TextEncoder().encode(str), however it appears that TextEncoder is only available in node 16+, and if I read it correctly you're supporting 12+? Unfortunately I don't know enough about node to say what's the best way to replace Buffer and/or what's the ideal minimum version to support.

0x0ece commented 10 months ago

Actually I spoke too early, it seems I was able to do the trick: https://github.com/dhensby/node-http-message-signatures/pull/160.

Assuming the change Buffer -> Uint8Array is good (or anyway we can discuss any additional tweak/fix in the PR), the other thing that remains to do is to "remove" algorithms. I'm not entirely sure how to do that.

In my tests I simply commented out algorithms and so far it's compiling. Next I'll do some additional tests to verify I can properly sign on the client.