test: Add Wycheproof-based AES-CBC-PKCS5 tests#363
Conversation
| // The main requirement is that Valid tests pass | ||
| assert_eq!(failed, 0, "Some valid Wycheproof tests failed"); | ||
|
|
||
| session.close()?; | ||
| pkcs11.finalize()?; |
There was a problem hiding this comment.
I think it's better to close and then assert, since if the assert panics the finalization will not happen (maybe I'm paranoid 😅).
| // The main requirement is that Valid tests pass | |
| assert_eq!(failed, 0, "Some valid Wycheproof tests failed"); | |
| session.close()?; | |
| pkcs11.finalize()?; | |
| session.close()?; | |
| pkcs11.finalize()?; | |
| // The main requirement is that Valid tests pass | |
| assert_eq!(failed, 0, "Some valid Wycheproof tests failed"); |
Implements comprehensive AES-CBC-PKCS5 testing using official Wycheproof test vectors from Google. Tests 216 valid cryptographic operations across multiple key sizes (128/192/256-bit) with PKCS#7 padding. Fixes parallaxsecond#187 Signed-off-by: James Eilers <eilersjames15@gmail.com>
Move session.close() and pkcs11.finalize() before the assert so that if the assert panics, resources are guaranteed to be cleaned up properly. Signed-off-by: James Eilers <eilersjames15@gmail.com>
| passed += 1; | ||
| } | ||
| // Invalid test that succeeded - Note: HSM may not catch all invalid cases | ||
| (wycheproof::TestResult::Invalid, Ok(_)) => { |
There was a problem hiding this comment.
Does that actually happen with some of the test cases? Maybe it would be safe to double-check those and filter-out on invalid cases we do not support?
There was a problem hiding this comment.
I think most of the Invalid tasks have BadPadding flag. which can be used to filter it more precisely.
https://github.com/randombit/wycheproof-rs/blob/master/src/data/aes_cbc_pkcs5_test.json#L424
| failed += 1; | ||
| } | ||
| // Acceptable tests can go either way | ||
| (wycheproof::TestResult::Acceptable, Ok(_)) => { |
There was a problem hiding this comment.
Can we move the | condition on Acceptable above and move it here like:
// Acceptable tests can go either way
(wycheproof::TestResult::Acceptable, Ok(_) | Err(_)) => {There was a problem hiding this comment.
Going through the source file for this input, I do not see any tests marked Acceptable:
https://github.com/randombit/wycheproof-rs/blob/master/src/data/aes_cbc_pkcs5_test.json
Could we prune the branches that are not executed?
4 participants
Implements comprehensive AES-CBC-PKCS5 testing using official Wycheproof test vectors from Google. Tests 216 valid cryptographic operations across multiple key sizes (128/192/256-bit) with PKCS#7 padding.
Fixes #187