Skip to content

Fix stale key_method pointer after private key switch in CERT#3085

Merged
justsmth merged 3 commits intoaws:mainfrom
justsmth:fix-ssl-stale-key_method
Mar 16, 2026
Merged

Fix stale key_method pointer after private key switch in CERT#3085
justsmth merged 3 commits intoaws:mainfrom
justsmth:fix-ssl-stale-key_method

Conversation

@justsmth
Copy link
Contributor

Description of changes:

key_method and per-slot privatekey on CERT are mutually exclusive — key_method
wins when non-NULL. But several paths didn't clear the other when setting one, so stale
values could silently take effect after switching between a real key and a callback.

Adds SetKeyMethod and SetSlotPrivateKey on CERT that enforce the invariant, and
routes all mutation paths through them.

Call-outs:

  • dc_key_method / dc_privatekey have the same issue — left for a follow-up.
  • SSL_set_private_key_method / SSL_CTX_set_private_key_method stay void for API
    compat; failures go to the error stack.

Testing:

Added SSLTest.KeyMethodPrivateKeyMutualExclusivity — 6 sub-cases covering invariant
checks across all mutation paths and functional handshake checks after key switches in
both directions.

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.

@justsmth justsmth requested a review from a team as a code owner March 10, 2026 17:44
@justsmth justsmth changed the title Fix stale pointer after private key switch in Fix stale key_method pointer after private key switch in CERT Mar 10, 2026
@justsmth justsmth requested a review from skmcgrail March 11, 2026 14:59
@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 88.60759% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.18%. Comparing base (f3c6fff) to head (e9589e6).

Files with missing lines Patch % Lines
ssl/ssl_cert.cc 73.68% 5 Missing ⚠️
ssl/ssl_privkey.cc 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3085      +/-   ##
==========================================
- Coverage   78.36%   78.18%   -0.19%     
==========================================
  Files         689      689              
  Lines      121743   121814      +71     
  Branches    16997    16995       -2     
==========================================
- Hits        95402    95238     -164     
- Misses      25452    25692     +240     
+ Partials      889      884       -5     

☔ 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 enabled auto-merge (squash) March 16, 2026 15:00
@justsmth justsmth merged commit 32bba49 into aws:main Mar 16, 2026
457 of 461 checks passed
@justsmth justsmth deleted the fix-ssl-stale-key_method branch March 16, 2026 17:52
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.

4 participants