-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
crypto: fix X509* leak in --use-system-ca #56832
base: main
Are you sure you want to change the base?
Conversation
Review requested:
|
The X509 structures are never freed. Use ncrypto::X509Pointer to manage it automatically and move the X509* to PEM conversion into a helper to be reused by integration in other systems.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #56832 +/- ##
==========================================
- Coverage 89.20% 89.20% -0.01%
==========================================
Files 663 663
Lines 192012 192028 +16
Branches 36929 36934 +5
==========================================
+ Hits 171286 171291 +5
- Misses 13582 13597 +15
+ Partials 7144 7140 -4
|
// TODO(joyeecheung): it is a bit excessive to do this X509 -> PEM -> X509 | ||
// dance when we could've just pass everything around in binary. Change the | ||
// root_certs to be embedded as DER so that we can save the serialization | ||
// and deserialization. |
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.
💯
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.
Thanks for the follow up!
The X509 structures are never freed. Use ncrypto::X509Pointer to manage it automatically and move the X509* to PEM conversion into a helper to be reused by integration in other systems.