From: Christos Tsantilas Date: Thu, 26 Jan 2017 16:22:30 +0000 (+0200) Subject: SSLv2 records force SslBump bumping despite a matching step2 peek rule. X-Git-Tag: M-staged-PR71~296 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6744c1a8d7e44ec5553ebde76c4a81dc0785043e;p=thirdparty%2Fsquid.git SSLv2 records force SslBump bumping despite a matching step2 peek rule. If Squid receives a valid TLS Hello encapsulated into ancient SSLv2 records (observed on Solaris 10), the old code ignored the step2 peek decision and bumped the transaction instead. Now Squid peeks (or stares) at the origin server as configured, even after detecting (and parsing) SSLv2 records. This is a Measurement Factory project. --- diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc index d3e6249f95..645e000b3e 100644 --- a/src/ssl/PeekingPeerConnector.cc +++ b/src/ssl/PeekingPeerConnector.cc @@ -171,18 +171,16 @@ Ssl::PeekingPeerConnector::initialize(Security::SessionPointer &serverSession) BIO *bc = SSL_get_rbio(clientSession); Ssl::ClientBio *cltBio = static_cast(BIO_get_data(bc)); Must(cltBio); - if (details && details->tlsVersion.protocol != AnyP::PROTO_NONE) { + if (details && details->tlsVersion.protocol != AnyP::PROTO_NONE) applyTlsDetailsToSSL(serverSession.get(), details, csd->sslBumpMode); - // Should we allow it for all protocols? - if (details->tlsVersion.protocol == AnyP::PROTO_TLS || details->tlsVersion == AnyP::ProtocolVersion(AnyP::PROTO_SSL, 3, 0)) { - BIO *b = SSL_get_rbio(serverSession.get()); - Ssl::ServerBio *srvBio = static_cast(BIO_get_data(b)); - // Inherite client features, like SSL version, SNI and other - srvBio->setClientFeatures(details, cltBio->rBufData()); - srvBio->recordInput(true); - srvBio->mode(csd->sslBumpMode); - } - } + + BIO *b = SSL_get_rbio(serverSession.get()); + Ssl::ServerBio *srvBio = static_cast(BIO_get_data(b)); + Must(srvBio); + // inherit client features such as TLS version and SNI + srvBio->setClientFeatures(details, cltBio->rBufData()); + srvBio->recordInput(true); + srvBio->mode(csd->sslBumpMode); } else { // Set client SSL options SSL_set_options(serverSession.get(), ::Security::ProxyOutgoingConfig.parsedOptions); diff --git a/src/ssl/bio.cc b/src/ssl/bio.cc index 3a6e0a48f2..a91b03cbbd 100644 --- a/src/ssl/bio.cc +++ b/src/ssl/bio.cc @@ -268,7 +268,7 @@ void Ssl::ServerBio::setClientFeatures(Security::TlsDetails::Pointer const &details, SBuf const &aHello) { clientTlsDetails = details; - clientHelloMessage = aHello; + clientSentHello = aHello; }; int @@ -372,6 +372,9 @@ static bool adjustSSL(SSL *ssl, Security::TlsDetails::Pointer const &details, SBuf &helloMessage) { #if SQUID_USE_OPENSSL_HELLO_OVERWRITE_HACK + if (!details) + return false; + if (!ssl->s3) { debugs(83, 5, "No SSLv3 data found!"); return false; @@ -475,33 +478,37 @@ Ssl::ServerBio::write(const char *buf, int size, BIO *table) } if (!helloBuild && (bumpMode_ == Ssl::bumpPeek || bumpMode_ == Ssl::bumpStare)) { - if ( - buf[1] >= 3 //it is an SSL Version3 message - && buf[0] == 0x16 // and it is a Handshake/Hello message - ) { - - //Hello message is the first message we write to server - assert(helloMsg.isEmpty()); - - auto ssl = fd_table[fd_].ssl.get(); - if (ssl) { - if (bumpMode_ == Ssl::bumpPeek) { - if (adjustSSL(ssl, clientTlsDetails, clientHelloMessage)) - allowBump = true; - allowSplice = true; - helloMsg.append(clientHelloMessage); - debugs(83, 7, "SSL HELLO message for FD " << fd_ << ": Random number is adjusted for peek mode"); - } else { /*Ssl::bumpStare*/ + // buf contains OpenSSL-generated ClientHello. We assume it has a + // complete ClientHello and nothing else, but cannot fully verify + // that quickly. We only verify that buf starts with a v3+ record + // containing ClientHello. + Must(size >= 2); // enough for version and content_type checks below + Must(buf[1] >= 3); // record's version.major; determines buf[0] meaning + Must(buf[0] == 22); // TLSPlaintext.content_type == handshake in v3+ + + //Hello message is the first message we write to server + assert(helloMsg.isEmpty()); + + if (auto ssl = fd_table[fd_].ssl.get()) { + if (bumpMode_ == Ssl::bumpPeek) { + // we should not be here if we failed to parse the client-sent ClientHello + Must(!clientSentHello.isEmpty()); + if (adjustSSL(ssl, clientTlsDetails, clientSentHello)) allowBump = true; - if (adjustSSL(ssl, clientTlsDetails, clientHelloMessage)) { - allowSplice = true; - helloMsg.append(clientHelloMessage); - debugs(83, 7, "SSL HELLO message for FD " << fd_ << ": Random number is adjusted for stare mode"); - } + allowSplice = true; + // Replace OpenSSL-generated ClientHello with client-sent one. + helloMsg.append(clientSentHello); + debugs(83, 7, "FD " << fd_ << ": Using client-sent ClientHello for peek mode"); + } else { /*Ssl::bumpStare*/ + allowBump = true; + if (!clientSentHello.isEmpty() && adjustSSL(ssl, clientTlsDetails, clientSentHello)) { + allowSplice = true; + helloMsg.append(clientSentHello); + debugs(83, 7, "FD " << fd_ << ": Using client-sent ClientHello for stare mode"); } } } - // If we do not build any hello message, copy the current + // if we did not use the client-sent ClientHello, then use the OpenSSL-generated one if (helloMsg.isEmpty()) helloMsg.append(buf, size); diff --git a/src/ssl/bio.h b/src/ssl/bio.h index 1c13938fec..512a3b5a45 100644 --- a/src/ssl/bio.h +++ b/src/ssl/bio.h @@ -182,7 +182,7 @@ private: /// SSL client features extracted from ClientHello message or SSL object Security::TlsDetails::Pointer clientTlsDetails; /// TLS client hello message, used to adapt our tls Hello message to the server - SBuf clientHelloMessage; + SBuf clientSentHello; SBuf helloMsg; ///< Used to buffer output data. mb_size_t helloMsgSize; bool helloBuild; ///< True if the client hello message sent to the server