On Thu, Aug 21, 2014 at 1:04 PM, Justin Erenkrantz
<justin_at_erenkrantz.com> wrote:
> On Tue, Aug 19, 2014 at 2:53 PM, Lieven Govaerts <lgo_at_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.
]]]
Received on 2014-08-21 18:30:59 CEST