Interesting. I'd probably recommend not pushing this patch out then
until this can be fixed as it will be trivial to resource-exploit a
haproxy instance that is exhibiting a client-controlled retry. A quick
try with a script that generates randomized SNI names shows I can open
connmax and crash the haproxy from a single instance pretty readily.
If there's other errors that the client can control that lead to a retry
like this, they're probably worthy of a CVE.
It takes approximately 5s per connection to clear the connection in this
condition.
I'll see if retries 0 will work for our use case, but I'd hate to think
we'd have to give up non-client-controlled retry support entirely (ie
for a backend apache restart, retry to another app server...) due to this.
--
Kevin
On 2017-07-26 9:26 AM, Willy Tarreau wrote:
On Wed, Jul 26, 2017 at 09:14:08AM -0700, Kevin McArthur wrote:
Looks like this patch works re verifyhost but I think there's still a
problem here. A browser that tries to load an invalid sni name now produces
4 tries to the backend with about a second delay between each attempt,
amplifying the problem. It also takes a good 5 seconds for the connections
to cleanup/close on failure.
That's the normal behaviour here which confirms it works for you (the
previous code does exactly this for me, though after discussing with
Christopher we still fail to understand precisely why it works with
my version of openssl and none with his nor yours but that's another
story).
The thing is that the layer deciding to retry the connection does it
when there is a connection error. An SSL handshake failure is one of
the many possible connection errors. This could be caused by various
things including a server which is a bit slow to start or to load its
certificates.
For now we have no way to say "don't retry if there is this or that
specific type of connection error". Also passing the information that a
failed handshake is caused by a non-matching SNI is further complicated
as everything is done using callbacks at these layers.
The best I can recommend you for now is to set "retries 0" in your
backend to disable connection retries. Ideally we should try to
enumerate the type of errors that should lead to no retry because they
may be controlled by the client.
Regards,
Willy
