-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
caddytls: Evict internal certs from cache based on issuer #6266
Conversation
During a config reload, we would keep certs in the cache fi they were used by the next config. If one config uses InternalIssuer and the other uses a public CA, this behavior is problematic / unintuitive, because there is a big difference between private/public CAs. This change should ensure that internal issuers are considered when deciding whether to keep or evict from the cache during a reload, by making them distinct from each other and certs from public CAs.
hmm I don't think this works yet. Commenting On Now, Caddy fails to serve that vhost at all, and does not try to issue a certificate against the new issuer. ❯ cat Caddyfile
example.com
tls internal
❯ curl --resolve example.com:443:127.0.0.1 https://example.com -ki
HTTP/2 200
alt-svc: h3=":443"; ma=2592000
server: Caddy
content-length: 0
date: Fri, 26 Apr 2024 16:58:56 GMT ❯ cat Caddyfile
example.com
#tls internal
❯ curl --resolve example.com:443:127.0.0.1 https://example.com -ki
curl: (35) quictls/3.1.4: error:0A000438:SSL routines::tlsv1 alert internal error On the current ❯ curl --resolve example.com:443:127.0.0.1 https://example.com -ki
HTTP/2 200
alt-svc: h3=":443"; ma=2592000
server: Caddy
content-length: 0
date: Fri, 26 Apr 2024 17:06:21 GMT
❯ curl --resolve example.com:443:127.0.0.1 https://example.com -ki
HTTP/2 200
alt-svc: h3=":443"; ma=2592000
server: Caddy
content-length: 0
date: Fri, 26 Apr 2024 17:06:26 GMT |
Thanks for the reproducer, currently investigating! |
@emilylange I was able to see the behavior with those instructions and pushed a fix that worked for me. Please verify it works for your use case and then we'll merge this! 😃 |
Now the following chain of configs will leave the vhost without any certificate at all 🫣 example.com
tls internal example.com
# tls internal Certificate cannot be obtained from LE, because
(which is to be expected) *Manually rolling back to example.com
tls internal now throws
and checking against it with
|
@emilylange Ah, indeed. However it only seems to do that if getting the cert from the production CA (in that second config) fails. If it succeeds, going back to Dinner time, but I'll try to figure out why soon. |
@emilylange Ok, I got it working in the case that the first reload (so, the second config) doesn't have an error getting or loading the certificate. But I'm still stumped by the context canceled error when a cert error is had, I need to keep working that, but am out of time for tonight. |
@emilylange Actually the context canceled is a red herring -- that is the previous config saying it's giving up trying to get a production cert for So I think things are working and ready for your final check! |
Going to merge this soon so it gets in beta 1. |
During a config reload, we would keep certs in the cache if they were used by the next config. If one config uses InternalIssuer and the other uses a public CA, this behavior is problematic / unintuitive, because there is a big difference between private/public CAs: the new config would either just keep using the same certs from before the reload, which is unexpected in this case.
This change should ensure that internal issuers are considered when deciding whether to keep or evict from the cache during a reload, by making them distinct from each other and certs from public CAs.