voxpupuli / puppet-openssl

Puppet OpenSSL module
Apache License 2.0
38 stars 84 forks source link

add ability to certificate provider to get signed against a CA cert #153

Closed zilchms closed 1 year ago

zilchms commented 1 year ago

Pull Request (PR) description

This Pull requests adresses parts of #152 and should allow users to provide a CA certificate and key to sign generated certificates.

This Pull Request (PR) fixes the following issues

n/a

zilchms commented 1 year ago

öhhhm, no idea why the static validation is failing. has someone suggestions?

smortex commented 1 year ago

@zilchms the CI report says:

REFERENCE.md is outdated

Make sure your bundle is up-to-date and re-generate the doc:

bundle update
bundle exec rake reference
bastelfreak commented 1 year ago

@zilchms is this ready for review or still a draft?

zilchms commented 1 year ago

i would say this needs some valdation checks on the inputs and rspec tests. however i have never written any rspec tests and that would need some (or a lot, depending on life stuff) time to finish. I havent come around to testing the module either, but maybe i find some time tomorrow/tonight.

zilchms commented 1 year ago

ok, so. couple of things: there seems to be some more stuff that is inconsistent/ needs to be addressed. The x509_cert provider uses the openssl req command with the -new x509 option (that works for many certs, but not all. especially this does not allow the usage of the -ca* flags) openssl x509 seems to be the correct alternative (especially considering x509 is hardcoded in the x509_cert provider) Changing this however of course breaks tests... and maybe backward compatability...

As the x509 req command builds the certificate request while building the cert, once we change the command we need to be able to pass the csr to the cert via parameter. This should hopefully be no problem, as the certificate::x509 class already ensures the csr is created.

For some of the parameters previously used parameters, the openssl x509 command has no equivalents (as far as i could tell). Maybe someone wants to have a look at that, so I didnt overlook something. For now I am going to have a look at the rspec tests.

zilchms commented 1 year ago

This unfortunately only ductapes the CA signing onto the x509_cert provider. And signing the cert against a CA needs a csr to be specified. I know the solution is not really clean, however it preserves backwards compatibility.

To clean this up, the openssl::certificate::x509 class should always hand the csr it generates through to the crt it tries to generate. Then the provider does not need to differentiate between getting a key or a csr, getting rid of the big if statement in the creation clause. This however breaks backwards compatibility, as the private key the cert provider receives is no longer necessary and should be removed.

I can provide the relevant pull-request for this, should this PR go through, just let me know. Additionally: Yes, the CA signing feature is not feature-complete in regards to all possible options yet. I will come around to it, when I find the time.

Edit: currently extkeyusage, altnames and other config specific functionalities dont work in conjunction with ca signing for the openssl::certificate::x509 class. this would require the openssl::certificate::x509 class to hand the csr to the crt provider as mentioned above.

zilchms commented 1 year ago

@bastelfreak @smortex maybe someone of you finds the time to review this? :)