Drop buffers in idle SSLClientSockets (and SSLServerSockets).

net/socket/socket_bio_adapter.cc

+// Copyright 2016 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.
+
+#include "net/socket/socket_bio_adapter.h"
+
+#include <openssl/bio.h>
+#include <string.h>
+
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/socket/stream_socket.h"
+#include "net/ssl/openssl_ssl_util.h"
+
+namespace net {
+
+SocketBIOAdapter::SocketBIOAdapter(StreamSocket* socket,
+                                   int read_buffer_capacity,
+                                   int write_buffer_capacity,
+                                   Delegate* delegate)
+    : socket_(socket),
+      read_buffer_capacity_(read_buffer_capacity),
+      read_offset_(0),
+      read_result_(0),
+      write_buffer_capacity_(write_buffer_capacity),
+      write_buffer_used_(0),
+      write_error_(OK),
+      delegate_(delegate),
+      weak_factory_(this) {
+  bio_.reset(BIO_new(&kBIOMethod));
+  bio_->ptr = this;
+  bio_->init = 1;
+
+  read_callback_ = base::Bind(&SocketBIOAdapter::OnSocketReadComplete,
+                              weak_factory_.GetWeakPtr());
+  write_callback_ = base::Bind(&SocketBIOAdapter::OnSocketWriteComplete,
+                               weak_factory_.GetWeakPtr());
+}
+
+SocketBIOAdapter::~SocketBIOAdapter() {
+  // BIOs are reference-counted and may outlive the adapter. Clear the pointer
+  // so future operations fail.
+  bio_->ptr = nullptr;
+}
+
+bool SocketBIOAdapter::HasPendingReadData() {
+  return read_result_ > 0;
+}
+
+int SocketBIOAdapter::BIORead(char* out, int len) {
+  if (len <= 0)
+    return len;
+
+  // If there is no result available synchronously, report any Write() errors
+  // that were observed. Otherwise the application may have encountered a socket
+  // error while writing that would otherwise not be reported until the
+  // application attempted to write again - which it may never do. See
+  // https://crbug.com/249848.
+  if (write_error_ != OK && write_error_ != ERR_IO_PENDING &&
+      (read_result_ == 0 || read_result_ == ERR_IO_PENDING)) {
+    OpenSSLPutNetError(FROM_HERE, write_error_);
+    return -1;
+  }
+
+  if (read_result_ == 0) {
+    // Instantiate the read buffer and read from the socket. Although only |len|
+    // bytes were requested, intentionally read to the full buffer size. The SSL
+    // layer reads the record header and body in separate reads to avoid
+    // overreading, but issuing one is more efficient. SSL sockets are not
+    // reused after shutdown for non-SSL traffic, so overreading is fine.
+    DCHECK(!read_buffer_);
+    DCHECK_EQ(0, read_offset_);
+    read_buffer_ = new IOBuffer(read_buffer_capacity_);
+    int result = socket_->Read(read_buffer_.get(), read_buffer_capacity_,
+                               read_callback_);
+    if (result == ERR_IO_PENDING) {
+      read_result_ = ERR_IO_PENDING;
+    } else {
+      HandleSocketReadResult(result);
+    }
+  }
+
+  // There is a pending Read(). Inform the caller to retry when it completes.
+  if (read_result_ == ERR_IO_PENDING) {
+    BIO_set_retry_read(bio());
+    return -1;
+  }
+
+  // If the last Read() failed, report the error.
+  if (read_result_ < 0) {
+    OpenSSLPutNetError(FROM_HERE, read_result_);
+    return -1;
+  }
+
+  // Report the result of the last Read() if non-empty.
+  CHECK_LT(read_offset_, read_result_);
+  len = std::min(len, read_result_ - read_offset_);
+  memcpy(out, read_buffer_->data() + read_offset_, len);
+  read_offset_ += len;
+
+  // Release the buffer when empty.
+  if (read_offset_ == read_result_) {
+    read_buffer_ = nullptr;
+    read_offset_ = 0;
+    read_result_ = 0;
+  }
+
+  return len;
+}
+
+void SocketBIOAdapter::HandleSocketReadResult(int result) {
+  DCHECK_NE(ERR_IO_PENDING, result);
+
+  // If an EOF, canonicalize to ERR_CONNECTION_CLOSED here, so that higher
+  // levels don't report success.
+  if (result == 0)
+    result = ERR_CONNECTION_CLOSED;
+
+  read_result_ = result;
+
+  // The read buffer is no longer needed.
+  if (read_result_ <= 0)
+    read_buffer_ = nullptr;
+}
+
+void SocketBIOAdapter::OnSocketReadComplete(int result) {
+  DCHECK_EQ(ERR_IO_PENDING, read_result_);
+
+  HandleSocketReadResult(result);
+  delegate_->OnReadReady();
+}
+
+int SocketBIOAdapter::BIOWrite(const char* in, int len) {
+  if (len <= 0)
+    return len;
+
+  // If the write buffer is not empty, there must be a pending Write() to flush
+  // it.
+  DCHECK(write_buffer_used_ == 0 || write_error_ == ERR_IO_PENDING);
+
+  // If a previous Write() failed, report the error.
+  if (write_error_ != OK && write_error_ != ERR_IO_PENDING) {
+    OpenSSLPutNetError(FROM_HERE, write_error_);
+    return -1;
+  }
+
+  // Instantiate the write buffer if needed.
+  if (!write_buffer_) {
+    DCHECK_EQ(0, write_buffer_used_);
+    write_buffer_ = new GrowableIOBuffer;
+    write_buffer_->SetCapacity(write_buffer_capacity_);
+  }
+
+  // If the ring buffer is full, inform the caller to try again later.
+  if (write_buffer_used_ == write_buffer_->capacity()) {
+    BIO_set_retry_write(bio());
+    return -1;
+  }
+
+  len = std::min(len, write_buffer_->capacity() - write_buffer_used_);
+
+  int to_copy = len;
+  while (to_copy > 0) {
+    // Determine where to write.
+    int write_offset = write_buffer_->offset() + write_buffer_used_;
+    if (write_offset >= write_buffer_->capacity())
+      write_offset -= write_buffer_->capacity();

[Ryan Sleevi, 2016/10/18 01:30:17]

This approach makes me *super* nervous from a security perspective, because
you're assuming no overflows here, and that's a subtle sort of assumption (ok,
yes, 17K means in practice no overflows, but you get what I mean - it's a
pattern match to a dangerous pattern)

It's also subtle to parse out and understand what's going on here - that
offset() represents the 'start' of the ring buffer, but that write_buffer_used_
may have wrapped around.

One way would be to write it as an expression without the risk of overflow (that
is, detect wrap-around by determining if write_buffer_->offset() >=
write_buffer_->capacity() - write_buffer_used_ ; which indicates that start is
after end), another would be to use the safe_numerics just to make the invariant
clearly explicit.

I know this is safe in practice, but it's just the trigger that it's dangerous
code w/o any safety net.

[davidben, 2016/10/18 20:49:11]

Every way to write this involves different subtle invariants. :-) I rewrote it
as a fixed version of the first iteration. How's this verison?

+    CHECK_LE(0, write_offset);
+    CHECK_LT(write_offset, write_buffer_->capacity());
+
+    // Determine how many bytes may be written.
+    int chunk = (write_offset < write_buffer_->offset())
+                    ? write_buffer_->offset() - write_offset
+                    : write_buffer_->capacity() - write_offset;
+    chunk = std::min(chunk, to_copy);
+
+    memcpy(write_buffer_->StartOfBuffer() + write_offset, in, chunk);
+    in += chunk;
+    to_copy -= chunk;
+    write_buffer_used_ += chunk;
+  }
+
+  // Schedule a socket Write() if necessary. (The ring buffer may previously
+  // have been empty.)
+  SocketWrite();
+
+  // If a read-interrupting write error was synchronously discovered,
+  // asynchronously notify OnReadReady. See https://crbug.com/249848. Avoid
+  // reentrancy by deferring it to a later event loop iteration.
+  if (write_error_ != OK && write_error_ != ERR_IO_PENDING &&
+      read_result_ == ERR_IO_PENDING) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(&SocketBIOAdapter::CallOnReadReady,
+                              weak_factory_.GetWeakPtr()));
+  }
+
+  return len;
+}
+
+void SocketBIOAdapter::SocketWrite() {
+  while (write_error_ == OK && write_buffer_used_ > 0) {
+    int write_size =
+        std::min(write_buffer_used_, write_buffer_->RemainingCapacity());
+    int result =
+        socket_->Write(write_buffer_.get(), write_size, write_callback_);
+    if (result == ERR_IO_PENDING) {
+      write_error_ = ERR_IO_PENDING;
+      return;
+    }
+
+    HandleSocketWriteResult(result);
+  }
+}
+
+void SocketBIOAdapter::HandleSocketWriteResult(int result) {
+  DCHECK_NE(ERR_IO_PENDING, result);
+
+  if (result < 0) {
+    write_error_ = result;
+
+    // The write buffer is no longer needed.
+    write_buffer_ = nullptr;
+    write_buffer_used_ = 0;
+    return;
+  }
+
+  // Advance the ring buffer.
+  write_buffer_->set_offset(write_buffer_->offset() + result);
+  write_buffer_used_ -= result;
+  if (write_buffer_->RemainingCapacity() == 0)
+    write_buffer_->set_offset(0);
+  write_error_ = OK;
+
+  // Release the write buffer if empty.
+  if (write_buffer_used_ == 0)
+    write_buffer_ = nullptr;
+}
+
+void SocketBIOAdapter::OnSocketWriteComplete(int result) {
+  DCHECK_EQ(ERR_IO_PENDING, write_error_);
+
+  bool was_full = write_buffer_used_ == write_buffer_->capacity();
+
+  HandleSocketWriteResult(result);
+  SocketWrite();
+
+  // If transitioning from being unable to accept data to being able to, signal
+  // OnWriteReady.
+  if (was_full) {
+    base::WeakPtr<SocketBIOAdapter> guard(weak_factory_.GetWeakPtr());
+    delegate_->OnWriteReady();
+    // OnWriteReady may delete the adapter.
+    if (!guard)
+      return;
+  }
+
+  // Write errors are fed back into BIO_read once the read buffer is empty. If
+  // BIO_read is currently blocked, signal early that a read result is ready.
+  if (result < 0 && read_result_ == ERR_IO_PENDING)
+    delegate_->OnReadReady();
+}
+
+void SocketBIOAdapter::CallOnReadReady() {
+  if (read_result_ == ERR_IO_PENDING)
+    delegate_->OnReadReady();
+}
+
+SocketBIOAdapter* SocketBIOAdapter::GetAdapter(BIO* bio) {
+  DCHECK_EQ(&kBIOMethod, bio->method);
+  SocketBIOAdapter* adapter = reinterpret_cast<SocketBIOAdapter*>(bio->ptr);
+  if (adapter)
+    DCHECK_EQ(bio, adapter->bio());
+  return adapter;
+}
+
+int SocketBIOAdapter::BIOWriteWrapper(BIO* bio, const char* in, int len) {
+  BIO_clear_retry_flags(bio);
+
+  SocketBIOAdapter* adapter = GetAdapter(bio);
+  if (!adapter) {
+    OpenSSLPutNetError(FROM_HERE, ERR_UNEXPECTED);
+    return -1;
+  }
+
+  return adapter->BIOWrite(in, len);
+}
+
+int SocketBIOAdapter::BIOReadWrapper(BIO* bio, char* out, int len) {
+  BIO_clear_retry_flags(bio);
+
+  SocketBIOAdapter* adapter = GetAdapter(bio);
+  if (!adapter) {
+    OpenSSLPutNetError(FROM_HERE, ERR_UNEXPECTED);
+    return -1;
+  }
+
+  return adapter->BIORead(out, len);
+}
+
+long SocketBIOAdapter::BIOCtrlWrapper(BIO* bio,
+                                      int cmd,
+                                      long larg,
+                                      void* parg) {
+  switch (cmd) {
+    case BIO_CTRL_FLUSH:
+      // The SSL stack requires BIOs handle BIO_flush.
+      return 1;
+  }
+
+  NOTIMPLEMENTED();
+  return 0;
+}
+
+const BIO_METHOD SocketBIOAdapter::kBIOMethod = {
+    0,        // type (unused)
+    nullptr,  // name (unused)
+    SocketBIOAdapter::BIOWriteWrapper,
+    SocketBIOAdapter::BIOReadWrapper,
+    nullptr,  // puts
+    nullptr,  // gets
+    SocketBIOAdapter::BIOCtrlWrapper,
+    nullptr,  // create
+    nullptr,  // destroy
+    nullptr,  // callback_ctrl
+};
+
+}  // namespace net