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

Caddy self-signed certs have invalid KeyUsage for ECDSA keys #2969

Closed
cpu opened this issue Jan 9, 2020 · 9 comments
Closed

Caddy self-signed certs have invalid KeyUsage for ECDSA keys #2969

cpu opened this issue Jan 9, 2020 · 9 comments

Comments

@cpu
Copy link

cpu commented Jan 9, 2020

👋 Hi folks,

I wanted to open a quick issue on the Caddy repo about something that came up on the IETF LAMPS WG mailing list. The short summary is that Caddy's self-signed certificates for ECDSA subject public keys specify an inappropriate key usage: dataEncipherment.

RFC 5480 describes the usage of elliptic curve keys with X509. There's an update to this document in WG last call, draft-ietf-lamps-5480-ku-clarifications-00 that clarifies the allowed Key Usages extension values for certificates with ECDSA subject public keys. (See this MDSP thread for more background).

The primary change is to clarify two values that do not make sense in the context of elliptic curve cryptography and MUST NOT be used:

If the keyUsage extension is present in a certificate that indicates id-ecPublicKey as algorithm of AlgorithmIdentifier [RFC2986] in SubjectPublicKeyInfo, then following values MUST NOT be present:

keyEncipherment; and
dataEncipherment.

A question was raised on the LAMPS list about how many certificates exist that use an EC public key and specify one of those key usages incorrectly. The results of a Censys query that was shared by Ryan Sleevi indicate there are 9,942 Caddy Self-Signed certificates that don't adhere to the updated RFC 5480 clarification.

I believe the reason is that Caddy's self-signed certificate generation code sets the KeyUsage field of the &x509.Certificate template to a static set of values, not considering the type of the associated keypair:

KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,

You might consider changing this code to not specify x509.KeyUsageKeyEncipherment when the ssconfig.KeyType is for an EC algorithm and not RSA.

I suspect this isn't causing any issues for your users today but once draft-ietf-lamps-5480-ku-clarifications-00 replaces RFC 5480 it's possible that browsers and other user agents will start to enforce the new language and reject Caddy's self-signed certificates.

Hope this helps! Thanks!

@mholt
Copy link
Member

mholt commented Jan 9, 2020

Thanks for pointing this out @cpu -- and I appreciate the links/references.

Caddy 2 does not have the self-signed certificates feature at all, as we're replacing it with a proper internal PKI management system (eventually). Caddy 1 will probably be deprecated later this year, and already we only advise the self-signed usage for short-term local development as it is. Production use of that is not supported.

So then, I might get around to fixing it, but if not, it'll be going away either way. I'm mostly focusing my efforts on Caddy 2 at this point due to limited development resources and time constraints.

Thank you for documenting this, I'll leave it open either until it's fixed or v1 is deprecated.

@mholt
Copy link
Member

mholt commented Jan 9, 2020

@cpu By the way, that certificate generation code was borrowed from the Go standard library. You might want to file an issue with Go as well, as it has the same problem: https://golang.org/src/crypto/tls/generate_cert.go

@cpu
Copy link
Author

cpu commented Jan 9, 2020

So then, I might get around to fixing it, but if not, it'll be going away either way. I'm mostly focusing my efforts on Caddy 2 at this point due to limited development resources and time constraints.

Understood, that seems like a super sensible prioritization. 👍

@cpu By the way, that certificate generation code was borrowed from the Go standard library. You might want to file an issue with Go as well, as it has the same problem:

Oh interesting! Thanks for flagging that. I will chase this down with the Go stdlib too.

@cpu
Copy link
Author

cpu commented Jan 10, 2020

Oh interesting! Thanks for flagging that. I will chase this down with the Go stdlib too.

golang/go#36499 & golang/go#36500

@mholt
Copy link
Member

mholt commented Jan 14, 2020

Closing this, since this feature will be removed/rewritten for v2.

@mholt mholt closed this as completed Jan 14, 2020
@CMCDragonkai
Copy link

CMCDragonkai commented Jan 17, 2020

Self signed certs are important for internal mutual TLS. We have a internal CA system that is generates certs for internal use. Surely Caddy v2 should still support self signed certs. If not, that limits its utilisation incredibly.

@mholt
Copy link
Member

mholt commented Jan 17, 2020

@CMCDragonkai

Self signed certs are important for internal mutual TLS.

Self-signed is an ambiguous term. I know what you mean, but the truly self-signed certificates that Caddy 1 implemented were not useful for mTLS. I don't know if you have used v1's self-signed feature, but it was not intended or useful for mutual TLS in a cluster: only for quick https on a local dev site. Once the server was stopped, the key was disposed of, so you had to re-trust a new key every time or bypass browser warnings every time.

Truly "self-signed" is a bad way to implement mTLS, so Caddy 2 will not be doing this.

Surely Caddy v2 should still support self signed certs.

Of course it will. Just better.

I'm currently in discussions with some Go CA experts to collaborate on a special integration in Caddy 2 that will make mTLS just about as easy as Caddy makes public HTTPS already. It will involve a proper, managed internal PKI that "just works" (as much as possible, we're hoping). I'm still in the early design phase here and it will take some time, but I believe it will be the simplest, most flexible, and most automated internal managed PKI system available.

I only have so much development bandwidth, however -- please allow a little time. :) I'd rather get this feature right than hack something together like I did in v1.

@CMCDragonkai
Copy link

CMCDragonkai commented Jan 17, 2020 via email

@mholt
Copy link
Member

mholt commented Jan 17, 2020

Yes, nothing has changed in that sense.

Btw you don't have to manage the planned Caddy 2 pki, at least, not much. The point of the integration is to automate most of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants