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.
]]]
Index: subversion/include/svn_config.h
===================================================================
--- subversion/include/svn_config.h     (revision 1618848)
+++ subversion/include/svn_config.h     (working copy)
@@ -97,6 +97,8 @@ typedef struct svn_config_t svn_config_t;
 #define SVN_CONFIG_OPTION_HTTP_MAX_CONNECTIONS      "http-max-connections"
 /** @since New in 1.9. */
 #define SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS     "http-chunked-requests"
+/** @since New in 1.9. */
+#define SVN_CONFIG_OPTION_HTTP_PIPELINING           "http-pipelining"
 
 /** @since New in 1.9. */
 #define SVN_CONFIG_OPTION_SERF_LOG_COMPONENTS       "serf-log-components"
Index: subversion/libsvn_ra_serf/ra_serf.h
===================================================================
--- subversion/libsvn_ra_serf/ra_serf.h (revision 1618848)
+++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
@@ -144,6 +144,13 @@ struct svn_ra_serf__session_t {
      i.e. is there a (reverse) proxy that does not support them?  */
   svn_boolean_t detect_chunking;
 
+  /* Can serf use HTTP pipelining, or should it send requests one by one.
+     HTTP pipelining is enabled by default. The only known case where it should
+     be disabled is when the server triggers SSL renegotiations in the middle
+     of HTTP traffic on a connection, which OpenSSL currently doesn't handle
+     well. See serf issue #135. */
+  svn_boolean_t http_pipelining;
+
   /* Our Version-Controlled-Configuration; may be NULL until we know it. */
   const char *vcc_url;
 
Index: subversion/libsvn_ra_serf/serf.c
===================================================================
--- subversion/libsvn_ra_serf/serf.c    (revision 1618848)
+++ subversion/libsvn_ra_serf/serf.c    (working copy)
@@ -244,6 +244,12 @@ load_config(svn_ra_serf__session_t *session,
                                   SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS,
                                   "auto", svn_tristate_unknown));
 
+  /* Should we use HTTP pipelining. */
+  SVN_ERR(svn_config_get_bool(config, &session->http_pipelining,
+                              SVN_CONFIG_SECTION_GLOBAL,
+                              SVN_CONFIG_OPTION_HTTP_PIPELINING,
+                              TRUE));
+
 #if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
   SVN_ERR(svn_config_get_int64(config, &log_components,
                                SVN_CONFIG_SECTION_GLOBAL,
@@ -311,6 +317,12 @@ load_config(svn_ra_serf__session_t *session,
                                       SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS,
                                       "auto", chunked_requests));
 
+      /* Should we use HTTP pipelining. */
+      SVN_ERR(svn_config_get_bool(config, &session->http_pipelining,
+                                  server_group,
+                                  SVN_CONFIG_OPTION_HTTP_PIPELINING,
+                                  session->http_pipelining));
+
 #if SERF_VERSION_AT_LEAST(1, 4, 0) && !defined(SVN_SERF_NO_LOGGING)
       SVN_ERR(svn_config_get_int64(config, &log_components,
                                    server_group,
@@ -570,6 +582,10 @@ svn_ra_serf__open(svn_ra_session_t *session,
   if (status)
     return svn_ra_serf__wrap_err(status, NULL);
 
+  if (!serf_sess->http_pipelining) {
+      serf_connection_set_max_outstanding_requests(serf_sess->conns[0]->conn, 
1);
+  }
+
   /* Set the progress callback. */
   serf_context_set_progress_cb(serf_sess->context, svn_ra_serf__progress,
                                serf_sess);
@@ -771,6 +787,10 @@ ra_serf_dup_session(svn_ra_session_t *new_session,
   if (status)
     return svn_ra_serf__wrap_err(status, NULL);
 
+  if (!new_sess->http_pipelining) {
+      serf_connection_set_max_outstanding_requests(new_sess->conns[0]->conn, 
1);
+  }
+
   /* Set the progress callback. */
   serf_context_set_progress_cb(new_sess->context, svn_ra_serf__progress,
                                new_sess);
Index: subversion/libsvn_ra_serf/update.c
===================================================================
--- subversion/libsvn_ra_serf/update.c  (revision 1618848)
+++ subversion/libsvn_ra_serf/update.c  (working copy)
@@ -723,6 +723,11 @@ open_connection_if_needed(svn_ra_serf__session_t *
       if (status)
         return svn_ra_serf__wrap_err(status, NULL);
 
+      if (!sess->http_pipelining) {
+          serf_connection_set_max_outstanding_requests(sess->conns[cur]->conn,
+                                                       1);
+      }
+
       sess->num_conns++;
     }
 
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c        (revision 1618848)
+++ subversion/libsvn_subr/config_file.c        (working copy)
@@ -940,6 +940,8 @@ svn_config_ensure(const char *config_dir, apr_pool
         "###                              HTTP operation."                   NL
         "###   http-chunked-requests      Whether to use chunked transfer"   NL
         "###                              encoding for HTTP requests body."  NL
+        "###   http-pipelining            Whether to use HTTP pipelining "   NL
+        "###                              or send requests one by one."      NL
         "###   neon-debug-mask            Debug mask for Neon HTTP library"  NL
         "###   ssl-authority-files        List of files, each of a trusted CA"
                                                                              NL

Reply via email to