SHA 512/384 padding is 128 bit (flutter#77)

The SHA algorithms were fine; but the padding in HashSink was hardcoded
to 64-bit signatures. While we still only generate a 64-bit signature,
the signature space is 128-bit.

Fixes flutter#69.

Special thanks to @Nico04 for providing the test cases that lead to this
discovery.

Author: jtmcdole

CHANGELOG.md

@@ -1,3 +1,6 @@
+## 2.1.4
+  * BugFix: padding was incorrect for some SHA-512/328.
+
 ## 2.1.3
   * **Security vulnerability**: Fixed constant-time comparison in `Digest`.
 

lib/src/hash_sink.dart

@@ -44,12 +44,23 @@ abstract class HashSink implements Sink<List<int>> {
   /// This should be updated each time [updateHash] is called.
   Uint32List get digest;
 
+  /// The number of signature bytes emitted at the end of the message.
+  ///
+  /// An encrypted message is followed by a signature which depends
+  /// on the encryption algorithm used. This value specifies the
+  /// number of bytes used by this signature. It must always be
+  /// a power of 2 and no less than 8.
+  final int _signatureBytes;
+
   /// Creates a new hash.
   ///
   /// [chunkSizeInWords] represents the size of the input chunks processed by
   /// the algorithm, in terms of 32-bit words.
-  HashSink(this._sink, int chunkSizeInWords, {Endian endian = Endian.big})
+  HashSink(this._sink, int chunkSizeInWords,
+      {Endian endian = Endian.big, int signatureBytes = 8})
       : _endian = endian,
+        assert(signatureBytes >= 8),
+        _signatureBytes = signatureBytes,
         _currentChunk = Uint32List(chunkSizeInWords);
 
   /// Runs a single iteration of the hash computation, updating [digest] with
@@ -82,10 +93,12 @@ abstract class HashSink implements Sink<List<int>> {
   Uint8List _byteDigest() {
     if (_endian == Endian.host) return digest.buffer.asUint8List();
 
-    var byteDigest = Uint8List(digest.lengthInBytes);
-    var byteData = byteDigest.buffer.asByteData();
-    for (var i = 0; i < digest.length; i++) {
-      byteData.setUint32(i * bytesPerWord, digest[i]);
+    // Cache the digest locally as `get` could be expensive.
+    final cachedDigest = digest;
+    final byteDigest = Uint8List(cachedDigest.lengthInBytes);
+    final byteData = byteDigest.buffer.asByteData();
+    for (var i = 0; i < cachedDigest.length; i++) {
+      byteData.setUint32(i * bytesPerWord, cachedDigest[i]);
     }
     return byteDigest;
   }
@@ -116,11 +129,14 @@ abstract class HashSink implements Sink<List<int>> {
   /// This adds a 1 bit to the end of the message, and expands it with 0 bits to
   /// pad it out.
   void _finalizeData() {
-    // Pad out the data with 0x80, eight 0s, and as many more 0s as we need to
-    // land cleanly on a chunk boundary.
+    // Pad out the data with 0x80, eight or sixteen 0s, and as many more 0s
+    // as we need to land cleanly on a chunk boundary.
     _pendingData.add(0x80);
-    var contentsLength = _lengthInBytes + 9;
-    var finalizedLength = _roundUp(contentsLength, _currentChunk.lengthInBytes);
+
+    final contentsLength = _lengthInBytes + 1 /* 0x80 */ + _signatureBytes;
+    final finalizedLength =
+        _roundUp(contentsLength, _currentChunk.lengthInBytes);
+
     for (var i = 0; i < finalizedLength - contentsLength; i++) {
       _pendingData.add(0);
     }
@@ -133,9 +149,11 @@ abstract class HashSink implements Sink<List<int>> {
     var lengthInBits = _lengthInBytes * bitsPerByte;
 
     // Add the full length of the input data as a 64-bit value at the end of the
-    // hash.
-    var offset = _pendingData.length;
-    _pendingData.addAll(Uint8List(8));
+    // hash. Note: we're only writing out 64 bits, so skip ahead 8 if the
+    // signature is 128-bit.
+    final offset = _pendingData.length + (_signatureBytes - 8);
+
+    _pendingData.addAll(Uint8List(_signatureBytes));
     var byteData = _pendingData.buffer.asByteData();
 
     // We're essentially doing byteData.setUint64(offset, lengthInBits, _endian)

lib/src/sha512_fastsinks.dart

@@ -30,7 +30,8 @@ abstract class _Sha64BitSink extends HashSink {
   /// used across invocations of [updateHash].
   final _extended = Uint64List(80);
 
-  _Sha64BitSink(Sink<Digest> sink, this._digest) : super(sink, 32);
+  _Sha64BitSink(Sink<Digest> sink, this._digest)
+      : super(sink, 32, signatureBytes: 16);
   // The following helper functions are taken directly from
   // http://tools.ietf.org/html/rfc6234.
 

lib/src/sha512_slowsinks.dart

@@ -71,7 +71,8 @@ abstract class _Sha64BitSink extends HashSink {
   /// used across invocations of [updateHash].
   final _extended = Uint32List(160);
 
-  _Sha64BitSink(Sink<Digest> sink, this._digest) : super(sink, 32);
+  _Sha64BitSink(Sink<Digest> sink, this._digest)
+      : super(sink, 32, signatureBytes: 16);
   // The following helper functions are taken directly from
   // http://tools.ietf.org/html/rfc6234.
 

pubspec.yaml

@@ -1,5 +1,5 @@
 name: crypto
-version: 2.1.3
+version: 2.1.4
 author: Dart Team <misc@dartlang.org>
 description: Library of cryptographic functions.
 homepage: https://www.github.com/dart-lang/crypto

test/sha512_test.dart

@@ -75,5 +75,27 @@ void main() {
         outer.close();
       });
     });
+
+    test('128 bit padding', () {
+      final salts = [
+        'AAAA{3FXhiiyc5gGWlRrVQ2RlJ.6xj.DKvf6l0bJxqh0BzA}'.codeUnits,
+        'AAAA{3FXhiiyc5gGWlRrVQ.2RlJ6xj.DKvf6l0bJxqh0BzA}'.codeUnits,
+        'AAAA{rFXhiiyc5gGWlVQ.2RlJ6xj.DKvf6lFXhiiyc5gGWl0}'.codeUnits,
+      ];
+
+      const results = [
+        'nYg7eEsF/P7/l1AO0w8JFNNomS1gC76VE7Eg7Dpet+Dh6XiScDntYEU4tVItXp67evaLFvtMpW2uVJBZVKrBPw==',
+        'TXNM4uk1Iwr2cYisWSdFifXdjfNiJTGEmNaMtqYrwJoS3JXpL1rebPKPfKudbFQGpcgJkLLhhpfnLzULBqq8KA==',
+        'ckPYMDuPJjc73qHXQZiJgCskNG8mj9cPqFNsqYqxcBbQESgkWChoibAN7ssJrnoMFIpz9HwsBwMtt3z/KDUh9w==',
+      ];
+
+      for (int i = 0; i < salts.length; i++) {
+        var digest = <int>[];
+        for (int run = 0; run < 2000; run++) {
+          digest = sha512.convert([]..addAll(digest)..addAll(salts[i])).bytes;
+        }
+        expect(base64.encode(digest), results[i]);
+      }
+    });
   });
 }