feat: moving AES-128-CBC AES-256-CBC to the list of insecure algorithms

BREAKING CHANGE: CBC algorithms are now marked insecure

Author: julienwoll

README.md

@@ -78,15 +78,32 @@ Currently the library supports:
   * http://www.w3.org/2001/04/xmlenc#rsa-1_5 (Insecure Algorithm)
 
 * EncryptedData using:
-  * http://www.w3.org/2001/04/xmlenc#aes128-cbc
-  * http://www.w3.org/2001/04/xmlenc#aes256-cbc
+  * http://www.w3.org/2001/04/xmlenc#aes128-cbc (Insecure Algorithm)
+  * http://www.w3.org/2001/04/xmlenc#aes256-cbc (Insecure Algorithm)
   * http://www.w3.org/2009/xmlenc11#aes128-gcm
   * http://www.w3.org/2009/xmlenc11#aes256-gcm
   * http://www.w3.org/2001/04/xmlenc#tripledes-cbc (Insecure Algorithm)
 
-Insecure Algorithms can be disabled via `disallowEncryptionWithInsecureAlgorithm`/`disallowDecryptionWithInsecureAlgorithm` flags when encrypting/decrypting. This flag is off by default in 0.x versions.
+Insecure Algorithms can be used via `disallowEncryptionWithInsecureAlgorithm`/`disallowDecryptionWithInsecureAlgorithm` flags when encrypting/decrypting by setting them to false. In version 4.0 onwards, these flags are true by default (forbidding insecure algorithms).
 
-A warning will be piped to `stderr` using console.warn() by default when the aforementioned algorithms are used. This can be disabled via the `warnInsecureAlgorithm` flag.
+A warning will be piped to `stderr` using console.warn() by default when the aforementioned algorithms are used and above mentioned flags are false. This can be disabled via the `warnInsecureAlgorithm` flag.
+
+We recommend usage of AES-256-GCM (Galois/Counter Mode) for the strongest security posture and to align with current industry best practices.
+
+Note that `xml-encryption` versions prior to 4.0 supported AES-128-CBC and AES-256-CBC as secure algorithms. In version 4.0 onwards, these are treated as insecure because they use the Cipher Block Chaining (CBC) mode of encryption, which does not provide integrity guarantees. To continue using AES128-CBC and AES256-CBC, enable support for insecure algorithms via `disallowEncryptionWithInsecureAlgorithm/disallowDecryptionWithInsecureAlgorithm`.
+
+### Allow listing specific algorithms when decrypting
+
+If decrypting with `disallowEncryptionWithInsecureAlgorithm: true`, you may wish to only support a subset of insecure algorithms (for example, supporting AES-256-CBC only). This can be achieved by extracting the encryption algorithm using the following code and applying validation as required.
+
+~~~js
+const xmldom  = require('@xmldom/xmldom');
+const xpath   = require('xpath');
+
+const doc = new xmldom.DOMParser().parseFromString(xmlString);
+const encryptionMethod = xpath.select("//*[local-name(.)='EncryptedData']/*[local-name(.)='EncryptionMethod']", doc)[0];
+const encryptionAlgorithm = encryptionMethod.getAttribute('Algorithm');
+~~~
 
 ## Issue Reporting
 

lib/xmlenc.js

@@ -7,7 +7,11 @@ const insecureAlgorithms = [
   //https://www.w3.org/TR/xmlenc-core1/#rsav15note
   'http://www.w3.org/2001/04/xmlenc#rsa-1_5',
   //https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
-  'http://www.w3.org/2001/04/xmlenc#tripledes-cbc'];
+  'http://www.w3.org/2001/04/xmlenc#tripledes-cbc',
+  //https://www.w3.org/TR/xmlenc-core1/#sec-edata-attacks
+  'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
+  'http://www.w3.org/2001/04/xmlenc#aes128-cbc',
+];
 
 function encryptKeyInfoWithScheme(symmetricKey, options, padding, callback) {
   const symmetricKeyBuffer = Buffer.isBuffer(symmetricKey) ? symmetricKey : Buffer.from(symmetricKey, 'utf-8');
@@ -44,7 +48,7 @@ function encryptKeyInfo(symmetricKey, options, callback) {
 
   if (!options.keyEncryptionAlgorithm)
     return callback(new Error('encryption without encrypted key is not supported yet'));
-  if (options.disallowEncryptionWithInsecureAlgorithm
+  if (options.disallowEncryptionWithInsecureAlgorithm !== false 
     && insecureAlgorithms.indexOf(options.keyEncryptionAlgorithm) >= 0) {
     return callback(new Error('encryption algorithm ' + options.keyEncryptionAlgorithm + 'is not secure'));
   }
@@ -71,19 +75,24 @@ function encrypt(content, options, callback) {
     return callback(new Error('rsa_pub option is mandatory and you should provide a valid RSA public key'));
   if (!options.pem)
     return callback(new Error('pem option is mandatory and you should provide a valid x509 certificate encoded as PEM'));
-  if (options.disallowEncryptionWithInsecureAlgorithm
-    && (insecureAlgorithms.indexOf(options.keyEncryptionAlgorithm) >= 0
-      || insecureAlgorithms.indexOf(options.encryptionAlgorithm) >= 0)) {
-    return callback(new Error('encryption algorithm ' + options.keyEncryptionAlgorithm + ' is not secure'));
+  if (options.disallowEncryptionWithInsecureAlgorithm !== false) {
+    if (insecureAlgorithms.indexOf(options.keyEncryptionAlgorithm) >= 0) {
+      return callback(new Error('encryption algorithm ' + options.keyEncryptionAlgorithm + ' is not secure'));
+    }
+    if (insecureAlgorithms.indexOf(options.encryptionAlgorithm) >= 0) {
+      return callback(new Error('encryption algorithm ' + options.encryptionAlgorithm + ' is not secure'));
+    }
   }
   options.input_encoding = options.input_encoding || 'utf8';
 
   function generate_symmetric_key(cb) {
     switch (options.encryptionAlgorithm) {
       case 'http://www.w3.org/2001/04/xmlenc#aes128-cbc':
+        utils.warnInsecureAlgorithm(options.encryptionAlgorithm, options.warnInsecureAlgorithm);
         crypto.randomBytes(16, cb); // generate a symmetric random key 16 bytes length
         break;
       case 'http://www.w3.org/2001/04/xmlenc#aes256-cbc':
+        utils.warnInsecureAlgorithm(options.encryptionAlgorithm, options.warnInsecureAlgorithm);
         crypto.randomBytes(32, cb); // generate a symmetric random key 32 bytes length
         break;
       case 'http://www.w3.org/2009/xmlenc11#aes128-gcm':
@@ -104,12 +113,14 @@ function encrypt(content, options, callback) {
   function encrypt_content(symmetricKey, cb) {
     switch (options.encryptionAlgorithm) {
       case 'http://www.w3.org/2001/04/xmlenc#aes128-cbc':
+        utils.warnInsecureAlgorithm(options.encryptionAlgorithm, options.warnInsecureAlgorithm);
         encryptWithAlgorithm('aes-128-cbc', symmetricKey, 16, content, options.input_encoding, function (err, encryptedContent) {
           if (err) return cb(err);
           cb(null, encryptedContent);
         });
         break;
       case 'http://www.w3.org/2001/04/xmlenc#aes256-cbc':
+        utils.warnInsecureAlgorithm(options.encryptionAlgorithm, options.warnInsecureAlgorithm);
         encryptWithAlgorithm('aes-256-cbc', symmetricKey, 16, content, options.input_encoding, function (err, encryptedContent) {
           if (err) return cb(err);
           cb(null, encryptedContent);
@@ -190,7 +201,7 @@ function decrypt(xml, options, callback) {
     var encryptionMethod = xpath.select("//*[local-name(.)='EncryptedData']/*[local-name(.)='EncryptionMethod']", doc)[0];
     var encryptionAlgorithm = encryptionMethod.getAttribute('Algorithm');
 
-    if (options.disallowDecryptionWithInsecureAlgorithm
+    if (options.disallowDecryptionWithInsecureAlgorithm !== false 
       && insecureAlgorithms.indexOf(encryptionAlgorithm) >= 0) {
       return callback(new Error('encryption algorithm ' + encryptionAlgorithm + ' is not secure, fail to decrypt'));
     }
@@ -199,8 +210,10 @@ function decrypt(xml, options, callback) {
     var encrypted = Buffer.from(encryptedContent.textContent, 'base64');
     switch (encryptionAlgorithm) {
       case 'http://www.w3.org/2001/04/xmlenc#aes128-cbc':
+        utils.warnInsecureAlgorithm(encryptionAlgorithm, options.warnInsecureAlgorithm);
         return callback(null, decryptWithAlgorithm('aes-128-cbc', symmetricKey, 16, encrypted));
       case 'http://www.w3.org/2001/04/xmlenc#aes256-cbc':
+        utils.warnInsecureAlgorithm(encryptionAlgorithm, options.warnInsecureAlgorithm);
         return callback(null, decryptWithAlgorithm('aes-256-cbc', symmetricKey, 16, encrypted));
       case 'http://www.w3.org/2001/04/xmlenc#tripledes-cbc':
         utils.warnInsecureAlgorithm(encryptionAlgorithm, options.warnInsecureAlgorithm);
@@ -252,7 +265,7 @@ function decryptKeyInfo(doc, options) {
   }
 
   var keyEncryptionAlgorithm = keyEncryptionMethod.getAttribute('Algorithm');
-  if (options.disallowDecryptionWithInsecureAlgorithm
+  if (options.disallowDecryptionWithInsecureAlgorithm !== false 
     && insecureAlgorithms.indexOf(keyEncryptionAlgorithm) >= 0) {
     throw new Error('encryption algorithm ' + keyEncryptionAlgorithm + ' is not secure, fail to decrypt');
   }

test/xmlenc.encryptedkey.js

@@ -56,7 +56,7 @@ describe('encrypt', function() {
     name: 'des-ede3-cbc',
     encryptionOptions: {
       encryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#tripledes-cbc',
-      keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-1_5'
+      keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p'
     }
   }];
 
@@ -83,8 +83,15 @@ describe('encrypt', function() {
     options.key = fs.readFileSync(__dirname + '/test-auth0.key'),
     options.warnInsecureAlgorithm = false;
 
+    const disallowInsecureAlgorithm = ![
+      'http://www.w3.org/2001/04/xmlenc#tripledes-cbc',
+      'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
+      'http://www.w3.org/2001/04/xmlenc#aes128-cbc'
+    ].includes(options?.encryptionAlgorithm);
+    options.disallowEncryptionWithInsecureAlgorithm = disallowInsecureAlgorithm;
+
     xmlenc.encrypt(content, options, function(err, result) {
-      xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-auth0.key'), warnInsecureAlgorithm: false}, function (err, decrypted) {
+      xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-auth0.key'), warnInsecureAlgorithm: false, disallowDecryptionWithInsecureAlgorithm: disallowInsecureAlgorithm }, function (err, decrypted) {
         if (err) return done(err);
         assert.equal(decrypted, content);
         done();
@@ -93,7 +100,7 @@ describe('encrypt', function() {
   }
 
   describe('des-ede3-cbc fails', function() {
-    it('should fail encryption when disallowEncryptionWithInsecureAlgorithm is set', function(done) {
+    it('should fail encryption when disallowEncryptionWithInsecureAlgorithm is set to true', function(done) {
       const options = {
       rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'),
       pem: fs.readFileSync(__dirname + '/test-auth0.pem'),
@@ -133,7 +140,7 @@ describe('encrypt', function() {
   });
 
   describe('rsa-1.5 fails', function() {
-    it('should fail encryption when disallowEncryptionWithInsecureAlgorithm is set', function(done) {
+    it('should fail encryption when disallowEncryptionWithInsecureAlgorithm is set to true', function(done) {
       const options = {
       rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'),
       pem: fs.readFileSync(__dirname + '/test-auth0.pem'),
@@ -195,7 +202,7 @@ describe('encrypt', function() {
 
   it('should decrypt xml with odd padding (aes256-cbc)', function (done) {
     var encryptedContent = fs.readFileSync(__dirname + '/test-cbc256-padding.xml').toString()
-    xmlenc.decrypt(encryptedContent, { key: fs.readFileSync(__dirname + '/test-auth0.key')}, function(err, decrypted) {
+    xmlenc.decrypt(encryptedContent, { key: fs.readFileSync(__dirname + '/test-auth0.key'), disallowDecryptionWithInsecureAlgorithm: false }, function(err, decrypted) {
       assert.ifError(err);
       assert.equal(decrypted, 'content');
       done();
@@ -210,7 +217,7 @@ describe('encrypt', function() {
     });
   });
 
-  it('should fail encrypt when disallowEncryptionWithInsecureAlgorithm is set', function (done) {
+  it('should fail encrypt when disallowEncryptionWithInsecureAlgorithm is set to true', function (done) {
     var options = {
       rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'),
       pem: fs.readFileSync(__dirname + '/test-auth0.pem'),
@@ -229,7 +236,8 @@ describe('encrypt', function() {
     var options = {
       rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'),
       pem: fs.readFileSync(__dirname + '/test-auth0.pem'),
-      keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-1_5'
+      keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-1_5',
+      disallowEncryptionWithInsecureAlgorithm: false,
     };
 
     var plaintext = 'The quick brown fox jumps over the lazy dog';
@@ -248,4 +256,73 @@ describe('encrypt', function() {
       done();
     });
   });
+
+  it('should fail decrypt due to insecure algorithm (aes256-cbc)', function (done) {
+    var encryptedContent = fs.readFileSync(__dirname + '/test-cbc256-padding.xml').toString()
+    xmlenc.decrypt(encryptedContent, { key: fs.readFileSync(__dirname + '/test-auth0.key'), disallowDecryptionWithInsecureAlgorithm: true }, function(err, decrypted) {
+      assert(err);
+      done();
+    });
+  });
+
+  it('should decrypt with insecure algorithm (aes256-cbc) with warning when disallowDecryptionWithInsecureAlgorithm is false', function (done) {
+    var encryptedContent = fs.readFileSync(__dirname + '/test-cbc256-padding.xml').toString()
+    xmlenc.decrypt(encryptedContent, { key: fs.readFileSync(__dirname + '/test-auth0.key'), disallowDecryptionWithInsecureAlgorithm: false, warnInsecureAlgorithm: true }, function(err, decrypted) {
+      assert.ifError(err);
+      assert.equal(consoleSpy.called, true);
+      assert.equal(decrypted, 'content');
+      done();
+    });
+  });
+
+  it('should decrypt with insecure algorithm (aes256-cbc) without warning when disallowDecryptionWithInsecureAlgorithm is false', function (done) {
+    var encryptedContent = fs.readFileSync(__dirname + '/test-cbc256-padding.xml').toString()
+    xmlenc.decrypt(encryptedContent, { key: fs.readFileSync(__dirname + '/test-auth0.key'), disallowDecryptionWithInsecureAlgorithm: false, warnInsecureAlgorithm: false }, function(err, decrypted) {
+      assert.ifError(err);
+      assert.equal(consoleSpy.called, false);
+      assert.equal(decrypted, 'content');
+      done();
+    });
+  });
+
+  it('should fail decrypt with insecure algorithm (aes256-cbc)', function (done) {
+    var encryptedContent = fs.readFileSync(__dirname + '/test-cbc256-padding.xml').toString()
+    xmlenc.decrypt(encryptedContent, { key: fs.readFileSync(__dirname + '/test-auth0.key') }, function(err, decrypted) {
+      assert(err);
+      done();
+    });
+  });
+
+  it('should fail encrypt due to insecure algorithm (aes256-cbc)', function (done) {
+    const options = {
+      rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'),
+      pem: fs.readFileSync(__dirname + '/test-auth0.pem'),
+      key: fs.readFileSync(__dirname + '/test-auth0.key'),
+      encryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
+      keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p',
+    }
+    xmlenc.encrypt('encrypt', options, function(err, result) {
+      assert(err);
+      assert(!result);
+      done();
+    });
+  });
+
+  it('should encrypt with insecure algorithm (aes256-cbc) when disallowEncryptionWithInsecureAlgorithm is false', function (done) {
+    const options = {
+      rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'),
+      pem: fs.readFileSync(__dirname + '/test-auth0.pem'),
+      key: fs.readFileSync(__dirname + '/test-auth0.key'),
+      encryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
+      keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p',
+      disallowEncryptionWithInsecureAlgorithm: false,
+      warnInsecureAlgorithm: true
+    }
+    xmlenc.encrypt('encrypt', options, function(err, result) {
+      assert.ifError(err);
+      assert.equal(consoleSpy.called, true);
+      assert(result);
+      done();
+    });
+  });
 });

test/xmlenc.integration.js

@@ -11,7 +11,7 @@ describe('integration', function() {
   it('should decrypt assertion with aes128', function (done) {
     var result = fs.readFileSync(__dirname + '/assertion-sha1-128.xml').toString();
 
-    xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-cbc128.key')}, function (err, decrypted) {
+    xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-cbc128.key'), disallowDecryptionWithInsecureAlgorithm: false }, function (err, decrypted) {
       // decrypted content should finish with <saml2:Assertion>
       assert.equal(/<\/saml2:Assertion>$/.test(decrypted), true);
       done();
@@ -22,7 +22,7 @@ describe('integration', function() {
     var encryptedContent = fs.readFileSync(__dirname + '/test-okta-enc-response.xml').toString()
     xmlenc.decrypt(
       encryptedContent,
-      {key: fs.readFileSync(__dirname + '/test-okta.pem')},
+      {key: fs.readFileSync(__dirname + '/test-okta.pem'), disallowDecryptionWithInsecureAlgorithm: false},
       (err, res) => {
         assert.ifError(err);