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 and add dns name constraints #309

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

Conversation

taliesins
Copy link

@taliesins taliesins commented Jan 19, 2023

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 automaticaly 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/

@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.

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.

8 participants