Avoid SSL certificate db corruption with empty index.txt as a symptom.

* Detect cases where the size file is corrupted or has a clearly wrong
  value. Automatically rebuild the database in such cases.

* Teach ssl_crtd to keep running if it is unable to store the generated
  certificate in the database. Return the generated certificate to Squid
  and log an error message in such cases.

Background:

There are cases where ssl_crtd may corrupt its certificate database.
The known cases manifest themselves with an empty db index file.  When
that happens, ssl_crtd helpers quit, SSL bumping does not work any more,
and the certificate DB has to be deleted and re-initialized.

We do not know exactly what causes corruption in deployments, but one
known trigger that is easy to reproduce in a lab is the block size
change in the ssl_crtd configuration. That change has the following
side-effects:

1. When ssl_crtd removes certificates, it computes their size using a
   different block size than the one used to store the certificates.
   This is may result in negative database sizes.

2. Signed/unsigned conversion results in a huge number near LONG_MAX,
   which is then written to the "size" file.

3. The ssl_crtd helper remoces all certificates from database trying to make
   space for new certificates.

4. The ssl_crtd helper refuses to store new certificates because the
   database size (as described by the "size" file) still exceeds the
   configured limit.

5. The ssl_crtd helper exits because it cannot store a new certificates
   to the database. No helper response is sent to Squid in this case.

Most likely, there are other corruption triggers -- the database
management code is of an overall poor quality. This change resolves some
of the underlying problems in hope to address at least some of the
unknown triggers as well as the known one.

This is a Measurement Factory project.

diff --git a/src/ssl/certificate_db.cc b/src/ssl/certificate_db.cc
index 175243ce56d7d3bc8e7d59c59e0bdaec9bb2b7b6..4c4d6075bae3a469b173b9a50e4efb6b706f9e73 100644 (file)
--- a/src/ssl/certificate_db.cc
+++ b/src/ssl/certificate_db.cc
@@ -320,18 +320,25 @@ bool Ssl::CertificateDb::addCertAndPrivateKey(Ssl::X509_Pointer & cert, Ssl::EVP
     }
 
     // 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()) {
-                save(); // Some entries may have been removed. Update the index file.
-                return false; // errors prevented us from freeing enough space
-            }
-        } while (size() > max_db_size);
-        break;
+    size_t dbSize = size();
+    if ((dbSize == 0 && hasRows()) ||
+        (dbSize > 0 && !hasRows()) ||
+        (dbSize >  10 * max_db_size)) {
+        // Invalid database size, rebuild
+        dbSize = rebuildSize();
+    }
+    while (dbSize > max_db_size && deleteInvalidCertificate()) {
+        dbSize = size(); // get the current database size
+        // and try to find another invalid certificate if needed
+    }
+    // there are no more invalid ones, but there must be valid certificates
+    while (dbSize > max_db_size){
+        if (!deleteOldestCertificate()) {
+            rebuildSize(); // No certificates in database.Update the size file.
+            save(); // Some entries may have been removed. Update the index file.
+            return false; // errors prevented us from freeing enough space
+        }
+        dbSize = size(); // get the current database size
     }
 
     row.setValue(cnlType, "V");
@@ -456,7 +463,8 @@ void Ssl::CertificateDb::addSize(std::string const & filename) {
 void Ssl::CertificateDb::subSize(std::string const & filename) {
     // readSize will rebuild 'size' file if missing or it is corrupted
     size_t dbSize = readSize();
-    dbSize -= getFileSize(filename);
+    const size_t fileSize = getFileSize(filename);
+    dbSize = dbSize > fileSize ? dbSize - fileSize : 0;
     writeSize(dbSize);
 }
 
@@ -480,8 +488,10 @@ size_t Ssl::CertificateDb::getFileSize(std::string const & filename) {
     if (!file)
         return 0;
     file.seekg(0, std::ios_base::end);
-    size_t file_size = file.tellg();
-    return ((file_size + fs_block_size - 1) / fs_block_size) * fs_block_size;
+    const std::streampos file_size = file.tellg();
+    if (file_size < 0)
+        return 0;
+    return ((static_cast<size_t>(file_size) + fs_block_size - 1) / fs_block_size) * fs_block_size;
 }
 
 void Ssl::CertificateDb::load() {
@@ -561,15 +571,9 @@ bool Ssl::CertificateDb::deleteInvalidCertificate() {
     return true;
 }
 
-bool Ssl::CertificateDb::deleteOldestCertificate() {
-    if (!db)
-        return false;
-
-#if SQUID_SSLTXTDB_PSTRINGDATA
-    if (sk_OPENSSL_PSTRING_num(db.get()->data) == 0)
-#else
-    if (sk_num(db.get()->data) == 0)
-#endif
+bool Ssl::CertificateDb::deleteOldestCertificate()
+{
+    if (!hasRows())
         return false;
 
 #if SQUID_SSLTXTDB_PSTRINGDATA
@@ -610,6 +614,20 @@ bool Ssl::CertificateDb::deleteByHostname(std::string const & host) {
     return false;
 }
 
+bool Ssl::CertificateDb::hasRows() const
+{
+    if (!db)
+        return false;
+
+#if SQUID_SSLTXTDB_PSTRINGDATA
+    if (sk_OPENSSL_PSTRING_num(db.get()->data) == 0)
+#else
+    if (sk_num(db.get()->data) == 0)
+#endif
+        return false;
+    return true;
+}
+
 bool Ssl::CertificateDb::IsEnabledDiskStore() const {
     return enabled_disk_store;
 }

diff --git a/src/ssl/certificate_db.h b/src/ssl/certificate_db.h
index 3062fa6d41b85c47b8b37bc0b2f9f29efb22ba02..96e8e8bec9fd73ac976902dff0339c32f3ab3f43 100644 (file)
--- a/src/ssl/certificate_db.h
+++ b/src/ssl/certificate_db.h
@@ -127,6 +127,7 @@ private:
     bool deleteInvalidCertificate(); ///< Delete invalid certificate.
     bool deleteOldestCertificate(); ///< Delete oldest certificate.
     bool deleteByHostname(std::string const & host); ///< Delete using host name.
+    bool hasRows() const; ///< Whether the TXT_DB has stored items.
 
     /// Removes the first matching row from TXT_DB. Ignores failures.
     static void sq_TXT_DB_delete(TXT_DB *db, const char **row);

diff --git a/src/ssl/ssl_crtd.cc b/src/ssl/ssl_crtd.cc
index f0181fa42173f2971b2f106f7f8b5bba5eefbf5e..41042ebdbc82f60462ed386b0d00a7eac0d7ea20 100644 (file)
--- a/src/ssl/ssl_crtd.cc
+++ b/src/ssl/ssl_crtd.cc
@@ -205,7 +205,13 @@ static bool proccessNewRequest(Ssl::CrtdMessage & request_message, std::string c
     Ssl::EVP_PKEY_Pointer pkey;
     std::string &cert_subject = certProperties.dbKey();
 
-    db.find(cert_subject, cert, pkey);
+    bool dbFailed = false;
+    try {
+        db.find(cert_subject, cert, pkey);
+    } catch (std::runtime_error &err) {
+        dbFailed = true;
+        error = err.what();
+    }
 
     if (cert.get()) {
         if (!Ssl::certificateMatchesProperties(cert.get(), certProperties)) {
@@ -221,10 +227,22 @@ static bool proccessNewRequest(Ssl::CrtdMessage & request_message, std::string c
         if (!Ssl::generateSslCertificate(cert, pkey, certProperties))
             throw std::runtime_error("Cannot create ssl certificate or private key.");
 
-        if (!db.addCertAndPrivateKey(cert, pkey, cert_subject) && db.IsEnabledDiskStore())
-            throw std::runtime_error("Cannot add certificate to db.");
+        if (!dbFailed && db.IsEnabledDiskStore()) {
+            try {
+                if (!db.addCertAndPrivateKey(cert, pkey, cert_subject)) {
+                    dbFailed = true;
+                    error = "Cannot add certificate to db.";
+                }
+            } catch (const std::runtime_error &err) {
+                dbFailed = true;
+                error = err.what();
+            }
+        }
     }
 
+    if (dbFailed)
+        std::cerr << "ssl_crtd helper database '" << db_path  << "' failed: " << error << std::endl;
+
     std::string bufferToWrite;
     if (!Ssl::writeCertAndPrivateKeyToMemory(cert, pkey, bufferToWrite))
         throw std::runtime_error("Cannot write ssl certificate or/and private key to memory.");