Move to using padding like TLS

martinthomson · martinthomson · commit d8235267336e · 2017-03-13T10:52:56.000+11:00
diff --git a/circle.yml b/circle.yml
@@ -4,7 +4,7 @@ machine:
   python:
     version: 3.5.2
   post:
-    - pyenv global 2.7.12 3.4.4 3.5.2
+    - pyenv global 2.7.12 3.4.4 3.5.2 3.6.0
 
 dependencies:
   cache_directories:
diff --git a/nodejs/ece.js b/nodejs/ece.js
@@ -25,7 +25,7 @@ var saved = {
   keylabels: {}
 };
 var AES_GCM = 'aes-128-gcm';
-var PAD_SIZE = { 'aes128gcm': 2, 'aesgcm': 2, 'aesgcm128': 1 };
+var PAD_SIZE = { 'aes128gcm': 1, 'aesgcm': 2, 'aesgcm128': 1 };
 var TAG_LENGTH = 16;
 var KEY_LENGTH = 16;
 var NONCE_LENGTH = 12;
@@ -313,15 +313,8 @@ function readHeader(buffer, header) {
   return 21 + idsz;
 }
 
-function decryptRecord(key, counter, buffer, header) {
-  keylog('decrypt', buffer);
-  var nonce = generateNonce(key.nonce, counter);
-  var gcm = crypto.createDecipheriv(AES_GCM, key.key, nonce);
-  gcm.setAuthTag(buffer.slice(buffer.length - TAG_LENGTH));
-  var data = gcm.update(buffer.slice(0, buffer.length - TAG_LENGTH));
-  data = Buffer.concat([data, gcm.final()]);
-  keylog('decrypted', data);
-  var padSize = PAD_SIZE[header.version];
+function unpadLegacy(data, version) {
+  var padSize = PAD_SIZE[version];
   var pad = data.readUIntBE(0, padSize);
   if (pad + padSize > data.length) {
     throw new Error('padding exceeds block size');
@@ -335,6 +328,40 @@ function decryptRecord(key, counter, buffer, header) {
   return data.slice(padSize + pad);
 }
 
+function unpad(data, last) {
+  var i = data.length - 1;
+  while(i > 0) {
+    if (data[i]) {
+      if (last) {
+        if (data[i] !== 2) {
+          throw new Error('last record needs to start padding with a 2');
+        }
+      } else {
+        if (data[i] !== 1) {
+          throw new Error('last record needs to start padding with a 2');
+        }
+      }
+      return data.slice(0, i);
+    }
+    --i;
+  }
+  throw new Error('all zero plaintext');
+}
+
+function decryptRecord(key, counter, buffer, header, last) {
+  keylog('decrypt', buffer);
+  var nonce = generateNonce(key.nonce, counter);
+  var gcm = crypto.createDecipheriv(AES_GCM, key.key, nonce);
+  gcm.setAuthTag(buffer.slice(buffer.length - TAG_LENGTH));
+  var data = gcm.update(buffer.slice(0, buffer.length - TAG_LENGTH));
+  data = Buffer.concat([data, gcm.final()]);
+  keylog('decrypted', data);
+  if (header.version !== 'aes128gcm') {
+    return unpadLegacy(data, header.version);
+  }
+  return unpad(data, last);
+}
+
 // TODO: this really should use the node streams stuff
 
 /**
@@ -375,38 +402,51 @@ function decrypt(buffer, params) {
 
   for (var i = 0; start < buffer.length; ++i) {
     var end = start + chunkSize;
-    if (end === buffer.length) {
+    if (header.version !== 'aes128gcm' && end === buffer.length) {
       throw new Error('Truncated payload');
     }
     end = Math.min(end, buffer.length);
     if (end - start <= TAG_LENGTH) {
       throw new Error('Invalid block: too small at ' + i);
     }
     var block = decryptRecord(key, i, buffer.slice(start, end),
-                              header);
+                              header, end >= buffer.length);
     result = Buffer.concat([result, block]);
     start = end;
   }
   return result;
 }
 
-function encryptRecord(key, counter, buffer, pad, padSize) {
+function encryptRecord(key, counter, buffer, pad, header, last) {
   keylog('encrypt', buffer);
   pad = pad || 0;
   var nonce = generateNonce(key.nonce, counter);
   var gcm = crypto.createCipheriv(AES_GCM, key.key, nonce);
+
+  var ciphertext = [];
+  var padSize = PAD_SIZE[header.version];
   var padding = new Buffer(pad + padSize);
   padding.fill(0);
-  padding.writeUIntBE(pad, 0, padSize);
-  keylog('padding', padding);
-  var epadding = gcm.update(padding);
-  var ebuffer = gcm.update(buffer);
+
+  if (header.version !== 'aes128gcm') {
+    padding.writeUIntBE(pad, 0, padSize);
+    keylog('padding', padding);
+    ciphertext.push(gcm.update(padding));
+    ciphertext.push(gcm.update(buffer));
+  } else {
+    ciphertext.push(gcm.update(buffer));
+    padding.writeUIntBE(last ? 2 : 1, 0, 1);
+    keylog('padding', padding);
+    ciphertext.push(gcm.update(padding));
+  }
+
   gcm.final();
   var tag = gcm.getAuthTag();
   if (tag.length !== TAG_LENGTH) {
     throw new Error('invalid tag generated');
   }
-  return keylog('encrypted', Buffer.concat([epadding, ebuffer, tag]));
+  ciphertext.push(tag);
+  return keylog('encrypted', Buffer.concat(ciphertext));
 }
 
 function writeHeader(header) {
@@ -471,19 +511,30 @@ function encrypt(buffer, params) {
   }
   var pad = isNaN(parseInt(params.pad, 10)) ? 0 : parseInt(params.pad, 10);
 
-  // Note the <= here ensures that we write out a padding-only block at the end
-  // of a buffer.
-  for (var i = 0; start <= buffer.length; ++i) {
+  var counter = 0;
+  var last = false;
+  while (!last) {
     // Pad so that at least one data byte is in a block.
-    var recordPad = Math.min((1 << (padSize * 8)) - 1, // maximum padding
-                             Math.min(header.rs - overhead - 1, pad));
+    var recordPad = Math.min(header.rs - overhead - 1, pad);
+    if (header.version !== 'aes128gcm') {
+      recordPad = Math.min((1 << (padSize * 8)) - 1, recordPad);
+    }
     pad -= recordPad;
 
-    var end = Math.min(start + header.rs - overhead - recordPad, buffer.length);
-    var block = encryptRecord(key, i, buffer.slice(start, end),
-                              recordPad, padSize);
+    var end = start + header.rs - overhead - recordPad;
+    if (header.version !== 'aes128gcm') {
+      // The > here ensures that we write out a padding-only block at the end
+      // of a buffer.
+      last = end > buffer.length;
+    } else {
+      last = end >= buffer.length && pad <= 0;
+    }
+    var block = encryptRecord(key, counter, buffer.slice(start, end),
+                              recordPad, header, last);
     result = Buffer.concat([result, block]);
-    start += header.rs - overhead - recordPad;
+
+    start = end;
+    ++counter;
   }
   if (pad) {
     throw new Error('Unable to pad by requested amount, ' + pad + ' remaining');
diff --git a/nodejs/package.json b/nodejs/package.json
@@ -1,6 +1,6 @@
 {
   "name": "http_ece",
-  "version": "0.6.4",
+  "version": "0.6.5",
   "description": "Encrypted Content-Encoding for HTTP",
   "homepage": "https://github.com/martinthomson/encrypted-content-encoding",
   "bugs": "https://github.com/martinthomson/encrypted-content-encoding/issues",
diff --git a/nodejs/test.js b/nodejs/test.js
@@ -26,7 +26,7 @@ args.forEach(function(arg) {
   if (arg === 'verbose') {
     log = console.log.bind(console);
   } else if (arg.substring(0, 5) === 'text=') {
-    plaintext = arg.substring(5);
+    plaintext = Buffer.from(arg.substring(5), 'utf8');
   } else if (arg.substring(0, 4) === 'max=') {
     var v = parseInt(arg.substring(4), 10);
     if (!isNaN(v) && v > minLen) {
@@ -136,7 +136,6 @@ function encryptDecrypt(input, encryptParams, decryptParams, keys) {
   var decrypted = ece.decrypt(encrypted, decryptParams);
   logbuf('Decrypted', decrypted);
   assert.equal(Buffer.compare(input, decrypted), 0);
-  log('----- OK');
 
   saveDump({
     version: encryptParams.version,
@@ -183,6 +182,9 @@ function exactlyOneRecord(version) {
 }
 
 function detectTruncation(version) {
+  if (version === 'aes128gcm') {
+    return;
+  }
   var input = generateInput(2);
   var params = {
     version: version,
@@ -281,37 +283,39 @@ function useDH(version) {
 }
 
 // Use the examples from the draft as a sanity check.
-function checkExamples() {
+function checkExamples(version) {
   [
     {
       args: {
         version: 'aes128gcm',
-        key: base64.decode('6Aqf1aDH8lSxLyCpoCnAqg'),
+        key: base64.decode('yqdlZ-tYemfogSmv7Ws5PQ'),
         keyid: '',
-        salt: base64.decode('sJvlboCWzB5jr8hI_q9cOQ'),
+        salt: base64.decode('I1BsxtFttlv3u_Oo94xnmw'),
         rs: 4096
       },
       plaintext: Buffer.from('I am the walrus'),
-      ciphertext: base64.decode('sJvlboCWzB5jr8hI_q9cOQAAEAAANSmx' +
-                                'kSVa0-MiNNuF77YHSs-iwaNe_OK0qfmO' +
-                                'c7NT5WSW'),
+      ciphertext: base64.decode('I1BsxtFttlv3u_Oo94xnmwAAEAAA-NAV' +
+                                'ub2qFgBEuQKRapoZu-IxkIva3MEB1PD-' +
+                                'ly8Thjg'),
     },
     {
       args: {
         version: 'aes128gcm',
         key: base64.decode('BO3ZVPxUlnLORbVGMpbT1Q'),
         keyid: 'a1',
         salt: base64.decode('uNCkWiNYzKTnBN9ji3-qWA'),
-        rs: 26,
+        rs: 25,
         pad: 1
       },
       plaintext: Buffer.from('I am the walrus'),
-      ciphertext: base64.decode('uNCkWiNYzKTnBN9ji3-qWAAAABoCYTGH' +
-                                'OqYFz-0in3dpb-VE2GfBngkaPy6bZus_' +
-                                'qLF79s6zQyTSsA0iLOKyd3JqVIwprNzV' +
-                                'atRCWZGUx_qsFbJBCQu62RqQuR2d')
+      ciphertext: base64.decode('uNCkWiNYzKTnBN9ji3-qWAAAABkCYTHO' +
+                                'G8chz_gnvgOqdGYovxyjuqRyJFjEDyoF' +
+                                '1Fvkj6hQPdPHI51OEUKEpgz3SsLWIqS_' +
+                                'uA')
     }
-  ].forEach(function (v, i) {
+  ].filter(function(v) {
+    return v.args.version === version;
+  }).forEach(function (v, i) {
     log('decrypt ' + v.args.version + ' example ' + (i + 1));
     var decrypted = ece.decrypt(v.ciphertext, v.args);
     logbuf('decrypted', decrypted);
@@ -333,17 +337,17 @@ filterTests([ 'aesgcm128', 'aesgcm', 'aes128gcm' ])
                   detectTruncation,
                   useKeyId,
                   useDH,
+                  checkExamples,
                 ])
       .forEach(function(test) {
         log(version + ' Test: ' + test.name);
         test(version);
+        log('----- OK');
       });
   });
-checkExamples();
 
 log('All tests passed.');
 
-
 if (dumpFile) {
   require('fs').writeFileSync(dumpFile, JSON.stringify(dumpData, undefined, '  '));
 }
diff --git a/python/http_ece/__init__.py b/python/http_ece/__init__.py
@@ -22,7 +22,7 @@
 
 # Valid content types (ordered from newest, to most obsolete)
 versions = {
-    "aes128gcm": {"pad": 2},
+    "aes128gcm": {"pad": 1},
     "aesgcm": {"pad": 2},
     "aesgcm128": {"pad": 1},
 }
@@ -217,16 +217,29 @@ def decrypt_record(key, nonce, counter, content):
             modes.GCM(iv(nonce, counter), tag=content[-TAG_LENGTH:]),
             backend=default_backend()
         ).decryptor()
-        data = decryptor.update(content[:-TAG_LENGTH]) + decryptor.finalize()
+        return decryptor.update(content[:-TAG_LENGTH]) + decryptor.finalize()
+
+    def unpad_legacy(data):
         pad = functools.reduce(
             lambda x, y: x << 8 | y, struct.unpack(
                 "!" + ("B" * pad_size), data[0:pad_size])
         )
         if pad_size + pad > len(data) or \
            data[pad_size:pad_size+pad] != (b"\x00" * pad):
             raise ECEException(u"Bad padding")
-        data = data[pad_size + pad:]
-        return data
+        return data[pad_size + pad:]
+
+    def unpad(data, last):
+        i = len(data) - 1
+        for i in range(len(data) - 1, -1, -1):
+            if data[i] != 0:
+                if not last and data[i] != 1:
+                    raise ECEException(u'record delimiter != 1')
+                if last and data[i] != 2:
+                    raise ECEException(u'last record delimiter != 2')
+                return data[0:i]
+            i -= 1
+        raise ECEException(u'all zero record plaintext')
 
     if version not in versions:
         raise ECEException(u"Invalid version")
@@ -269,14 +282,19 @@ def decrypt_record(key, nonce, counter, content):
         chunk = rs + 16  # account for tags in old versions
     else:
         chunk = rs
-    if len(content) % chunk == 0:
+    if len(content) % chunk == 0 and version != 'aes128gcm':
         raise ECEException(u"Message truncated")
 
     result = b''
     counter = 0
     try:
         for i in list(range(0, len(content), chunk)):
-            result += decrypt_record(key_, nonce_, counter, content[i:i + chunk])
+            data = decrypt_record(key_, nonce_, counter, content[i:i + chunk])
+            if version == 'aes128gcm':
+                last = (i + chunk) >= len(content)
+                result += unpad(data, last)
+            else:
+                result += unpad_legacy(data)
             counter += 1
     except InvalidTag as ex:
         raise ECEException("Decryption error: {}".format(repr(ex)))
@@ -312,14 +330,17 @@ def encrypt(content, salt=None, key=None,
     :rtype str
 
     """
-    def encrypt_record(key, nonce, counter, buf):
+    def encrypt_record(key, nonce, counter, buf, last):
         encryptor = Cipher(
             algorithms.AES(key),
             modes.GCM(iv(nonce, counter)),
             backend=default_backend()
         ).encryptor()
 
-        data = encryptor.update((b"\0" * pad_size) + buf)
+        if version == 'aes128gcm':
+            data = encryptor.update(buf + (b'\x02' if last else b'\x01'))
+        else:
+            data = encryptor.update((b"\0" * pad_size) + buf)
         data += encryptor.finalize()
         data += encryptor.tag
         return data
@@ -372,6 +393,9 @@ def compose_aes128gcm(salt, content, rs, keyid):
     overhead = pad_size
     if version == 'aes128gcm':
         overhead += 16
+        end = len(content)
+    else:
+        end = len(content) + 1
     if rs <= pad_size:
         raise ECEException(u"Record size too small")
     chunk_size = rs - overhead
@@ -381,9 +405,10 @@ def compose_aes128gcm(salt, content, rs, keyid):
 
     # the extra one on the loop ensures that we produce a padding only
     # record if the data length is an exact multiple of the chunk size
-    for i in list(range(0, len(content) + 1, chunk_size)):
+    for i in list(range(0, end, chunk_size)):
         result += encrypt_record(key_, nonce_, counter,
-                                 content[i:i + chunk_size])
+                                 content[i:i + chunk_size],
+                                 (i + chunk_size) >= end)
         counter += 1
     if version == "aes128gcm":
         if keyid is None and private_key is not None:
diff --git a/python/http_ece/tests/test_ece.py b/python/http_ece/tests/test_ece.py
diff --git a/python/tox.ini b/python/tox.ini

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "http_ece",`
`3`		`- "version": "0.6.4",`
	`3`	`+ "version": "0.6.5",`
`4`	`4`	`"description": "Encrypted Content-Encoding for HTTP",`
`5`	`5`	`"homepage": "https://github.com/martinthomson/encrypted-content-encoding",`
`6`	`6`	`"bugs": "https://github.com/martinthomson/encrypted-content-encoding/issues",`