-
-
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
Caddy does not support non-SNI clients anymore #2438
Comments
Duplicate of #2414. Perhaps some of the antagonists from #1303 would like to submit a PR that satisfies everyone, and if not, their complaints won't be justified, because their demands broke a lot of people unnecessarily. @ghoeffner @orenyomtov @Zenexer
This is where you're going to rub a certain crowd the wrong way because of "information leakage." |
@mholt I don’t really see this as a duplicate. The other issue is about Caddy not serving a self signed cert that doesn’t contain the domain name as a Common Name or SANS. This is is about Caddy not serving a signed certificate that matches the domain name to a non-SNI client when there is only one TLS site defined in the Caddyfile. Besides, the issue you said mine is a duplicate of is closed too. |
It's the same issue: Caddy won't serve a certificate unless the SNI value (or the listening IP address) matches a name (or IP) on the certificate. Who signed it doesn't matter. The linked issue is closed because until better solutions are proposed, that's expected -- and by some, demanded -- behavior. |
I've been following issues related to HTTPS the last couple of weeks following #1303 and related PRs. Would it be possible to revert these changes? Could this option possibly be considered? It was mentioned as an:
What was the level of risk that users or websites had before the proposed fix to this issue? What was the severity score of the CVE it fixes? |
What about a configuration for fallback to a default cert? |
Possible -- how should that be done? |
Of course, but it's a vulnerability, like it or not. It's also entirely unnecessary behavior: there is zero benefit to serving a random certificate. Serving the first available certificate would work just as well and wouldn't be considered an information disclosure vulnerability, at least on Caddy's part. It's the randomization that's the problem.
I'm doing my best, @mholt. I spent about 20 hours writing the initial PR with almost no knowledge of Go; it was mostly trial and error. I took a day off from work to do it. My understanding is that although the fix worked, you were already in the process of moving away from the code that I patched, which I wasn't aware of (and had no way of knowing--it would've been nice if you had told me before asking me to fix it). Given the backlog of work I currently have, I can't commit to spending that much time on this anytime soon. I'm afraid you're on your own from here. I reported the vulnerability and mostly ignored the matter when you chose not to fix it. When you complained after security experts started commenting on the matter, I did my best to provide a patch, despite your continuous retaliations toward myself and other security researchers. I tried very hard to ignore your poor choice of wording and aggressive attitude for the sake of the internet community, and I did my best to convince security researchers behind-the-scenes to lay low, but I'm still seeing aggressive outbursts like this. Sorry, but you're on your own from here. You can revert the changes if you so desire, but it will continue to be treated as a significant information disclosure vulnerability by the security community. |
Both sides could do with less sniping at each other, but I wouldn't try to tell the developers that they shouldn't simply point to the off-topic latter half of #1303 every time a valid use case is broken because of that fix. The code is what it is, and it is that way because you demanded it. Resultingly, we now see users with legitimate use cases experiencing unique usability issues. If you don't have the time to - or even if you just don't want to - contribute further, that's absolutely fine, and the Caddy project is not entitled to your time or work. But to leverage very vocal demands against an open source project (while repeatedly misconstruing the facts of the situation, and without providing any kind of implementation for months, despite the fact that nobody knew how to satisfy all of the various demands on Caddy's functionality), and then wash your hands of the matter afterwards is rude in the extreme. Let's not presume that either side were without aggression or retaliation, or that either side is without responsibility for the current state. The final comment of #1303 sums up very well a number of misconceptions you still seem to hold (that the issue was ignored, that the developer refused to fix the problem, that the developer was retaliating against you without cause). I'd caution against accusing the developer of holding an "aggressive attitude" in light of the tone and manner in which you prosecuted your demands in that issue thread. His "aggressive outburst" in this thread is equally as accurate as you found it to be antagonistic. Quibbling about it further is not useful to the Caddy project. Ideally this thread should remain focused on reaching a solution for this issue, rather than continuing off-topic. I appreciate that you're busy, so thank you for advising us as much. The topic at hand is Caddy's ability to support non-SNI clients. The direction of the conversation is whether this can be provided as an option for users to configure. The question currently posed in this thread: how should this be done? @mholt, are you asking what it should look like in the Caddyfile, or are you seeking proposals on how it should be implemented in code? |
What do you think of this?
where host is the domain or IP that matches a Common Name or SAN on the certificate you want served by default when the standard conditions aren't met. |
#2452) * caddytls: Fix empty SNI handling (new -default-sni flag) vendor: update certmagic, needed to support this Hopefully fixes #2451, fixes #2438, and fixes #2414 * caddytls: Don't overwrite certmagic Manager (fixes #2407) Supersedes #2447 * vendor: Update certmagic to fix nil pointer deref and TLS-ALPN cleanup * Improve -default-sni flag help text
1. What version of Caddy are you using (
caddy -version
)?Caddy (untracked dev build) (unofficial)
commit a7aeb97
2. What are you trying to do?
Request a https url using a non-SNI client. There is only one site defined in my Caddyfile, so Caddy should not be confused about which cert to serve.
This was caused by #2339. Caddy serves the TLS cert to non-SNI clients up to and including the commit right before this TLS change, 22dfb14
If there is only one TLS site defined in the Caddyfile, Caddy should serve that cert to non-SNI clients.
Additionally, maybe there could be an option to set a default cert when multiple TLS sites are defined in the Caddyfile.
3. What is your entire Caddyfile?
4. How did you run Caddy (give the full command and describe the execution environment)?
caddy
5. Please paste any relevant HTTP request(s) here.
The command I used to test is
which simulates a non-SNI client by not using the -servername argument.
6. What did you expect to see?
I would expect to see the certificate chain served from Caddy:
7. What did you see instead (give full error messages and/or log)?
I see this handshake failure:
8. How can someone who is starting from scratch reproduce the bug as minimally as possible?
Create a Caddyfile with a single domain name.
Start Caddy and make sure it issued a TLS cert from Letsencrypt.
Use this command to test a non-SNI client:
The text was updated successfully, but these errors were encountered: