Use private_key parameter when creating certificate

[vasilevalex]

Pull Request (PR) description

Use private key created before, not the default value.

This Pull Request (PR) fixes the following issues

Fixes #185

[kenyon]

Seems like we need more test coverage too, since this was broken and no tests failed.

[zilchms]

I am still not sure if we want to allow direct private key passing into the certificate. CSRs exist for exactly this reason. The regression for no longer allowing directly passing keys into certs and using CSRs instead was made specifically for this reason.

To be clear: I am not against allowing passing the private key directly, I just would like to get a good reason to do so

I may also be completely wrong here, been a while since I made those changes

[vasilevalex]

In the simplest case without CA we need some private key to sign the certificate. So openssl is invokeg with -signkey:

puppet-openssl/lib/puppet/provider/x509_cert/openssl.rb

Line 84 in dcd89b5

options << ['-signkey', resource[:private_key]]

with the private key as parameter. And if we don't pass the private key generated earlier, default value would be used.

[fmichea]

From @zilchms : I am still not sure if we want to allow direct private key passing into the certificate. CSRs exist for exactly this reason. The regression for no longer allowing directly passing keys into certs and using CSRs instead was made specifically for this reason.

Just hit this issue as well, I'm a bit of two minds on this. On one hand this bug currently prevents using the key/csr/cert parameters in order to control their naming convention. Plus if these parameters are not used, then the key is indeed passed to the certificate for signing anyway, with the correct name this time so no failure happens. So in the current state of things it doesn't appear to me that this fix should necessarily be blocked short term.

On the other hand I take your point about CSR, and for those that use a CA to then sign the certificates, maybe the right solution would be to allow creating the CNF + KEY + CSR without the certificate, and let that be generated by the CA?

At least in my setup I am working on, I get the certificate because its generated by default, but I dont use it and instead create signed versions from the CSR and my CA to then be uploaded. I could totally use an option to turn of cert generation off since those files are not necessary to me.

Any thoughts?

[ashish1099]

any update on how we go ahead, I just hit the same issue

[yakatz]

It looks like the PR needs a rebase. Running through the comments very quickly, I don't see any objections, so once it can be cleanly merged, we should be able to do so.

[yakatz]

Actually, @zilchms might be opposed. Can you clarify?

[zilchms]

No further objections, feel free to merge after rebasing and passing CI :) At some point I will write some tests and refactor a bit of this module, so bugs/regressions like this dont sneak in unnoticed/without discussion beforehand

[vasilevalex]

Rebased and passed CI

[ekohl]

@vasilevalex sorry, but we've been working on this module today and that introduced another merge conflict. Good news is that we should have acceptance tests now so it should be easier to add an acceptance test to it.

[ekohl]

I took the liberty to rebase your PR.