------------------------------------------------------------ revno: 13619 revision-id: chtsanti@users.sourceforge.net-20141001123158-m6e3c8t98q7l25jf parent: squid3@treenet.co.nz-20140930131158-ajzas7x8d3dfmigm committer: Christos Tsantilas branch nick: trunk timestamp: Wed 2014-10-01 15:31:58 +0300 message: Polish peek-and-splice related code - Record SSL bump action at each bumping step in the Ssl::ServerBump. The new Ssl::ServerBump::act member added for this purpose. - Split Ssl::PeerConnector::checkForPeekAndSplice to two methods (checkForPeekAndSplice and checkForPeekAndSpliceDone) add some documentation, and polish the code. - Polish httpsSslBumpStep2AccessCheckDone function (client_side.cc file) This is a Measurement Factory project ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: chtsanti@users.sourceforge.net-20141001123158-\ # m6e3c8t98q7l25jf # target_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # testament_sha1: 9ea5ef7bf86b1e3e26142d34408f727b20162cf4 # timestamp: 2014-10-02 09:21:34 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/trunk/ # base_revision_id: squid3@treenet.co.nz-20140930131158-\ # ajzas7x8d3dfmigm # # Begin patch === modified file 'src/client_side.cc' --- src/client_side.cc 2014-09-29 05:13:17 +0000 +++ src/client_side.cc 2014-10-01 12:31:58 +0000 @@ -3954,7 +3954,7 @@ debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " cannot be generated. ssl_crtd response: " << reply_message.getBody()); } else { debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " was successfully recieved from ssl_crtd"); - if (sslServerBump && (sslServerBump->mode == Ssl::bumpPeek || sslServerBump->mode == Ssl::bumpStare)) { + if (sslServerBump && (sslServerBump->act.step1 == Ssl::bumpPeek || sslServerBump->act.step1 == Ssl::bumpStare)) { doPeekAndSpliceStep(); SSL *ssl = fd_table[clientConnection->fd].ssl; bool ret = Ssl::configureSSLUsingPkeyAndCertFromMemory(ssl, reply_message.getBody().c_str(), *port); @@ -4071,7 +4071,7 @@ assert(sslBumpCertKey.size() > 0 && sslBumpCertKey[0] != '\0'); // Disable caching for bumpPeekAndSplice mode - if (!(sslServerBump && (sslServerBump->mode == Ssl::bumpPeek || sslServerBump->mode == Ssl::bumpStare))) { + if (!(sslServerBump && (sslServerBump->act.step1 == Ssl::bumpPeek || sslServerBump->act.step1 == Ssl::bumpStare))) { debugs(33, 5, "Finding SSL certificate for " << sslBumpCertKey << " in cache"); Ssl::LocalContextStorage * ssl_ctx_cache = Ssl::TheGlobalContextStorage.getLocalStorage(port->s); SSL_CTX * dynCtx = NULL; @@ -4111,7 +4111,7 @@ #endif // USE_SSL_CRTD debugs(33, 5, HERE << "Generating SSL certificate for " << certProperties.commonName); - if (sslServerBump && (sslServerBump->mode == Ssl::bumpPeek || sslServerBump->mode == Ssl::bumpStare)) { + if (sslServerBump && (sslServerBump->act.step1 == Ssl::bumpPeek || sslServerBump->act.step1 == Ssl::bumpStare)) { doPeekAndSpliceStep(); SSL *ssl = fd_table[clientConnection->fd].ssl; if (!Ssl::configureSSL(ssl, certProperties, *port)) @@ -4280,16 +4280,25 @@ return; debugs(33, 5, "Answer: " << answer << " kind:" << answer.kind); - if (answer == ACCESS_ALLOWED && answer.kind != Ssl::bumpNone && answer.kind != Ssl::bumpSplice) { - if (answer.kind == Ssl::bumpTerminate) - comm_close(connState->clientConnection->fd); - else { - if (answer.kind != Ssl::bumpPeek && answer.kind != Ssl::bumpStare) - connState->sslBumpMode = Ssl::bumpBump; - else - connState->sslBumpMode = (Ssl::BumpMode)answer.kind; - connState->startPeekAndSpliceDone(); - } + assert(connState->serverBump()); + Ssl::BumpMode bumpAction; + if (answer == ACCESS_ALLOWED) { + if (answer.kind == Ssl::bumpNone) + bumpAction = Ssl::bumpSplice; + else if (answer.kind == Ssl::bumpClientFirst || answer.kind == Ssl::bumpServerFirst) + bumpAction = Ssl::bumpBump; + else + bumpAction = (Ssl::BumpMode)answer.kind; + } else + bumpAction = Ssl::bumpSplice; + + connState->serverBump()->act.step2 = bumpAction; + connState->sslBumpMode = bumpAction; + + if (bumpAction == Ssl::bumpTerminate) { + comm_close(connState->clientConnection->fd); + } else if (bumpAction != Ssl::bumpSplice) { + connState->startPeekAndSpliceDone(); } else { //Normally we can splice here, because we just got client hello message SSL *ssl = fd_table[connState->clientConnection->fd].ssl; @@ -4298,8 +4307,6 @@ MemBuf const &rbuf = bio->rBufData(); debugs(83,5, "Bio for " << connState->clientConnection << " read " << rbuf.contentSize() << " helo bytes"); // Do splice: - - connState->sslBumpMode = Ssl::bumpSplice; fd_table[connState->clientConnection->fd].read_method = &default_read_method; fd_table[connState->clientConnection->fd].write_method = &default_write_method; === modified file 'src/ssl/PeerConnector.cc' --- src/ssl/PeerConnector.cc 2014-09-25 10:34:22 +0000 +++ src/ssl/PeerConnector.cc 2014-10-01 12:31:58 +0000 @@ -287,14 +287,14 @@ void switchToTunnel(HttpRequest *request, int *status_ptr, Comm::ConnectionPointer & clientConn, Comm::ConnectionPointer &srvConn); void -Ssl::PeerConnector::cbCheckForPeekAndSplice(allow_t answer, void *data) +Ssl::PeerConnector::cbCheckForPeekAndSpliceDone(allow_t answer, void *data) { Ssl::PeerConnector *peerConnect = (Ssl::PeerConnector *) data; - peerConnect->checkForPeekAndSplice(true, (Ssl::BumpMode)answer.kind); + peerConnect->checkForPeekAndSpliceDone((Ssl::BumpMode)answer.kind); } -bool -Ssl::PeerConnector::checkForPeekAndSplice(bool checkDone, Ssl::BumpMode peekMode) +void +Ssl::PeerConnector::checkForPeekAndSplice() { SSL *ssl = fd_table[serverConn->fd].ssl; // Mark Step3 of bumping @@ -306,44 +306,48 @@ } } - if (!checkDone) { - ACLFilledChecklist *acl_checklist = new ACLFilledChecklist( - ::Config.accessList.ssl_bump, - request.getRaw(), NULL); - acl_checklist->nonBlockingCheck(Ssl::PeerConnector::cbCheckForPeekAndSplice, this); - return false; - } + ACLFilledChecklist *acl_checklist = new ACLFilledChecklist( + ::Config.accessList.ssl_bump, + request.getRaw(), NULL); + acl_checklist->nonBlockingCheck(Ssl::PeerConnector::cbCheckForPeekAndSpliceDone, this); +} +void +Ssl::PeerConnector::checkForPeekAndSpliceDone(Ssl::BumpMode const action) +{ + SSL *ssl = fd_table[serverConn->fd].ssl; BIO *b = SSL_get_rbio(ssl); Ssl::ServerBio *srvBio = static_cast(b->ptr); debugs(83,5, "Will check for peek and splice on FD " << serverConn->fd); - // bump, peek, stare, server-first,client-first are all mean bump the connection - if (peekMode < Ssl::bumpSplice) - peekMode = Ssl::bumpBump; - - if (peekMode == Ssl::bumpSplice && !srvBio->canSplice()) - peekMode = Ssl::bumpPeek; - else if (peekMode == Ssl::bumpBump && !srvBio->canBump()) - peekMode = Ssl::bumpSplice; - - if (peekMode == Ssl::bumpTerminate) { + Ssl::BumpMode finalAction = action; + // adjust the final bumping mode if needed + if (finalAction < Ssl::bumpSplice) + finalAction = Ssl::bumpBump; + + if (finalAction == Ssl::bumpSplice && !srvBio->canSplice()) + finalAction = Ssl::bumpBump; + else if (finalAction == Ssl::bumpBump && !srvBio->canBump()) + finalAction = Ssl::bumpSplice; + + // Record final decision + if (request->clientConnectionManager.valid()) + request->clientConnectionManager->sslBumpMode = finalAction; + + if (finalAction == Ssl::bumpTerminate) { comm_close(serverConn->fd); comm_close(clientConn->fd); - } else if (peekMode != Ssl::bumpSplice) { + } else if (finalAction != Ssl::bumpSplice) { //Allow write, proceed with the connection srvBio->holdWrite(false); srvBio->recordInput(false); Comm::SetSelect(serverConn->fd, COMM_SELECT_WRITE, &NegotiateSsl, this, 0); debugs(83,5, "Retry the fwdNegotiateSSL on FD " << serverConn->fd); - return true; } else { static int status_code = 0; debugs(83,5, "Revert to tunnel FD " << clientConn->fd << " with FD " << serverConn->fd); switchToTunnel(request.getRaw(), &status_code, clientConn, serverConn); - return false; } - return false; } void @@ -496,7 +500,7 @@ case SSL_ERROR_WANT_WRITE: if ((request->clientConnectionManager->sslBumpMode == Ssl::bumpPeek || request->clientConnectionManager->sslBumpMode == Ssl::bumpStare) && srvBio->holdWrite()) { debugs(81, DBG_IMPORTANT, "hold write on SSL connection on FD " << fd); - checkForPeekAndSplice(false, Ssl::bumpNone); + checkForPeekAndSplice(); return; } Comm::SetSelect(fd, COMM_SELECT_WRITE, &NegotiateSsl, this, 0); @@ -514,7 +518,7 @@ #if 1 if ((request->clientConnectionManager->sslBumpMode == Ssl::bumpPeek || request->clientConnectionManager->sslBumpMode == Ssl::bumpStare) && srvBio->holdWrite()) { debugs(81, 3, "Error (" << ERR_error_string(ssl_lib_error, NULL) << ") but, hold write on SSL connection on FD " << fd); - checkForPeekAndSplice(false, Ssl::bumpNone); + checkForPeekAndSplice(); return; } #endif === modified file 'src/ssl/PeerConnector.h' --- src/ssl/PeerConnector.h 2014-09-13 13:59:43 +0000 +++ src/ssl/PeerConnector.h 2014-10-01 12:31:58 +0000 @@ -116,7 +116,13 @@ /// It is called multiple times untill the negotiation finish or aborted. void negotiateSsl(); - bool checkForPeekAndSplice(bool, Ssl::BumpMode); + /// Initiates the ssl_bump acl check in step3 SSL bump step to decide + /// about bumping, splicing or terminating the connection. + void checkForPeekAndSplice(); + + /// Callback function for ssl_bump acl check in step3 SSL bump step. + /// Handles the final bumping decision. + void checkForPeekAndSpliceDone(Ssl::BumpMode const); /// Called when the SSL negotiation step aborted because data needs to /// be transferred to/from SSL server or on error. In the first case @@ -149,8 +155,8 @@ /// A wrapper function for negotiateSsl for use with Comm::SetSelect static void NegotiateSsl(int fd, void *data); - /// A wrapper function for checkForPeekAndSplice for use with acl - static void cbCheckForPeekAndSplice(allow_t answer, void *data); + /// A wrapper function for checkForPeekAndSpliceDone for use with acl + static void cbCheckForPeekAndSpliceDone(allow_t answer, void *data); HttpRequestPointer request; ///< peer connection trigger or cause Comm::ConnectionPointer serverConn; ///< TCP connection to the peer === modified file 'src/ssl/ServerBump.cc' --- src/ssl/ServerBump.cc 2014-09-13 13:59:43 +0000 +++ src/ssl/ServerBump.cc 2014-10-01 12:31:58 +0000 @@ -22,10 +22,12 @@ Ssl::ServerBump::ServerBump(HttpRequest *fakeRequest, StoreEntry *e, Ssl::BumpMode md): request(fakeRequest), sslErrors(NULL), - mode(md), step(bumpStep1) { debugs(33, 4, HERE << "will peek at " << request->GetHost() << ':' << request->port); + act.step1 = md; + act.step2 = act.step3 = Ssl::bumpNone; + const char *uri = urlCanonical(request.getRaw()); if (e) { entry = e; === modified file 'src/ssl/ServerBump.h' --- src/ssl/ServerBump.h 2014-09-22 19:06:19 +0000 +++ src/ssl/ServerBump.h 2014-10-01 12:31:58 +0000 @@ -36,8 +36,12 @@ StoreEntry *entry; ///< for receiving Squid-generated error messages Ssl::X509_Pointer serverCert; ///< HTTPS server certificate Ssl::CertErrors *sslErrors; ///< SSL [certificate validation] errors - Ssl::BumpMode mode; ///< The SSL server bump mode - Ssl::BumpStep step; ///< The SSL server bumping step + struct { + Ssl::BumpMode step1; ///< The SSL bump mode at step1 + Ssl::BumpMode step2; ///< The SSL bump mode at step2 + Ssl::BumpMode step3; ///< The SSL bump mode at step3 + } act; ///< bumping actions at various bumping steps + Ssl::BumpStep step; ///< The SSL bumping step SBuf clientSni; ///< the SSL client SNI name private: