]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Cleanup and fixes
authorChristos Tsantilas <chtsanti@users.sourceforge.net>
Wed, 6 Apr 2016 19:28:28 +0000 (22:28 +0300)
committerChristos Tsantilas <chtsanti@users.sourceforge.net>
Wed, 6 Apr 2016 19:28:28 +0000 (22:28 +0300)
- 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

src/client_side.cc
src/security/Handshake.cc
src/security/Handshake.h
src/security/NegotiationHistory.cc
src/security/NegotiationHistory.h
src/ssl/PeekingPeerConnector.cc
src/ssl/bio.h

index b26f118af8db6857b8c7846c2744c11cae8ff510..9fe5dff5330d617a255dccff7425088d1574cd78 100644 (file)
@@ -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<Ssl::ClientBio *>(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<Ssl::ClientBio *>(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<Ssl::ClientBio *>(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);
index afac430347aefe45a82b52e138c0c39891385d79..c978e5430f2a3060c0bba71d58cc87c32200bdf2 100644 (file)
@@ -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");
index 5041d83aa10b8c739823ba10e6ce41f9fb9a4f71..fbb9f5972c895af769fd7af2d5df7cd43714fb79 100644 (file)
@@ -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
 };
 
 }
index 65e5a8e16401351798cdd6ba914f1833085f13b9..f3aa2de906b163579e32479c98df7b035606114b 100644 (file)
@@ -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<Ssl::Bio *>(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<Ssl::Bio *>(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;
index 44edaf5a1aa30906a6be3c97a44f8042453a24e1..68fb8363fe153140a22fff7562ec76d0db0137b6 100644 (file)
@@ -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_);}
index 9b2fe56591dd5e3cecaad34ae48f5e3a7869392d..964b6a7b5dbaa78752404f83baa29f546349ec19 100644 (file)
@@ -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<Ssl::ServerBio *>(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();
         }
index b94cd7938663cf0162d63ecdc0328b0f1a00979f..f29ac52b25956df0f5aa7d7090803ad797f5dafb 100644 (file)
@@ -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