Reject imported EC and RSA keys with trailing bytes#251
Reject imported EC and RSA keys with trailing bytes#251harrshita123 wants to merge 2 commits intogoogle:masterfrom
Conversation
| final k = ssl.EVP_parse_private_key(cbs); | ||
|
|
||
| _checkData( | ||
| k.address != 0 && cbs.ref.len == 0, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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.