Convert Ssl::Bio::rbuf from MemBuf to SBuf

-The Ssl::Bio::rBufData fixed to return an SBuf too
-Required changes to the tunnel.cc:
  * The TunnelStateData::preReadServerData added to hold received data from
    the SSL server (Server hello message)
  * The TunnelStateData::copyServerBytes() added and provides similar
    functionality to TunnelStateData::copyClientBytes() but for client side:
    It acts as a wrapper for copyRead() method when copies data from server
    to client side, checks if the TunnelStateData::preReadServerData is not
    empty and if yes copy data from this buffer to the client side
-Other required changes

diff --git a/src/client_side.cc b/src/client_side.cc
index 0e4612df09d732d4fde0272c7ab89d0217c116e3..b5b7d52e8cf990b35e5c8df016844a3eda08dfe2 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3875,7 +3875,7 @@ clientPeekAndSpliceSSL(int fd, void *data)
         return;
     }
 
-    if (bio->rBufData().contentSize() > 0)
+    if (!bio->rBufData().isEmpty())
         conn->receivedFirstByte();
 
     if (bio->gotHello()) {
@@ -3959,8 +3959,8 @@ ConnStateData::splice()
     auto ssl = fd_table[clientConnection->fd].ssl;
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ClientBio *bio = static_cast<Ssl::ClientBio *>(b->ptr);
-    MemBuf const &rbuf = bio->rBufData();
-    debugs(83,5, "Bio for  " << clientConnection << " read " << rbuf.contentSize() << " helo bytes");
+    SBuf const &rbuf = bio->rBufData();
+    debugs(83,5, "Bio for  " << clientConnection << " read " << rbuf.length() << " helo bytes");
     // Do splice:
     fd_table[clientConnection->fd].read_method = &default_read_method;
     fd_table[clientConnection->fd].write_method = &default_write_method;
@@ -3969,17 +3969,14 @@ ConnStateData::splice()
         // set the current protocol to something sensible (was "HTTPS" for the bumping process)
         // we are sending a faked-up HTTP/1.1 message wrapper, so go with that.
         transferProtocol = Http::ProtocolVersion();
-        // XXX: copy from MemBuf reallocates, not a regression since old code did too
-        SBuf temp;
-        temp.append(rbuf.content(), rbuf.contentSize());
-        fakeAConnectRequest("intercepted TLS spliced", temp);
+        fakeAConnectRequest("intercepted TLS spliced", rbuf);
     } else {
         // XXX: assuming that there was an HTTP/1.1 CONNECT to begin with...
 
         // reset the current protocol to HTTP/1.1 (was "HTTPS" for the bumping process)
         transferProtocol = Http::ProtocolVersion();
         // inBuf still has the "CONNECT ..." request data, reset it to SSL hello message
-        inBuf.append(rbuf.content(), rbuf.contentSize());
+        inBuf.append(rbuf);
         ClientSocketContext::Pointer context = pipeline.front();
         ClientHttpRequest *http = context->http;
         tunnelStart(http);

diff --git a/src/ssl/bio.cc b/src/ssl/bio.cc
index f3b2aa24b79d2f3fe5af440660265ca524020a6b..3f82cc01b4fcf78c7d1d413dfbf7994dce027534 100644 (file)
--- a/src/ssl/bio.cc
+++ b/src/ssl/bio.cc
@@ -354,17 +354,9 @@ Ssl::Bio::read(char *buf, int size, BIO *table)
 int
 Ssl::Bio::readAndBuffer(BIO *table, const char *description)
 {
-    prepReadBuf();
     char buf[SQUID_TCP_SO_RCVBUF ];
-    size_t size = min(rbuf.potentialSpaceSize(), (mb_size_t)SQUID_TCP_SO_RCVBUF);
-    if (size <= 0) {
-        debugs(83, DBG_IMPORTANT, "Not enough space to hold " <<
-               rbuf.contentSize() << "+ byte " << description);
-        return -1;
-    }
-
     const int bytes = Ssl::Bio::read(buf, sizeof(buf), table);
-    debugs(83, 5, "read " << bytes << " out of " << size << " bytes"); // move to Ssl::Bio::read()
+    debugs(83, 5, "read " << bytes << " bytes"); // move to Ssl::Bio::read()
 
     if (bytes > 0) {
         rbuf.append(buf, bytes);
@@ -391,13 +383,6 @@ Ssl::Bio::stateChanged(const SSL *ssl, int where, int ret)
            SSL_state_string(ssl) << " (" << SSL_state_string_long(ssl) << ")");
 }
 
-void
-Ssl::Bio::prepReadBuf()
-{
-    if (rbuf.isNull())
-        rbuf.init(4096, 65536);
-}
-
 bool
 Ssl::ClientBio::isClientHello(int state)
 {
@@ -451,9 +436,9 @@ Ssl::ClientBio::read(char *buf, int size, BIO *table)
     }
 
     if (helloState == atHelloStarted) {
-        debugs(83, 7, "SSL Header: " << Raw(nullptr, rbuf.content(), rbuf.contentSize()).hex());
+        debugs(83, 7, "SSL Header: " << Raw(nullptr, rbuf.rawContent(), rbuf.length()).hex());
 
-        if (helloSize > rbuf.contentSize()) {
+        if (helloSize > (int)rbuf.length()) {
             BIO_set_retry_read(table);
             return -1;
         }
@@ -468,9 +453,9 @@ Ssl::ClientBio::read(char *buf, int size, BIO *table)
     }
 
     if (helloState == atHelloReceived) {
-        if (rbuf.hasContent()) {
-            int bytes = (size <= rbuf.contentSize() ? size : rbuf.contentSize());
-            memcpy(buf, rbuf.content(), bytes);
+        if (!rbuf.isEmpty()) {
+            int bytes = (size <= (int)rbuf.length() ? size : rbuf.length());
+            memcpy(buf, rbuf.rawContent(), bytes);
             rbuf.consume(bytes);
             return bytes;
         } else
@@ -500,9 +485,7 @@ Ssl::ServerBio::readAndBufferServerHelloMsg(BIO *table, const char *description)
     if (ret <= 0)
         return ret;
 
-    // XXX: Replace Bio::MemBuf with SBuf to avoid this performance overhead.
-    const SBuf rbuf2(rbuf.content(), rbuf.contentSize());
-    if (!parser_.parseServerHello(rbuf2)) {
+    if (!parser_.parseServerHello(rbuf)) {
         if (!parser_.parseError) 
             BIO_set_retry_read(table);
         return -1;
@@ -516,7 +499,7 @@ Ssl::ServerBio::read(char *buf, int size, BIO *table)
 {
     if (!parser_.parseDone || record_) {
         int ret = readAndBufferServerHelloMsg(table, "TLS server Hello");
-        if (!rbuf.contentSize() && parser_.parseDone && ret <= 0)
+        if (!rbuf.length() && parser_.parseDone && ret <= 0)
             return ret;
     }
 
@@ -527,10 +510,10 @@ Ssl::ServerBio::read(char *buf, int size, BIO *table)
     }
 
     if (parser_.parseDone && !parser_.parseError) {
-        int unsent = rbuf.contentSize() - rbufConsumePos;
+        int unsent = rbuf.length() - rbufConsumePos;
         if (unsent > 0) {
             int bytes = (size <= unsent ? size : unsent);
-            memcpy(buf, rbuf.content()+rbufConsumePos, bytes);
+            memcpy(buf, rbuf.rawContent() + rbufConsumePos, bytes);
             rbufConsumePos += bytes;
             debugs(83, 7, "Pass " << bytes << " bytes to openSSL as read");
             return bytes;
@@ -949,17 +932,17 @@ Ssl::Bio::sslFeatures::get(const SSL *ssl)
 }
 
 int
-Ssl::Bio::sslFeatures::parseMsgHead(const MemBuf &buf)
+Ssl::Bio::sslFeatures::parseMsgHead(const SBuf &buf)
 {
-    debugs(83, 7, "SSL Header: " << Raw(nullptr, buf.content(), buf.contentSize()).hex());
+    debugs(83, 7, "SSL Header: " << Raw(nullptr, buf.rawContent(), buf.length()).hex());
 
-    if (buf.contentSize() < 5)
+    if (buf.length() < 5)
         return 0;
 
     if (helloMsgSize > 0)
         return helloMsgSize;
 
-    const unsigned char *head = (const unsigned char *)buf.content();
+    const unsigned char *head = (const unsigned char *)buf.rawContent();
     // Check for SSLPlaintext/TLSPlaintext record
     // RFC6101 section 5.2.1
     // RFC5246 section 6.2.1
@@ -991,7 +974,7 @@ Ssl::Bio::sslFeatures::parseMsgHead(const MemBuf &buf)
 }
 
 bool
-Ssl::Bio::sslFeatures::get(const MemBuf &buf, bool record)
+Ssl::Bio::sslFeatures::get(const SBuf &buf, bool record)
 {
     int msgSize;
     if ((msgSize = parseMsgHead(buf)) <= 0) {
@@ -999,22 +982,20 @@ Ssl::Bio::sslFeatures::get(const MemBuf &buf, bool record)
         return false;
     }
 
-    if (msgSize > buf.contentSize()) {
+    if (msgSize > (int)buf.length()) {
         debugs(83, 2, "Partial SSL handshake message, can not parse!");
         return false;
     }
 
-    if (record) {
-        helloMessage.clear();
-        helloMessage.append(buf.content(), buf.contentSize());
-    }
+    if (record)
+        helloMessage = buf;
 
-    const unsigned char *msg = (const unsigned char *)buf.content();
+    const unsigned char *msg = (const unsigned char *)buf.rawContent();
     if (msg[0] & 0x80)
         return parseV23Hello(msg, (size_t)msgSize);
     else {
         // Hello messages require 5 bytes header + 1 byte Msg type + 3 bytes for Msg size
-        if (buf.contentSize() < 9)
+        if (buf.length() < 9)
             return false;
 
         // Check for the Handshake/Message type

diff --git a/src/ssl/bio.h b/src/ssl/bio.h
index 70e57b97dc1db79ea9198d6c7e5f36820d675710..3fc79f70b93a424b2be2f5f4f4fba726278273d6 100644 (file)
--- a/src/ssl/bio.h
+++ b/src/ssl/bio.h
@@ -248,7 +248,7 @@ public:
         bool get(const SSL *ssl); ///< Retrieves the features from SSL object
         /// Retrieves features from raw SSL Hello message.
         /// \param record  whether to store Message to the helloMessage member
-        bool get(const MemBuf &, bool record = true);
+        bool get(const SBuf &, bool record = true);
         /// Parses a v3 ClientHello message
         bool parseV3Hello(const unsigned char *hello, size_t helloSize);
         /// Parses a v23 ClientHello message
@@ -265,7 +265,7 @@ public:
         /// \retval >0 if the hello size is retrieved
         /// \retval 0 if the contents of the buffer are not enough
         /// \retval <0 if the contents of buf are not SSLv3 or TLS hello message
-        int parseMsgHead(const MemBuf &);
+        int parseMsgHead(const SBuf &);
     public:
         int sslVersion; ///< The requested/used SSL version
         int compressMethod; ///< The requested/used compressed  method
@@ -310,16 +310,13 @@ public:
     /// Tells ssl connection to use BIO and monitor state via stateChanged()
     static void Link(SSL *ssl, BIO *bio);
 
-    /// Prepare the rbuf buffer to accept hello data
-    void prepReadBuf();
-
     /// Reads data from socket and record them to a buffer
     int readAndBuffer(BIO *table, const char *description);
 
-    const MemBuf &rBufData() {return rbuf;}
+    const SBuf &rBufData() {return rbuf;}
 protected:
     const int fd_; ///< the SSL socket we are reading and writing
-    MemBuf rbuf;  ///< Used to buffer input data.
+    SBuf rbuf;  ///< Used to buffer input data.
 };
 
 /// BIO node to handle socket IO for squid client side

diff --git a/src/tunnel.cc b/src/tunnel.cc
index 36339d748f6e3e6693f0ce43f271070a34269463..f68b6261d1bc8e2c97e7ffdf229e7c621864c341 100644 (file)
--- a/src/tunnel.cc
+++ b/src/tunnel.cc
@@ -166,6 +166,7 @@ public:
     MemBuf *connectRespBuf; ///< accumulates peer CONNECT response when we need it
     bool connectReqWriting; ///< whether we are writing a CONNECT request to a peer
     SBuf preReadClientData;
+    SBuf preReadServerData;
     time_t started;         ///< when this tunnel was initiated.
 
     void copyRead(Connection &from, IOCB *completion);
@@ -216,6 +217,7 @@ public:
     static void ReadConnectResponseDone(const Comm::ConnectionPointer &, char *buf, size_t len, Comm::Flag errcode, int xerrno, void *data);
     void readConnectResponseDone(char *buf, size_t len, Comm::Flag errcode, int xerrno);
     void copyClientBytes();
+    void copyServerBytes();
 };
 
 static const char *const conn_established = "HTTP/1.1 200 Connection established\r\n\r\n";
@@ -739,7 +741,7 @@ TunnelStateData::writeClientDone(char *, size_t len, Comm::Flag flag, int xerrno
     CbcPointer<TunnelStateData> safetyLock(this);   /* ??? should be locked by the caller... */
 
     if (cbdataReferenceValid(this))
-        copyRead(server, ReadServer);
+        copyServerBytes();
 }
 
 static void
@@ -831,6 +833,20 @@ TunnelStateData::copyClientBytes()
         copyRead(client, ReadClient);
 }
 
+void
+TunnelStateData::copyServerBytes()
+{
+    if (preReadServerData.length()) {
+        size_t copyBytes = preReadServerData.length() > SQUID_TCP_SO_RCVBUF ? SQUID_TCP_SO_RCVBUF : preReadServerData.length();
+        memcpy(server.buf, preReadServerData.rawContent(), copyBytes);
+        preReadServerData.consume(copyBytes);
+        server.bytesIn(copyBytes);
+        if (keepGoingAfterRead(copyBytes, Comm::OK, 0, server, client))
+            copy(copyBytes, server, client, TunnelStateData::WriteClientDone);
+    } else
+        copyRead(server, ReadServer);
+}
+
 /**
  * Set the HTTP status for this request and sets the read handlers for client
  * and server side connections.
@@ -846,7 +862,7 @@ tunnelStartShoveling(TunnelStateData *tunnelState)
 
         // Shovel any payload already pushed into reply buffer by the server response
         if (!tunnelState->server.len)
-            tunnelState->copyRead(tunnelState->server, TunnelStateData::ReadServer);
+            tunnelState->copyServerBytes();
         else {
             debugs(26, DBG_DATA, "Tunnel server PUSH Payload: \n" << Raw("", tunnelState->server.buf, tunnelState->server.len) << "\n----------");
             tunnelState->copy(tunnelState->server.len, tunnelState->server, tunnelState->client, TunnelStateData::WriteClientDone);
@@ -1298,11 +1314,8 @@ switchToTunnel(HttpRequest *request, Comm::ConnectionPointer &clientConn, Comm::
     assert(ssl);
     BIO *b = SSL_get_rbio(ssl);
     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(b->ptr);
-    const MemBuf &buf = srvBio->rBufData();
-
-    AsyncCall::Pointer call = commCbCall(5,5, "tunnelConnectedWriteDone",
-                                         CommIoCbPtrFun(tunnelConnectedWriteDone, tunnelState));
-    tunnelState->client.write(buf.content(), buf.contentSize(), call, NULL);
+    tunnelState->preReadServerData = srvBio->rBufData();
+    tunnelStartShoveling(tunnelState);
 }
 #endif //USE_OPENSSL