BREAKING CHANGE: Fix digest method algorithm id by adamjmcgrath · Pull Request #135 · auth0/node-xml-encryption

adamjmcgrath · 2026-06-30T12:41:32Z

Description

Replaces #118

http://www.w3.org/2000/09/xmldsig#sha256 and http://www.w3.org/2000/09/xmldsig#sha512 are not valid DigestMethod algorithm values, the correct DigestMethod algorithm identifier’s should be http://www.w3.org/2001/04/xmlenc#sha256 and http://www.w3.org/2001/04/xmlenc#sha512. (see XML Encryption Syntax and Processing Version 1.1 )

Update the decryption logic to support the new 2001/04/xmlenc identifiers for encryption and decryption and the existing 2000/09/xmldsig identifiers for decryption (for backwards compatibility)

This is a breaking change for people doing rsa-oaep with sha256 or sha512

References

https://www.w3.org/TR/xmlenc-core1/#sec-SHA256

Testing

This change adds test coverage for new/changed/fixed functionality

Checklist

I have added documentation for new/changed functionality in this PR or in auth0.com/docs
All active GitHub checks for tests, formatting, and security are passing
The correct base branch is being used, if not the default branch

shane-tomlinson · 2026-07-01T04:57:16Z

+    const digestUri = DIGEST_ALGORITHMS[keyEncryptionDigest] || keyEncryptionDigest;
+
+    // RSA-OAEP requires it. RSA-1.5 must NOT have it.
+    const isOAEP = keyEncryptionMethod && keyEncryptionMethod.includes('rsa-oaep');


Can you add a link to the relevant portion of the spec that says RSA-1.5 must not have a DigestMethod?

There isn't really a relevant portion of the spec, but I updated the comment 8bc66ed

Is that a breaking change ?

I don't know how anyone could rely on the DigestMethod when using RSA 1.5, but the shape of the XML is changing, so strictly speaking yes

shane-tomlinson · 2026-07-01T04:58:32Z

        oaepHash = 'sha256';
        break;
+      case 'http://www.w3.org/2001/04/xmlenc#sha512':
      case 'http://www.w3.org/2000/09/xmldsig#sha512':


Should we have a similar comment here about // backwards compatibility for previous wrong usage?

Added a comment 8bc66ed

shane-tomlinson · 2026-07-01T05:26:38Z

+    const digestUri = DIGEST_ALGORITHMS[keyEncryptionDigest] || keyEncryptionDigest;
+
+    // RSA-OAEP requires it. RSA-1.5 must NOT have it.
+    const isOAEP = keyEncryptionMethod && keyEncryptionMethod.includes('rsa-oaep');


Is there a previous check on whether the key encryption method is on an allow list? The check here is pretty lax in that rsa-oaep can appear anywhere within the string.

There's one here https://github.com/auth0/node-xml-encryption/blob/master/lib/xmlenc.js#L56-L66

shane-tomlinson · 2026-07-01T05:27:22Z

+var xmlenc = require('../lib');
+var keyinfoTemplate = require('../lib/templates/keyinfo.tpl.xml');
+
+var RSA_OAEP = 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p';


Should we try both http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p and http://www.w3.org/2001/04/xmlenc#rsa-oaep?

I believe there's a http://www.w3.org/2009/xmlenc11#rsa-oaep but we don't support that https://github.com/auth0/node-xml-encryption/blob/master/lib/xmlenc.js#L56-L66

shane-tomlinson · 2026-07-01T05:29:13Z

+      assert(xml.includes('<DigestMethod Algorithm="' + custom + '" />'));
+    });
+
+    it('includes a DigestMethod for RSA-OAEP', function () {


All the previous tests are implicitly RSA-OAEP, make sense to ditch this test and add an explicit keyEncryptionMethod to those?

ditched 👍 8bc66ed

shane-tomlinson · 2026-07-01T05:29:49Z

+    }
+
+    ['sha1', 'sha256', 'sha512'].forEach(function (digest) {
+      it('round trips with ' + digest, function (done) {


Would this test have failed prior to this fix?

No, but it seems like a reasonable regression test to have

shane-tomlinson · 2026-07-01T05:30:54Z

+      });
+    });
+
+    it('emits the correct xmlenc URI in the produced XML for sha256', function (done) {


I don't see similar tests for sha1/sha512?

Added tests for sha1 and sha512 8bc66ed

shane-tomlinson · 2026-07-01T05:32:56Z


-module.exports = ({ encryptionPublicCert, encryptedKey, keyEncryptionMethod, keyEncryptionDigest }) => `
+const DIGEST_ALGORITHMS = {
+    'sha1': 'http://www.w3.org/2000/09/xmldsig#sha1',


Maybe add a comment about why sha1 uses xmldsig and the others use xmlenc within the URI.

Added a comment 8bc66ed

julienwoll · 2026-07-01T18:44:39Z

    switch (keyDigestMethodAlgorithm) {
-      case 'http://www.w3.org/2000/09/xmldsig#sha256':
+      case 'http://www.w3.org/2001/04/xmlenc#sha256':
+      case 'http://www.w3.org/2000/09/xmldsig#sha256': // backwards compatibility for previous wrong usage


Is this what we want ?

github-actions · 2026-07-02T15:00:32Z

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

adamjmcgrath requested a review from a team as a code owner June 30, 2026 12:41

adamjmcgrath mentioned this pull request Jun 30, 2026

fix digest support #118

Closed

4 tasks

elbuo8 and others added 2 commits June 30, 2026 13:43

fix!: digest support

cd68a98

chore: add tests

7f68d30

adamjmcgrath force-pushed the digest-method-alg branch from b9f4e4c to 7f68d30 Compare June 30, 2026 12:43

shane-tomlinson reviewed Jul 1, 2026

View reviewed changes

chore: comment and test updates

8bc66ed

adamjmcgrath force-pushed the digest-method-alg branch from 719ec21 to 8bc66ed Compare July 1, 2026 08:55

julienwoll reviewed Jul 1, 2026

View reviewed changes

julienwoll approved these changes Jul 2, 2026

View reviewed changes

shane-tomlinson approved these changes Jul 2, 2026

View reviewed changes

adamjmcgrath merged commit e226c22 into auth0:master Jul 2, 2026
4 checks passed

github-actions Bot added the released label Jul 2, 2026

Uh oh!

Conversation

adamjmcgrath commented Jun 30, 2026

Description

References

Testing

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamjmcgrath Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamjmcgrath Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamjmcgrath Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamjmcgrath Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamjmcgrath Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adamjmcgrath Jul 1, 2026 •

edited

Loading

adamjmcgrath Jul 1, 2026 •

edited

Loading

adamjmcgrath Jul 1, 2026 •

edited

Loading

adamjmcgrath Jul 1, 2026 •

edited

Loading

adamjmcgrath Jul 1, 2026 •

edited

Loading