Skip to content
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

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

Merged
merged 12 commits into from May 21, 2023

Conversation

zilchms
Copy link
Contributor

@zilchms zilchms commented Feb 20, 2023

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
Copy link
Contributor Author

zilchms commented Feb 21, 2023

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

@smortex
Copy link
Member

smortex commented Feb 22, 2023

@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
Copy link
Member

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

@zilchms
Copy link
Contributor Author

zilchms commented Feb 24, 2023

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
Copy link
Contributor Author

zilchms commented Feb 26, 2023

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 zilchms marked this pull request as ready for review March 10, 2023 22:13
@zilchms
Copy link
Contributor Author

zilchms commented Mar 10, 2023

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
Copy link
Contributor Author

zilchms commented Mar 20, 2023

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

@zilchms zilchms changed the title CA signing add ability to certificate provider to get signed against a CA cert May 21, 2023
@bastelfreak bastelfreak added the enhancement New feature or request label May 21, 2023
@bastelfreak bastelfreak merged commit 0a6cc8b into voxpupuli:master May 21, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants