-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Extended Key Usage: incompatible key usage #846
Comments
Hi @tam7t At the moment I don't have a good answer for this, for a few reasons:
So it's hard to say at this moment what's going on, but, (3) suggests that a missing piece of information is knowing what key usages are actually being requested by the verifying function -- and in fact, I don't know from your output above whether you're seeing this error from your client or from a Golang server. Any chance that code is share-able? |
Sure, here is a gist of the example code with the generated certificates: https://gist.github.com/tam7t/a1e8a1fedabd48caeb97 I believe that the issue occurs because go certificate validations verifies that the signing certs have the requested leaf extended usage. So in this example the leaf wants to use |
Oh, @$!%#
I didn't notice that bit in the comment. One of those things you pass over a million times but don't think about until it becomes a problem. So I think we're basically both right - the spec doesn't require it, but Go does because it's not spec compliant. In the short term, if the code is your own, can you set the verify options to |
So the question now is, should we just add all usages to CA certs, or add toggleable flags. @tam7t @james-masson do you have thoughts on this? On the one hand it seems like we shouldn't be doing that if it's not necessary especially since not all crypto stacks will behave the way that Go is. On the other hand, if Windows stacks behave this way, and we anticipate many Go clients, without seeing significant downsides we might as well just add all of them. That removes user confusion and/or mistakes from causing an issue down the line. |
Thinking about it, I'm leaning towards just adding all of them, and in addition, adding all of them to intermediate CAs being signed. Thoughts? |
That seams reasonable to me |
Well, is there a reason to specify them at all on CA certificates? Since the RFC says |
@tam7t I'd be worried about other crypto stacks not behaving the same way, and also, I think it's nice for the leaf certs being issued to be able to be constrained for organizations that want a bit of extra security, e.g. it's nice for an org to be able to issue certs for email encryption/signing with a DNS SAN for that user's machine that a user can't then try to put onto a web server on their machine to run unauthorized services. I think we're probably better off forcing an ExtKeyUsageAny value for CAs than dropping back to only using basic keyusage constraints. Does that make sense/do you agree? |
* Move to one place for both code paths * Assign ExtKeyUsageAny to CA certs to help with validation with the Windows Crypto API and Go's validation logic Fixes #846
@tam7t See what you think of that commit ^ -- any chance you can make sure that fixes your issue? |
👍 yeah that makes sense. I also tried out the branch and its working, thanks! |
Great! |
Wasted days trying to figure out why Consul health checks wouldn't accept certs generated by Vault. Finally tied what I was seeing to this issue. :( When is the 0.5 release scheduled to be available? Need this fix ASAP. Thx! |
Hi Jordan,
This fix is in master, so you can easily build it and run it if you like.
|
(edit: oops, wrong issue) |
I'm having difficulty validating certificates generated with the
pki
backend as described in the documentation.When I use the
pki/root/generate/internal
endpoint to generate a root certificate it has the following properties:and the cert generated from
pki/issue/example-dot-com
:however, the client code
errors with
My understanding is that since the root cert specifies
x509.ExtKeyUsageOCSPSigning
(but notx509.ExtKeyUsageServerAuth
) the validation fails.I've 'correct' this locally by removing
certTemplate.ExtKeyUsage = append(certTemplate.ExtKeyUsage, x509.ExtKeyUsageOCSPSigning)
sense it seems that most Root CA's don't have any extended key usage properties set. Another solution might be add a roles parameter to the api, but not sure if there's something I'm doing wrong/don't understand or what solution makes the most sense.The text was updated successfully, but these errors were encountered: