Fixes #5379 - Better handling for wrong SNI.

[sbordet]

Now returning 400 when SNI does not match any certificate
and SecureRequestCustomizer.sniHostCheck=true.

Signed-off-by: Simone Bordet simone.bordet@gmail.com

[gregw]

@sbordet lots of CI failures?

[sbordet]

@gregw yes the problem is when you have a KeyStore that has only 1 certificate.
In this case, we don't wrap the KeyManagers with the SNI enabled ones, so we don't select SNI, but we add the SecureRequestCustomizer, which by default does a SNI host check, which of course fails because SNI was never chosen.

I think we can "force" SNI if there is only one certificate (but we always need to wrap), or we drop this change.
I'm leaning towards dropping it.

[gregw]

So if we drop it, is there a way with a single certificate to force the an SNI check is done on the host header?
I think that is desirable, but happy if t hat requires a manual addition of the SecureRequestCustomizer. Just so long as it can be done.

[sbordet]

@gregw I updated the PR after discussion with @joakime as follows.

jetty.sslContext.sniRequired=true checks the SNI against the certificate at the TLS level. If no match, TLS failure.

jetty.ssl.sniRequired=true checks the SNI against the certificate at the HTTP level. If no match, HTTP 400.

So the 2 properties above are equivalent, the second just allows for a nicer error to users.

jetty.ssl.sniHostCheck=true checks the Host header against the certificate that has been sent to the client. If no match, HTTP 400.

Before the sniHostCheck was skipped most of the times (e.g. when the KeyStore only had 1 certificate), but now is consistent.
This required to change a number of tests that were using bogus Host headers.

However, is it ok to have sniHostCheck=true by default? For example, Google does not and behaves as if the 3 properties above are set to false.

WDYT?

[gregw]

@sbordet sounds good.
I say leave sniHostCheck=true by default as it has been that way and we don't want to change it on too many people.
As it is, his change will turn it on for some users that previously didn't have it... so that will be a surprise... but better to turn on than to turn off by surprise.

[gregw]

Wow that was bigger than expected!