Skip to content

Make some more half-empty EVP_PKEY states impossible#3056

Open
nebeid wants to merge 2 commits intoaws:mainfrom
nebeid:bssl-cherry-pick-5b7171f2
Open

Make some more half-empty EVP_PKEY states impossible#3056
nebeid wants to merge 2 commits intoaws:mainfrom
nebeid:bssl-cherry-pick-5b7171f2

Conversation

@nebeid
Copy link
Copy Markdown
Contributor

@nebeid nebeid commented Mar 2, 2026

Note: to be rebased on #3055

Description of changes:

BoringSSL commit google/boringssl@5b7171f: "Make some more half-empty EVP_PKEY states impossible"

Changes (3 files):

  1. crypto/fipsmodule/evp/evp.cEVP_PKEY_assign_RSA, EVP_PKEY_assign_DSA, EVP_PKEY_assign_EC_KEY: check for NULL key before calling evp_pkey_set_method, return 0 early instead of setting the method and then returning key != NULL

  2. crypto/evp_extra/p_dh_asn1.c — Same fix for EVP_PKEY_assign_DH

  3. crypto/evp_extra/evp_extra_test.cc — Added NoHalfEmptyKeys test verifying that EVP_PKEY_set1_*(pkey, nullptr) fails without changing the key type

Adaptations for AWS-LC:

AWS-LC didn't take RSA/DSA/EC_KEY out of the FIPS module (unlike BoringSSL's google/boringssl@044fbc8), so the changes go in crypto/fipsmodule/evp/evp.c rather than separate p_*_asn1.cc files.

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.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@nebeid nebeid changed the title Bssl cherry pick 5b7171f2 Make some more half-empty EVP_PKEY states impossible Mar 2, 2026
@nebeid nebeid marked this pull request as ready for review March 2, 2026 16:44
@nebeid nebeid requested a review from a team as a code owner March 2, 2026 16:44
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.01%. Comparing base (1a308a7) to head (b102b9b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3056   +/-   ##
=======================================
  Coverage   78.00%   78.01%           
=======================================
  Files         689      689           
  Lines      122405   122415   +10     
  Branches    17083    17088    +5     
=======================================
+ Hits        95482    95498   +16     
+ Misses      26026    26020    -6     
  Partials      897      897           

☔ 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.

justsmth
justsmth previously approved these changes Mar 4, 2026
@justsmth justsmth requested a review from skmcgrail March 4, 2026 15:14
@nebeid nebeid requested a review from a team March 4, 2026 15:50
sgmenda
sgmenda previously approved these changes Mar 6, 2026
Cherry-picked from BoringSSL 5b7171f2da10d5dad52d0a2e4426fbc71d4ff146.

EVP_PKEY_{assign,set1}_FOO would check for NULL, return an error, but
still leave the EVP_PKEY assigned to that type on failure. Check for
NULL first, so that we don't leave it in that state.

Adapted for AWS-LC:
- RSA/DSA/EC_KEY assign functions are in crypto/fipsmodule/evp/evp.c
  (not split out into separate p_*_asn1.cc files as in BoringSSL)
- DH assign function is in crypto/evp_extra/p_dh_asn1.c
- Added NoHalfEmptyKeys test (AWS-LC did not have the BoringSSL
  equivalent)
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