Author: rhuijben
Date: Wed Oct 28 13:53:25 2015
New Revision: 1711013

URL: http://svn.apache.org/viewvc?rev=1711013&view=rev
Log:
Resolve a theoretical bug in our use of SSL_read() and SSL_write() in the
ssl buckets. I'm unable to reproduce an actual problem here, but the
documentation is very clear that moving the buffer is not allowed unless
we set this flag.

And the fix is luckily not that hard.

The reason for this strange requirement is that SSL_write() may already have
started transmitting data from the buffer when returning a SSL_ERROR_WANT_READ
or SSL_ERROR_WANT_WRITE. The next call will then continue where it left last
time.

If we would enable SSL_MODE_ENABLE_PARTIAL_WRITE we would be allowed to
shrink the buffer between calls, at the cost of copying the remaining
data to a smaller memory chunk.

* buckets/ssl_buckets.c
  (ssl_decrypt): Document SSL_read() requirement.
  (ssl_encrypt): Document SSL_write() requirement.
  (ssl_init_context): Set flag.

Modified:
    serf/trunk/buckets/ssl_buckets.c

Modified: serf/trunk/buckets/ssl_buckets.c
URL: 
http://svn.apache.org/viewvc/serf/trunk/buckets/ssl_buckets.c?rev=1711013&r1=1711012&r2=1711013&view=diff
==============================================================================
--- serf/trunk/buckets/ssl_buckets.c (original)
+++ serf/trunk/buckets/ssl_buckets.c Wed Oct 28 13:53:25 2015
@@ -952,6 +952,12 @@ static apr_status_t ssl_decrypt(void *ba
     ctx->want_read = FALSE; /* Reading now */
     ctx->crypt_status = APR_SUCCESS; /* Clear before calling SSL */
 
+    /* When an SSL_read() operation has to be repeated because of
+       SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE, it must be repeated
+       with the same arguments.
+
+       Luckily we can assume that we are called from the databuffer
+       implementation */
     /* Is there some data waiting to be read? */
     ssl_len = SSL_read(ctx->ssl, buf, bufsize);
     if (ssl_len < 0) {
@@ -1116,6 +1122,13 @@ static apr_status_t ssl_encrypt(void *ba
                           "ssl_encrypt: bucket read %d bytes; "\
                           "status %d\n", interim_len, status);
 
+                /* When an SSL_write() operation has to be repeated because of
+                   SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE, it MUST be
+                   repeated with the same arguments.
+
+                   Unless SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER is set...
+                   ... which we now do.
+                */
                 ctx->crypt_status = APR_SUCCESS; /* Clear before calling SSL */
                 ssl_len = SSL_write(ctx->ssl, vecs_data, interim_len);
 
@@ -1539,6 +1552,9 @@ static serf_ssl_context_t *ssl_init_cont
     ssl_ctx->ctx = SSL_CTX_new(SSLv23_client_method());
     SSL_CTX_set_options(ssl_ctx->ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
 
+    /* Allow calling SSL_write() with different buffer pointers */
+    SSL_CTX_set_mode(ssl_ctx->ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
+
     SSL_CTX_set_client_cert_cb(ssl_ctx->ctx, ssl_need_client_cert);
     ssl_ctx->cached_cert = 0;
     ssl_ctx->cached_cert_pw = 0;


Reply via email to