Implement zero-copy SSL buffers.

net/socket/ssl_client_socket_openssl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // OpenSSL binding for SSLClientSocket. The class layout and general principle
 // of operation is derived from SSLClientSocketNSS.
 
 #include "net/socket/ssl_client_socket_openssl.h"
 
 #include <openssl/err.h>
@@ skipping 16 matching lines @@
 #include "net/socket/ssl_session_cache_openssl.h"
 #include "net/ssl/openssl_client_key_store.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_info.h"
 
 namespace net {
 
 namespace {
 
+// This IOBuffer version is to be used with OpenSSL's non-copying interface,
+// to guarantee that the BIO object remains alive as long as the socket keeps
+// a reference to it's internal data buffer.
+//
+// |bio_buffer| must be retrieved from |bio| using the non-copying interface
+// BIO_nread0() or BIO_nwrite0(). The reference count of |bio| is increased to
+// make sure |bio| is not free'd if it is referenced from elsewhere.
+class WrappedBIOAndIOBuffer : public IOBuffer {
+ public:
+
+  explicit WrappedBIOAndIOBuffer(const char* bio_buffer, BIO* bio)

[Ryan Sleevi, 2014/04/30 21:33:32]

nit: delete newline on 46

+      : IOBuffer(const_cast<char*>(bio_buffer)), bio_(bio) {
+    CHECK(bio);
+
+    CRYPTO_add(&bio_->references, 1, CRYPTO_LOCK_BIO);
+  }
+
+ protected:
+  virtual ~WrappedBIOAndIOBuffer() {
+    data_ = NULL;
+    // Will count down the reference count, and free if the count reaches zero.
+    BIO_free_all(bio_);
+  }
+
+  BIO* bio_;
+};
+
 // Enable this to see logging for state machine state transitions.
 #if 0
 #define GotoState(s) do { DVLOG(2) << (void *)this << " " << __FUNCTION__ << \
                            " jump to state " << s; \
                            next_handshake_state_ = s; } while (0)
 #else
 #define GotoState(s) next_handshake_state_ = s
 #endif
 
 // This constant can be any non-negative/non-zero value (eg: it does not
@@ skipping 1222 matching lines @@
 }
 
 int SSLClientSocketOpenSSL::BufferSend(void) {
   if (transport_send_busy_)
     return ERR_IO_PENDING;
 
   if (!send_buffer_.get()) {
     // Get a fresh send buffer out of the send BIO.
     size_t max_read = BIO_ctrl_pending(transport_bio_);
     if (!max_read)
       return 0;  // Nothing pending in the OpenSSL write BIO.

[Ryan Sleevi, 2014/04/30 21:33:32]

Is this still necessary, given that BIO_nread0 with a NULL buf (second arg) will
return the size available for reading?

Is this even needed to check, now that we follow a non-copying pattern? If
available_read_bytes is 0, we can fall through, otherwise we can continue as
normal.

[haavardm, 2014/05/01 16:55:19]

The problem with letting 0 fall through is that calling  BIO_nread0 when there's
no data triggers the retry flag and the request for one byte. This happens
through the call to bio_read(bio, &dummy, 1) to avoid "code duplication " (see
bss_bio.c:303). That's a side effect that doesn't happen today and to me this
seems like something we want to avoid. 

On 2014/04/30 21:33:32, Ryan Sleevi wrote:

[Ryan Sleevi, 2014/05/08 22:59:28]

Yeah, I suppose the alternative would be to reset the retry flag, but I agree,
that seems less than ideal.

-    send_buffer_ = new DrainableIOBuffer(new IOBuffer(max_read), max_read);
-    int read_bytes = BIO_read(transport_bio_, send_buffer_->data(), max_read);
-    DCHECK_GT(read_bytes, 0);
-    CHECK_EQ(static_cast<int>(max_read), read_bytes);
+
+    char* bio_transport_buf;
+    // Get the internal buffer and number of bytes available for non-copying
+    // read.
+    // BIO_ctrl_pending() is still called above. Calling BIO_nread0() when
+    // there is no data triggers a call to BIO_nread() to read one byte. This
+    // has side effect that the retry flag is set.
+    int available_read_bytes = BIO_nread0(transport_bio_, &bio_transport_buf);
+    // BIO_nread0() might report less than BIO_ctrl_pending() due to the buffer
+    // offset and no ring buffer wrap-around for non-copying interface.
+    CHECK_GE(static_cast<int>(max_read), available_read_bytes);
+    if (available_read_bytes <= 0)
+      return 0;
+
+    send_buffer_ = new DrainableIOBuffer(

[Ryan Sleevi, 2014/04/30 21:33:32]

Huh. I'm not really sure why we use a DrainableIOBuffer for the OpenSSL case (we
don't for NSS, which also uses a circular buffer).

[haavardm, 2014/05/01 16:55:19]

I believe the drainable buffer is needed to handle partial write.

It's useful to be able to consume data if TransportWriteComplete() gets called
with only partial data sent. According to the socket documentation this may
happen :"Note: data may be written partially."

The calls BIO_nread() and Consume() in TransportWriteComplete() keeps the read
and write indexes of transport_bio_ and send_buffer_  in sync.

[Ryan Sleevi, 2014/05/08 22:59:28]

Yeah, but why create a DrainableIOBuffer (and have custom logic for spinning the
loop), as opposed to using BIO_nread0 to get the available, and then once data
is written - partial or not - using BIO_nread to commit that many bytes
(1421-1422)

In the code prior to this, once we used BIO_read, we had committed to the BIO
that we'd write that much data, so we had to use a DrainableIOBuffer to keep
track of that. However, with the copy-less approach, I *think* it should be
sufficient to BIO_nread0 the maximum, and then, based on what the transport
wrote, just advance the circular buffer with the BIO_nread.

eg: assume we have 10 bytes in the circular buffer
BIO_ctrl_pending == 10
BIO_nread0(bio, buf) == 10
transport_->Write(..., 10) == 5 [only half the buffer was written]
BIO_nread(bio, buf, 5) // what to do with ret here - currently dropping to the
floor?

Which will then cause
BIO_ctrl_pending == 5
BIO_nread(bio, buf) == 5
transport_->Write(..., 5) == 5 [the remaining half]
BIO_nread(bio, buf, 5);

which will then cause
BIO_ctrl_pending == 0

The DrainableIOBuffer was our poor-man's way of simulating that.

[haavardm, 2014/05/09 07:23:41]

Yes, I had a look into NSS yesterday and saw how you do it there. In NSS (as
opposed to  OpenSSL standard copy interface) you are able to report how much you
have written.

I agree we can remove the this for non-copy interface since OpenSSL takes care
of the accounting. It will also simplify the code.

+        new WrappedBIOAndIOBuffer(bio_transport_buf, transport_bio_),
+        available_read_bytes);
   }
 
   int rv = transport_->socket()->Write(
       send_buffer_.get(),
       send_buffer_->BytesRemaining(),
       base::Bind(&SSLClientSocketOpenSSL::BufferSendComplete,
                  base::Unretained(this)));
   if (rv == ERR_IO_PENDING) {
     transport_send_busy_ = true;
   } else {
@@ skipping 21 matching lines @@
   // One read for the SSL record header (~5 bytes) and one read for the SSL
   // record body. Rather than issuing these reads to the underlying socket
   // (and constantly allocating new IOBuffers), a single Read() request to
   // fill |transport_bio_| is issued. As long as an SSL client socket cannot
   // be gracefully shutdown (via SSL close alerts) and re-used for non-SSL
   // traffic, this over-subscribed Read()ing will not cause issues.
   size_t max_write = BIO_ctrl_get_write_guarantee(transport_bio_);
   if (!max_write)
     return ERR_IO_PENDING;
 
-  recv_buffer_ = new IOBuffer(max_write);
+  char* bio_transport_buf;
+  // Get the internal buffer and number of bytes available for non-copying
+  // write.
+  int available_write_bytes = BIO_nwrite0(transport_bio_, &bio_transport_buf);
+  // BIO_nwrite0() might report less than BIO_ctrl_get_write_guarantee() due to
+  // the buffer offset and no ring buffer wrap-around for non-copying interface.
+  DCHECK_GE(static_cast<int>(max_write), available_write_bytes);
+  if (available_write_bytes <= 0)
+    return ERR_IO_PENDING;
+
+  recv_buffer_ = new WrappedBIOAndIOBuffer(bio_transport_buf, transport_bio_);
   int rv = transport_->socket()->Read(
       recv_buffer_.get(),
-      max_write,
+      available_write_bytes,
       base::Bind(&SSLClientSocketOpenSSL::BufferRecvComplete,
                  base::Unretained(this)));
   if (rv == ERR_IO_PENDING) {
     transport_recv_busy_ = true;
   } else {
     rv = TransportReadComplete(rv);
   }
   return rv;
 }
 
@@ skipping 21 matching lines @@
 
     // Match the fix for http://crbug.com/249848 in NSS by erroring future reads
     // from the socket after a write error.
     //
     // TODO(davidben): Avoid having read and write ends interact this way.
     transport_write_error_ = result;
     (void)BIO_shutdown_wr(transport_bio_);
     send_buffer_ = NULL;
   } else {
     DCHECK(send_buffer_.get());
+    char* bio_transport_buf;
+    // Advance the buffer index.
+    int ret = BIO_nread(transport_bio_, &bio_transport_buf, result);
+    CHECK_EQ(ret, result);
+    CHECK_EQ(bio_transport_buf, send_buffer_->data());
     send_buffer_->DidConsume(result);
     DCHECK_GE(send_buffer_->BytesRemaining(), 0);
     if (send_buffer_->BytesRemaining() <= 0)
       send_buffer_ = NULL;
   }
 }
 
 int SSLClientSocketOpenSSL::TransportReadComplete(int result) {
   DCHECK(ERR_IO_PENDING != result);
   if (result <= 0) {
     DVLOG(1) << "TransportReadComplete result " << result;
     // Received 0 (end of file) or an error. Either way, bubble it up to the
     // SSL layer via the BIO. TODO(joth): consider stashing the error code, to
     // relay up to the SSL socket client (i.e. via DoReadCallback).
     if (result == 0)
       transport_recv_eof_ = true;
     (void)BIO_shutdown_wr(transport_bio_);
   } else if (transport_write_error_ < 0) {
     // Mirror transport write errors as read failures; transport_bio_ has been
     // shut down by TransportWriteComplete, so the BIO_write will fail, failing
     // the CHECK. http://crbug.com/335557.
     result = transport_write_error_;
   } else {
-    DCHECK(recv_buffer_.get());
-    int ret = BIO_write(transport_bio_, recv_buffer_->data(), result);
+    char* bio_transport_buf;
+    // Set the amount of data read into buffer.
+    int ret = BIO_nwrite(transport_bio_, &bio_transport_buf, result);
+    CHECK_EQ(ret, result);
+    CHECK_EQ(bio_transport_buf, recv_buffer_->data());
     // A write into a memory BIO should always succeed.
     // Force values on the stack for http://crbug.com/335557
     base::debug::Alias(&result);
     base::debug::Alias(&ret);
     CHECK_EQ(result, ret);
   }
   recv_buffer_ = NULL;
   transport_recv_busy_ = false;
   return result;
 }
@@ skipping 160 matching lines @@
   DVLOG(2) << "next protocol: '" << npn_proto_ << "' status: " << npn_status_;
   return SSL_TLSEXT_ERR_OK;
 }
 
 scoped_refptr<X509Certificate>
 SSLClientSocketOpenSSL::GetUnverifiedServerCertificateChain() const {
   return server_cert_;
 }
 
 }  // namespace net