Add support for certificates with client and server auth and URL SANs #166
Conversation
|
Thanks for the feedback! |
|
@FiloSottile I updated the PR according to your requests: It consists of 2 commits now, the first only adds support for URLs as SANs, the second adds the extended key usage server auth for client certs. |
main.go
Outdated
| @@ -194,6 +195,9 @@ func (m *mkcert) Run(args []string) { | |||
| if email, err := mail.ParseAddress(name); err == nil && email.Address == name { | |||
| continue | |||
| } | |||
| if _, err := url.Parse(name); err == nil { | |||
| continue | |||
| } | |||
| punycode, err := idna.ToASCII(name) | |||
| if err != nil { | |||
| log.Fatalf("ERROR: %q is not a valid hostname, IP, or email: %s", name, err) | |||
There was a problem hiding this comment.
Here and below, update the error.
main.go
Outdated
| @@ -194,6 +195,9 @@ func (m *mkcert) Run(args []string) { | |||
| if email, err := mail.ParseAddress(name); err == nil && email.Address == name { | |||
| continue | |||
| } | |||
| if _, err := url.Parse(name); err == nil { | |||
There was a problem hiding this comment.
This is missing the extra validation that happens in cert.go.
cert.go
Outdated
| @@ -70,13 +71,15 @@ func (m *mkcert) makeCert(hosts []string) { | |||
| tpl.IPAddresses = append(tpl.IPAddresses, ip) | |||
| } else if email, err := mail.ParseAddress(h); err == nil && email.Address == h { | |||
| tpl.EmailAddresses = append(tpl.EmailAddresses, h) | |||
| } else if uriName, err := url.Parse(h); err == nil && uriName.Scheme != "" { | |||
There was a problem hiding this comment.
I'm a bit worried this will catch things some people want as hostnames. Anything with a : will parse as an URI with Scheme != "", and sometimes people use : in hostnames, for example.
Are there other filters we can apply while still working for your use case?
There was a problem hiding this comment.
For my concrete use case I want Spiffe URIs, so checking for value URLs plus the prefix spiffe:// would be fine too (for me)
There was a problem hiding this comment.
I'll update the PR to require the scheme spiffe.
There was a problem hiding this comment.
No need to make it Spiffe specific. Looks like those are URLs with the full ://, so we can just check Scheme != "" && Host != "" (because without :// only Scheme and Opaque get populated).
There was a problem hiding this comment.
Thanks! Just updated the PR with this check.
|
Just updated the PR by fixing the error message and only supporting spiffe URIs for now. |
|
LGTM with a minor nit (see suggestions). |
Co-Authored-By: Filippo Valsorda <hi@filippo.io>
Co-Authored-By: Filippo Valsorda <hi@filippo.io>
|
Thank you! Sorry for forgetting about this. |
Thank you for this awesome tool! :-)
I wanted to use it to create certificates for a Service Mesh, that is for using Istio with mTLS without using Citadel.
Therefore I needed to create certificates that have an extended key usage of client and server auth.
I also needed to have Spiffe URIs in the Subject Alternate Names.
Therefore I did the following changes:
-serverwhich is enabled by default. If set tofalseand setting-client=truea client-only certificate would be generated, with-client=trueand-server=true(the default) a client and server certificate would be created.