From: Christos Tsantilas Date: Wed, 6 Apr 2016 19:28:28 +0000 (+0300) Subject: Cleanup and fixes X-Git-Tag: SQUID_4_0_11~29^2~31 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8abcff997be2bf3acc96a6933e282d3a7096713d;p=thirdparty%2Fsquid.git Cleanup and fixes - remove uneeded members from classes - Remove the TlsDetails object from ClientBio - Enable SSL Server Hello parsing code - remove uneeded code - Fix bump client first and bump server first bumping modes --- diff --git a/src/client_side.cc b/src/client_side.cc index b26f118af8..9fe5dff533 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2703,7 +2703,7 @@ clientNegotiateSSL(int fd, void *data) } // Connection established. Retrieve TLS connection parameters for logging. - conn->clientConnection->tlsNegotiations()->fillWith(ssl); + conn->clientConnection->tlsNegotiations()->retrieveNegotiatedInfo(ssl); client_cert = SSL_get_peer_certificate(ssl); @@ -3109,11 +3109,6 @@ ConnStateData::getSslContextDone(Security::ContextPtr sslContext, bool isNew) Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0); Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, clientNegotiateSSL, this, 0); switchedToHttps_ = true; - auto ssl = fd_table[clientConnection->fd].ssl.get(); - BIO *b = SSL_get_rbio(ssl); - Ssl::ClientBio *bio = static_cast(b->ptr); - bio->setReadBufData(inBuf); - bio->parsedDetails(tlsParser.details); } void @@ -3192,6 +3187,9 @@ void ConnStateData::startPeekAndSplice() } receivedFirstByte(); + // Record parsed info + clientConnection->tlsNegotiations()->retrieveParsedInfo(tlsParser.details); + if (serverBump()) { Security::TlsDetails::Pointer const &details = tlsParser.details; if (!details->serverName.isEmpty()) { @@ -3239,24 +3237,18 @@ ConnStateData::splice() { // normally we can splice here, because we just got client hello message - if (auto ssl = fd_table[clientConnection->fd].ssl.get()) { - // We built - //retrieve received TLS client information - clientConnection->tlsNegotiations()->fillWith(ssl); + if (fd_table[clientConnection->fd].ssl.get()) { + // The following block does not needed, inBuf and rbuf have the same content. // BIO *b = SSL_get_rbio(ssl); // Ssl::ClientBio *bio = static_cast(b->ptr); // SBuf const &rbuf = bio->rBufData(); - // // The following do not needed, inBuf and rbuf has the same content. // inBuf.assign(rbuf); // 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; - - } else { - clientConnection->tlsNegotiations()->fillWith(tlsParser.details); } if (transparent()) { @@ -3312,7 +3304,6 @@ ConnStateData::startPeekAndSpliceDone() BIO *b = SSL_get_rbio(ssl); Ssl::ClientBio *bio = static_cast(b->ptr); bio->setReadBufData(inBuf); - bio->parsedDetails(tlsParser.details); bio->hold(true); // Here squid should have all of the client hello message so the @@ -3329,7 +3320,7 @@ ConnStateData::startPeekAndSpliceDone() } // Do we need to reset inBuf here? - // inBuf.clear(); + inBuf.clear(); debugs(83, 5, "Peek and splice at step2 done. Start forwarding the request!!! "); FwdState::Start(clientConnection, sslServerBump->entry, sslServerBump->request.getRaw(), http ? http->al : NULL); diff --git a/src/security/Handshake.cc b/src/security/Handshake.cc index afac430347..c978e5430f 100644 --- a/src/security/Handshake.cc +++ b/src/security/Handshake.cc @@ -83,17 +83,16 @@ Security::Extension::Extension(BinaryTokenizer &tk): commit(tk); } -Security::SSL2Record::SSL2Record(BinaryTokenizer &tk): - FieldGroup(tk, "SSL2Record") +Security::Sslv2Record::Sslv2Record(BinaryTokenizer &tk): + FieldGroup(tk, "Sslv2Record") { uint16_t head = tk.uint16(".head(Record+Length)"); length = head & 0x7FFF; - type = tk.uint8(".type"); - if ((head & 0x8000) == 0 || length == 0 || type != 0x01) + if ((head & 0x8000) == 0 || length == 0) throw TexcHere("Not an SSLv2 message"); version = 0x02; // The remained message has length of length-sizeof(type)=(length-1); - fragment = tk.area(length - 1, ".fragment"); + fragment = tk.area(length, ".fragment"); commit(tk); } @@ -144,37 +143,48 @@ operator <<(std::ostream &os, const DebugFrame &frame) return os << frame.size << "-byte type-" << frame.type << ' ' << frame.name; } +void +Security::HandshakeParser::parseVersion2Record() +{ + const Sslv2Record record(tkRecords); + Must(details); + details->tlsVersion = record.version; + parseVersion2HandshakeMessage(record.fragment); + state = atHelloReceived; + parseDone = true; +} + bool -Security::HandshakeParser::parseRecordVersion2Try() +Security::HandshakeParser::isSslv2Record() { - try { - const SSL2Record record(tkRecords); - details->tlsVersion = record.version; - parseVersion2HandshakeMessage(record.fragment); - state = atHelloReceived; - parseDone = true; - return true; - } catch (const BinaryTokenizer::InsufficientInput &) { - throw BinaryTokenizer::InsufficientInput(); - } catch (const std::exception &ex) { - debugs(83, 5, "fallback to the TLS records parser: " << ex.what()); - useTlsParser = true; - tkRecords.rollback(); + uint16_t head = tkRecords.uint16(".head(Record+Length)"); + uint16_t length = head & 0x7FFF; + uint8_t type = tkRecords.uint8(".type"); + tkRecords.rollback(); + if ((head & 0x8000) == 0 || length == 0 || type != 0x01) return false; - } - return false; + // It is an SSLv2 Client Hello Message + return true; } -/// parses a single TLS Record Layer frame void Security::HandshakeParser::parseRecord() { - if (details == NULL) + if (details == NULL) { details = new TlsDetails; + expectingModernRecords = !isSslv2Record(); + } - if (!useTlsParser && parseRecordVersion2Try()) - return; + if (expectingModernRecords) + parseModernRecord(); + else + parseVersion2Record(); +} +/// parses a single TLS Record Layer frame +void +Security::HandshakeParser::parseModernRecord() +{ const TLSPlaintext record(tkRecords); Must(record.length <= (1 << 14)); // RFC 5246: length MUST NOT exceed 2^14 @@ -252,16 +262,13 @@ Security::HandshakeParser::parseHandshakeMessage() switch (message.msg_type) { case HandshakeType::hskClientHello: Must(state < atHelloReceived); - // TODO: Parse ClientHello in message.body; extract version/session Security::HandshakeParser::parseClientHelloHandshakeMessage(message.body); state = atHelloReceived; parseDone = true; return; case HandshakeType::hskServerHello: Must(state < atHelloReceived); - // TODO: Parse ServerHello in message.body; extract version/session - // If the server is resuming a session, stop parsing w/o certificates - // because all subsequent [Finished] messages will be encrypted, right? + parseServerHelloHandshakeMessage(message.body); state = atHelloReceived; return; case HandshakeType::hskCertificate: @@ -293,6 +300,9 @@ Security::HandshakeParser::parseVersion2HandshakeMessage(const SBuf &raw) BinaryTokenizer tkHsk(raw); Must(details); + uint8_t type = tkHsk.uint8("type"); + Must(type == hskClientHello); // Only client hello supported. + details->tlsSupportedVersion = tkHsk.uint16("tlsSupportedVersion"); uint16_t ciphersLen = tkHsk.uint16(".cipherSpecLength"); uint16_t sessionIdLen = tkHsk.uint16(".sessionIdLength"); diff --git a/src/security/Handshake.h b/src/security/Handshake.h index 5041d83aa1..fbb9f5972c 100644 --- a/src/security/Handshake.h +++ b/src/security/Handshake.h @@ -60,12 +60,11 @@ struct TLSPlaintext: public FieldGroup SBuf fragment; ///< exactly length bytes }; -struct SSL2Record: public FieldGroup +struct Sslv2Record: public FieldGroup { - explicit SSL2Record(BinaryTokenizer &tk); + explicit Sslv2Record(BinaryTokenizer &tk); uint16_t version; uint16_t length; - uint8_t type; SBuf fragment; }; @@ -180,7 +179,7 @@ public: /// The parsing states typedef enum {atHelloNone = 0, atHelloStarted, atHelloReceived, atCertificatesReceived, atHelloDoneReceived, atNstReceived, atCcsReceived, atFinishReceived} ParserState; - HandshakeParser(): state(atHelloNone), ressumingSession(false), parseDone(false), parseError(false), currentContentType(0), unParsedContent(0), parsingPos(0), currentMsg(0), currentMsgSize(0), certificatesMsgPos(0), certificatesMsgSize(0), useTlsParser(false) {} + HandshakeParser(): state(atHelloNone), ressumingSession(false), parseDone(false), parseError(false), currentContentType(0), expectingModernRecords(false) {} /// Parses the initial sequence of raw bytes sent by the SSL server. /// Returns true upon successful completion (HelloDone or Finished received). @@ -205,19 +204,11 @@ public: bool parseError; ///< Set to tru by parse on parse error. private: - unsigned int currentContentType; ///< The current SSL record content type - size_t unParsedContent; ///< The size of current SSL record, which is not parsed yet - size_t parsingPos; ///< The parsing position from the beginning of parsed data - size_t currentMsg; ///< The current handshake message possition from the beginning of parsed data - size_t currentMsgSize; ///< The current handshake message size. - - size_t certificatesMsgPos; ///< The possition of certificates message from the beggining of parsed data - size_t certificatesMsgSize; ///< The size of certificates message - -private: - void parseServerHelloTry(); + bool isSslv2Record(); void parseRecord(); + void parseModernRecord(); + void parseVersion2Record(); void parseMessages(); void parseChangeCipherCpecMessage(); @@ -240,13 +231,14 @@ private: static X509 *ParseCertificate(const SBuf &raw); #endif + unsigned int currentContentType; ///< The current SSL record content type /// concatenated TLSPlaintext.fragments of TLSPlaintext.type SBuf fragments; BinaryTokenizer tkRecords; // TLS record layer (parsing uninterpreted data) BinaryTokenizer tkMessages; // TLS message layer (parsing fragments) - bool useTlsParser; // Whether to use TLS parser or a V2 compatible parser + bool expectingModernRecords; // Whether to use TLS parser or a V2 compatible parser }; } diff --git a/src/security/NegotiationHistory.cc b/src/security/NegotiationHistory.cc index 65e5a8e164..f3aa2de906 100644 --- a/src/security/NegotiationHistory.cc +++ b/src/security/NegotiationHistory.cc @@ -51,7 +51,7 @@ Security::NegotiationHistory::printTlsVersion(int v) const } void -Security::NegotiationHistory::fillWith(Security::SessionPtr ssl) +Security::NegotiationHistory::retrieveNegotiatedInfo(Security::SessionPtr ssl) { #if USE_OPENSSL if ((cipher = SSL_get_current_cipher(ssl)) != NULL) { @@ -61,19 +61,18 @@ Security::NegotiationHistory::fillWith(Security::SessionPtr ssl) version_ = ssl->version; } - BIO *b = SSL_get_rbio(ssl); - Ssl::Bio *bio = static_cast(b->ptr); - if (const Security::TlsDetails::Pointer &details = bio->receivedHelloDetails()) - fillWith(details); - - debugs(83, 5, "SSL connection info on FD " << bio->fd() << - " SSL version " << version_ << - " negotiated cipher " << cipherName()); + if (do_debug(83, 5)) { + BIO *b = SSL_get_rbio(ssl); + Ssl::Bio *bio = static_cast(b->ptr); + debugs(83, 5, "SSL connection info on FD " << bio->fd() << + " SSL version " << version_ << + " negotiated cipher " << cipherName()); + } #endif } void -Security::NegotiationHistory::fillWith(Security::TlsDetails::Pointer const &details) +Security::NegotiationHistory::retrieveParsedInfo(Security::TlsDetails::Pointer const &details) { helloVersion_ = details->tlsVersion; supportedVersion_ = details->tlsSupportedVersion; diff --git a/src/security/NegotiationHistory.h b/src/security/NegotiationHistory.h index 44edaf5a1a..68fb8363fe 100644 --- a/src/security/NegotiationHistory.h +++ b/src/security/NegotiationHistory.h @@ -18,8 +18,13 @@ class NegotiationHistory { public: NegotiationHistory(); - void fillWith(Security::SessionPtr); ///< Extract negotiation information from TLS object - void fillWith(Security::TlsDetails::Pointer const &details); ///< Extract negotiation information from parser TlsDetails object + + /// Extract negotiation information from TLS object + void retrieveNegotiatedInfo(Security::SessionPtr); + + /// Extract information from parser stored in TlsDetails object + void retrieveParsedInfo(Security::TlsDetails::Pointer const &details); + const char *cipherName() const; ///< The name of negotiated cipher /// String representation of TLS negotiated version const char *negotiatedVersion() const {return printTlsVersion(version_);} diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc index 9b2fe56591..964b6a7b5d 100644 --- a/src/ssl/PeekingPeerConnector.cc +++ b/src/ssl/PeekingPeerConnector.cc @@ -252,14 +252,17 @@ Ssl::PeekingPeerConnector::noteNegotiationDone(ErrorState *error) } } - // retrieve TLS server information if any - serverConnection()->tlsNegotiations()->fillWith(ssl); + // retrieve TLS server negotiated information if any + serverConnection()->tlsNegotiations()->retrieveNegotiatedInfo(ssl); + // retrieve TLS parsed extra info + BIO *b = SSL_get_rbio(ssl); + Ssl::ServerBio *bio = static_cast(b->ptr); + if (const Security::TlsDetails::Pointer &details = bio->receivedHelloDetails()) + serverConnection()->tlsNegotiations()->retrieveParsedInfo(details); + if (!error) { serverCertificateVerified(); if (splice) { - //retrieved received TLS client informations - auto clientSsl = fd_table[clientConn->fd].ssl.get(); - clientConn->tlsNegotiations()->fillWith(clientSsl); switchToTunnel(request.getRaw(), clientConn, serverConn); tunnelInsteadOfNegotiating(); } diff --git a/src/ssl/bio.h b/src/ssl/bio.h index b94cd79386..f29ac52b25 100644 --- a/src/ssl/bio.h +++ b/src/ssl/bio.h @@ -52,9 +52,6 @@ public: /// appears, or an error occurs. See SSL_set_info_callback(). virtual void stateChanged(const SSL *ssl, int where, int ret); - /// Return the TLS Details advertised by TLS server. - virtual const Security::TlsDetails::Pointer &receivedHelloDetails() const = 0; - /// Creates a low-level BIO table, creates a high-level Ssl::Bio object /// for a given socket, and then links the two together via BIO_C_SET_FD. static BIO *Create(const int fd, Type type); @@ -88,22 +85,15 @@ public: /// If the holdRead flag is true then it does not write any data /// to socket and sets the "read retry" flag of the BIO to true virtual int read(char *buf, int size, BIO *table); - /// Return true if the client hello message received and analized - //bool gotHello() const { return (parser_.parseDone && !parser_.parseError); } /// Prevents or allow writting on socket. void hold(bool h) {holdRead_ = holdWrite_ = h;} void setReadBufData(SBuf &data) {rbuf = data;} - const Security::TlsDetails::Pointer &receivedHelloDetails() const {return details;} - void parsedDetails(Security::TlsDetails::Pointer &someDetails) {details = someDetails;} private: /// True if the SSL state corresponds to a hello message bool isClientHello(int state); bool holdRead_; ///< The read hold state of the bio. bool holdWrite_; ///< The write hold state of the bio. int helloSize; ///< The SSL hello message sent by client size - - /// SSL client features extracted from ClientHello message or SSL object - Security::TlsDetails::Pointer details; }; /// BIO node to handle socket IO for squid server side