You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Lieven Govaerts <lg...@mobsol.be> on 2014/08/19 15:25:22 UTC
[PATCH] Add a configuration option to disable HTTP pipelining.
Hi,
attached patch adds a new configuration flag to disable HTTP pipelining.
There's a bug in OpenSSL's SSL renegotiation algorithm. When it's
initiated by the server to request a client certificate, it'll fail
when on the connection pipelined requests are incoming at the server
side.
Short summary of the root cause: during renegotiation, OpenSSL reads
data from the TCP connection expecting it to be a proper client
certificate. However, if an HTTP request was still pending on the
connection or in the server's receive buffer, OpenSSL will read that
request's data, recognise its not a proper client certificate, discard
the data and report an error. Apache will then abort the connection in
response to that OpenSSL error.
Given that there's no fix planned in OpenSSL, the only available
mitigation is to disable HTTP pipelining on connections where a SSL
renegotiation can happen. Since that depends on the configuration of
the server, we can't really know or predict when such renegotiation
will happen.
Conclusion: give the user the option to disable HTTP pipelining, which
she can use in case of problems caused by renegotiation.
Attached patch implements just that.
Objections anyone? Other remarks?
Lieven
[[[
Add an option "http-pipelining" to the servers configuration, so that a user
can disable HTTP pipelining in case that causes problems, e.g. during SSL
renegotiation triggered by the server to request a client certificate.
* subversion/include/svn_config.h
(SVN_CONFIG_OPTION_HTTP_PIPELINING): New boolean config option.
* subversion/libsvn_ra_serf/ra_serf.h
(struct svn_ra_serf__session_t): New member variable http_pipelining.
* subversion/libsvn_ra_serf/serf.c
(load_config): Load the value of the new option from the servers file. If not
set, use 'HTTP pipelining is enabled' by default.
(svn_ra_serf__open,
ra_serf_dup_session): Set the max. nr. of outstanding requests to 1
if HTTP pipelining is disabled.
* subversion/libsvn_ra_serf/update.c
(open_connection_if_needed): Set the max. nr. of outstanding requests to 1
if HTTP pipelining is disabled.
* subversion/libsvn_subr/config_file.c
(svn_config_ensure): Add the 'http-pipelining' option in the comment section
of the initial servers file.
]]]
Re: [PATCH] Add a configuration option to disable HTTP pipelining.
Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Aug 19, 2014 at 9:53 AM, Lieven Govaerts <lg...@mobsol.be> wrote:
> On Tue, Aug 19, 2014 at 2:34 PM, Mark Phippard <ma...@gmail.com> wrote:
> > On Tue, Aug 19, 2014 at 9:25 AM, Lieven Govaerts <lg...@mobsol.be> wrote:
> >
> >> There's a bug in OpenSSL's SSL renegotiation algorithm. When it's
> >> initiated by the server to request a client certificate, it'll fail
> >> when on the connection pipelined requests are incoming at the server
> >> side.
> >>
> >> Short summary of the root cause: during renegotiation, OpenSSL reads
> >> data from the TCP connection expecting it to be a proper client
> >> certificate. However, if an HTTP request was still pending on the
> >> connection or in the server's receive buffer, OpenSSL will read that
> >> request's data, recognise its not a proper client certificate, discard
> >> the data and report an error. Apache will then abort the connection in
> >> response to that OpenSSL error.
> >>
> >> Given that there's no fix planned in OpenSSL, the only available
> >> mitigation is to disable HTTP pipelining on connections where a SSL
> >> renegotiation can happen. Since that depends on the configuration of
> >> the server, we can't really know or predict when such renegotiation
> >> will happen.
> >>
> >> Conclusion: give the user the option to disable HTTP pipelining, which
> >> she can use in case of problems caused by renegotiation.
> >> Attached patch implements just that.
> >>
> >> Objections anyone? Other remarks?
> >>
> >
> > I assume it is not possible for Serf to know that it is using client
> > connections and disable it automatically?
>
> Serf knows that the application set the client cert callback, but it
> only knows that the server requires a client certificate for a certain
> resource when OpenSSL asks serf for a client certificate. Then it's
> already too late to disable HTTP pipelining.
>
> So indeed, it's not possible.
>
> > I assume this problem does not happen all the time? How does it manifest
> > currently? What error does user see.
>
> The details can be found in serf issue #135:
> https://code.google.com/p/serf/issues/detail?id=135
>
> The user gets this error message:
> svn: E120171: Unable to connect to a repository at URL
> 'https://svn.company.com/ssl/repo/Math/trunk'
> svn: E120171: Error running context: An error occurred during SSL
> communication
>
> > We have a few customers that use client certificates, and AFAIK, they
> have
> > been using SVN 1.8 OK.
>
> It has been reported a couple of times to the serf list by svn users,
> and I can simulate it (see issue #135) repeatedly.
>
> It does depend on the server configuration though. If the server is
> configured to request for a client certificate on the root of the
> repository, then it will request the client certificate in response to
> one of the first requests on the connection. Typically - at least for
> the first connection - the first requests will be sent one by one (see
> svn's broken proxy detector), so this problem never triggers.
>
> However, when the client certificate is requested for a resource
> deeper in the repository, it's likely that say during a large
> checkout, many (pipelined) requests will already be sent by the client
> before the request for the protected resource. This is the scenario
> that'll lead to the problem.
>
> So with the combination of a general error message and a need for a
> specific server configuration, it's not surprising that you won't have
> heard of this issue before.
>
> Thanks for the clarification. Based on this, I think an option is the way
to go. It sounds like most users will never need this, even if using
client certs. So making it an option for those impacted sounds like a good
plan.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [serf-dev] Re: [PATCH] Add a configuration option to disable HTTP pipelining.
Posted by Lieven Govaerts <lg...@mobsol.be>.
On Thu, Aug 21, 2014 at 5:47 PM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> On Thu, Aug 21, 2014 at 5:30 PM, Lieven Govaerts <lg...@mobsol.be> wrote:
>> Attached patch implements a slightly different approach but I think
>> it's a valid workaround for the problem.
>
> It may be useful to note why we register apps_ssl_info_callback in the
> re-negotiate info callback. (It's because there can only be one
> callback.)
Fixed.
> I'd also like to confirm whether we always re-init the SSL context
> when we reset the connection - I think so, but didn't confirm...
Management of the lifecycle the ssl context is handled by the
application, not by serf. The SSL context is created when the
encrypt/decrypt buckets are created, which is done by the application
in the connection_setup handler. By just looking at the
connection_setup handler it seems at first that Subversion is reusing
the ssl context, but it will reset it to NULL in the connection_closed
handler, so that should be ok.
>
>> What it does is detect when a server triggers a renegotiation, and
>> when that happens switch from a pipelined to a non-pipelined
>> connection. It doesn't check if the application is actually using the
>> pipelining, just if it's enabled or not.
>
> As we discussed, this is the only thing that bothers me a bit - any
> time we see a renegotiation, we'll reset the connection and try again
> with pipelining turned off. I think we could do something to factor
> in the state of serf's connection state - that is, something like
> (conn->completed_requests - conn->completed_responses > 1) to not
> reset the connection when there is only just the one pending response
> that triggers the renegotiation. Ostensibly, after the renegotiation
> succeeds, we *could* do pipelining again - we just can't handle
> renegotiation when there is more than one outstanding request pending.
I propose we implement this after we switch to the new
protocol/connection implementation. The separation between context and
buckets makes it very difficult to find out how many outstanding
requests there are on a connection from within the BIO read/write
code.
>> I'm not convinced this completely removes the need for the
>> "http-pipelining" option, because I'm not convinced this will cover
>> all possible situations, but it should at least cover the ones
>> reported to the serf dev list.
>
> Agreed. But, I'm tempted to apply this patch and then see if we still
> get real-world complaints before adding yet-another-knob. -- justin
>
Fine by me.
Lieven
Re: [serf-dev] Re: [PATCH] Add a configuration option to disable HTTP pipelining.
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Thu, Aug 21, 2014 at 5:30 PM, Lieven Govaerts <lg...@mobsol.be> wrote:
> Attached patch implements a slightly different approach but I think
> it's a valid workaround for the problem.
It may be useful to note why we register apps_ssl_info_callback in the
re-negotiate info callback. (It's because there can only be one
callback.)
I'd also like to confirm whether we always re-init the SSL context
when we reset the connection - I think so, but didn't confirm...
> What it does is detect when a server triggers a renegotiation, and
> when that happens switch from a pipelined to a non-pipelined
> connection. It doesn't check if the application is actually using the
> pipelining, just if it's enabled or not.
As we discussed, this is the only thing that bothers me a bit - any
time we see a renegotiation, we'll reset the connection and try again
with pipelining turned off. I think we could do something to factor
in the state of serf's connection state - that is, something like
(conn->completed_requests - conn->completed_responses > 1) to not
reset the connection when there is only just the one pending response
that triggers the renegotiation. Ostensibly, after the renegotiation
succeeds, we *could* do pipelining again - we just can't handle
renegotiation when there is more than one outstanding request pending.
> I'm not convinced this completely removes the need for the
> "http-pipelining" option, because I'm not convinced this will cover
> all possible situations, but it should at least cover the ones
> reported to the serf dev list.
Agreed. But, I'm tempted to apply this patch and then see if we still
get real-world complaints before adding yet-another-knob. -- justin
Re: [PATCH] Add a configuration option to disable HTTP pipelining.
Posted by Lieven Govaerts <lg...@mobsol.be>.
On Thu, Aug 21, 2014 at 1:04 PM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> On Tue, Aug 19, 2014 at 2:53 PM, Lieven Govaerts <lg...@mobsol.be> wrote:
>> However, when the client certificate is requested for a resource
>> deeper in the repository, it's likely that say during a large
>> checkout, many (pipelined) requests will already be sent by the client
>> before the request for the protected resource. This is the scenario
>> that'll lead to the problem.
>
> I know we already do something similar in a number of other places -
> what if we can flag that we have sent the client cert, see an error
> with pipelining, and then retry the requests/connections without
> pipelining? It'd mean the performance would suffer for those with
> renegotiations - and if there is a real failure, it'd force us to fail
> twice - but, not require a config option.
>
> I also wonder if we retry the first request that triggered
> renegotiation and then turn back on pipelining...
>
> WDYT? -- justin
Attached patch implements a slightly different approach but I think
it's a valid workaround for the problem.
What it does is detect when a server triggers a renegotiation, and
when that happens switch from a pipelined to a non-pipelined
connection. It doesn't check if the application is actually using the
pipelining, just if it's enabled or not.
I'm not convinced this completely removes the need for the
"http-pipelining" option, because I'm not convinced this will cover
all possible situations, but it should at least cover the ones
reported to the serf dev list.
Lieven
[[[
Add a workaround for issue #135: SSL renegotiation over a connection that
uses HTTP pipelining will fail with OpenSSL.
The workaround: when a connection has pipelining enabled, detect when a server
initiates a SSL renegotiation via the SSL alert info callback, reset the
connection, disable pipelining on the connection and reconnect to the server.
* serf.h
(SERF_ERROR_SSL_NEGOTIATE_IN_PROGRESS): New error code.
(SERF_CONFIG_CONN_PIPELINING): New config key, its value is "Y" or "N" to
indicate if HTTP pipelining is enabled on the connection.
(SERF_CONFIG_*) Renumber to clarify that their code is per category.
* buckets/ssl_buckets.c
(struct serf_ssl_context_t): New flag renegotiation.
(detect_renegotiate): New OpenSSL state callback, listens for the
"SSL renegotiate ciphers" alert and raises an error flag.
(bio_bucket_read,
bio_bucket_write): When the renegotiation flag is raised bailed out
immediately.
(ssl_init_context): Initiate the renegotiation flag.
(serf_ssl_set_config): When the SERF_CONFIG_CONN_PIPELINING key is set to "Y",
detect renegotiation events.
* outgoing.c
(serf__open_connections): Set the SERF_CONFIG_CONN_PIPELINING config value to
"Y" if HTTP pipelining is enabled on the connection.
(read_from_connection): Handle SERF_ERROR_SSL_NEGOTIATE_IN_PROGRESS errors.
TODO: we probably have to do this in write_to_connection as well.
* test/test_ssl.c
(test_ssl_renegotiate): Enable and finish the already written test for this
issue.
* test/MockHTTPinC/MockHTTP_server.c
(setupTCPServer,
_mhRunServerLoop): Handle sudden connection aborts initiated by the client.
(initSSLCtx): Add some logging.
]]]
Re: [PATCH] Add a configuration option to disable HTTP pipelining.
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Tue, Aug 19, 2014 at 2:53 PM, Lieven Govaerts <lg...@mobsol.be> wrote:
> However, when the client certificate is requested for a resource
> deeper in the repository, it's likely that say during a large
> checkout, many (pipelined) requests will already be sent by the client
> before the request for the protected resource. This is the scenario
> that'll lead to the problem.
I know we already do something similar in a number of other places -
what if we can flag that we have sent the client cert, see an error
with pipelining, and then retry the requests/connections without
pipelining? It'd mean the performance would suffer for those with
renegotiations - and if there is a real failure, it'd force us to fail
twice - but, not require a config option.
I also wonder if we retry the first request that triggered
renegotiation and then turn back on pipelining...
WDYT? -- justin
Re: [PATCH] Add a configuration option to disable HTTP pipelining.
Posted by Lieven Govaerts <lg...@mobsol.be>.
On Tue, Aug 19, 2014 at 2:34 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Tue, Aug 19, 2014 at 9:25 AM, Lieven Govaerts <lg...@mobsol.be> wrote:
>
>> There's a bug in OpenSSL's SSL renegotiation algorithm. When it's
>> initiated by the server to request a client certificate, it'll fail
>> when on the connection pipelined requests are incoming at the server
>> side.
>>
>> Short summary of the root cause: during renegotiation, OpenSSL reads
>> data from the TCP connection expecting it to be a proper client
>> certificate. However, if an HTTP request was still pending on the
>> connection or in the server's receive buffer, OpenSSL will read that
>> request's data, recognise its not a proper client certificate, discard
>> the data and report an error. Apache will then abort the connection in
>> response to that OpenSSL error.
>>
>> Given that there's no fix planned in OpenSSL, the only available
>> mitigation is to disable HTTP pipelining on connections where a SSL
>> renegotiation can happen. Since that depends on the configuration of
>> the server, we can't really know or predict when such renegotiation
>> will happen.
>>
>> Conclusion: give the user the option to disable HTTP pipelining, which
>> she can use in case of problems caused by renegotiation.
>> Attached patch implements just that.
>>
>> Objections anyone? Other remarks?
>>
>
> I assume it is not possible for Serf to know that it is using client
> connections and disable it automatically?
Serf knows that the application set the client cert callback, but it
only knows that the server requires a client certificate for a certain
resource when OpenSSL asks serf for a client certificate. Then it's
already too late to disable HTTP pipelining.
So indeed, it's not possible.
> I assume this problem does not happen all the time? How does it manifest
> currently? What error does user see.
The details can be found in serf issue #135:
https://code.google.com/p/serf/issues/detail?id=135
The user gets this error message:
svn: E120171: Unable to connect to a repository at URL
'https://svn.company.com/ssl/repo/Math/trunk'
svn: E120171: Error running context: An error occurred during SSL communication
> We have a few customers that use client certificates, and AFAIK, they have
> been using SVN 1.8 OK.
It has been reported a couple of times to the serf list by svn users,
and I can simulate it (see issue #135) repeatedly.
It does depend on the server configuration though. If the server is
configured to request for a client certificate on the root of the
repository, then it will request the client certificate in response to
one of the first requests on the connection. Typically - at least for
the first connection - the first requests will be sent one by one (see
svn's broken proxy detector), so this problem never triggers.
However, when the client certificate is requested for a resource
deeper in the repository, it's likely that say during a large
checkout, many (pipelined) requests will already be sent by the client
before the request for the protected resource. This is the scenario
that'll lead to the problem.
So with the combination of a general error message and a need for a
specific server configuration, it's not surprising that you won't have
heard of this issue before.
Lieven
>
> --
> Thanks
>
> Mark Phippard
> http://markphip.blogspot.com/
Re: [PATCH] Add a configuration option to disable HTTP pipelining.
Posted by Mark Phippard <ma...@gmail.com>.
On Tue, Aug 19, 2014 at 9:25 AM, Lieven Govaerts <lg...@mobsol.be> wrote:
There's a bug in OpenSSL's SSL renegotiation algorithm. When it's
> initiated by the server to request a client certificate, it'll fail
> when on the connection pipelined requests are incoming at the server
> side.
>
> Short summary of the root cause: during renegotiation, OpenSSL reads
> data from the TCP connection expecting it to be a proper client
> certificate. However, if an HTTP request was still pending on the
> connection or in the server's receive buffer, OpenSSL will read that
> request's data, recognise its not a proper client certificate, discard
> the data and report an error. Apache will then abort the connection in
> response to that OpenSSL error.
>
> Given that there's no fix planned in OpenSSL, the only available
> mitigation is to disable HTTP pipelining on connections where a SSL
> renegotiation can happen. Since that depends on the configuration of
> the server, we can't really know or predict when such renegotiation
> will happen.
>
> Conclusion: give the user the option to disable HTTP pipelining, which
> she can use in case of problems caused by renegotiation.
> Attached patch implements just that.
>
> Objections anyone? Other remarks?
>
>
I assume it is not possible for Serf to know that it is using client
connections and disable it automatically?
I assume this problem does not happen all the time? How does it manifest
currently? What error does user see.
We have a few customers that use client certificates, and AFAIK, they have
been using SVN 1.8 OK.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/