From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 22 Apr 2016 04:51:24 +0000 (-0600)
Subject: Do not allocate TlsDetails until throwing isSslv2Record() returns.
X-Git-Tag: SQUID_4_0_11~29^2~21
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5ea53436a725b2068fdb72757c0dbf16756807f1;p=thirdparty%2Fsquid.git

Do not allocate TlsDetails until throwing isSslv2Record() returns.

If isSslv2Record() throws InsufficientInput, then we must re-parse these
first few bytes later. Nil details triggers that parsing.
---

diff --git a/src/security/Handshake.cc b/src/security/Handshake.cc
index c02d58817d..393095c94c 100644
--- a/src/security/Handshake.cc
+++ b/src/security/Handshake.cc
@@ -154,27 +154,21 @@ Security::HandshakeParser::parseVersion2Record()
     parseDone = true;
 }
 
+/// RFC 5246. Appendix E.2. Compatibility with SSL 2.0
+/// And draft-hickman-netscape-ssl-00. Section 4.1 SSL Record Header Format
 bool
-Security::HandshakeParser::isSslv2Record()
+Security::HandshakeParser::isSslv2Record(const SBuf &raw) const
 {
-    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;
-    // It is an SSLv2 Client Hello Message
-    return true;
+    BinaryTokenizer tk(raw, true);
+    const uint16_t head = tk.uint16("V2Hello.msg_length+");
+    const uint8_t type = tk.uint8("V2Hello.msg_type");
+    const uint16_t length = head & 0x7FFF;
+    return (head & 0x8000) && length && type == 1;
 }
 
 void
 Security::HandshakeParser::parseRecord()
 {
-    if (details == NULL) {
-        details = new TlsDetails;
-        expectingModernRecords = !isSslv2Record();
-    }
-
     if (expectingModernRecords)
         parseModernRecord();
     else
@@ -436,6 +430,11 @@ bool
 Security::HandshakeParser::parseHello(const SBuf &data)
 {
     try {
+        if (!details) {
+            expectingModernRecords = !isSslv2Record(data);
+            details = new TlsDetails; // after expectingModernRecords is known
+        }
+
         // data contains everything read so far, but we may read more later
         tkRecords.reinput(data, true);
         tkRecords.rollback();
diff --git a/src/security/Handshake.h b/src/security/Handshake.h
index 813f388a9d..1cad347460 100644
--- a/src/security/Handshake.h
+++ b/src/security/Handshake.h
@@ -199,8 +199,7 @@ public:
     bool parseError; ///< Set to tru by parse on parse error.
 
 private:
-
-    bool isSslv2Record();
+    bool isSslv2Record(const SBuf &raw) const;
     void parseRecord();
     void parseModernRecord();
     void parseVersion2Record();