From: Amos Jeffries <squid3@treenet.co.nz>
Date: Tue, 30 Jun 2015 10:39:39 +0000 (-0700)
Subject: Move tls-options= parser into Security::PeerOptions
X-Git-Tag: merge-candidate-3-v1~38^2~21^2~5
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c62717bdac6c981fd58c07ed38b49cd3852613f0;p=thirdparty%2Fsquid.git

Move tls-options= parser into Security::PeerOptions

It no longer needs to be called by the SSL code, so can be a private
member without OpenSSL library dependency.

Also, remove c_str() performance regression by upgrading to use
::Parser::Tokenizer.
---

diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc
index 27595c929d..0903979f12 100644
--- a/src/security/PeerOptions.cc
+++ b/src/security/PeerOptions.cc
@@ -61,12 +61,7 @@ Security::PeerOptions::parse(const char *token)
         tlsMinVersion = SBuf(token + 12);
     } else if (strncmp(token, "options=", 8) == 0) {
         sslOptions = SBuf(token + 8);
-#if USE_OPENSSL
-        // Pre-parse SSL client options to be applied when the client SSL objects created.
-        // Options must not used in the case of peek or stare bump mode.
-        // XXX: performance regression. c_str() can reallocate
-        parsedOptions = Security::ParseOptions(sslOptions.c_str());
-#endif
+        parsedOptions = parseOptions();
     } else if (strncmp(token, "cipher=", 7) == 0) {
         sslCipher = SBuf(token + 7);
     } else if (strncmp(token, "cafile=", 7) == 0) {
@@ -325,60 +320,50 @@ static struct ssl_option {
     }
 };
 
+/**
+ * Pre-parse TLS options= parameter to be applied when the TLS objects created.
+ * Options must not used in the case of peek or stare bump mode.
+ */
 long
-Security::ParseOptions(const char *options)
+Security::PeerOptions::parseOptions()
 {
     long op = 0;
-    char *tmp;
-    char *option;
-
-    if (options) {
-
-    tmp = xstrdup(options);
-    option = strtok(tmp, ":,");
-
-    while (option) {
+    ::Parser::Tokenizer tok(sslOptions);
 
+    do {
         enum {
             MODE_ADD, MODE_REMOVE
         } mode;
 
-        switch (*option) {
-
-        case '!':
-
-        case '-':
+        if (tok.skip('-') || tok.skip('!'))
             mode = MODE_REMOVE;
-            ++option;
-            break;
-
-        case '+':
+        else {
+            (void)tok.skip('+'); // default action is add. ignore if missing operator
             mode = MODE_ADD;
-            ++option;
-            break;
+        }
 
-        default:
-            mode = MODE_ADD;
-            break;
+        static const CharacterSet optChars = CharacterSet("TLS-option", "_") + CharacterSet::ALPHA + CharacterSet::DIGIT;
+        int64_t hex = 0;
+        SBuf option;
+        long value = 0;
+
+        if (tok.int64(hex, 16, false)) {
+            /* Special case.. hex specification */
+            value = hex;
         }
 
-        struct ssl_option *opt = NULL;
-        for (struct ssl_option *opttmp = ssl_options; opttmp->name; ++opttmp) {
-            if (strcmp(opttmp->name, option) == 0) {
-                opt = opttmp;
-                break;
+        else if (tok.prefix(option, optChars)) {
+            // find the named option in our supported set
+            for (struct ssl_option *opttmp = ssl_options; opttmp->name; ++opttmp) {
+                if (option.cmp(opttmp->name) == 0) {
+                    value = opttmp->value;
+                    break;
+                }
             }
         }
 
-        long value = 0;
-        if (opt)
-            value = opt->value;
-        else if (strncmp(option, "0x", 2) == 0) {
-            /* Special case.. hex specification */
-            value = strtol(option + 2, NULL, 16);
-        } else {
-            fatalf("Unknown TLS option '%s'", option);
-            value = 0;      /* Keep GCC happy */
+        if (!value) {
+            fatalf("Unknown TLS option '" SQUIDSBUFPH "'", SQUIDSBUFPRINT(option));
         }
 
         switch (mode) {
@@ -392,11 +377,12 @@ Security::ParseOptions(const char *options)
             break;
         }
 
-        option = strtok(NULL, ":,");
-    }
+        static const CharacterSet delims("TLS-option-delim",":,");
+        if (!tok.skipAll(delims) && !tok.atEnd()) {
+            fatalf("Unknown TLS option '" SQUIDSBUFPH "'", SQUIDSBUFPRINT(tok.remaining()));
+        }
 
-    safe_free(tmp);
-    }
+    } while (!tok.atEnd());
 
 #if SSL_OP_NO_SSLv2
     // compliance with RFC 6176: Prohibiting Secure Sockets Layer (SSL) Version 2.0
diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h
index cf5aefec37..13e5fa8db6 100644
--- a/src/security/PeerOptions.h
+++ b/src/security/PeerOptions.h
@@ -40,6 +40,10 @@ public:
     /// output squid.conf syntax with 'pfx' prefix on parameters for the stored settings
     void dumpCfg(Packable *, const char *pfx) const;
 
+private:
+    long parseOptions();
+
+public:
     SBuf certFile;       ///< path of file containing PEM format X509 certificate
     SBuf privateKeyFile; ///< path of file containing private key in PEM format
     SBuf sslOptions;     ///< library-specific options string
@@ -67,11 +71,6 @@ public:
 /// configuration options for DIRECT server access
 extern PeerOptions ProxyOutgoingConfig;
 
-/**
- * Parses the TLS options squid.conf parameter
- */
-long ParseOptions(const char *options);
-
 /**
  * Parses the TLS flags squid.conf parameter
  */
diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc
index 6d2396664c..e1e80cbd90 100644
--- a/src/tests/stub_libsecurity.cc
+++ b/src/tests/stub_libsecurity.cc
@@ -22,7 +22,7 @@ void Security::PeerOptions::parse(char const*) STUB
 Security::ContextPointer Security::PeerOptions::createClientContext(bool) STUB_RETVAL(NULL)
 void Security::PeerOptions::updateTlsVersionLimits() STUB
 void Security::PeerOptions::dumpCfg(Packable*, char const*) const STUB
+long Security::PeerOptions::parseOptions() STUB_RETVAL(0)
 void parse_securePeerOptions(Security::PeerOptions *) STUB
-long Security::ParseOptions(const char *) STUB_RETVAL(0)
 long Security::ParseFlags(const SBuf &) STUB_RETVAL(0)