diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 00e74cf..4170b43 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -91,6 +91,29 @@ jobs: - name: Run tests run: dart test -p ${{ matrix.platform }} ${{ matrix.sdk != 'stable' && '-x stable-only' || '' }} + wasm: + needs: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: dart-lang/setup-dart@v1 + with: + sdk: stable + + - name: Cache dependencies + uses: actions/cache@v6 + with: + path: | + ~/.pub-cache + .dart_tool/package_config.json + key: ${{ runner.os }}-test-${{ hashFiles('**/pubspec.lock') }} + + - name: Install dependencies + run: dart pub get + + - name: Run tests (dart2wasm on Chrome) + run: dart test -P wasm + integration: needs: lint strategy: diff --git a/CHANGELOG.md b/CHANGELOG.md index 6df051d..2431543 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # 0.7.2 +- 🐛 Fix **AES-GCM** producing wrong ciphertext on `dart2wasm` for messages of + about 1 MiB or larger + ([#28](https://github.com/bitanon/cipherlib/issues/28)). The block counter is + stored in a `Uint8List` + view over another buffer, and dart2wasm skips the modulo-256 truncation when + such a view is assigned an out-of-range value, so `counter[i]++` let the carry + bleed into the neighbouring byte. From the first 32-bit carry onward the + keystream desynchronised, silently breaking interoperability with every other + platform (and with other AES-GCM implementations). All stored counter bytes + are now masked explicitly. +- ✅ The `inc32` counter behaviour is now pinned against raw AES-ECB at every + byte-carry boundary, plus an end-to-end check across the 1 MiB boundary, and + the `dart2wasm` suite runs on every push instead of only at release time. +- ✅ New cross-cutting `test/counter_boundary_test.dart` asserts counter + continuity — block `j` of a run started at counter `C` equals block `0` of a + run started at `C + j` — for AES/Twofish/Blowfish CTR, ChaCha20 (32- and + 64-bit counters), XChaCha20, Salsa20 and XSalsa20. Because the starting + counter is an input, this reaches the 32- and 64-bit carries that no + achievable message length could walk to, alongside a 1 MiB run per cipher. - Improve the benchmark harness (`benchmark/_base.dart`): it now reports the median per-iteration time sampled across ~25ms batches instead of the arithmetic mean, making results robust against GC pauses and keeping each diff --git a/lib/src/algorithms/aes_modes/gcm.dart b/lib/src/algorithms/aes_modes/gcm.dart index 6e62bf2..abb6146 100644 --- a/lib/src/algorithms/aes_modes/gcm.dart +++ b/lib/src/algorithms/aes_modes/gcm.dart @@ -79,17 +79,21 @@ class AESInGCMModeCipherCore { M[3] = t3; } - // Increment a 32-bit counter in little-endian order + /// Increment a 32-bit counter in little-endian order + /// + /// [counter] is a [Uint8List] view over another buffer. on wasm it silently + /// desynchronises the keystream past the first 32-bit carry. + /// See https://github.com/bitanon/cipherlib/issues/28 @pragma('vm:prefer-inline') @pragma('dart2js:tryInline') static void _increment32(Uint8List counter) { - counter[15]++; + counter[15] = (counter[15] + 1) & 0xFF; if (counter[15] == 0) { - counter[14]++; + counter[14] = (counter[14] + 1) & 0xFF; if (counter[14] == 0) { - counter[13]++; + counter[13] = (counter[13] + 1) & 0xFF; if (counter[13] == 0) { - counter[12]++; + counter[12] = (counter[12] + 1) & 0xFF; } } } diff --git a/test/aes_gcm_test.dart b/test/aes_gcm_test.dart index ebc19ea..b3f2376 100644 --- a/test/aes_gcm_test.dart +++ b/test/aes_gcm_test.dart @@ -104,6 +104,86 @@ void main() { }); group('AESInGCMModeCipherCore', () { + // NIST SP 800-38D section 6.2: GCM's counter uses inc32, which increments + // only the rightmost 32 bits of the counter block modulo 2^32 and leaves + // the leading 96 bits untouched. + // + // Each boundary is checked against a raw AES-ECB encryption of the counter + // block that inc32 is required to produce, so this pins the counter value + // itself rather than merely that some keystream came out. Regression for + // https://github.com/bitanon/cipherlib/issues/28, where `counter[i]++` on a + // Uint8List view skipped the modulo-256 truncation under dart2wasm and let + // the carry bleed into the next byte, desynchronising the keystream from + // the first 32-bit carry (~1 MiB) onward on that platform only. + test('counter follows inc32 across every byte-carry boundary', () { + // NIST SP 800-38D, Appendix B, Test Case 2 key/IV + final key = fromHex('feffe9928665731c6d6a8f9467308308'); + final iv = fromHex('cafebabefacedbaddecaf888'); + final ecb = AES.noPadding(key).ecb(); + + const boundaries = { + '000000fe': '000000ff', // no carry + '000000ff': '00000100', // carry into byte 14 + '0000ffff': '00010000', // carry into byte 13 + '00ffffff': '01000000', // carry into byte 12 + 'ffffffff': '00000000', // wrap around modulo 2^32 + }; + + boundaries.forEach((before, after) { + final core = AESInGCMModeCipherCore(key, iv, null, 16)..initialize(); + core.counter.setRange(12, 16, fromHex(before)); + + // encrypting an all-zero block exposes the keystream block verbatim + final keystream = core.encrypt(Uint8List(16)).sublist(0, 16); + + final expectedBlock = Uint8List(16); + expectedBlock.setRange(0, 12, iv); + expectedBlock.setRange(12, 16, fromHex(after)); + + expect( + toHex(keystream), + equals(toHex(ecb.encrypt(expectedBlock))), + reason: '[counter: $before -> $after]', + ); + expect( + toHex(core.counter.sublist(12)), + equals(after), + reason: '[counter: $before -> $after]', + ); + expect( + toHex(core.counter.sublist(0, 12)), + equals(toHex(iv)), + reason: 'inc32 must not touch the leading 96 bits', + ); + }); + }); + // End-to-end guard at the message size reported in issue #28. Block k of + // the ciphertext is keyed by counter k + 2 (the counter starts at 1 and is + // incremented before each block), so the first carry out of the low 16 bits + // lands on block 65534, at byte offset 1048544 of a 1 MiB message. + test('keystream stays aligned past the 1 MiB counter carry', () { + final key = fromHex('feffe9928665731c6d6a8f9467308308'); + final iv = fromHex('cafebabefacedbaddecaf888'); + final ecb = AES.noPadding(key).ecb(); + + final cipher = AES(key).gcm(iv).encrypt(Uint8List(1048576)); + expect(cipher.length, equals(1048576 + 16)); + + for (final k in [65532, 65533, 65534, 65535]) { + final counter = k + 2; + final block = Uint8List(16); + block.setRange(0, 12, iv); + block[12] = (counter >>> 24) & 0xFF; + block[13] = (counter >>> 16) & 0xFF; + block[14] = (counter >>> 8) & 0xFF; + block[15] = counter & 0xFF; + expect( + toHex(cipher.sublist(k * 16, k * 16 + 16)), + equals(toHex(ecb.encrypt(block))), + reason: '[block: $k, counter: $counter]', + ); + } + }); test('core counter increment propagates carry to highest byte', () { final core = AESInGCMModeCipherCore(Uint8List(16), Uint8List(12), null, 16); diff --git a/test/counter_boundary_test.dart b/test/counter_boundary_test.dart new file mode 100644 index 0000000..3295574 --- /dev/null +++ b/test/counter_boundary_test.dart @@ -0,0 +1,221 @@ +// Copyright (c) 2026, Sudipto Chandra +// All rights reserved. Check LICENSE file for details. + +import 'dart:typed_data'; + +import 'package:cipherlib/cipherlib.dart'; +import 'package:cipherlib/codecs.dart'; +import 'package:test/test.dart'; + +/// Cross-cutting counter checks for every counter-based cipher. +/// +/// A block counter that mis-carries produces a keystream that is still +/// perfectly reversible by the same implementation, so round-trips and the +/// size sweeps in `correctness_test.dart` (which top out at 124568 bytes) can +/// not see it. That is how the dart2wasm AES-GCM carry bug in issue #28 +/// survived to a release: it only showed up past 1 MiB, and only on a platform +/// the everyday suite did not run. +/// +/// The property asserted here is *counter continuity*: block `j` of a run +/// started at counter `C` must equal block `0` of a run started at `C + j`. +/// It holds for every counter mode, needs no external vector, and — because +/// the starting counter is an input — it reaches the 32-bit and 64-bit carries +/// that no achievable message length could ever walk to. + +/// A block counter held as two 32-bit halves. +/// +/// The interesting boundaries sit at 2^32 and 2^64, and an `int` literal past +/// 2^53 is not exactly representable on the JS platform this suite also runs +/// on. Splitting the value keeps every intermediate small enough to be exact +/// everywhere. +class _Counter { + final int high; + final int low; + + const _Counter(this.high, this.low); + + /// This counter advanced by [n], wrapping at [bits]. + _Counter plus(int n, int bits) { + var l = low + n; + var h = high; + if (l > 0xFFFFFFFF) { + l -= 0x100000000; + h = (h + 1) & 0xFFFFFFFF; + } + return _Counter(bits == 32 ? 0 : h, l); + } + + @override + String toString() => '0x' + '${high.toRadixString(16).padLeft(8, '0')}' + '${low.toRadixString(16).padLeft(8, '0')}'; +} + +/// Produces [blocks] blocks of raw keystream starting from counter [c]. +typedef _Keystream = Uint8List Function(_Counter c, int blocks); + +class _Subject { + final String name; + final int blockSize; + final int counterBits; + final _Keystream keystream; + + const _Subject(this.name, this.blockSize, this.counterBits, this.keystream); +} + +Uint8List _fill(int size, int mul, int add) { + final out = Uint8List(size); + for (int i = 0; i < size; ++i) { + out[i] = (i * mul + add) & 0xFF; + } + return out; +} + +/// Builds a CTR-mode IV of [size] bytes: a fixed nonce prefix with [c] packed +/// big-endian into the low [counterBits] bits, which is how every CTR +/// implementation here loads its counter block. +Uint8List _ctrIV(int size, int counterBits, _Counter c) { + final iv = _fill(size, 3, 5); + final packed = [ + (c.high >>> 24) & 0xFF, + (c.high >>> 16) & 0xFF, + (c.high >>> 8) & 0xFF, + c.high & 0xFF, + (c.low >>> 24) & 0xFF, + (c.low >>> 16) & 0xFF, + (c.low >>> 8) & 0xFF, + c.low & 0xFF, + ]; + final n = counterBits >>> 3; + iv.setRange(size - n, size, packed.sublist(8 - n)); + return iv; +} + +/// Counters positioned two blocks below each carry boundary reachable within +/// [bits], so a short run steps over the carry. +List<_Counter> _boundaries(int bits) { + final out = <_Counter>[ + const _Counter(0, 0x000000FD), // 8-bit carry + const _Counter(0, 0x0000FFFD), // 16-bit carry + const _Counter(0, 0x00FFFFFD), // 24-bit carry + const _Counter(0, 0xFFFFFFFD), // 32-bit carry + ]; + if (bits > 32) { + out.addAll(const [ + _Counter(0x00000001, 0xFFFFFFFD), // carry inside the high word + _Counter(0x0000FFFF, 0xFFFFFFFD), // 48-bit carry + _Counter(0xFFFFFFFF, 0xFFFFFFFD), // wrap around modulo 2^64 + ]); + } + return out; +} + +void main() { + final key32 = _fill(32, 7, 1); + final key56 = _fill(56, 7, 1); + + final subjects = <_Subject>[ + _Subject( + 'AES-256/CTR-32', + 16, + 32, + (c, b) => + AES(key32).ctr(_ctrIV(16, 32, c), 32).encrypt(Uint8List(b * 16)), + ), + _Subject( + 'AES-256/CTR-64', + 16, + 64, + (c, b) => + AES(key32).ctr(_ctrIV(16, 64, c), 64).encrypt(Uint8List(b * 16)), + ), + _Subject( + 'Twofish/CTR-64', + 16, + 64, + (c, b) => + Twofish(key32).ctr(_ctrIV(16, 64, c), 64).encrypt(Uint8List(b * 16)), + ), + _Subject( + 'Blowfish/CTR-64', + 8, + 64, + (c, b) => + Blowfish(key56).ctr(_ctrIV(8, 64, c), 64).encrypt(Uint8List(b * 8)), + ), + _Subject( + 'ChaCha20/32-bit counter', + 64, + 32, + (c, b) => ChaCha20(key32, _fill(12, 3, 5), Nonce64.int32(c.low)) + .convert(Uint8List(b * 64)), + ), + _Subject( + 'ChaCha20/64-bit counter', + 64, + 64, + (c, b) => ChaCha20(key32, _fill(8, 3, 5), Nonce64.int32(c.low, c.high)) + .convert(Uint8List(b * 64)), + ), + _Subject( + 'XChaCha20', + 64, + 64, + (c, b) => XChaCha20(key32, _fill(24, 3, 5), Nonce64.int32(c.low, c.high)) + .convert(Uint8List(b * 64)), + ), + _Subject( + 'Salsa20', + 64, + 64, + (c, b) => Salsa20(key32, _fill(8, 3, 5), Nonce64.int32(c.low, c.high)) + .convert(Uint8List(b * 64)), + ), + _Subject( + 'XSalsa20', + 64, + 64, + (c, b) => XSalsa20(key32, _fill(24, 3, 5), Nonce64.int32(c.low, c.high)) + .convert(Uint8List(b * 64)), + ), + ]; + + for (final s in subjects) { + group(s.name, () { + test('keystream is continuous across every carry boundary', () { + const run = 4; + for (final start in _boundaries(s.counterBits)) { + final actual = s.keystream(start, run); + expect(actual.length, equals(run * s.blockSize)); + for (int j = 0; j < run; ++j) { + expect( + toHex(actual.sublist(j * s.blockSize, (j + 1) * s.blockSize)), + equals(toHex(s.keystream(start.plus(j, s.counterBits), 1))), + reason: '[start: $start, block: $j]', + ); + } + } + }); + + test('keystream stays aligned over a 1 MiB message', () { + const size = 1048576; + // Positioned so the 16-bit carry lands eight blocks in; the remainder + // of the message exercises large-index output addressing, which is + // where the counter and the output offset can drift apart. + const start = _Counter(0, 0x0000FFF8); + final blocks = size ~/ s.blockSize; + + final actual = s.keystream(start, blocks); + expect(actual.length, equals(size)); + + for (final j in [0, 7, 8, 9, 15, blocks - 2, blocks - 1]) { + expect( + toHex(actual.sublist(j * s.blockSize, (j + 1) * s.blockSize)), + equals(toHex(s.keystream(start.plus(j, s.counterBits), 1))), + reason: '[block: $j]', + ); + } + }); + }); + } +}