Bug 3497: Bad ssl_crtd db size file causes infinite loop.

The db size file may become empty when Squid runs out of disk space. Ignoring
db size reading errors led to bogus db sizes used as looping condition. This
fix honors reading errors and also terminates the loop when no more
certificates can be removed. Both errors and removal failure are fatal to
ssl_crtd.

A positive side-effect of this fix is one less call to the relatively
expensive file-reading size()/readSize() methods under normal conditions.

I also removed "minimum db size" check because it did not seem to be in sync
with other ssl_crtd parameters such as fs block size and because its overall
purpose was unclear. The check was also removed by the original bug reporter.

TODO: Remaining problems include: ssl_crtd should not exit just because it
cannot write something to disk. A proper reporting/debugging API is missing.

Same as trunk r12062.

diff --git a/src/ssl/certificate_db.cc b/src/ssl/certificate_db.cc
index 220619d369451c4b24087f4b43f45698bd455a30..708d2f07f01da519735411fceafe91dfc74dede7 100644 (file)
--- a/src/ssl/certificate_db.cc
+++ b/src/ssl/certificate_db.cc
@@ -174,7 +174,6 @@ int Ssl::CertificateDb::index_name_cmp(const char **a, const char **b)
 const std::string Ssl::CertificateDb::db_file("index.txt");
 const std::string Ssl::CertificateDb::cert_dir("certs");
 const std::string Ssl::CertificateDb::size_file("size");
-const size_t Ssl::CertificateDb::min_db_size(4096);
 
 Ssl::CertificateDb::CertificateDb(std::string const & aDb_path, size_t aMax_db_size, size_t aFs_block_size)
         :  db_path(aDb_path),
@@ -218,7 +217,7 @@ bool Ssl::CertificateDb::addCertAndPrivateKey(Ssl::X509_Pointer & cert, Ssl::EVP
 {
     const Locker locker(dbLock, Here);
     load();
-    if (!db || !cert || !pkey || min_db_size > max_db_size)
+    if (!db || !cert || !pkey)
         return false;
     Row row;
     ASN1_INTEGER * ai = X509_get_serialNumber(cert.get());
@@ -241,14 +240,18 @@ bool Ssl::CertificateDb::addCertAndPrivateKey(Ssl::X509_Pointer & cert, Ssl::EVP
         if (pure_find(useName.empty() ? subject.get() : useName, cert, pkey))
             return true;
     }
-    // check db size.
-    while (max_db_size < size()) {
-        if (!deleteInvalidCertificate())
-            break;
-    }
 
-    while (max_db_size < size()) {
-        deleteOldestCertificate();
+    // check db size while trying to minimize calls to size()
+    while (size() > max_db_size) {
+        if (deleteInvalidCertificate())
+            continue; // try to find another invalid certificate if needed
+
+        // there are no more invalid ones, but there must be valid certificates
+        do {
+            if (!deleteOldestCertificate())
+                return false; // errors prevented us from freeing enough space
+        } while (size() > max_db_size);
+        break;
     }
 
     row.setValue(cnlType, "V");
@@ -355,11 +358,12 @@ void Ssl::CertificateDb::subSize(std::string const & filename)
 
 size_t Ssl::CertificateDb::readSize() const
 {
-    size_t db_size;
     std::ifstream size_file(size_full.c_str());
     if (!size_file && enabled_disk_store)
-        throw std::runtime_error("cannot read \"" + size_full + "\" file");
-    size_file >> db_size;
+        throw std::runtime_error("cannot open for reading: " + size_full);
+    size_t db_size = 0;
+    if (!(size_file >> db_size))
+        throw std::runtime_error("error while reading " + size_full);
     return db_size;
 }