diff --git a/.github/workflows/flutter_ci.yaml b/.github/workflows/flutter_ci.yaml index 93cf400..fafd880 100644 --- a/.github/workflows/flutter_ci.yaml +++ b/.github/workflows/flutter_ci.yaml @@ -2,6 +2,11 @@ name: '[CI]' on: pull_request: + # Also run on direct pushes to main. Without this, anything pushed straight + # to main skips CI entirely - which is how the encrypt 5.0.3 IV regression + # shipped despite the existing round-trip tests catching it. + push: + branches: [main] workflow_dispatch: jobs: @@ -13,20 +18,19 @@ jobs: - uses: subosito/flutter-action@v2 with: - flutter-version: "3.10.0" + flutter-version: "3.44.2" channel: 'stable' cache: true - - uses: dart-lang/setup-dart@v1 - with: - sdk: 3.0.0 - - name: Install dependencies run: flutter pub get + # NOTE: `dart format --fix` was removed in newer Dart SDKs; formatting + # is written in place by default. Applying lint fixes is now a separate + # command (`dart fix --apply`). - name: Verify formatting - run: dart format . --fix + run: dart format . - name: Analyze project source run: flutter analyze . --fatal-infos --no-pub diff --git a/.github/workflows/flutter_pub_cd.yaml b/.github/workflows/flutter_pub_cd.yaml index 1501bc2..6eb8e66 100644 --- a/.github/workflows/flutter_pub_cd.yaml +++ b/.github/workflows/flutter_pub_cd.yaml @@ -17,7 +17,7 @@ jobs: - uses: subosito/flutter-action@v2 with: - flutter-version: "3.7.0" + flutter-version: "3.44.2" channel: 'stable' cache: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 880c854..6dc2716 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # qr_plus changelog +## 1.4.0 + +- fix: snowden mode could not decrypt its own output. `encrypt` 5.0.3 changed + `IV.fromLength(16)` to return RANDOM bytes instead of zeros, so the renderer + and the reader each generated a different IV and every code failed to decrypt + with "Invalid or corrupted pad block". Now uses `IV.allZerosOfLength(16)`, + which restores the previous behaviour and keeps the wire format + byte-identical to 1.3.0 and earlier, so older clients still interoperate. +- feat: added `onError` to `QrPlusReader`, reporting a `QrPlusReadError` when a + code is detected but cannot be decoded. Previously such failures were + swallowed silently, making an undecryptable code indistinguishable from the + camera seeing nothing at all. +- test: added crypto regression tests pinning the wire format, and fixed a TTL + test that asserted nothing because it used a mode without a TTL. +- ci: run on pushes to `main`, not just pull requests. The 1.3.0 dependency + update was pushed straight to `main` and skipped CI entirely, which is why + the existing round-trip tests never flagged the regression. + ## 1.3.0 - fix: fix issues with snowden mode diff --git a/example/pubspec.lock b/example/pubspec.lock index c22ff14..7753d6e 100644 --- a/example/pubspec.lock +++ b/example/pubspec.lock @@ -542,7 +542,7 @@ packages: path: ".." relative: true source: path - version: "1.3.0" + version: "1.4.0" screen_capture_event: dependency: transitive description: diff --git a/lib/qr_plus.dart b/lib/qr_plus.dart index 378f92f..4ca2aa0 100644 --- a/lib/qr_plus.dart +++ b/lib/qr_plus.dart @@ -6,4 +6,4 @@ export 'src/feature/reader/controller/controller.dart'; export 'src/feature/reader/view/view.dart' show QrPlusReader; export 'src/feature/renderer/view/view.dart' show QrPlusRenderer; export 'src/model/model.dart' show QrPlusMode; -export 'src/utility/utility.dart' show QrPlusAuthenticity; +export 'src/utility/utility.dart' show QrPlusAuthenticity, QrPlusReadError; diff --git a/lib/src/feature/reader/cubit/qr_plus_reader_cubit.dart b/lib/src/feature/reader/cubit/qr_plus_reader_cubit.dart index 65a580b..dcaa8f4 100644 --- a/lib/src/feature/reader/cubit/qr_plus_reader_cubit.dart +++ b/lib/src/feature/reader/cubit/qr_plus_reader_cubit.dart @@ -10,6 +10,7 @@ class QrPlusReaderCubit extends Cubit { required this.mode, required this.onData, required this.ntpRepository, + this.onReadError, this.allowDuplicates = false, }) : super(const QrPlusReaderState()); @@ -25,6 +26,11 @@ class QrPlusReaderCubit extends Cubit { List authenticity, ) onData; + /// Called when a QR code was detected but could not be turned into usable + /// data. Without this the failure is invisible: the reader ignores the code + /// and the user sees a camera that never reacts. + final void Function(QrPlusReadError error)? onReadError; + /// Whether to call [onData] on duplicate detections or not. final bool allowDuplicates; @@ -42,7 +48,14 @@ class QrPlusReaderCubit extends Cubit { /// [id] being null means the data is [UnknownQrPlusData], which we won't process. final uid = data.maybeUid; - if (uid == null) return; + if (uid == null) { + /// The code was readable as a QR code but its contents could not be + /// decrypted or parsed. In snowden mode this usually means the reader and + /// the renderer disagree on the encryption key. + onReadError?.call(QrPlusReadError.unreadable); + + return; + } final cachedData = state.cache[uid]; @@ -82,6 +95,12 @@ class QrPlusReaderCubit extends Cubit { requiredMode: mode, ); + if (isWhole && !valid) { + /// All crumbs arrived but they were produced with a different mode than + /// this reader is configured with. + onReadError?.call(QrPlusReadError.modeMismatch); + } + if (valid) { /// Converts the crumbs into a string final dataList = [...newCrumbs]..sort( diff --git a/lib/src/feature/reader/view/qr_plus_reader.dart b/lib/src/feature/reader/view/qr_plus_reader.dart index 1c27988..5197f16 100644 --- a/lib/src/feature/reader/view/qr_plus_reader.dart +++ b/lib/src/feature/reader/view/qr_plus_reader.dart @@ -10,6 +10,7 @@ class QrPlusReader extends StatefulWidget { required this.onData, this.mode = const QrPlusMode.plain(), super.key, + this.onError, this.controller, this.fit = BoxFit.cover, @Deprecated('Use DetectionSpeed on QrPlusReaderController instead') @@ -42,6 +43,14 @@ class QrPlusReader extends StatefulWidget { List authenticity, ) onData; + /// Called when a QR code was detected but could not be turned into usable + /// data, with the reason why. See [QrPlusReadError]. + /// + /// Handling this is strongly recommended: without it a decryption or parsing + /// failure is completely silent, and an unreadable code is indistinguishable + /// from the camera seeing nothing at all. + final void Function(QrPlusReadError error)? onError; + /// Handles how the widget should fit the screen. final BoxFit fit; @@ -83,6 +92,7 @@ class _QrPlusReaderState extends State { mode: widget.mode, ntpRepository: _ntpRepository, onData: widget.onData, + onReadError: widget.onError, ), child: BlocBuilder( buildWhen: (_, __) => false, diff --git a/lib/src/model/src/qr_plus_data_crumb.dart b/lib/src/model/src/qr_plus_data_crumb.dart index f55da4f..977b7de 100644 --- a/lib/src/model/src/qr_plus_data_crumb.dart +++ b/lib/src/model/src/qr_plus_data_crumb.dart @@ -185,10 +185,17 @@ class QrPlusDataCrumb with _$QrPlusDataCrumb { return null; } + // NOTE: must be IV.allZerosOfLength, NOT IV.fromLength. As of encrypt + // 5.0.3 IV.fromLength returns RANDOM bytes rather than zeros, so the + // renderer and the reader each generate a different IV and decryption + // fails with "Invalid or corrupted pad block". allZerosOfLength also keeps + // the wire format byte-identical to encrypt 5.0.1, so older clients still + // interop. Covered by test/model/qr_plus_crypto_regression_test.dart. try { final encrypted = Encrypted.from64(data); final encrypter = Encrypter(AES(Key.fromUtf8(encryptionKey))); - final decrypted = encrypter.decrypt(encrypted, iv: IV.fromLength(16)); + final decrypted = + encrypter.decrypt(encrypted, iv: IV.allZerosOfLength(16)); return tryParseJson(decrypted); } catch (e) { @@ -210,7 +217,7 @@ class QrPlusDataCrumb with _$QrPlusDataCrumb { final encrypted = encrypter.encrypt( jsonEncode(toJson()), - iv: IV.fromLength(16), + iv: IV.allZerosOfLength(16), ); return encrypted.base64; diff --git a/lib/src/utility/src/qr_plus_read_error.dart b/lib/src/utility/src/qr_plus_read_error.dart new file mode 100644 index 0000000..1a50eb0 --- /dev/null +++ b/lib/src/utility/src/qr_plus_read_error.dart @@ -0,0 +1,26 @@ +/// {@template qr_plus_read_error} +/// Describes why a scanned QR code could not be turned into usable data. +/// +/// Without this, a failure to decode is indistinguishable from the camera +/// simply not seeing anything: the reader silently ignores the code and the +/// user is left staring at a viewfinder that never reacts. Surfacing the reason +/// lets the app tell the user what is wrong, and makes misconfiguration +/// (notably a mismatched encryption key) obvious instead of invisible. +/// {@endtemplate} +enum QrPlusReadError { + /// {@macro qr_plus_read_error} + /// + /// The QR code was read, but its contents could not be decrypted or parsed. + /// + /// In snowden mode this almost always means the reader and the renderer + /// disagree on the encryption key, or the code was not produced by + /// package:qr_plus at all. + unreadable, + + /// {@macro qr_plus_read_error} + /// + /// The data was decoded, but it was created with a different QrPlusMode + /// than the reader is configured with. The renderer and reader must use the + /// same mode. + modeMismatch, +} diff --git a/lib/src/utility/utility.dart b/lib/src/utility/utility.dart index 1e7d1e1..f96439c 100644 --- a/lib/src/utility/utility.dart +++ b/lib/src/utility/utility.dart @@ -1,4 +1,5 @@ export 'src/connectivity_result_extension.dart'; export 'src/qr_plus_authenticity.dart'; +export 'src/qr_plus_read_error.dart'; export 'src/screen_recorder_status.dart'; export 'src/string_extension.dart'; diff --git a/pubspec.yaml b/pubspec.yaml index 7b43af8..26efb4a 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,6 @@ name: qr_plus description: Advanced all-in-one qr code package with support for safe qr codes. -version: 1.3.0 +version: 1.4.0 homepage: https://mankeli.co repository: https://github.com/Mankeli-Software/qr_plus issue_tracker: https://github.com/Mankeli-Software/qr_plus/issues diff --git a/test/feature/reader/controller/qr_plus_reader_controller_test.dart b/test/feature/reader/controller/qr_plus_reader_controller_test.dart index d80425a..7b88a19 100644 --- a/test/feature/reader/controller/qr_plus_reader_controller_test.dart +++ b/test/feature/reader/controller/qr_plus_reader_controller_test.dart @@ -8,13 +8,18 @@ void main() { TestWidgetsFlutterBinding.ensureInitialized(); group('QrPlusReaderController', () { test( - 'barcodes will return empty stream ' + 'barcodes forwards the underlying scanner stream ' 'always', () { - expect( - QrPlusReaderController().barcodes, - const Stream.empty(), - ); + // This used to return const Stream.empty(). 44e26b0 ("fixes issues + // where reader is too slow") deliberately changed it to forward + // super.barcodes, but this test was never updated and had been failing + // ever since. The getter stays deprecated because the values it emits + // are raw, still-encoded scanner data: consumers want onData instead. + final controller = QrPlusReaderController(); + + expect(controller.barcodes, isA>()); + expect(controller.barcodes.isBroadcast, isTrue); }, ); }); diff --git a/test/feature/reader/cubit/qr_plus_reader_error_test.dart b/test/feature/reader/cubit/qr_plus_reader_error_test.dart new file mode 100644 index 0000000..7c2af60 --- /dev/null +++ b/test/feature/reader/cubit/qr_plus_reader_error_test.dart @@ -0,0 +1,130 @@ +// Tests for the onReadError path. +// +// Before this existed, a QR code that could not be decrypted was discarded in +// silence: no callback, no log, nothing. That made a total crypto failure look +// identical to "the camera isn't seeing the code", which is exactly what made +// the encrypt 5.0.3 IV regression so slow to diagnose. + +import 'package:bloc_test/bloc_test.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:qr_plus/qr_plus.dart'; +import 'package:qr_plus/src/feature/reader/cubit/cubit.dart'; +import 'package:qr_plus/src/model/model.dart'; +import 'package:qr_plus/src/repository/repository.dart'; + +class DataCallback { + void call(String s, List a) {} +} + +class ErrorCallback { + void call(QrPlusReadError error) {} +} + +class MockDataCallback extends Mock implements DataCallback {} + +class MockErrorCallback extends Mock implements ErrorCallback {} + +class MockNtpRepository extends Mock implements NtpRepository {} + +void main() { + late DataCallback onData; + late ErrorCallback onReadError; + late NtpRepository ntpRepository; + + const mode = QrPlusMode.snowden( + encryptionKey: 'abcdefghijklmnopqrstuvwx', + ttl: Duration(minutes: 5), + crumbs: 2, + ); + + setUp(() { + registerFallbackValue(QrPlusReadError.unreadable); + onData = MockDataCallback(); + onReadError = MockErrorCallback(); + ntpRepository = MockNtpRepository(); + + when(() => ntpRepository.now).thenReturn(DateTime(2026)); + when(() => onReadError.call(any())).thenAnswer((_) {}); + }); + + group('QrPlusReaderCubit onReadError', () { + blocTest( + 'is called with unreadable ' + 'when the code cannot be decrypted', + build: () => QrPlusReaderCubit( + mode: mode, + onData: onData.call, + onReadError: onReadError.call, + ntpRepository: ntpRepository, + ), + act: (cubit) => cubit.onRawData('not-a-qr-plus-code'), + verify: (_) => verify( + () => onReadError.call(QrPlusReadError.unreadable), + ).called(1), + ); + + blocTest( + 'is called with unreadable ' + 'when the code was encrypted with a different key', + build: () => QrPlusReaderCubit( + mode: mode, + onData: onData.call, + onReadError: onReadError.call, + ntpRepository: ntpRepository, + ), + act: (cubit) { + final foreign = QrPlusDataCrumb.authentic( + uid: 'uid', + data: 'data', + mode: const QrPlusMode.snowden( + encryptionKey: 'XXXXXXXXXXXXXXXXXXXXXXXX', + ), + timestamp: DateTime.utc(2026), + index: 0, + crumbs: 2, + ).toQrString(); + + return cubit.onRawData(foreign); + }, + verify: (_) => verify( + () => onReadError.call(QrPlusReadError.unreadable), + ).called(1), + ); + + blocTest( + 'is not called ' + 'when the code decrypts successfully', + build: () => QrPlusReaderCubit( + mode: mode, + onData: onData.call, + onReadError: onReadError.call, + ntpRepository: ntpRepository, + ), + act: (cubit) { + final valid = QrPlusDataCrumb.authentic( + uid: 'uid', + data: 'data', + mode: mode, + timestamp: DateTime.utc(2026), + index: 0, + crumbs: 2, + ).toQrString(); + + return cubit.onRawData(valid); + }, + verify: (_) => verifyNever(() => onReadError.call(any())), + ); + + blocTest( + 'does not throw ' + 'when no onReadError handler is provided', + build: () => QrPlusReaderCubit( + mode: mode, + onData: onData.call, + ntpRepository: ntpRepository, + ), + act: (cubit) => cubit.onRawData('not-a-qr-plus-code'), + ); + }); +} diff --git a/test/model/qr_plus_crypto_regression_test.dart b/test/model/qr_plus_crypto_regression_test.dart new file mode 100644 index 0000000..12b9123 --- /dev/null +++ b/test/model/qr_plus_crypto_regression_test.dart @@ -0,0 +1,110 @@ +// Regression tests for snowden-mode encryption. +// +// These exist because of a silent, total breakage: encrypt 5.0.3 changed +// IV.fromLength(16) from returning all zeros to returning RANDOM bytes. Since +// the renderer and the reader each build their own IV, every QR produced became +// undecryptable ("Invalid or corrupted pad block"), and because the failure was +// swallowed by a catch in tryParseBase64 the reader simply did nothing at all. +// +// The golden test below is the important one: it pins the exact wire format, so +// ANY change in crypto behaviour (package upgrade, IV, mode, padding) fails here +// instead of silently shipping QR codes that no device can read. + +import 'package:flutter_test/flutter_test.dart'; +import 'package:qr_plus/src/model/model.dart'; + +void main() { + group('snowden mode crypto', () { + const encryptionKey = 'abcdefghijklmnopqrstuvwx'; // 24 bytes / AES-192 + const mode = QrPlusMode.snowden( + encryptionKey: encryptionKey, + ttl: Duration(minutes: 5), + crumbs: 2, + ); + + QrPlusDataCrumb buildCrumb() => QrPlusDataCrumb.authentic( + uid: '11111111-2222-3333-4444-555555555555', + data: '{"id":"16","na', + mode: mode, + timestamp: DateTime.utc(2026, 1, 2, 3, 4, 5), + index: 0, + crumbs: 2, + ); + + // Produced by a known-good implementation (encrypt 5.0.1, and 5.0.3 with + // IV.allZerosOfLength). Both render byte-identical output, so an app on the + // older release can still read codes from a newer one. If this test fails, + // the wire format has changed and older clients WILL break. + const golden = + 'gGg5rNbtIlMGT/AXNYPg73ytUXZYArBomNW39yUlRbH/ZZjcvEJMk57uFgh29oFmZHRmC6kv' + 'WJs/i+uGrVS4wJUssyQE0+WGN4pxC3Bd1+0yP1jKU3X+CTfg9EtYJT5sIcczio1zOzf1cNz0' + 'KcR5BxEThJ/nptU9hLQfUV11rb3CLhjfo+I15zTLAQabk5u4czkh18tSVQ8Cz/cPX0HkX0io' + '+d+SNLt3O4yzG88BzYiXuCBE2Xhuscv0zHnrHvErULkT6D0IcXacRE4BwiUnybeEu7rifIFT' + 'ZLW+m3kk3AU='; + + test( + 'produces the pinned wire format ' + 'so older clients can still decrypt new codes', + () { + expect(buildCrumb().toQrString(), golden); + }, + ); + + test( + 'decrypts the pinned wire format ' + 'so new clients can still read codes from older releases', + () { + final parsed = QrPlusDataCrumb.fromQrString(golden, mode); + + expect(parsed, isA()); + expect(parsed.maybeUid, '11111111-2222-3333-4444-555555555555'); + expect(parsed.maybeData, '{"id":"16","na'); + expect(parsed.maybeIndex, 0); + expect(parsed.maybeCrumbs, 2); + }, + ); + + test( + 'round trips ' + 'when encrypting and decrypting with the same key', + () { + final parsed = QrPlusDataCrumb.fromQrString( + buildCrumb().toQrString(), + mode, + ); + + expect(parsed.maybeUid, buildCrumb().maybeUid); + expect(parsed.maybeData, buildCrumb().maybeData); + }, + ); + + test( + 'is deterministic ' + 'so the rendered QR does not change between frames', + () { + // A random IV would make every render a different image, which is what + // encrypt 5.0.3 silently introduced. + final outputs = List.generate(5, (_) => buildCrumb().toQrString()); + + expect(outputs.toSet(), hasLength(1)); + }, + ); + + test( + 'returns unknown ' + 'when decrypting with a different key', + () { + final parsed = QrPlusDataCrumb.fromQrString( + golden, + const QrPlusMode.snowden( + encryptionKey: 'XXXXXXXXXXXXXXXXXXXXXXXX', + ttl: Duration(minutes: 5), + crumbs: 2, + ), + ); + + expect(parsed, isA()); + }, + ); + }); +} diff --git a/test/model/qr_plus_data_crumb_test.dart b/test/model/qr_plus_data_crumb_test.dart index 63bd0ff..b454f79 100644 --- a/test/model/qr_plus_data_crumb_test.dart +++ b/test/model/qr_plus_data_crumb_test.dart @@ -34,14 +34,48 @@ void main() { 'is false ' 'when ttl has passed', () { + // QrPlusMode.safe carries no ttl at all, so isTTLValid is always + // true for it. Use a mode that actually defines one (robust + // defaults to 20 seconds) or this asserts nothing. + final expiring = QrPlusDataCrumb.authentic( + uid: 'uid', + data: 'data', + mode: const QrPlusMode.robust(), + timestamp: DateTime(2026), + index: 1, + crumbs: 2, + ); + expect( - crumb.isTTLValid( - now: DateTime.now().add(const Duration(seconds: 11)), + expiring.isTTLValid( + now: DateTime(2026).add(const Duration(seconds: 21)), ), isFalse, ); }, ); + + test( + 'is true ' + 'when ttl has not passed', + () { + final fresh = QrPlusDataCrumb.authentic( + uid: 'uid', + data: 'data', + mode: const QrPlusMode.robust(), + timestamp: DateTime(2026), + index: 1, + crumbs: 2, + ); + + expect( + fresh.isTTLValid( + now: DateTime(2026).add(const Duration(seconds: 19)), + ), + isTrue, + ); + }, + ); }); group('fromAuthenticity', () {