]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Polishing fixes
authorChristos Tsantilas <chtsanti@users.sourceforge.net>
Wed, 18 May 2016 16:26:16 +0000 (19:26 +0300)
committerChristos Tsantilas <chtsanti@users.sourceforge.net>
Wed, 18 May 2016 16:26:16 +0000 (19:26 +0300)
Code formatting, variables fixing, comments and debug messages

Most of them proposed by Amos on squid-dev review procedure.

12 files changed:
src/anyp/ProtocolType.h
src/client_side.cc
src/client_side.h
src/parser/BinaryTokenizer.cc
src/parser/BinaryTokenizer.h
src/security/Handshake.cc
src/security/Handshake.h
src/security/NegotiationHistory.cc
src/ssl/PeekingPeerConnector.cc
src/ssl/PeerConnector.cc
src/ssl/bio.cc
src/ssl/bio.h

index c08f84c3b1e2c874f990e4d01ca71c5a600625f2..425575203e436528a1ce600af780a3821e2c4fec 100644 (file)
@@ -36,8 +36,8 @@ typedef enum {
     PROTO_URN,
     PROTO_WHOIS,
     PROTO_ICY,
-    PROTO_SSL,
     PROTO_TLS,
+    PROTO_SSL,
     PROTO_UNKNOWN,
     PROTO_MAX
 } ProtocolType;
index 4c355dfc13a033006e784e6fada2ab588e1258bf..8e826f47127f1dc9643cc5d45d179e8fb05e84bf 100644 (file)
@@ -3208,7 +3208,8 @@ ConnStateData::spliceOnError(const err_type err)
     return false;
 }
 
-void ConnStateData::startPeekAndSplice(const bool unsupportedProtocol)
+void
+ConnStateData::startPeekAndSplice(const bool unsupportedProtocol)
 {
     if (unsupportedProtocol) {
         if (!spliceOnError(ERR_PROTOCOL_UNKNOWN))
@@ -3260,15 +3261,7 @@ ConnStateData::splice()
     // normally we can splice here, because we just got client hello message
 
     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();
-        // inBuf.assign(rbuf);
-        // debugs(83,5, "Bio for  " << clientConnection << " read " << rbuf.length() << " helo bytes");
-
-        // Do splice:
+        // Restore default read methods
         fd_table[clientConnection->fd].read_method = &default_read_method;
         fd_table[clientConnection->fd].write_method = &default_write_method;
     }
index e5944774aea4a962051779f84d1be77da803b5ff..c3d0761d6874b4b8f042f373968baaf1b1825997 100644 (file)
@@ -256,6 +256,9 @@ public:
     bool serveDelayedError(Http::Stream *);
 
     Ssl::BumpMode sslBumpMode; ///< ssl_bump decision (Ssl::bumpEnd if n/a).
+
+    /// Tls parser to use for client HELLO messages parsing on bumped
+    /// connections.
     Security::HandshakeParser tlsParser;
 #else
     bool switchedToHttps() const { return false; }
index 0bf042ab65a53f150468f500a895b0331b736306..d9370678d5f97984a52e19cb5856eeeb27ed1992 100644 (file)
@@ -9,7 +9,7 @@
 /* DEBUG: section 24    SBuf */
 
 #include "squid.h"
-#include "BinaryTokenizer.h"
+#include "parser/BinaryTokenizer.h"
 
 Parser::BinaryTokenizer::BinaryTokenizer(): BinaryTokenizer(SBuf())
 {
index 6309990f7c3a2a1da0f2cd810ee5edfcd14ba158..61807b768311a18bfe81c1842c7fed7b69ae9752 100644 (file)
@@ -6,8 +6,8 @@
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
-#ifndef SQUID_PARSER_BINARY_TOKENIZER_H
-#define SQUID_PARSER_BINARY_TOKENIZER_H
+#ifndef SQUID_SRC_PARSER_BINARYTOKENIZER_H
+#define SQUID_SRC_PARSER_BINARYTOKENIZER_H
 
 #include "sbuf/SBuf.h"
 
@@ -21,7 +21,7 @@ class BinaryTokenizerContext
 {
 public:
     /// starts parsing named object
-    inline explicit BinaryTokenizerContext(BinaryTokenizer &tk, const char *aName);
+    explicit BinaryTokenizerContext(BinaryTokenizer &tk, const char *aName);
     ~BinaryTokenizerContext() { close(); }
 
     /// ends parsing named object; repeated calls OK
@@ -31,12 +31,13 @@ public:
     inline void success();
 
     BinaryTokenizer &tokenizer; ///< tokenizer being used for parsing
-    const BinaryTokenizerContext *parent; ///< enclosing context or nullptr
-    const char *name; ///< this context description or nullptr
+    const BinaryTokenizerContext * const parent; ///< enclosing context or nullptr
+    const char *const name; ///< this context description or nullptr
     uint64_t start; ///< context parsing begins at this tokenizer position
 };
 
 /// Safely extracts byte-oriented (i.e., non-textual) fields from raw input.
+/// Assume that the integers are stored in network byte order.
 /// Supports commit points for atomic incremental parsing of multi-part fields.
 /// Throws InsufficientInput when more input is needed to parse the next field.
 /// Throws on errors.
@@ -69,13 +70,13 @@ public:
     /// parse a single-byte unsigned integer
     uint8_t uint8(const char *description);
 
-    // parse a two-byte unsigned integer
+    /// parse a two-byte unsigned integer
     uint16_t uint16(const char *description);
 
-    // parse a three-byte unsigned integer (returned as uint32_t)
+    /// parse a three-byte unsigned integer (returned as uint32_t)
     uint32_t uint24(const char *description);
 
-    // parse a four-byte unsigned integer
+    /// parse a four-byte unsigned integer
     uint32_t uint32(const char *description);
 
     /// parse size consecutive bytes as an opaque blob
@@ -144,5 +145,5 @@ BinaryTokenizerContext::success() {
 
 } /* namespace Parser */
 
-#endif // SQUID_PARSER_BINARY_TOKENIZER_H
+#endif // SQUID_SRC_PARSER_BINARYTOKENIZER_H
 
index 09e80183ef371b65f1f955acc802dc483b9fe343..cd8f418d7f81ffce03cffd2d88514db9eb86d2e4 100644 (file)
@@ -190,7 +190,7 @@ Security::TlsDetails::TlsDetails():
 Security::HandshakeParser::HandshakeParser():
     details(new TlsDetails),
     state(atHelloNone),
-    ressumingSession(false),
+    resumingSession(false),
     currentContentType(0),
     done(nullptr),
     expectingModernRecords(false)
@@ -275,7 +275,7 @@ Security::HandshakeParser::parseMessages()
             parseApplicationDataMessage();
             continue;
         }
-        skipMessage("unknown ContentType msg");
+        skipMessage("unknown ContentType msg [fragment]");
     }
 }
 
@@ -284,11 +284,11 @@ Security::HandshakeParser::parseChangeCipherCpecMessage()
 {
     Must(currentContentType == ContentType::ctChangeCipherSpec);
     // We are currently ignoring Change Cipher Spec Protocol messages.
-    skipMessage("ChangeCipherCpec msg");
+    skipMessage("ChangeCipherCpec msg [fragment]");
 
     // Everything after the ChangeCipherCpec message may be encrypted.
     // Continuing parsing is pointless. Stop here.
-    ressumingSession = true;
+    resumingSession = true;
     done = "ChangeCipherCpec";
 }
 
@@ -344,7 +344,7 @@ void
 Security::HandshakeParser::parseApplicationDataMessage()
 {
     Must(currentContentType == ContentType::ctApplicationData);
-    skipMessage("app data");
+    skipMessage("app data [fragment]");
 }
 
 void
index 4bb4c5b23127dda07f921f7f1c667d4cab791f6e..d3ff94a1463c326135344270718579c07253f5b9 100644 (file)
 #define SQUID_SECURITY_HANDSHAKE_H
 
 #include "anyp/ProtocolVersion.h"
-#include "base/RefCount.h"
 #include "base/YesNoNone.h"
 #include "parser/BinaryTokenizer.h"
-#include "sbuf/SBuf.h"
 #if USE_OPENSSL
 #include "ssl/gadgets.h"
 #endif
@@ -56,15 +54,16 @@ std::ostream &operator <<(std::ostream &os, Security::TlsDetails const &details)
     return details.print(os);
 }
 
-/// Incremental SSL Handshake parser.
-class HandshakeParser {
+/// Incremental TLS/SSL Handshake parser.
+class HandshakeParser
+{
 public:
     /// The parsing states
     typedef enum {atHelloNone = 0, atHelloStarted, atHelloReceived, atCertificatesReceived, atHelloDoneReceived, atNstReceived, atCcsReceived, atFinishReceived} ParserState;
 
     HandshakeParser();
 
-    /// Parses the initial sequence of raw bytes sent by the SSL agent.
+    /// Parses the initial sequence of raw bytes sent by the TLS/SSL agent.
     /// Returns true upon successful completion (e.g., got HelloDone).
     /// Returns false if more data is needed.
     /// Throws on errors.
@@ -78,7 +77,7 @@ public:
 
     ParserState state; ///< current parsing state.
 
-    bool ressumingSession; ///< True if this is a resumming session
+    bool resumingSession; ///< True if this is a resuming session
 
 private:
     bool isSslv2Record(const SBuf &raw) const;
@@ -110,17 +109,21 @@ private:
     static X509 *ParseCertificate(const SBuf &raw);
 #endif
 
-    unsigned int currentContentType; ///< The current SSL record content type
+    unsigned int currentContentType; ///< The current TLS/SSL record content type
 
-    const char *done; ///< not nil iff we got what we were looking for
+    const char *done; ///< not nil if we got what we were looking for
 
     /// concatenated TLSPlaintext.fragments of TLSPlaintext.type
     SBuf fragments;
 
-    Parser::BinaryTokenizer tkRecords; // TLS record layer (parsing uninterpreted data)
-    Parser::BinaryTokenizer tkMessages; // TLS message layer (parsing fragments)
+    /// TLS record layer (parsing uninterpreted data)
+    Parser::BinaryTokenizer tkRecords;
+
+     /// TLS message layer (parsing fragments)
+    Parser::BinaryTokenizer tkMessages;
 
-    YesNoNone expectingModernRecords; // Whether to use TLS parser or a V2 compatible parser
+     /// Whether to use TLS parser or a V2 compatible parser
+    YesNoNone expectingModernRecords;
 };
 
 }
index 2004f18d91f48eb1a87db0f31c4947e8303b4898..57fcdecf6c095cf878799c7ebfe4f2a8880bf2a8 100644 (file)
@@ -17,7 +17,7 @@
 
 Security::NegotiationHistory::NegotiationHistory()
 #if USE_OPENSSL
-    : cipher(NULL)
+    : cipher(nullptr)
 #endif
 {
 }
@@ -38,18 +38,26 @@ static AnyP::ProtocolVersion
 toProtocolVersion(const int v)
 {
     switch(v) {
-#if OPENSSL_VERSION_NUMBER >= 0x10001000L
+#if defined(TLS1_2_VERSION)
     case TLS1_2_VERSION:
         return AnyP::ProtocolVersion(AnyP::PROTO_TLS, 1, 2);
+#endif
+#if defined(TLS1_1_VERSION)
     case TLS1_1_VERSION:
         return AnyP::ProtocolVersion(AnyP::PROTO_TLS, 1, 1);
 #endif
+#if defined(TLS1_VERSION)
     case TLS1_VERSION:
         return AnyP::ProtocolVersion(AnyP::PROTO_TLS, 1, 0);
+#endif
+#if defined(SSL3_VERSION)
     case SSL3_VERSION:
         return AnyP::ProtocolVersion(AnyP::PROTO_SSL, 3, 0);
+#endif
+#if defined(SSL2_VERSION)
     case SSL2_VERSION:
         return AnyP::ProtocolVersion(AnyP::PROTO_SSL, 2, 0);
+#endif
     default:
         return AnyP::ProtocolVersion();
     }
index 3679bb2fe95771601164a7017f30b2383a8e2e0b..f1b7b97b0ae64e5ed35a3e142a972925cd2f6c80 100644 (file)
@@ -269,6 +269,7 @@ Ssl::PeekingPeerConnector::noteWantWrite()
     Ssl::ServerBio *srvBio = static_cast<Ssl::ServerBio *>(b->ptr);
 
     if ((srvBio->bumpMode() == Ssl::bumpPeek || srvBio->bumpMode() == Ssl::bumpStare) && srvBio->holdWrite()) {
+        debugs(81, DBG_IMPORTANT, "hold write on SSL connection on FD " << fd);
         checkForPeekAndSplice();
         return;
     }
index 44a0e292dfca7bef95284772de011f77652a01cf..0e22b8ce701bd7e9a9e1d1e7f50cb7671db08864 100644 (file)
@@ -352,7 +352,7 @@ Ssl::PeerConnector::handleNegotiateError(const int ret)
         break;
     }
 
-    // Log connections details if there is any
+    // Log connection details, if any
     recordNegotiationDetails();
     noteSslNegotiationError(ret, ssl_error, ssl_lib_error);
 }
@@ -360,8 +360,8 @@ Ssl::PeerConnector::handleNegotiateError(const int ret)
 void
 Ssl::PeerConnector::noteWantRead()
 {
-    const int fd = serverConnection()->fd;
     setReadTimeout();
+    const int fd = serverConnection()->fd;
     Comm::SetSelect(fd, COMM_SELECT_READ, &NegotiateSsl, this, 0);
 }
 
index a81ea132656b170849467413607005e757606de0..dca2b430ca0090a282871e8c49a71e96a3e20c22 100644 (file)
@@ -57,8 +57,6 @@ static BIO_METHOD SquidMethods = {
     NULL // squid_callback_ctrl not supported
 };
 
-/* Ssl:Bio */
-
 BIO *
 Ssl::Bio::Create(const int fd, Ssl::Bio::Type type)
 {
@@ -198,8 +196,6 @@ Ssl::ClientBio::read(char *buf, int size, BIO *table)
     return -1;
 }
 
-/* ServerBio */
-
 Ssl::ServerBio::ServerBio(const int anFd):
     Bio(anFd),
     helloMsgSize(0),
@@ -329,7 +325,7 @@ adjustSSL(SSL *ssl, Security::TlsDetails::Pointer const &details, SBuf &helloMes
     // If the client supports compression but our context does not support
     // we can not adjust.
 #if !defined(OPENSSL_NO_COMP)
-    const bool requireCompression = (details->compressionSupported && ssl->ctx->comp_methods == NULL);
+    const bool requireCompression = (details->compressionSupported && ssl->ctx->comp_methods == nullptr);
 #else
     const bool requireCompression = details->compressionSupported;
 #endif
@@ -497,7 +493,7 @@ Ssl::ServerBio::flush(BIO *table)
 bool
 Ssl::ServerBio::resumingSession()
 {
-    return parser_.ressumingSession;
+    return parser_.resumingSession;
 }
 
 /// initializes BIO table after allocation
@@ -625,8 +621,9 @@ applyTlsDetailsToSSL(SSL *ssl, Security::TlsDetails::Pointer const &details, Ssl
     // To increase the possibility for bumping after peek mode selection or
     // splicing after stare mode selection it is good to set the
     // SSL protocol version.
-    // The SSL_set_ssl_method is not the correct method because it will strict
-    // SSL version which can be used to the SSL version used for client hello message.
+    // The SSL_set_ssl_method is wrong here because it will restrict the
+    // permitted transport version to be identical to the version used in the
+    // ClientHello message.
     // For example will prevent comunnicating with a tls1.0 server if the
     // client sent and tlsv1.2 Hello message.
 #if defined(TLSEXT_NAMETYPE_host_name)
index 5df82116e04ae9477921b39ed4fcca7ecbdd752c..a06592660d26150124ffa6258d00fa1630ed485a 100644 (file)
@@ -10,7 +10,6 @@
 #define SQUID_SSL_BIO_H
 
 #include "fd.h"
-#include "sbuf/SBuf.h"
 #include "security/Handshake.h"
 
 #include <iosfwd>
@@ -58,7 +57,7 @@ public:
     /// Tells ssl connection to use BIO and monitor state via stateChanged()
     static void Link(SSL *ssl, BIO *bio);
 
-    const SBuf &rBufData() {return rbuf;}
+    const SBuf &rBufData() {return rbuf;} ///< The buffered input data
 protected:
     const int fd_; ///< the SSL socket we are reading and writing
     SBuf rbuf;  ///< Used to buffer input data.
@@ -70,7 +69,7 @@ protected:
 class ClientBio: public Bio
 {
 public:
-    explicit ClientBio(const int anFd): Bio(anFd), holdRead_(false), holdWrite_(false), helloSize(0)/*, wrongProtocol(false)*/ {}
+    explicit ClientBio(const int anFd): Bio(anFd), holdRead_(false), holdWrite_(false), helloSize(0) {}
 
     /// The ClientBio version of the Ssl::Bio::stateChanged method
     /// When the client hello message retrieved, fill the
@@ -84,6 +83,10 @@ public:
     virtual int read(char *buf, int size, BIO *table);
     /// Prevents or allow writting on socket.
     void hold(bool h) {holdRead_ = holdWrite_ = h;}
+
+    /// Sets the buffered input data (Bio::rbuf).
+    /// Used to pass payload data (normally client HELLO data) retrieved
+    /// by the caller.
     void setReadBufData(SBuf &data) {rbuf = data;}
 private:
     /// True if the SSL state corresponds to a hello message
@@ -145,7 +148,7 @@ public:
     void mode(Ssl::BumpMode m) {bumpMode_ = m;}
     Ssl::BumpMode bumpMode() {return bumpMode_;} ///< return the bumping mode
 
-    /// Return the TLS Details advertised by TLS server.
+    /// \return the TLS Details advertised by TLS server.
     const Security::TlsDetails::Pointer &receivedHelloDetails() const {return parser_.details;}
 
     const Ssl::X509_STACK_Pointer &serverCertificatesIfAny() { return parser_.serverCertificates; } /* XXX: may be nil */
@@ -170,9 +173,9 @@ private:
     bool parsedHandshake; ///< whether we are done parsing TLS Hello
     Ssl::BumpMode bumpMode_;
 
-    ///< The size of data stored in rbuf which passed to the openSSL
+    /// The size of data stored in rbuf which passed to the openSSL
     size_t rbufConsumePos;
-    Security::HandshakeParser parser_; ///< The SSL messages parser.
+    Security::HandshakeParser parser_; ///< The TLS/SSL messages parser.
 };
 
 } // namespace Ssl