caddytls: Fix handling of IP-only TLS configs and empty-SNI handshakes

[mholt]

1. What does this change do, exactly?

• Always choose a TLS config during a handshake, even if there is no match. Random selection (as a last resort) is OK; any config should allow the TLS-ALPN challenge to be completed.

• Add -default-sni flag. If a ClientHello is received with no ServerName (SNI), then this value will be assumed when matching TLS configs and choosing a certificate.

• When no ServerName is given, certificates may be chosen based on a matching IP address of the listener.

• Update CertMagic dependency, which supports this new matching logic with a default ServerName.

• If SNI is present in a ClientHello but does not match any certificates, a TLS alert is raised (in keeping with previous behavior before this change and other recent changes)

2. Please link to the relevant issues.

Hopefully fixes #2451, fixes #2438, fixes #2414, and fixes #2407

3. Which documentation changes (if any) need to be made because of this PR?

Add note about the -default-sni flag.

4. Checklist

• I have written tests and verified that they fail without my change
• I have squashed any insignificant commits
• This change has comments for package types, values, functions, and non-obvious lines of code
• I am willing to help maintain this change if there are issues with it later

Calling on: @fr33tux, @oscartbeaumont, @rmoriz, @whitestrake, and @magikstm if you could help test this please, and ensure it works for you, since I was not able to reproduce all the various issues myself (like the Docker stuff).

[mholt]

Better get @ghoeffner @orenyomtov @Zenexer to try this out while they still have a chance, before it lands -- since they apparently have opinions on the matter. And if not, they would not have made such a stink about it before. So I expect to hear from them, but if not it's on them.

[xombiemp]

@mholt I can confirm that this change has fixed #2438
Thank you!

[Zenexer]

@mholt None of this is on me. You're responsible for the security of your own product. I reported the vulnerability and provided a fix. What you do from here is on you. I don't use Caddy and I'm not going to spend any more time testing and coding for someone who's just going to get angry at me. If I see another vulnerability while pentesting, I'll report it, but don't expect me to contribute beyond that.

[luisalvarado]

Hi @mholt , awesome work buddy. Here is some feedback about my experience to see if this helps with a couple of deployment issues.

If I may, I think I see several minor issues here that when using -service install should be addressed to make it easier for faster deployment. When using the hook.service, caddy -service install should do the following to fix several issues at once:

When it creates the service (using systemd for example) it should also create a default folder for the SSL certificates and a default for the caddyserver file. Like /etc/caddy/ssl for the ssl files and /etc/caddy for the conf file. (As a bonus it should by default be renamed to caddy.service, not Caddy.service (Capital C there).

The default conf folder is to avoid having to guess or manually add the conf parameter.

The default ssl folder is to avoid having several issues when doing the whole systemd process as a it fails if it cant find the folders.

There should be a default log in /var/log/caddy (default with option to set somewhere else, same for conf and ssl folders).

Did not check but an option to automatically restart if the caddyserver file is updated or changed (same option to check if caddyserver file has correct format, if not, do not restart service but send a warning to the logs. If all is good, automatically restart service)

This are just a couple of things that would help in deployment.

[mholt]

@luisalvarado The service plugin is third-party (not by me), so you might want to take it up over there instead: https://github.com/hacdias/caddy-service

[luisalvarado]

Don't know why it took me more than a week to reply. Thank you @mholt will do.