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.

diff --git a/src/ssl/helper.cc b/src/ssl/helper.cc
index db822c8254f275b73bcab9cf07586dc9ff0f8218..80514394dd56bfaa456d54291a70a7a7aad3927e 100644 (file)
--- 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 f8ec8bd4a9f5d0807ef5d0adbf8b5771fe4c4007..475ef89f279498882292f1ae9d08bb03f707b793 100644 (file)
--- 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() + "\".");
             }