Partially fix raw IP request regression (#2356)

[Zenexer]

1. What does this change do, exactly?

Partially resolves regression that resulted from a security patch, as described in #2356

2. Please link to the relevant issues.

#2356

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

This still needs to be discussed.

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

[CLAassistant]

All committers have signed the CLA.

[Zenexer]

I did my best to minimize the potential for unintended side effects. Initially, I tried to get it to intelligently fall back to the most likely intended certificate under certain circumstances, but the changes were too disruptive, and I couldn't easily gauge the fallout.

[Zenexer]

Force-pushed formatting adjustment for handshake_test.go (gofmt -s)

[jung-kurt]

Nice work, @Zenexer. Thanks for your prompt attention to this issue. Your solution addresses some subtleties that I would have certainly missed. My simple example and my more complicated working setup both work properly with your changes.

[lukenowak]

Thank you @Zenexer , I checked your PR code and it works perfectly on my quite complex configuration (a lot of sites with many name-based certificates and few endpoints with IP based -- IPv4, IPv6 and others).

For the ip access I have caddyfile like:

https:[::1]:5555, https:127.0.0.1:5555 {
  bind 127.0.0.1
  tls ip.crt ip.key
}

And my cert has two IPs in altSubjectName with meaningless common name. All works well.

Of course! If at least one entry for IP-based access has wrong certificate, like adding to my Caddyfile:

https:[::1]:5555/cous, https:127.0.0.1:5555/cous {
  bind 127.0.0.1
  tls other.crt other.key
}

all endpoints returns error.

This is expected, as it is said in the docs, that all configurations shall share exactly the same TLS stanzas. Improving error reporting in this case is subject to another thing I think.

[mholt]

Thank you for your pull request. This is a good change, and I would like to see it make its way into Caddy.

Unfortunately, the handshake logic has moved to CertMagic. If you could migrate the relevant changes in this pull request in that repo (linking to the same issues over in this repo), that would be appreciated, and I'll get it reviewed and merged in.

Then update this PR with the Caddy-specific changes and I'll merge this in after that new one.

[Zenexer]

Will do ASAP.

@mholt Apologies for bringing this here, but I want to be absolutely sure I get this message to you. I received a suspicious message claiming to be from you. I've sent you more info via Keybase chat. Looks like you might not actively use Keybase, so I'll follow up via email.

[mholt]

@Zenexer Thanks! Also, I got your email. Am swamped today but will take care of it.

[mholt]

Mostly superseded by a7aeb97 - but I would like to at least merge in your tests, @Zenexer, either here or CertMagic, wherever they belong.