From 7e43e88e084402fea629e9fb3b4465f9e8dd9c57 Mon Sep 17 00:00:00 2001
From: Nathan Hoad <nathan@getoffmalawn.com>
Date: Fri, 6 May 2016 20:15:36 +1200
Subject: [PATCH] Prevent Squid forcing -b 2048 into the arguments for
 sslcrtd_program

Previously Squid assumed it was running with the default sslcrtd_program, which
takes an argument for the FS block size. This causes issues for administrators
that use their own helpers that happen to take a -b argument that means
something else entirely, causing confusion and preventing them from removing
this argument.

A summary of the changes:

* Move the block size retrieval from Squid into security_file_certgen. It
  does not use fsBlockSize as that introduces a lot of dependencies on
  unrelated Squid code, e.g. fde, Debug, MemBuf.

* Make the -b argument mostly redundant, but leave it there so
  administrators can overrule xstatvfs.

* Fix a small typo.

This work is submitted on behalf of Bloomberg L.P.
---
 src/ssl/helper.cc   | 19 +------------------
 src/ssl/ssl_crtd.cc | 32 +++++++++++++++++++++-----------
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/src/ssl/helper.cc b/src/ssl/helper.cc
index db822c8254..80514394dd 100644
--- a/src/ssl/helper.cc
+++ b/src/ssl/helper.cc
@@ -63,26 +63,9 @@ void Ssl::Helper::Init()
     {
         char *tmp = xstrdup(Ssl::TheConfig.ssl_crtd);
         char *tmp_begin = tmp;
-        char * token = NULL;
-        bool db_path_was_found = false;
-        bool block_size_was_found = false;
-        char buffer[20] = "2048";
+        char *token = NULL;
         while ((token = strwordtok(NULL, &tmp))) {
             wordlistAdd(&ssl_crtd->cmdline, token);
-            if (!strcmp(token, "-b"))
-                block_size_was_found = true;
-            if (!strcmp(token, "-s")) {
-                db_path_was_found = true;
-            } else if (db_path_was_found) {
-                db_path_was_found = false;
-                int fs_block_size = 0;
-                storeDirGetBlkSize(token, &fs_block_size);
-                snprintf(buffer, sizeof(buffer), "%i", fs_block_size);
-            }
-        }
-        if (!block_size_was_found) {
-            wordlistAdd(&ssl_crtd->cmdline, "-b");
-            wordlistAdd(&ssl_crtd->cmdline, buffer);
         }
         safe_free(tmp_begin);
     }
diff --git a/src/ssl/ssl_crtd.cc b/src/ssl/ssl_crtd.cc
index f8ec8bd4a9..475ef89f27 100644
--- a/src/ssl/ssl_crtd.cc
+++ b/src/ssl/ssl_crtd.cc
@@ -24,8 +24,8 @@
  \defgroup ssl_crtd ssl_crtd
  \ingroup ExternalPrograms
  \par
-    Because the standart generation of ssl certificate for
-    sslBump feature, Squid must use external proccess to
+    Because the standard generation of ssl certificate for
+    sslBump feature, Squid must use external process to
     actually make these calls. This process generate new ssl
     certificates and worked with ssl certificates disk cache.
     Typically there will be five ssl_crtd processes spawned
@@ -188,11 +188,8 @@ static void usage()
     std::cerr << help_string << std::endl;
 }
 
-/**
- \ingroup ssl_crtd
- * Proccess new request message.
- */
-static bool proccessNewRequest(Ssl::CrtdMessage & request_message, std::string const & db_path, size_t max_db_size, size_t fs_block_size)
+/// Process new request message.
+static bool processNewRequest(Ssl::CrtdMessage & request_message, std::string const & db_path, size_t max_db_size, size_t fs_block_size)
 {
     Ssl::CertificateProperties certProperties;
     std::string error;
@@ -265,11 +262,11 @@ int main(int argc, char *argv[])
 {
     try {
         size_t max_db_size = 0;
-        size_t fs_block_size = 2048;
+        size_t fs_block_size = 0;
         int8_t c;
         bool create_new_db = false;
         std::string db_path;
-        // proccess options.
+        // process options.
         while ((c = getopt(argc, argv, "dcghvs:M:b:n:")) != -1) {
             switch (c) {
             case 'd':
@@ -310,13 +307,26 @@ int main(int argc, char *argv[])
             exit(0);
         }
 
+        if (fs_block_size == 0) {
+            struct statvfs sfs;
+
+            if (xstatvfs(db_path.c_str(), &sfs)) {
+                fs_block_size = 2048;
+            } else {
+                fs_block_size = sfs.f_frsize;
+                // Sanity check; make sure we have a meaningful value.
+                if (fs_block_size < 512)
+                    fs_block_size = 2048;
+            }
+        }
+
         {
             Ssl::CertificateDb::check(db_path, max_db_size, fs_block_size);
         }
         // Initialize SSL subsystem
         SSL_load_error_strings();
         SSLeay_add_ssl_algorithms();
-        // proccess request.
+        // process request.
         for (;;) {
             char request[HELPER_INPUT_BUFFER];
             Ssl::CrtdMessage request_message(Ssl::CrtdMessage::REQUEST);
@@ -332,7 +342,7 @@ int main(int argc, char *argv[])
             if (parse_result == Ssl::CrtdMessage::ERROR) {
                 throw std::runtime_error("Cannot parse request message.");
             } else if (request_message.getCode() == Ssl::CrtdMessage::code_new_certificate) {
-                proccessNewRequest(request_message, db_path, max_db_size, fs_block_size);
+                processNewRequest(request_message, db_path, max_db_size, fs_block_size);
             } else {
                 throw std::runtime_error("Unknown request code: \"" + request_message.getCode() + "\".");
             }
-- 
2.47.2