-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Service Indicator to handle custom crypto through *_METHOD structs #1857
Conversation
e4b7afc
to
e41c2ae
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1857 +/- ##
==========================================
+ Coverage 78.46% 78.47% +0.01%
==========================================
Files 585 585
Lines 99457 99511 +54
Branches 14236 14248 +12
==========================================
+ Hits 78038 78094 +56
+ Misses 20784 20781 -3
- Partials 635 636 +1 ☔ View full report in Codecov by Sentry. |
ASSERT_TRUE(EVP_DigestSignFinal(md_ctx.get(), | ||
signature.data(), | ||
&sig_len))); | ||
ASSERT_STREQ(static_cast<const char*>(EC_KEY_get_ex_data(eckey.get(), 0)), "ecdsa_sign"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the custom EC sign is well defined and checked for the data it generates while this is not the case for RSA test? Is it more tedious for RSA?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is more complex to emulate the signing behavior for RSA keys. Even more so because we'd have to define both the sign and sign_raw functionalities (since RSA_PSS keys directly call sign_raw). It shouldn't matter either way, all we care about is the existence of custom function pointers (sort of hand waving what actually happens in those custom pointers however).
Issues:
CryptoAlg-2633
Description of changes:
The service indicator "update" functions were modified to check for the existence of custom crypto in RSA_METHOD and EC_KEY_METHOD structs. If custom crypto is utilized for a given operation, the service indicator should not be updated. The checks are conservative and do not update the service indicator for a signing operation for EC/RSA keys if either custom sign or sign_sig/sign_raw functionality is provided - regardless of whether these may be invoked.
Testing:
Tested both EC_KEY_METHOD and RSA_METHOD with custom crypto and ensured the service indicator was not updating.
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.