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

Fix intermediate ca, add dns name constraints and provide data sources to convert between different certificate formats #309

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

taliesins
Copy link

@taliesins taliesins commented Jan 19, 2023

Fix intermediate ca and add dns name constraints

When creating an intermediate CA certificate we need to be able to set set_authority_key_id otherwise authority key id is excluded from certificate when creating an intermediate CA certificate, which causes problems for validation of chain as we don't know the parent certificate. This relates to issues reported in: #66 (a normal certificate does automatically set authority key id but intermediate does not.

Add support for certificate domain name certificate constraints. This is useful as we can limit the domains that people can issue domains for. Note: this verification is done on the client and it does not stop people from issuing an invalid certificate. To verify the client use https://bettertls.com/. Most modern browsers support it.

Also included #301 9f57550 for max length path. This is useful as we can prevent people from using the CA or intermediate CA from creating more intermediate CAs. Note: the verification is done on the client, see not for domain name constraints/

Added datasources to convert between pfx and pem certificates

When dealing with AWS, pem format is commonly used, but when dealing with Azure, pfx format is commonly used. We wanted to provide a way to easily convert between different certificate formats, as there does not seem to be an existing provider that allows two way conversion or has limited coverage. This solves #29 , #36 , #91.

We have added tls_pfx_to_pem and tls_pem_to_pfx datasources so that we can convert easily between the pem and pfx/pkcs12 certificate formats. The supported cryptographic algorithms are: RSA, DSA, ECDSA, ED25519 .

We did not expose some of the internal conversion settings as we wanted to simplify usage and be as secure as possible. These settings are defaulted internally to the most secure methods available. This still allows us to read less secure certificates, it just affects the newly created certificates.

@taliesins taliesins requested a review from a team as a code owner January 19, 2023 20:59
@taliesins
Copy link
Author

@bendbennett are you the right guy to approve the workflow and start the journey of getting this merged in?

@taliesins
Copy link
Author

@evanphx @hornbeck @dylanegan do you know who could approve the workflow for this? Its relatively small and makes a couple of cool use cases available.

@taliesins
Copy link
Author

@bendbennett could you please approve the workflow and review this PR

@taliesins
Copy link
Author

Hi @bendbennett @kmoe @apparentlymart @bflad @radeksimko

How do I get someone to approve the workflow? This PR fixes a bug in a relatively common use case (Intermediate CA). Is this provider still an active project?

@bendbennett
Copy link
Contributor

Hi @taliesins 👋

I have approved the workflow but I'm afraid that reviewing this PR is currently on-hold as we need to triage the changes and our team’s focus is currently elsewhere at the moment.

@taliesins
Copy link
Author

@bendbennett I am keeping this PR up to date. Honestly this shouldn't take someone more that an hour to get merged in.

@taliesins
Copy link
Author

@bendbennett keeping this guy up to date. Using this update to the provider and a secret store I have successfully run private key infrastracture across AWS and EKS for shared services, non-production and production accounts. Being able to delegate Intermediate CAs to teams as I can limit the scope has helped a lot.

Would be great if you could get someone to spare an hour to review this and merge it in

@bendbennett
Copy link
Contributor

Hi @taliesins 👋

Sorry for the delay in getting to this but I'm afraid that reviewing this PR is currently on-hold as we need to triage the changes and our team’s focus is currently elsewhere at the moment.

@whiskeysierra
Copy link

@bendbennett Any chance to put limited focus on this one? It's a really useful change and the community would really appreciate it if this contribution would make it into the provider. As an open source maintainer I know that quality of PRs can differ quite a lot, but this one looks really rock solid to me. Well thought out changes, cleanly implemented and good test coverage.
@taliesins Looks like your PR collected some conflicts in the mean time. Any chance to get up and running again?

@bendbennett
Copy link
Contributor

Hi @whiskeysierra 👋

Again, apologies for the delay. I have marked the issue for triage. We will discuss it at our next triage meeting and determine whether this can be prioritised.

@whiskeysierra
Copy link

We will discuss it at our next triage meeting

@bendbennett When is your next triage meeting scheduled?

@NoseyNick
Copy link

Nearly a year later... any update on this important and useful feature?

@taliesins
Copy link
Author

@bendbennett let me know when you are going to consider this pr and I will fix the pr to pass the build.

@dsantanu
Copy link

hi! anyfurther development on this?

@philipstreet
Copy link

I could really do with this being fixed as well.

@taliesins
Copy link
Author

@bendbennett could you approve the workflow again. I have pulled the latest changes into this branch.

Copy link

hashicorp-cla-app bot commented Nov 21, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes


2 out of 3 committers have signed the CLA.

  • taliesins
  • sestegra
  • manavglad

Have you signed the CLA already but the status is still pending? Recheck it.

Add properties to model
Generate documentation
Add named constraints for dns domains
Add meta data for authority key so that we can set it
@taliesins taliesins force-pushed the fix_intermediate_ca branch from 3e956be to d5f36bf Compare January 6, 2025 10:49
Add utility classes to implement conversion of PEM to PFX and PFX to PEM
Added documentation
@taliesins taliesins force-pushed the fix_intermediate_ca branch from c2f4ae2 to 97be97f Compare January 7, 2025 13:10
@taliesins taliesins changed the title Fix intermediate ca and add dns name constraints Fix intermediate ca, add dns name constraints and provide data sources to convert between different certificate formats Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants