Using native crypto module

[nikeee]

Node offers a native crypto module that provides sha256 and HMAC. Is it possible to use this instead of the hand-rolled JS version?

[BitPatty]

Yes, but that wouldn't work in the browser (which only supports WebCrypto). And the libraries I looked at back then either only support ESM or CJS but not both, or again only NodeJS or browser.

Technically SubtleCrypto could be used, but that makes it asynchronous which is rather inconvenient for this library.

Of course I'm open to suggestions.

[nikeee]

Pardon my ignorance, but what is the use case for signing URLs on the client? Why do any signing at all when the client has access to the secrets?

Would it be possible to offer some interface (or single signing function) that can be implemented by the stuff in the crypto part? That way, it would be possible to switch out the implementation with the one that node has.

[BitPatty]

Pardon my ignorance, but what is the use case for signing URLs on the client? Why do any signing at all when the client has access to the secrets?

The client consuming the URL must not necessarily be the client generating the URL. Then again, I'm in no position to judge what should be considered a valid use case and what shouldn't. This library is made for building imgproxy URL's and using signed URL's is a feature of imgproxy, so it's supported for the environments this library targets.

Would it be possible to offer some interface (or single signing function) that can be implemented by the stuff in the crypto part? That way, it would be possible to switch out the implementation with the one that node has.

This I would consider a good compromise :+1: Providing a callback parameter of some sort on the build function to specify the signing function to use and exporting template functions separately.

[nikeee]

Is this open for a PR with a proposed implementation? If so, I'd do one

[BitPatty]

Absolutely 👍

[nikeee]

Question: I think it's a good idea to provide the signing options when creating a new pb instance like this: https://github.com/BitPatty/imgproxy-url-builder/commit/0e9bae3395134fc331a129b04da90c207a79b1dd

This way, the lib user can create an instance and pass around the URL creation, so that one doesn't need the actual keys when building the URL in a different place. Basically, allowing something like this:

const signingOptions = {
  key: '73757065722d7365637265742d6b6579', // super-secret-key
  salt: '73757065722d7365637265742d73616c74', // super-secret-salt
};

pb(undefined, signingOptions).background({
  r: 255,
  g: 0,
  b: 0
}).build({
  path: 's3://mybucket/myimage.png',
});

If we add this, we can also add the signing functions there: https://github.com/BitPatty/imgproxy-url-builder/commit/a27eb972b05e9231e803418e08a999dc197f60ab

This would allow this:

const signingOptions = {
  key: '73757065722d7365637265742d6b6579', // super-secret-key
  salt: '73757065722d7365637265742d73616c74', // super-secret-salt
  crypto: {
      encodeFilePath: (path: string) => Buffer.from((path).toString("base64url"),
      generateSignature: () => ...,
  }
};

pb(undefined, signingOptions).background({
  r: 255,
  g: 0,
  b: 0
}).build({
  path: 's3://mybucket/myimage.png',
});

I'm not sure about this approach, since this still loads the ./crypto/common.js module even if it is not needed. Also, I think it doesn't play that well with the object-oriented approach of this lib. Do you have better suggestion?

[BitPatty]

Sorry for the late response.

I think in this case it would be better to just not have an automatic fallback to the current implementation (ref https://github.com/BitPatty/imgproxy-url-builder/commit/a27eb972b05e9231e803418e08a999dc197f60ab#diff-02a29244b206e24420e4ab859666106afebd6339aecfd27f83275ae1418dc486R172).

i.e. the signature algorithms should always specified together with the credentials (as in your example). Default implementations could still be loaded separately from the library.

E.g. something like this:

import { nodeCrypto as crypto } from '<package>';

or

import crypto from '<package>/crypto/node';

with

const signingOptions = {
  key: '73757065722d7365637265742d6b6579', // super-secret-key
  salt: '73757065722d7365637265742d73616c74', // super-secret-salt
+ crypto // <-- this being a required property
- crypto: {
-     encodeFilePath: (path: string) => Buffer.from((path).toString("base64url"),
-     generateSignature: () => ...,
- }
};

This way you can choose the algorithm for your platform but still override if necessary.

[nikeee]

Thanks for you feedback! I tried not to introduce a breaking change. Let me do another draft.