-
Notifications
You must be signed in to change notification settings - Fork 346
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
pki: T4026: Only emit private keys when available #3655
Conversation
👍 |
Can you please share the before and after op-mode output? |
Before the fix is applied, if you try to run
(I'm copying the certreq from the ticket so I didn't have to generate my own) It's not a problem if the certificate request has been generated internally as well - we've already set the private key in those instances and the user needs to see it. If we're signing a request without knowledge of the private key (which shouldn't be a requirement for a CA), that's when there's a problem. Self-signing and root CA creation always generate a private_key internally, so they're OK. |
With the fix applied, we get what we expect, just a signed certificate:
|
I just realised looking at the output, that the private key passphrase prompt is not relevant when private_key is None. I'll update the patch and re-run tests. |
* install_certificate() code path handles private_key=None & key_passphrase=None OK already * file and console output paths will error trying to encode None as a key * This is only an issue for a couple of the generate_*_sign() functions, where having a null private key is possible * Self-signing and CA creation always generate a private key * Certreqs will generate a private key if not already provided * Do not prompt for a private key passphrase if we aren't giving back a private key
Test scenarios work as expected, starting with a clean 1.5-rolling-202406130020, the patch applied:
Passphrase prompt doesn't show up unless a private key is known. There's no more backtrace without a private key. I can paste the full output if required but there's a lot of it, I'm hoping you get the gist. |
@Mergifyio backport sagitta-stream |
✅ Backports have been created
|
✅ Backports have been created
|
@Mergifyio backport circinus-stream |
✅ Backports have been created
|
Change Summary
Adding a simple check before printing out the private key to a file or console output that it exists, where the code path permits a None private_key.
Types of changes
Related Task(s)
Related PR(s)
Component(s) name
Proposed changes
How to test
Also ran through ca_cert and install combinations successfully.
Smoketest result
Checklist: