Skip to content

Add a test that arbitrary curves can be wrapped in EVP_PKEY#3055

Merged
justsmth merged 3 commits intoaws:mainfrom
nebeid:bssl-cherrypick-2
Mar 6, 2026
Merged

Add a test that arbitrary curves can be wrapped in EVP_PKEY#3055
justsmth merged 3 commits intoaws:mainfrom
nebeid:bssl-cherrypick-2

Conversation

@nebeid
Copy link
Copy Markdown
Contributor

@nebeid nebeid commented Mar 2, 2026

Description of changes:

BoringSSL commit google/boringssl@4d48736:
"Add a test that arbitrary curves can be wrapped in EVP_PKEY"

Changes made (6 files):

  1. crypto/test/test_util.h / test_util.cc — Added shared HexToBIGNUM(const char*) helper (declaration + implementation + #include <openssl/bn.h>)

  2. crypto/fipsmodule/ec/ec_test.cc — Removed duplicate local HexToBIGNUM (now uses the shared one from test_util.h, already included)

  3. crypto/fipsmodule/ecdsa/ecdsa_test.cc — Same removal of duplicate

  4. crypto/fipsmodule/bn/bn_test.cc — Renamed the different-signature HexToBIGNUM(UniquePtr<BIGNUM>*, const char*)HexToBIGNUMWithReturn to avoid conflict (this one returns the
    BN_hex2bn return value, so it's a distinct function)

  5. crypto/evp_extra/evp_extra_test.cc — Added the new TEST(EVPExtraTest, CustomCurve) test

Adaptations for AWS-LC:

  • Path: crypto/evp_extra/ instead of BoringSSL's crypto/evp/
  • Replaced bssl::StringAsBytes("hello") (doesn't exist in AWS-LC) with a plain static const uint8_t msg[] = "hello" + sizeof(msg) - 1

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.

Cherry-picked from BoringSSL 4d48736c2acd6878caa93ac0a628b46bda1227b0.

Conscrypt relies on both this working at the EC_KEY level, but also
EVP_PKEY. Make sure this keeps working, even as we mess around with
EVP_PKEY's EC bits to separate the curves out.

While I'm here, move the common HexToBIGNUM wrapper into test_util.h.

Adapted for AWS-LC:
- File paths differ (crypto/evp_extra/ vs crypto/evp/)
- Replaced bssl::StringAsBytes with plain uint8_t array (not available
  in AWS-LC)
- Renamed bn_test.cc's different-signature HexToBIGNUM to
  HexToBIGNUMWithReturn to avoid conflict with the shared one
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.55%. Comparing base (e0cf5f8) to head (e5ef5fd).
⚠️ Report is 48 commits behind head on main.

Files with missing lines Patch % Lines
crypto/fipsmodule/bn/bn_test.cc 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3055      +/-   ##
==========================================
+ Coverage   78.38%   78.55%   +0.16%     
==========================================
  Files         689      689              
  Lines      121129   121149      +20     
  Branches    16968    16973       +5     
==========================================
+ Hits        94948    95165     +217     
+ Misses      25287    25084     -203     
- Partials      894      900       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nebeid nebeid marked this pull request as ready for review March 2, 2026 16:42
@nebeid nebeid requested a review from a team as a code owner March 2, 2026 16:42
// compatibility with one legacy application, and supported in only a limited
// form. (E.g. such keys cannot be serialized.)
TEST(EVPExtraTest, CustomCurve) {
// Coefficients for brainpoolp256r1, not supported by BoringSSL.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: "..., not supported by AWS-LC."?


// HexToBIGNUM decodes |hex| as a hexadecimal, big-endian, unsigned integer and
// returns it as a |BIGNUM|, or nullptr on error.
bssl::UniquePtr<BIGNUM> HexToBIGNUM(const char *hex);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: We should also add #include <openssl/bn.h> to this header since we're referencing BIGNUM.

@sgmenda sgmenda self-assigned this Mar 6, 2026
@justsmth justsmth merged commit 88045d3 into aws:main Mar 6, 2026
444 of 455 checks passed
WillChilds-Klein pushed a commit to WillChilds-Klein/aws-lc that referenced this pull request Mar 11, 2026
### Description of changes: 
BoringSSL commit
google/boringssl@4d48736:
"Add a test that arbitrary curves can be wrapped in EVP_PKEY"

Changes made (6 files):

1. crypto/test/test_util.h / test_util.cc — Added shared
`HexToBIGNUM(const char*)` helper (declaration + implementation +
#include <openssl/bn.h>)

2. crypto/fipsmodule/ec/ec_test.cc — Removed duplicate local
`HexToBIGNUM` (now uses the shared one from test_util.h, already
included)

3. crypto/fipsmodule/ecdsa/ecdsa_test.cc — Same removal of duplicate

4. crypto/fipsmodule/bn/bn_test.cc — Renamed the different-signature
`HexToBIGNUM(UniquePtr<BIGNUM>*, const char*)` → `HexToBIGNUMWithReturn`
to avoid conflict (this one returns the
BN_hex2bn return value, so it's a distinct function)

5. crypto/evp_extra/evp_extra_test.cc — Added the new
`TEST(EVPExtraTest, CustomCurve)` test

### Adaptations for AWS-LC:
- Path: crypto/evp_extra/ instead of BoringSSL's crypto/evp/
- Replaced `bssl::StringAsBytes("hello")` (doesn't exist in AWS-LC) with
a plain `static const uint8_t msg[] = "hello" + sizeof(msg) - 1`

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.

Co-authored-by: David Benjamin <davidben@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants