Allow smaller amounts of padding and smaller records with padding

Author: martinthomson

nodejs/ece.js

@@ -330,7 +330,7 @@ function unpadLegacy(data, version) {
 
 function unpad(data, last) {
   var i = data.length - 1;
-  while(i > 0) {
+  while(i >= 0) {
     if (data[i]) {
       if (last) {
         if (data[i] !== 2) {
@@ -433,6 +433,10 @@ function encryptRecord(key, counter, buffer, pad, header, last) {
     keylog('padding', padding);
     ciphertext.push(gcm.update(padding));
     ciphertext.push(gcm.update(buffer));
+
+    if (!last && padding.length + buffer.length < header.rs) {
+      throw new Error('Unable to pad to record size');
+    }
   } else {
     ciphertext.push(gcm.update(buffer));
     padding.writeUIntBE(last ? 2 : 1, 0, 1);
@@ -520,6 +524,9 @@ function encrypt(buffer, params) {
     if (header.version !== 'aes128gcm') {
       recordPad = Math.min((1 << (padSize * 8)) - 1, recordPad);
     }
+    if (pad > 0 && recordPad === 0) {
+      ++recordPad; // Deal with perverse case of rs=overhead+1 with padding.
+    }
     pad -= recordPad;
 
     var end = start + header.rs - overhead - recordPad;
@@ -528,18 +535,16 @@ function encrypt(buffer, params) {
       // of a buffer.
       last = end > buffer.length;
     } else {
-      last = end >= buffer.length && pad <= 0;
+      last = end >= buffer.length;
     }
+    last = last && pad <= 0;
     var block = encryptRecord(key, counter, buffer.slice(start, end),
                               recordPad, header, last);
     result = Buffer.concat([result, block]);
 
     start = end;
     ++counter;
   }
-  if (pad) {
-    throw new Error('Unable to pad by requested amount, ' + pad + ' remaining');
-  }
   return result;
 }
 

nodejs/test.js

@@ -16,7 +16,7 @@ function usage() {
   console.log('  "dump[=file]" log info to ../encrypt_data.json or the specified file');
 }
 var args = process.argv.slice(2);
-var minLen = 3;
+var minLen = 1;
 var maxLen = 100;
 var plaintext;
 var dumpFile;
@@ -65,11 +65,12 @@ function logbuf(msg, buf) {
   }
 }
 
-function saveDump(data){
+function reallySaveDump(data){
   if (dumpFile && data.version) {
     dumpData.push(data);
   }
 }
+var saveDump = reallySaveDump;
 
 function validate() {
   ['hello', null, 1, NaN, [], {}].forEach(function(v) {
@@ -181,6 +182,48 @@ function exactlyOneRecord(version) {
   encryptDecrypt(input, params, params);
 }
 
+// If rs only allows one octet of data in each record AND padding is requested,
+// then we need to ensure that padding is added without infinitely looping.
+function padTinyRecord(version) {
+  var input = generateInput(1);
+  var params = {
+    version: version,
+    key: base64.encode(crypto.randomBytes(16)),
+    rs: rsoverhead(version) + 1,
+    pad: 2
+  };
+  encryptDecrypt(input, params, params);
+}
+
+// The earlier versions had a limit to how much padding they could include in
+// each record, which means that they could fail to encrypt if too much padding
+// was requested with a large record size.
+function tooMuchPadding(version) {
+  if (version === 'aes128gcm') {
+    return;
+  }
+  var padSize = rsoverhead(version);
+  var rs = Math.pow(256, padSize) + padSize + 1;
+  var input = generateInput(1);
+  var params = {
+    version: version,
+    key: base64.encode(crypto.randomBytes(16)),
+    rs: rs,
+    pad: rs
+  };
+  var ok = false;
+  try {
+    ece.encrypt(input, params);
+  } catch (e) {
+    log('----- OK: ' + e);
+    ok = true;
+  }
+  if (!ok) {
+    throw new Error('Encryption succeeded, but should not have');
+  }
+}
+
+
 function detectTruncation(version) {
   if (version === 'aes128gcm') {
     return;
@@ -334,13 +377,18 @@ filterTests([ 'aesgcm128', 'aesgcm', 'aes128gcm' ])
     filterTests([ useExplicitKey,
                   authenticationSecret,
                   exactlyOneRecord,
+                  padTinyRecord,
                   detectTruncation,
                   useKeyId,
                   useDH,
                   checkExamples,
                 ])
       .forEach(function(test) {
         log(version + ' Test: ' + test.name);
+        saveDump = data => {
+          data.test = test.name + ' ' + version;
+          reallySaveDump(data);
+        };
         test(version);
         log('----- OK');
       });

python/http_ece/tests/test_ece.py

@@ -475,7 +475,13 @@ def _run(self, mode):
             outp = 'input'
 
         for data in self.legacy_data:
+            logmsg('%s: %s' % (mode, data['test']))
             p = data['params'][mode]
+
+            if 'pad' in p and mode == 'encrypt':
+                # This library doesn't pad in exactly the same way.
+                continue
+
             if 'keys' in data:
                 key = None
                 private_key = pyelliptic.ECC(