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]);
+ }
+ });
});
}