Merge pull request #217 from toberndo/cfb_fix

Refactoring cfb.encrypt, fix twofish cipher

src/crypto/cfb.js

@@ -52,9 +52,10 @@ module.exports = {
     var FRE = new Uint8Array(block_size);
 
     prefixrandom = prefixrandom + prefixrandom.charAt(block_size - 2) + prefixrandom.charAt(block_size - 1);
-    util.print_debug("prefixrandom:" + util.hexstrdump(prefixrandom));
+    var ciphertext = new Uint8Array(plaintext.length + 2 + block_size * 2);
-    var ciphertext = '';
+    var i, n, begin;
-    var i, n;
+    var offset = resync ? 0 : 2;
+
     // 1.  The feedback register (FR) is set to the IV, which is all zeros.
     for (i = 0; i < block_size; i++) {
       FR[i] = 0;
@@ -67,13 +68,11 @@ module.exports = {
     //     the plaintext to produce C[1] through C[BS], the first BS octets
     //     of ciphertext.
     for (i = 0; i < block_size; i++) {
-      ciphertext += String.fromCharCode(FRE[i] ^ prefixrandom.charCodeAt(i));
+      ciphertext[i] = FRE[i] ^ prefixrandom.charCodeAt(i);
     }
 
     // 4.  FR is loaded with C[1] through C[BS].
-    for (i = 0; i < block_size; i++) {
+    FR.set(ciphertext.subarray(0, block_size));
-      FR[i] = ciphertext.charCodeAt(i);
-    }
 
     // 5.  FR is encrypted to produce FRE, the encryption of the first BS
     //     octets of ciphertext.
@@ -82,82 +81,44 @@ module.exports = {
     // 6.  The left two octets of FRE get xored with the next two octets of
     //     data that were prefixed to the plaintext.  This produces C[BS+1]
     //     and C[BS+2], the next two octets of ciphertext.
-    ciphertext += String.fromCharCode(FRE[0] ^ prefixrandom.charCodeAt(block_size));
+    ciphertext[block_size] = FRE[0] ^ prefixrandom.charCodeAt(block_size);
-    ciphertext += String.fromCharCode(FRE[1] ^ prefixrandom.charCodeAt(block_size + 1));
+    ciphertext[block_size + 1] = FRE[1] ^ prefixrandom.charCodeAt(block_size + 1);
 
     if (resync) {
-      // 7.  (The resync step) FR is loaded with C3-C10.
+      // 7.  (The resync step) FR is loaded with C[3] through C[BS+2].
-      for (i = 0; i < block_size; i++) {
+      FR.set(ciphertext.subarray(2, block_size + 2));
-        FR[i] = ciphertext.charCodeAt(i + 2);
-      }
     } else {
-      for (i = 0; i < block_size; i++) {
+      FR.set(ciphertext.subarray(0, block_size));
-        FR[i] = ciphertext.charCodeAt(i);
-      }
     }
     // 8.  FR is encrypted to produce FRE.
     FRE = cipherfn.encrypt(FR);
 
-    if (resync) {
+    // 9.  FRE is xored with the first BS octets of the given plaintext, now
-      // 9.  FRE is xored with the first 8 octets of the given plaintext, now
+    //     that we have finished encrypting the BS+2 octets of prefixed
-      //     that we have finished encrypting the 10 octets of prefixed data.
+    //     data.  This produces C[BS+3] through C[BS+(BS+2)], the next BS
-      //     This produces C11-C18, the next 8 octets of ciphertext.
+    //     octets of ciphertext.
+    for (i = 0; i < block_size; i++) {
+      ciphertext[block_size + 2 + i] = FRE[i + offset] ^ plaintext.charCodeAt(i);
+    }
+    for (n = block_size; n < plaintext.length + offset; n += block_size) {
+      // 10. FR is loaded with C[BS+3] to C[BS + (BS+2)] (which is C11-C18 for
+      // an 8-octet block).
+      begin = n + 2 - offset;
+      FR.set(ciphertext.subarray(begin, begin + block_size));
+
+      // 11. FR is encrypted to produce FRE.
+      FRE = cipherfn.encrypt(FR);
+
+      // 12. FRE is xored with the next BS octets of plaintext, to produce
+      // the next BS octets of ciphertext.  These are loaded into FR, and
+      // the process is repeated until the plaintext is used up.
       for (i = 0; i < block_size; i++) {
-        ciphertext += String.fromCharCode(FRE[i] ^ plaintext.charCodeAt(i));
+        ciphertext[block_size + begin + i] = FRE[i] ^ plaintext.charCodeAt(n + i - offset);
       }
-      for (n = block_size + 2; n < plaintext.length; n += block_size) {
-        // 10. FR is loaded with C11-C18
-        for (i = 0; i < block_size; i++) {
-          FR[i] = ciphertext.charCodeAt(n + i);
-        }
-
-        // 11. FR is encrypted to produce FRE.
-        FRE = cipherfn.encrypt(FR);
-
-        // 12. FRE is xored with the next 8 octets of plaintext, to produce the
-        // next 8 octets of ciphertext.  These are loaded into FR and the
-        // process is repeated until the plaintext is used up.
-        for (i = 0; i < block_size; i++) {
-          ciphertext += String.fromCharCode(FRE[i] ^ plaintext.charCodeAt((n - 2) + i));
-        }
-      }
-    } else {
-      plaintext = "  " + plaintext;
-      // 9.  FRE is xored with the first 8 octets of the given plaintext, now
-      //     that we have finished encrypting the 10 octets of prefixed data.
-      //     This produces C11-C18, the next 8 octets of ciphertext.
-      for (i = 2; i < block_size; i++) {
-        ciphertext += String.fromCharCode(FRE[i] ^ plaintext.charCodeAt(i));
-      }
-
-      var tempCiphertext = ciphertext.substring(0, 2 * block_size);
-      var tempCiphertextString = ciphertext.substring(block_size);
-      var tempCiphertextBlock;
-      for (n = block_size; n < plaintext.length; n += block_size) {
-        // 10. FR is loaded with C11-C18
-        for (i = 0; i < block_size; i++) {
-          FR[i] = tempCiphertextString.charCodeAt(i);
-        }
-        tempCiphertextString = '';
-
-        // 11. FR is encrypted to produce FRE.
-        FRE = cipherfn.encrypt(FR);
-
-        // 12. FRE is xored with the next 8 octets of plaintext, to produce the
-        //     next 8 octets of ciphertext.  These are loaded into FR and the
-        //     process is repeated until the plaintext is used up.
-        for (i = 0; i < block_size; i++) {
-          tempCiphertextBlock = String.fromCharCode(FRE[i] ^ plaintext.charCodeAt(n + i));
-          tempCiphertext += tempCiphertextBlock;
-          tempCiphertextString += tempCiphertextBlock;
-        }
-      }
-      ciphertext = tempCiphertext;
     }
 
-    ciphertext = ciphertext.substring(0, plaintext.length + 2 + block_size);
+    ciphertext = ciphertext.subarray(0, plaintext.length + 2 + block_size);
-
+    return util.Uint8Array2str(ciphertext);
-    return ciphertext;
   },
 
   /**
@@ -233,9 +194,9 @@ module.exports = {
     ablock = cipherfn.encrypt(ablock);
 
     // test check octets
-    if (iblock[block_size - 2] != (ablock[0] ^ ciphertext.charCodeAt(block_size)) || iblock[block_size - 1] != (ablock[
+    if (iblock[block_size - 2] != (ablock[0] ^ ciphertext.charCodeAt(block_size)) ||
-      1] ^ ciphertext.charCodeAt(block_size + 1))) {
+        iblock[block_size - 1] != (ablock[1] ^ ciphertext.charCodeAt(block_size + 1))) {
-      throw new Error('Invalid data.');
+      throw new Error('CFB decrypt: invalid key');
     }
 
     /*  RFC4880: Tag 18 and Resync:

src/crypto/cipher/twofish.js

@@ -337,7 +337,7 @@ var util = require('../../util.js');
 // added by Recurity Labs
 
 function TFencrypt(block, key) {
-  var block_copy = [].concat(block);
+  var block_copy = toArray(block);
   var tf = createTwofish();
   tf.open(util.str2bin(key), 0);
   var result = tf.encrypt(block_copy, 0);
@@ -350,10 +350,19 @@ function TF(key) {
   this.tf.open(util.str2bin(key), 0);
 
   this.encrypt = function(block) {
-    return this.tf.encrypt([].concat(block), 0);
+    return this.tf.encrypt(toArray(block), 0);
   };
 }
 
+function toArray(typedArray) {
+  // Array.apply([], typedArray) does not work in PhantomJS 1.9
+  var result = [];
+  for (var i = 0; i < typedArray.length; i++) {
+    result[i] = typedArray[i];
+  }
+  return result;
+}
+
 
 module.exports = TF;
 module.exports.keySize = TF.prototype.keySize = 32;

src/util.js

@@ -159,29 +159,23 @@ module.exports = {
    */
   bin2str: function (bin) {
     var result = [];
-
     for (var i = 0; i < bin.length; i++) {
-      result.push(String.fromCharCode(bin[i]));
+      result[i] = String.fromCharCode(bin[i]);
     }
-
     return result.join('');
   },
 
-  _str2bin: function (str, result) {
-    for (var i = 0; i < str.length; i++) {
-      result[i] = str.charCodeAt(i);
-    }
-
-    return result;
-  },
-
   /**
    * Convert a string to an array of integers(0.255)
    * @param {String} str String to convert
    * @return {Array<Integer>} An array of (binary) integers
    */
   str2bin: function (str) {
-    return this._str2bin(str, new Array(str.length));
+    var result = [];
+    for (var i = 0; i < str.length; i++) {
+      result[i] = str.charCodeAt(i);
+    }
+    return result;
   },
 
 
@@ -191,7 +185,11 @@ module.exports = {
    * @return {Uint8Array} The array of (binary) integers
    */
   str2Uint8Array: function (str) {
-    return this._str2bin(str, new Uint8Array(new ArrayBuffer(str.length)));
+    var result = new Uint8Array(str.length);
+    for (var i = 0; i < str.length; i++) {
+      result[i] = str.charCodeAt(i);
+    }
+    return result;
   },
 
   /**
@@ -202,7 +200,11 @@ module.exports = {
    * @return {String} String representation of the array
    */
   Uint8Array2str: function (bin) {
-    return this.bin2str(bin);
+    var result = '';
+    for (var i = 0; i < bin.length; i++) {
+      result += String.fromCharCode(bin[i]);
+    }
+    return result;
   },
 
   /**

test/crypto/crypto.js

@@ -252,26 +252,36 @@ describe('API functional testing', function() {
   });
 
   describe('Encrypt and decrypt', function () {
-    var symmAlgo = "aes256"; // AES256
+    var symmAlgos = Object.keys(openpgp.enums.symmetric);
-    var symmKey = openpgp.crypto.generateSessionKey(symmAlgo);
+    symmAlgos = symmAlgos.filter(function(algo) {
-    var symmencDataOCFB = openpgp.crypto.cfb.encrypt(openpgp.crypto.getPrefixRandom(symmAlgo), symmAlgo, "foobarfoobar1234567890", symmKey, true);
+      return algo !== 'idea' && algo !== 'plaintext';
-    var symmencDataCFB  = openpgp.crypto.cfb.encrypt(openpgp.crypto.getPrefixRandom(symmAlgo), symmAlgo, "foobarfoobar1234567890", symmKey, false);
-
-    it("Symmetric with OpenPGP CFB resync", function (done) {
-      var text = openpgp.crypto.cfb.decrypt(symmAlgo,symmKey,symmencDataOCFB,true);
-
-      expect(text).to.equal("foobarfoobar1234567890");
-      done();
     });
 
-    it.skip("Symmetric without OpenPGP CFB resync", function (done) {
+    function testCFB(plaintext, resync) {
-      var text = openpgp.crypto.cfb.decrypt(symmAlgo,symmKey,symmencDataCFB,false);
+      symmAlgos.forEach(function(algo) {
+        var symmKey = openpgp.crypto.generateSessionKey(algo);
+        var symmencData = openpgp.crypto.cfb.encrypt(openpgp.crypto.getPrefixRandom(algo), algo, plaintext, symmKey, resync);
+        var text = openpgp.crypto.cfb.decrypt(algo, symmKey, symmencData, resync);
+        expect(text).to.equal(plaintext);
+      });
+    }
 
-      expect(text).to.equal("foobarfoobar1234567890");
+    it("Symmetric with OpenPGP CFB resync", function () {
-      done();
+      testCFB("hello", true);
+      testCFB("1234567", true);
+      testCFB("foobarfoobar1234567890", true);
+      testCFB("12345678901234567890123456789012345678901234567890", true);
+    });
+
+    it("Symmetric without OpenPGP CFB resync", function () {
+      testCFB("hello", false);
+      testCFB("1234567", false);
+      testCFB("foobarfoobar1234567890", false);
+      testCFB("12345678901234567890123456789012345678901234567890", false);
     });
 
     it('Asymmetric using RSA with eme_pkcs1 padding', function (done) {
+      var symmKey = openpgp.crypto.generateSessionKey('aes256');
       var RSAUnencryptedData = new openpgp.MPI();
       RSAUnencryptedData.fromBytes(openpgp.crypto.pkcs1.eme.encode(symmKey, RSApubMPIs[0].byteLength()));
       var RSAEncryptedData = openpgp.crypto.publicKeyEncrypt("rsa_encrypt_sign", RSApubMPIs, RSAUnencryptedData);
@@ -282,6 +292,7 @@ describe('API functional testing', function() {
     });
 
     it('Asymmetric using Elgamal with eme_pkcs1 padding', function (done) {
+      var symmKey = openpgp.crypto.generateSessionKey('aes256');
       var ElgamalUnencryptedData = new openpgp.MPI();
       ElgamalUnencryptedData.fromBytes(openpgp.crypto.pkcs1.eme.encode(symmKey, ElgamalpubMPIs[0].byteLength()));
       var ElgamalEncryptedData = openpgp.crypto.publicKeyEncrypt("elgamal", ElgamalpubMPIs, ElgamalUnencryptedData);