SHA 512/384 padding is 128 bit (#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 #69. Special thanks to @Nico04 for providing the test cases that lead to this discovery.
diff --git a/CHANGELOG.md b/CHANGELOG.md index 472f96f..5cb220f 100644 --- a/CHANGELOG.md +++ b/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`.
diff --git a/lib/src/hash_sink.dart b/lib/src/hash_sink.dart index 434b99d..8cd4b99 100644 --- a/lib/src/hash_sink.dart +++ b/lib/src/hash_sink.dart
@@ -44,12 +44,23 @@ /// 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 @@ 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 @@ /// 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 @@ 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)
diff --git a/lib/src/sha512_fastsinks.dart b/lib/src/sha512_fastsinks.dart index 4bcf31e..5c4b876 100644 --- a/lib/src/sha512_fastsinks.dart +++ b/lib/src/sha512_fastsinks.dart
@@ -30,7 +30,8 @@ /// 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.
diff --git a/lib/src/sha512_slowsinks.dart b/lib/src/sha512_slowsinks.dart index a2af606..9fd17b2 100644 --- a/lib/src/sha512_slowsinks.dart +++ b/lib/src/sha512_slowsinks.dart
@@ -71,7 +71,8 @@ /// 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.
diff --git a/pubspec.yaml b/pubspec.yaml index 38b55c3..5339057 100644 --- a/pubspec.yaml +++ b/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
diff --git a/test/sha512_test.dart b/test/sha512_test.dart index 351a8b7..c5d1da6 100644 --- a/test/sha512_test.dart +++ b/test/sha512_test.dart
@@ -75,5 +75,27 @@ 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]); + } + }); }); }