Wrong certificate when mixing on-demand and manual TLS #1991

FiloSottile · 2018-01-07T02:27:29Z

1. What version of Caddy are you using (`caddy -version`)?

Caddy 0.10.10

2. What are you trying to do?

Serving both a manual certificate and a Let's Encrypt certificate on two different domains.

3. What is your entire Caddyfile?

whoami.filippo.io {
	tls [email protected]
	redir https://blog.filippo.io/ssh-whoami-filippo-io/ 302
}

https://cve-2016-2107.filippo.io {
	realip cloudflare
	tls cert.pem key.pem {
		protocols tls1.2 tls1.2
		ciphers ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-RSA-AES128-GCM-SHA256
		# clients origin-pull-ca.pem
	}
	proxy / localhost:31337
}

https://ticketbleed.filippo.io {
	realip cloudflare
	tls cert.pem key.pem {
		protocols tls1.2 tls1.2
		ciphers ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-RSA-AES128-GCM-SHA256
		# clients origin-pull-ca.pem
	}
	proxy / localhost:41337
}

4. How did you run Caddy (give the full command and describe the execution environment)?

cd /etc/caddy
ulimit -n 4096
setcap cap_net_bind_service=+ep /usr/local/bin/caddy
exec setuidgid www-data env HOME=/etc/caddy /usr/local/bin/caddy \
	-agree -pidfile /var/run/caddy.pid

6. What did you expect to see?

Let's Encrypt on whoami.filippo.io, my custom certificate on ticketbleed.filippo.io and cve-2016-2107.filippo.io.

7. What did you see instead (give full error messages and/or log)?

The custom certificate is served on whoami.filippo.io.

Even if I deleted the ACME certificate, Caddy obtained a new (correct) one and placed it at /etc/caddy/.caddy/acme/acme-v01.api.letsencrypt.org/sites/whoami.filippo.io/whoami.filippo.io.crt while starting. However it then doesn't use it.

This is a regression, the previous version (alas lost to the wind) was working correctly.

The text was updated successfully, but these errors were encountered:

mholt · 2018-01-07T16:26:11Z

Hey Filippo, thanks for the report. I'll take a look at this. What are all the names (CommonName and/or SAN) on the certificate in cert.pem?

FiloSottile · 2018-01-07T16:27:31Z

Aha, your intuition is correct, whoami.filippo.io is one of the SAN of cert.pem.

(You can see it live at https://whoami.filippo.io)

mholt · 2018-01-07T16:34:31Z

Excellent: I understand the nature of this bug now. This was a fix for a bug reported on the forums related to OCSP stapling. See #1821.

But apparently this is problematic. Hmm. We will need to find another way to prevent overlapping names while preserving OCSP stapling correctness and serving the right certificates...

FiloSottile · 2018-01-07T16:36:47Z

Yeah, you’ll probably have to key OCSP caches by certificate fingerprint.

mholt · 2018-01-07T16:39:42Z

Or, right now Caddy uses a single, global certificate cache. I wonder if the certificate cache should be divided up by site. In other words, each site uses a separate cert cache, and to maintain certificates I just keep a list of cert caches to iterate through (one more for loop in the auto-maintenance function).

Trying to think if this will have any negative consequences. (I should probably beef up testing around this after the fires are out.) Also need to find out if the overlap prevention would be needed if dividing the cert caches up.

FiloSottile · 2018-01-07T16:41:02Z

That also sounds good, in these GetConfigForClient days the sites TLS configurations can be well isolated, but you have a better understanding of the internals to make that call.

(Confirmed that removing whoami.filippo.io from cert.pem workarounds the bug.)

mholt · 2018-01-07T16:42:40Z

Cool. Thanks for your help. And yeah, the GetConfigForClient update in Go 1.8 was a godsend, and I suppose it's a good time to take full advantage of it.

I also need to find out if/how this will affect config reloads. I'll try to get on this today after church 😇 .

mholt · 2018-01-07T21:56:31Z

Alright, I think I can boil it down to this:

A handshake comes in for some site, example.com -- Caddy has to use only the information from SNI to load the right certificate. The problem is, although not necessarily in your case, that there can be multiple Caddy sites for example.com - consider this Caddyfile:

example.com/user1 {
    ...
}

example.com/user2 {
    ...
}

Suppose both those sites have different TLS certificates for some reason. Which one should be loaded?

I think I can fix the bug you've reported, but we need to decide how to handle this situation. There may be others; I'm just banging on the dents I can find so far.

FiloSottile · 2018-01-07T21:59:04Z

IMHO if those two sites have different TLS configurations it’s a configuration error and Caddy should not start.

mholt · 2018-01-07T22:24:04Z

Hmm, I think I agree. I will try to figure out the best way to implement this next.

- Expose the list of Caddy instances through caddy.Instances() - Added arbitrary storage to caddy.Instance - The cache of loaded certificates is no longer global; now scoped per-instance, meaning upon reload (like SIGUSR1) the old cert cache will be discarded entirely, whereas before, aggressively reloading config that added and removed lots of sites would cause unnecessary build-up in the cache over time. - Key certificates in the cache by their SHA-256 hash instead of by their names. This means certificates will not be duplicated in memory (within each instance), making Caddy much more memory-efficient for large-scale deployments with thousands of sites sharing certs. - Perform name-to-certificate lookups scoped per caddytls.Config instead of a single global lookup. This prevents certificates from stepping on each other when they overlap in their names. - Do not allow TLS configurations keyed by the same hostname to be different; this now throws an error. - Updated relevant tests, with a stark awareness that more tests are needed. - Change the NewContext function signature to include an *Instance. - Strongly recommend (basically require) use of caddytls.NewConfig() to create a new *caddytls.Config, to ensure pointers to the instance certificate cache are initialized properly. - Update the TLS-SNI challenge solver (even though TLS-SNI is disabled currently on the CA side). Store temporary challenge cert in instance cache, but do so directly by the ACME challenge name, not the hash. Modified the getCertificate function to check the cache directly for a name match if one isn't found otherwise. This will allow any caddytls.Config to be able to help solve a TLS-SNI challenge, with one extra side-effect that might actually be kind of interesting (and useless): clients could send a certificate's hash as the SNI and Caddy would be able to serve that certificate for the handshake. - Do not attempt to match a "default" (random) certificate when SNI is present but unrecognized; return no certificate so a TLS alert happens instead. - Store an Instance in the list of instances even while the instance is still starting up (this allows access to the cert cache for performing renewals at startup, etc). Will be removed from list again if instance startup fails. - Laid groundwork for ACMEv2 and Let's Encrypt wildcard support. Server type plugins will need to be updated slightly to accommodate minor adjustments to their API (like passing in an Instance). This commit includes the changes for the HTTP server. Certain Caddyfile configurations might error out with this change, if they configured different TLS settings for the same hostname. This change trades some complexity for other complexity, but ultimately this new complexity is more correct and robust than earlier logic. Fixes #1991 Fixes #1994 Fixes #1303

FiloSottile · 2018-02-25T18:51:59Z

@mholt Sorry for not testing this earlier, but the latest version seems to fix the issue! Thank you!

mholt · 2018-02-25T18:53:23Z

I am super relieved. :)

mholt added the bug 🐞 Something isn't working label Jan 7, 2018

mholt added this to the 0.10.11 milestone Jan 7, 2018

root360-AndreasUlm mentioned this issue Jan 12, 2018

old certificate is used after switching to on-demand let's encrypt #1994

Closed

mholt mentioned this issue Jan 12, 2018

caddy dies on startup with faulty DNS #1992

Closed

mholt added the in progress 🏃‍♂️ Being actively worked on label Jan 27, 2018

mholt mentioned this issue Feb 3, 2018

Upcoming change in Caddy will require a change in caddy-net pieterlouw/caddy-net#5

Closed

mholt mentioned this issue Feb 4, 2018

tls: Restructure and improve certificate management #2015

Merged

4 tasks

mholt closed this as completed in #2015 Feb 16, 2018

mholt removed the in progress 🏃‍♂️ Being actively worked on label Feb 17, 2018

Zenexer mentioned this issue Nov 12, 2018

Caddy serves random certificate from configuration for undefined subdomains #423

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrong certificate when mixing on-demand and manual TLS #1991

Wrong certificate when mixing on-demand and manual TLS #1991

FiloSottile commented Jan 7, 2018

mholt commented Jan 7, 2018

FiloSottile commented Jan 7, 2018 •

edited

Loading

mholt commented Jan 7, 2018 •

edited

Loading

FiloSottile commented Jan 7, 2018

mholt commented Jan 7, 2018 •

edited

Loading

FiloSottile commented Jan 7, 2018

mholt commented Jan 7, 2018

mholt commented Jan 7, 2018

FiloSottile commented Jan 7, 2018 via email

mholt commented Jan 7, 2018

FiloSottile commented Feb 25, 2018

mholt commented Feb 25, 2018

Wrong certificate when mixing on-demand and manual TLS #1991

Wrong certificate when mixing on-demand and manual TLS #1991

Comments

FiloSottile commented Jan 7, 2018

1. What version of Caddy are you using (caddy -version)?

2. What are you trying to do?

3. What is your entire Caddyfile?

4. How did you run Caddy (give the full command and describe the execution environment)?

6. What did you expect to see?

7. What did you see instead (give full error messages and/or log)?

mholt commented Jan 7, 2018

FiloSottile commented Jan 7, 2018 • edited Loading

mholt commented Jan 7, 2018 • edited Loading

FiloSottile commented Jan 7, 2018

mholt commented Jan 7, 2018 • edited Loading

FiloSottile commented Jan 7, 2018

mholt commented Jan 7, 2018

mholt commented Jan 7, 2018

FiloSottile commented Jan 7, 2018 via email

mholt commented Jan 7, 2018

FiloSottile commented Feb 25, 2018

mholt commented Feb 25, 2018

1. What version of Caddy are you using (`caddy -version`)?

FiloSottile commented Jan 7, 2018 •

edited

Loading

mholt commented Jan 7, 2018 •

edited

Loading

mholt commented Jan 7, 2018 •

edited

Loading