Reject imported EC and RSA keys with trailing bytes

[harrshita123]

This change ensures that all bytes supplied during key import are consumed by BoringSSL.

EC and RSA private/public key imports now verify that the CBS is empty after parsing, rejecting inputs with trailing bytes. This resolves the TODO in EC public key import and aligns behavior with WebCrypto implementations.

Tests for malformed key bytes are not included here, as generating invalid key material is currently blocked by #55.

Related to #60.

[jonasfj]

We need tests that confirm that this what all the browser implementations also do.

I wonder if we should simply write some hand written tests? that don't necessarily use the testrunner setup..

Maybe, we could do something like:

lib/src/testing/regression/issue_<id>_<name>.dart

And have such files do:

import '../utils/test.dart';

void runTests(void Function(String name, Future<void> Function() test) testFn) {
  test('something', () async {
    ...
  });
}

Then we can have lib/src/testing/testing.dart import and call runTests in its runAllTests function.

@HamdaanAliQuatil WDYT?

[HamdaanAliQuatil]

I like the idea of an issue-specific regression bucket.

Since runAllTests already aggregates non-TestRunner tests like random.dart and digest.dart, I’d follow that existing pattern and add something like lib/src/testing/regression/issue_60_trailing_bytes.dart exposing tests(), then append those in lib/src/testing/testing.dart.

That said, I wouldn’t want regression/issue_<id>_... to become the default pattern for lots of tests. I’d treat it as a narrow escape hatch for browser-compat regressions and other negative cases that don’t cleanly fit the current framework.

[jonasfj]

I'd love to fit as much as possible into TestRunner -- and I do think TestRunner (or something like it) could be used to test exceptions, but fact is also that TestRunner is a bit hard to keep track of today.

It's complicated, and adding support for testing a new kind of thing is just hard, very hard.

So maybe it's fine that we add more one-off test files like this. Also because otherwise how are we going to land these things we can't just land without tests, and we can't wait for TestRunner to support more features.

Also no matter how far we go with TestRunner it'll never be able to cover everything.

[jonasfj]

We could also hardcode these keys, to make tests faster and more consistent.

Maybe it might also be worth testing that before we append zero, we can't import, and when we've appended zero we can't import. This way we're sure it's not because we have hardcoded an invalid :D

[harrshita123]

Done. I replaced generated keys with hardcoded EC/RSA PKCS8 and SPKI keys to make the tests faster
and deterministic. Each test now first imports the valid key successfully, then appends a trailing
byte and verifies that import is rejected.

[jonasfj]

We could add something like this to utils/utils.dart

List<({String name, Future<void> Function() test})> defineTests(
  void Function(void Function(String name, Future<void> Function() testFn) test) tests
) {
  final tests_ = <({String name, Future<void> Function() test})>[];
  void test(String name, Future<void> Function() testFn) =>
      tests_.add((name: name, test: testFn));
  tests(test);
  return tests_;
}

Then this could just be:

void main() => tests.runTests();

final tests = defineTests((test) {
  test('...', () async {
    ...
  });

  ...
});

But we don't have to do that in this PR. I'm happy to land this as is.

[harrshita123]

Yes, the requested regression test coverage is added and wired into runAllTests. I also updated the
tests to use hardcoded keys and verify the original valid key imports before checking rejection
with trailing bytes. I left the defineTests helper out for now since that was marked as optional
for this PR.

[jonasfj]

What happens that is different on web?

[harrshita123]

On web, crypto.subtle.importKey() rejects the malformed PKCS8/SPKI input with trailing bytes, and our JS wrapper maps that browser DataError to FormatException. The FFI backend was previously more permissive because BoringSSL parsed the valid prefix and left the trailing bytes unread. This change makes FFI match the browser behavior by requiring the input to be fully consumed.