Fix a missing check in encryption for encrypt call (#64)

* Fix typo in readme and tests for for insecure algorithm options * Fix a missing check in encryption for encrypt call Fix a callback to match callback error signature Add additional tests Fix README and test typos
auth0 · Jan 29, 2020 · 9459c5a · 9459c5a
1 parent 4625cc3
commit 9459c5a
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 16 deletions.
diff --git a/README.md b/README.md
@@ -18,7 +18,7 @@ var options = {
   pem: fs.readFileSync(__dirname + '/your_public_cert.pem'),
   encryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
   keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p',
-  disallowInsecureEncryptionAlgorithm: true
+  disallowEncryptionWithInsecureAlgorithm: true
 };
 
 xmlenc.encrypt('content to encrypt', options, function(err, result) {
@@ -54,7 +54,7 @@ Result:
 ~~~js
 var options = {
     key: fs.readFileSync(__dirname + '/your_private_key.key'),
-    disallowInsecureDecryptionAlgorithm: true;
+    disallowDecryptionWithInsecureAlgorithm: true;
 };
 
 xmlenc.decrypt('<xenc:EncryptedData ..... </xenc:EncryptedData>', options, function(err, result) {
@@ -79,7 +79,7 @@ Currently the library supports:
   * http://www.w3.org/2001/04/xmlenc#aes256-cbc
   * http://www.w3.org/2001/04/xmlenc#tripledes-cbc (Insecure Algorithm)
 
-Insecure Algorithms can be disabled via disallowInsecureEncryptionAlgorithm/disallowInsecureDecryptionAlgorithm flags when encrypting/decrypting. This flag is off by default in 0.x versions.
+Insecure Algorithms can be disabled via disallowEncryptionWithInsecureAlgorithm/disallowDecryptionWithInsecureAlgorithm flags when encrypting/decrypting. This flag is off by default in 0.x versions.
 
 However, you can fork and implement your own algorithm. The code supports adding more algorithms easily
 

diff --git a/lib/xmlenc.js b/lib/xmlenc.js
@@ -64,8 +64,9 @@ function encrypt(content, options, callback) {
   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) {
-    return callback(new Error('encryption algorithm ' + options.keyEncryptionAlgorithm + 'is not secure'));
+    && (insecureAlgorithms.indexOf(options.keyEncryptionAlgorithm) >= 0
+      || insecureAlgorithms.indexOf(options.encryptionAlgorithm) >= 0)) {
+    return callback(new Error('encryption algorithm ' + options.keyEncryptionAlgorithm + ' is not secure'));
   }
   options.input_encoding = options.input_encoding || 'utf8';
 
@@ -164,7 +165,7 @@ function decrypt(xml, options, callback) {
 
     if (options.disallowDecryptionWithInsecureAlgorithm
       && insecureAlgorithms.indexOf(encryptionAlgorithm) >= 0) {
-      throw new Error('encryption algorithm ' + encryptionAlgorithm + ' is not secure, fail to decrypt');
+      return callback(new Error('encryption algorithm ' + encryptionAlgorithm + ' is not secure, fail to decrypt'));
     }
     var encryptedContent = xpath.select("//*[local-name(.)='EncryptedData']/*[local-name(.)='CipherData']/*[local-name(.)='CipherValue']", doc)[0];
 

diff --git a/test/xmlenc.encryptedkey.js b/test/xmlenc.encryptedkey.js
@@ -56,37 +56,75 @@ describe('encrypt', function() {
   }
 
   describe('des-ede3-cbc fails', function() {
-    it('should fail encryption when disallowInsecureEncryptionAlgorithm is set', function(done) {
+    it('should fail encryption when disallowEncryptionWithInsecureAlgorithm is set', 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'),
-      disallowInsecureEncryptionAlgorithm: true,
-      encryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#aes128-cbc',
+      disallowEncryptionWithInsecureAlgorithm: true,
+      encryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#tripledes-cbc',
       keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p'
     }
-      //options.encryptionAlgorithm = 'http://www.w3.org/2001/04/xmlenc#tripledes-cbc',
-      //options.keyEncryptionAlgorithm = 'http://www.w3.org/2001/04/xmlenc#rsa-1_5';
       xmlenc.encrypt('encrypt me', options, function(err, result) {
         assert(err);
+        assert(!result);
         done();
       });
     });
 
-    it('should fail decryption when disallowInsecureDecryptionAlgorithm is set', function(done) {
+    it('should fail decryption when disallowDecryptionWithInsecureAlgorithm is set', 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#aes128-cbc',
+      encryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#tripledes-cbc',
       keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p'
       }
       xmlenc.encrypt('encrypt me', options, function(err, result) {
         xmlenc.decrypt(result,
           { key: fs.readFileSync(__dirname + '/test-auth0.key'),
-            disallowInsecureDecryptionAlgorithm: true},
+            disallowDecryptionWithInsecureAlgorithm: true},
+          function (err, decrypted) {
+            assert(err);
+            assert(!decrypted);
+            done();
+        });
+      });
+    });
+  });
+
+  describe('rsa-1.5 fails', function() {
+    it('should fail encryption when disallowEncryptionWithInsecureAlgorithm is set', 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'),
+      disallowEncryptionWithInsecureAlgorithm: true,
+      encryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#aes256-cbc',
+      keyEncryptionAlgorithm: 'http://www.w3.org/2001/04/xmlenc#rsa-1_5'
+    }
+      xmlenc.encrypt('encrypt me', options, function(err, result) {
+        assert(err);
+        assert(!result);
+        done();
+      });
+    });
+
+    it('should fail decryption when disallowDecryptionWithInsecureAlgorithm is set', 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-1_5'
+      }
+      xmlenc.encrypt('encrypt me', options, function(err, result) {
+        xmlenc.decrypt(result,
+          { key: fs.readFileSync(__dirname + '/test-auth0.key'),
+            disallowDecryptionWithInsecureAlgorithm: true},
           function (err, decrypted) {
             assert(err);
+            assert(!decrypted);
             done();
         });
       });
@@ -133,12 +171,12 @@ describe('encrypt', function() {
     });
   });
 
-  it('should fail encrypt when disallowInsecureDecryptionAlgorithm is set', function (done) {
+  it('should fail encrypt when disallowEncryptionWithInsecureAlgorithm is set', function (done) {
     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',
-      disallowInsecureEncryptionAlgorithm: true
+      disallowEncryptionWithInsecureAlgorithm: true
     };
 
     var plaintext = 'The quick brown fox jumps over the lazy dog';