Conversation
nhatnghiho
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
nhatnghiho
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
nhatnghiho
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
nhatnghiho
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
nhatnghiho
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
nhatnghiho
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3024 +/- ##
==========================================
+ Coverage 78.31% 78.34% +0.02%
==========================================
Files 689 689
Lines 120995 121010 +15
Branches 16971 16990 +19
==========================================
+ Hits 94758 94803 +45
+ Misses 25341 25312 -29
+ Partials 896 895 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| BIGNUM *d = BN_new(); | ||
| BIGNUM *n = BN_new(); | ||
| BIGNUM *e = BN_new(); |
There was a problem hiding this comment.
This should use bssl::UniquePtr<BIGNUM> to avoid leaks and be consistent with others.
There was a problem hiding this comment.
These values will be then used in bssl::UniquePtr<RSA> RSA key so they will be taken care there I believe.
| bssl::UniquePtr<RSA> key(RSA_new()); | ||
| if (!BN_bin2bn(n_bytes.data(), n_bytes.size(), n) || | ||
| !BN_bin2bn(e_bytes.data(), e_bytes.size(), e) || | ||
| !BN_bin2bn(d_bytes.data(), d_bytes.size(), d) || | ||
| !RSA_set0_key(key.get(), n, e, d)) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Should we add || !RSA_check_key(key.get()) to this condition, similar to what's in RSADecryptionPrimitive?
There was a problem hiding this comment.
It's not obvious but RSA_new_private_key_large_e runs RSA_check_key internally (code). RSADecryptionPrimitive uses a public key instead. But we don't have a correspondingRSA_new_public_key_large_e so hence this manual check.
nhatnghiho
had a problem deploying
to
auto-approve
GitHub Actions
Error
nhatnghiho
had a problem deploying
to
auto-approve
GitHub Actions
Failure
nhatnghiho
had a problem deploying
to
auto-approve
GitHub Actions
Error
nhatnghiho
had a problem deploying
to
auto-approve
GitHub Actions
Error
nhatnghiho
had a problem deploying
to
auto-approve
GitHub Actions
Error
nhatnghiho
had a problem deploying
to
auto-approve
GitHub Actions
Error
3 participants
Issues:
Addresses P355857148
Description of changes:
This PR adds ACVP support for:
RSA signaturePrimitiveRSA decryptionPrimitiveKAS-ECC-SSC onePassDhECDSA sigGen componentTestCall-outs:
Due to the random nature of ECDH and ECDSA sigGen, no expected vector was added for these 2. To review, please run the script locally to verify the result.
Testing:
Run
check_expected.goon the new test vectors.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.