[mmorales-post]

PR checklist

• An issue/feature request has been created for this PR
• Pull Request title clearly describes the work in the pull request and the Pull Request description provides details about how to validate the work
• File the PR against the master branch
• The code in this PR is covered by unit tests

Link to issue/feature request: #105

Description

The JWE key wrap asked node's crypto.publicEncrypt for oaepHash: "sha256". node
honours that; the crypto-browserify polyfill silently ignores it and wraps with SHA-1,
producing a token whose header claims RSA-OAEP-256. It fails only at the recipient.

node callers are unaffected. The native path is retained and selected by a one-time
probe. Measured 0.055 ms against 0.050 ms on main for encryptData, within noise.

The probe. node validates the digest name and throws on an unrecognised one; a
polyfill that ignores oaepHash accepts anything. Attempting a wrap under a deliberately
bogus digest therefore distinguishes them, costs one call, and is memoised. It also treats
an absent or stubbed crypto module as unsupported, which is what
resolve.fallback.crypto = false in this repo's webpack.config.js produces.

The fallback goes through node-forge with SHA-256 set for the OAEP digest, which
forge otherwise defaults to SHA-1. MGF1 follows that digest when unset; it is set
explicitly anyway to make the intent clear. The helper mirrors createOAEPOptions
in field-level-crypto.js. Forge keys are parsed lazily, so node callers never parse them.

decryptData gets the same treatment, so a browser build can read responses as well as
send requests.

Scope. This fixes the digest, which is the failure that is silent. It does not make
the library work under resolve.fallback.crypto = false, because randomBytes,
createCipheriv and createDecipheriv still require the module. Under a bundler that
polyfills crypto the full JWE path now works. Happy to raise the remaining piece
separately if that is useful.

Validation

Three tests added to test/jwe-crypto.test.js:

• still wraps with SHA-256 when the runtime ignores oaepHash stubs publicEncrypt to
  behave like the polyfill, then asserts the emitted key unwraps with SHA-256 and not
  SHA-1. Fails on main.
• detects whether the runtime honours oaepHash covers the probe against real node, a
  polyfill, and an empty module. Fails on main.
• wraps the content encryption key with RSA-OAEP-256, not SHA-1 locks the digest for the
  native path. This one passes on main as well, by design: under node the old code was
  already correct, which is why the bug survived. It is there to stop a regression.

Cross-checked that forge and native interoperate in both directions on the same key, and
that a SHA-1 unwrap of the forge output is rejected.

Full suite: 200 passing. eslint: 0 errors, 5 warnings, unchanged from main.
Validated on Node 17, 22 and 24. Probe error code verified on node 13 through 24 and 26.

[talha-API]

Thanks for raising this PR and issue @mmorales-post and apologies for the delay in the response.

The changes are sound, just a few points to address. Please also update the PR with latest changes from main (if not done already) and add any relevant tests for the comments provided.

[talha-API]

Thanks for addressing the comments @mmorales-post , please see additional minor comments and I request you to provide some further context on the points below:

1. Which runtime did you run into this issue in? Since the issue specifiedcrypto-browserify and public-encrypt, I assume a browser bundle. If not, please share the runtime and the bundler/versions.
2. As you noted in the scope section, this change targets only the digest issue. A client must still polyfill process.version (≥ 13), fs, Buffer, crypto etc. How does your setup provide them?
3. I have tested the changes and they do not introduce any regressions, but please share evidence from the setup where you found this (browser/postman/insomnia, error logs etc) It will help us review and assess the need for JWE support outside native Node.

[mmorales-post]

Thanks @talha-API. Comment fixes are pushed. On your three questions:

1. Runtime. The Postman script runtime, which runs the same way in the Postman app, the Postman CLI and Postman monitors. It isn't a browser, but it has the same constraints: no require('crypto'), no fs, no process, no globalThis, no window. The only crypto primitive is crypto.getRandomValues. The library has to arrive as a pre-built bundle that the script evals, which is what Mastercard's own postman-encryption-lib does (browserify 17).

The bundle I validated with: webpack 5.112.2, target: "web", crypto-browserify 3.12.1 (public-encrypt 4.0.3), buffer 6.0.3, stream-browserify 3.0.0, node-forge 1.4.0.

2. The other polyfills.

Need	Provided by
crypto	resolve.fallback to crypto-browserify
fs.readFileSync (cert path)	alias to an in-memory virtual fs serving the PEM
process.version	ProvidePlugin stub with version only. Leave out versions.node, or forge decides it's on node and calls crypto.randomBytes, which forge's own browser field stubs out
randombytes	alias to a shim over crypto.getRandomValues
stream (create-hash in the publicEncrypt chain)	stream-browserify
Buffer	buffer, via ProvidePlugin
window	window = { crypto } before the eval. forge's globalScope needs window or self off node. postman-encryption-lib does the same, window = {}; // required for postman in src/index.js

None of that needs a change to this library. The digest is the only part a polyfill can't fix: the polyfill accepts oaepHash and doesn't use it, so nothing errors at encryption time.

3. Evidence.

a) The same bundle built from main and from this branch, run in a Node vm context stripped to match the Postman script runtime (no require, process, Buffer, globalThis), encrypting with test/res/test_certificate.cert, then unwrapping the JWE encrypted_key with native node and test_key.pem:

main header={"kid":"761b003c...","cty":"application/json","alg":"RSA-OAEP-256","enc":"A256GCM"}
     sha256: ERR_OSSL_RSA_OAEP_DECODING_ERROR | sha1: unwraps
pr   header={"kid":"761b003c...","cty":"application/json","alg":"RSA-OAEP-256","enc":"A256GCM"}
     sha256: unwraps | sha1: ERR_OSSL_RSA_OAEP_DECODING_ERROR

On main, the header says RSA-OAEP-256 and the key only unwraps with SHA-1. Happy to push the harness (webpack config, shims, runner) to a gist if that helps you reproduce it.

b) I also confirmed it end to end against Mastercard's sandbox environment: the SHA-1-wrapped token from main was rejected, and the same request with a SHA-256 wrap was accepted. Happy to share the details privately if that's useful.

c) In the same bundle, in the same Postman script runtime, the field-level path is already correct, because field-level-crypto.js goes through forge with an explicit digest. Only the JWE path depends on native crypto.

On the need for JWE outside native node: postman-encryption-lib uses this library for field-level encryption but brings in node-jose for JWE, so Postman users today run two implementations of the scheme. With this fix one library covers both, which is the outcome we want on our side as well.

[talha-API]

Thanks for the detailed reply on my queries, it is consistent with what I saw during review and is correct for client built bundles.

On your last point on the postman-encryption-lib, as you already know this change alone doesn't make it use this library's JWE. That would need a change in that library, swapping node-jose for JweEncryption and a few more like shimming fs.readFileSync (as you did in your custom bundle) and process.version in its build.

Have you tried using these updates through the postman-encryption-lib's own build? Did you face any other issues? And are you planning on raising a PR there with the required changes?

[mmorales-post]

Thanks @talha-API. No, I haven't run these changes through postman-encryption-lib's build. The evidence above is from my own webpack bundle.

I did check its current release for the same problem, and it's fine. Its jwe.js encrypts through node-jose with an explicit RSA-OAEP-256, so it never reaches the digest path this PR changes. I ran the v1.0.5 release bundle in the same stripped runtime, calling encryptRequest("jwe", pm), and the key unwraps with SHA-256. No other issues there.

Moving it onto JweEncryption would be a consolidation, not a fix: swap node-jose, then shim fs.readFileSync and process.version in the browserify build, as you said. My line yesterday about one library covering both overstated where things are. It's where I'd like this to end up, not something this PR does.

I can look into it and open a PR there once this one is merged, if your team wants it. I'd rather keep it out of this PR's scope.

Is anything still open here? If not, could you approve?

[talha-API]

Thanks for clarifying on your point! The change is valid for the specific OAEP issue for custom bundles so we can take up the postman library changes separately.

If you intend on raising the PR for it, do let us know. Otherwise we will plan and update ourselves.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud