One last thing; the health check process seems to be ignoring the cert
validation logic entirely. Could be the same pathway re default cert
though.
Its not actually important that we have tls protection on the health
check, but it should be explicitly configured I think, otherwise a
future version that corrects this will run into people needing to
generate certificates for internal servers or completely turn off checking.
Perhaps a check-ssl-verifypeer and check-ssl-verifyhost setting might
make sense to go with check-ssl?
--
Kevin
On 2017-07-26 9:57 AM, Kevin McArthur wrote:
On 2017-07-26 9:55 AM, Willy Tarreau wrote:
On Wed, Jul 26, 2017 at 09:39:03AM -0700, Kevin McArthur wrote:
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.
For now we're in development and the previous one was already merged, so
I'd rather merge Christopher's patch at least to benefit from his
analysis
and from the better solution than the one I came up with. For the
release
that's a different story though.
Fair enough, I just would be weary the release.
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.
What do you mean by "crash haproxy" ? Do you instead mean that it runs
out of connection and cannot connect to the server anymore or that you
managed to kill the process ? Because for me "crash" is this last one
and is a much more serious concern.
The former, just ties up all the connections.
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.
That's why I warned you that it was a tradeoff :-)
~_~
Willy
--
Kevin
