From 585af938287d329333d9c755698fb25aa6471dab Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Tue, 10 Mar 2015 00:11:11 +0900
Subject: [PATCH] nghttpx: Remove last write/read fields for TLS

It seems that we don't care about this since we don't change buffer
pointer between would-block write/read and next write/read.  Somehow
we decided we need these fields.  As a precaution, we set
SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER in SSL_set_mode() for both server
and client contexts.
---
 src/shrpx_connection.cc | 46 +++++++++++++----------------------------
 src/shrpx_connection.h  |  4 ----
 src/shrpx_ssl.cc        |  2 ++
 3 files changed, 16 insertions(+), 36 deletions(-)

diff --git a/src/shrpx_connection.cc b/src/shrpx_connection.cc
index 685f23ed..5edc480b 100644
--- a/src/shrpx_connection.cc
+++ b/src/shrpx_connection.cc
@@ -151,21 +151,13 @@ void Connection::update_tls_warmup_writelen(size_t n) {
 }
 
 ssize_t Connection::write_tls(const void *data, size_t len) {
-  // SSL_write requires the same arguments (buf pointer and its
-  // length) on SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE.
-  // get_write_limit() may return smaller length than previously
-  // passed to SSL_write, which violates OpenSSL assumption.  To avoid
-  // this, we keep last legnth passed to SSL_write to
-  // tls.last_writelen if SSL_write indicated I/O blocking.
-  if (tls.last_writelen == 0) {
-    len = std::min(len, wlimit.avail());
-    len = std::min(len, get_tls_write_limit());
-    if (len == 0) {
-      return 0;
-    }
-  } else {
-    len = tls.last_writelen;
-    tls.last_writelen = 0;
+  // We set SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER, so we don't have to
+  // care about parameters after SSL_ERROR_WANT_READ or
+  // SSL_ERROR_WANT_WRITE.
+  len = std::min(len, wlimit.avail());
+  len = std::min(len, get_tls_write_limit());
+  if (len == 0) {
+    return 0;
   }
 
   auto rv = SSL_write(tls.ssl, data, len);
@@ -185,7 +177,6 @@ ssize_t Connection::write_tls(const void *data, size_t len) {
       }
       return SHRPX_ERR_NETWORK;
     case SSL_ERROR_WANT_WRITE:
-      tls.last_writelen = len;
       wlimit.startw();
       ev_timer_again(loop, &wt);
       return 0;
@@ -205,21 +196,13 @@ ssize_t Connection::write_tls(const void *data, size_t len) {
 }
 
 ssize_t Connection::read_tls(void *data, size_t len) {
-  // SSL_read requires the same arguments (buf pointer and its
-  // length) on SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE.
-  // rlimit_.avail() or rlimit_.avail() may return different length
-  // than the length previously passed to SSL_read, which violates
-  // OpenSSL assumption.  To avoid this, we keep last legnth passed
-  // to SSL_read to tls_last_readlen_ if SSL_read indicated I/O
-  // blocking.
-  if (tls.last_readlen == 0) {
-    len = std::min(len, rlimit.avail());
-    if (len == 0) {
-      return 0;
-    }
-  } else {
-    len = tls.last_readlen;
-    tls.last_readlen = 0;
+  // Although SSL_read manual says it requires the same arguments (buf
+  // pointer and its length) on SSL_ERROR_WANT_READ or
+  // SSL_ERROR_WANT_WRITE.  But after reading OpenSSL source code,
+  // there is no such requirement.
+  len = std::min(len, rlimit.avail());
+  if (len == 0) {
+    return 0;
   }
 
   auto rv = SSL_read(tls.ssl, data, len);
@@ -228,7 +211,6 @@ ssize_t Connection::read_tls(void *data, size_t len) {
     auto err = SSL_get_error(tls.ssl, rv);
     switch (err) {
     case SSL_ERROR_WANT_READ:
-      tls.last_readlen = len;
       return 0;
     case SSL_ERROR_WANT_WRITE:
       if (LOG_ENABLED(INFO)) {
diff --git a/src/shrpx_connection.h b/src/shrpx_connection.h
index 61765499..fc6a14a3 100644
--- a/src/shrpx_connection.h
+++ b/src/shrpx_connection.h
@@ -42,10 +42,6 @@ struct TLSConnection {
   SSL *ssl;
   ev_tstamp last_write_time;
   size_t warmup_writelen;
-  // length passed to SSL_write and SSL_read last time.  This is
-  // required since these functions require the exact same parameters
-  // on non-blocking I/O.
-  size_t last_writelen, last_readlen;
   bool initial_handshake_done;
   bool reneg_started;
 };
diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc
index e7926b98..07e6081f 100644
--- a/src/shrpx_ssl.cc
+++ b/src/shrpx_ssl.cc
@@ -366,6 +366,7 @@ SSL_CTX *create_ssl_context(const char *private_key_file,
 
   SSL_CTX_set_mode(ssl_ctx, SSL_MODE_AUTO_RETRY);
   SSL_CTX_set_mode(ssl_ctx, SSL_MODE_RELEASE_BUFFERS);
+  SSL_CTX_set_mode(ssl_ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
   if (get_config()->private_key_passwd) {
     SSL_CTX_set_default_passwd_cb(ssl_ctx, ssl_pem_passwd_cb);
     SSL_CTX_set_default_passwd_cb_userdata(ssl_ctx, (void *)get_config());
@@ -467,6 +468,7 @@ SSL_CTX *create_ssl_client_context() {
 
   SSL_CTX_set_mode(ssl_ctx, SSL_MODE_AUTO_RETRY);
   SSL_CTX_set_mode(ssl_ctx, SSL_MODE_RELEASE_BUFFERS);
+  SSL_CTX_set_mode(ssl_ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
 
   if (SSL_CTX_set_default_verify_paths(ssl_ctx) != 1) {
     LOG(WARN) << "Could not load system trusted ca certificates: "