I just show that I had forgot to attach the patch here.
On 06/23/2015 06:30 PM, Tsantilas Christos wrote:
* 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 refuses to store new certificates because the
database size (as described by the "size" file) exceeds the configured
limit.
4. 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.
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 refuses to store new certificates because the
database size (as described by the "size" file) exceeds the
configured limit.
4. 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.
=== modified file 'src/ssl/certificate_db.cc'
--- src/ssl/certificate_db.cc 2015-04-14 07:26:12 +0000
+++ src/ssl/certificate_db.cc 2015-06-12 17:15:03 +0000
@@ -303,52 +303,59 @@
// TODO: check if the stored row is valid.
return true;
}
{
TidyPointer<char, tidyFree> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), NULL, 0));
Ssl::X509_Pointer findCert;
Ssl::EVP_PKEY_Pointer findPkey;
if (pure_find(useName.empty() ? subject.get() : useName, findCert, findPkey)) {
// Replace with database certificate
cert.reset(findCert.release());
pkey.reset(findPkey.release());
return true;
}
// pure_find may fail because the entry is expired, or because the
// certs file is corrupted. Remove any entry with given hostname
deleteByHostname(useName.empty() ? subject.get() : useName);
}
// 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");
ASN1_UTCTIME * tm = X509_get_notAfter(cert.get());
row.setValue(cnlExp_date, std::string(reinterpret_cast<char *>(tm->data), tm->length).c_str());
row.setValue(cnlFile, "unknown");
if (!useName.empty())
row.setValue(cnlName, useName.c_str());
else {
TidyPointer<char, tidyFree> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), NULL, 0));
row.setValue(cnlName, subject.get());
}
if (!TXT_DB_insert(db.get(), row.getRow())) {
// failed to add index (???) but we may have already modified
// the database so save before exit
save();
return false;
}
rrow = row.getRow();
@@ -439,66 +446,69 @@
readCertAndPrivateKeyFromFiles(cert, pkey, filename.c_str(), NULL);
if (!cert || !pkey)
return false;
return true;
}
size_t Ssl::CertificateDb::size() {
return readSize();
}
void Ssl::CertificateDb::addSize(std::string const & filename) {
// readSize will rebuild 'size' file if missing or it is corrupted
size_t dbSize = readSize();
dbSize += getFileSize(filename);
writeSize(dbSize);
}
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);
}
size_t Ssl::CertificateDb::readSize() {
std::ifstream ifstr(size_full.c_str());
size_t db_size = 0;
if (!ifstr || !(ifstr >> db_size))
return rebuildSize();
return db_size;
}
void Ssl::CertificateDb::writeSize(size_t db_size) {
std::ofstream ofstr(size_full.c_str());
if (!ofstr)
throw std::runtime_error("cannot write \"" + size_full + "\" file");
ofstr << db_size;
}
size_t Ssl::CertificateDb::getFileSize(std::string const & filename) {
std::ifstream file(filename.c_str(), std::ios::binary);
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() {
// Load db from file.
Ssl::BIO_Pointer in(BIO_new(BIO_s_file()));
if (!in || BIO_read_filename(in.get(), db_full.c_str()) <= 0)
throw std::runtime_error("Uninitialized SSL certificate database directory: " + db_path + ". To initialize, run \"ssl_crtd -c -s " + db_path + "\".");
bool corrupt = false;
Ssl::TXT_DB_Pointer temp_db(TXT_DB_read(in.get(), cnlNumber));
if (!temp_db)
corrupt = true;
// Create indexes in db.
if (!corrupt && !TXT_DB_create_index(temp_db.get(), cnlSerial, NULL, LHASH_HASH_FN(index_serial_hash), LHASH_COMP_FN(index_serial_cmp)))
corrupt = true;
if (!corrupt && !TXT_DB_create_index(temp_db.get(), cnlName, NULL, LHASH_HASH_FN(index_name_hash), LHASH_COMP_FN(index_name_cmp)))
corrupt = true;
@@ -544,73 +554,81 @@
#else
const char ** current_row = ((const char **)sk_OPENSSL_PSTRING_value(db.get()->data, i));
#endif
#else
for (int i = 0; i < sk_num(db.get()->data); ++i) {
const char ** current_row = ((const char **)sk_value(db.get()->data, i));
#endif
if (!sslDateIsInTheFuture(current_row[cnlExp_date])) {
deleteRow(current_row, i);
removed_one = true;
break;
}
}
if (!removed_one)
return false;
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
#if SQUID_STACKOF_PSTRINGDATA_HACK
const char **row = ((const char **)sk_value(CHECKED_STACK_OF(OPENSSL_PSTRING, db.get()->data), 0));
#else
const char **row = (const char **)sk_OPENSSL_PSTRING_value(db.get()->data, 0);
#endif
#else
const char **row = (const char **)sk_value(db.get()->data, 0);
#endif
deleteRow(row, 0);
return true;
}
bool Ssl::CertificateDb::deleteByHostname(std::string const & host) {
if (!db)
return false;
#if SQUID_SSLTXTDB_PSTRINGDATA
for (int i = 0; i < sk_OPENSSL_PSTRING_num(db.get()->data); ++i) {
#if SQUID_STACKOF_PSTRINGDATA_HACK
const char ** current_row = ((const char **)sk_value(CHECKED_STACK_OF(OPENSSL_PSTRING, db.get()->data), i));
#else
const char ** current_row = ((const char **)sk_OPENSSL_PSTRING_value(db.get()->data, i));
#endif
#else
for (int i = 0; i < sk_num(db.get()->data); ++i) {
const char ** current_row = ((const char **)sk_value(db.get()->data, i));
#endif
if (host == current_row[cnlName]) {
deleteRow(current_row, i);
return true;
}
}
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;
}
=== modified file 'src/ssl/certificate_db.h'
--- src/ssl/certificate_db.h 2015-01-13 09:13:49 +0000
+++ src/ssl/certificate_db.h 2015-06-12 16:32:50 +0000
@@ -110,40 +110,41 @@
bool IsEnabledDiskStore() const; ///< Check enabled of dist store.
private:
void load(); ///< Load db from disk.
void save(); ///< Save db to disk.
size_t size(); ///< Get db size on disk in bytes.
/// Increase db size by the given file size and update size_file
void addSize(std::string const & filename);
/// Decrease db size by the given file size and update size_file
void subSize(std::string const & filename);
size_t readSize(); ///< Read size from file size_file
void writeSize(size_t db_size); ///< Write size to file size_file.
size_t getFileSize(std::string const & filename); ///< get file size on disk.
size_t rebuildSize(); ///< Rebuild size_file
/// Only find certificate in current db and return it.
bool pure_find(std::string const & host_name, Ssl::X509_Pointer & cert, Ssl::EVP_PKEY_Pointer & pkey);
void deleteRow(const char **row, int rowIndex); ///< Delete a row from TXT_DB
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);
/// Remove the row on position idx from TXT_DB. Ignores failures.
static void sq_TXT_DB_delete_row(TXT_DB *db, int idx);
/// Callback hash function for serials. Used to create TXT_DB index of serials.
static unsigned long index_serial_hash(const char **a);
/// Callback compare function for serials. Used to create TXT_DB index of serials.
static int index_serial_cmp(const char **a, const char **b);
/// Callback hash function for names. Used to create TXT_DB index of names..
static unsigned long index_name_hash(const char **a);
/// Callback compare function for names. Used to create TXT_DB index of names..
static int index_name_cmp(const char **a, const char **b);
/// Definitions required by openSSL, to use the index_* functions defined above
///with TXT_DB_create_index.
#if SQUID_USE_SSLLHASH_HACK
static unsigned long index_serial_hash_LHASH_HASH(const void *a) {
return index_serial_hash((const char **)a);
=== modified file 'src/ssl/ssl_crtd.cc'
--- src/ssl/ssl_crtd.cc 2015-01-13 09:13:49 +0000
+++ src/ssl/ssl_crtd.cc 2015-06-12 14:25:42 +0000
@@ -188,60 +188,78 @@
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)
{
Ssl::CertificateProperties certProperties;
std::string error;
if (!request_message.parseRequest(certProperties, error))
throw std::runtime_error("Error while parsing the crtd request: " + error);
Ssl::CertificateDb db(db_path, max_db_size, fs_block_size);
Ssl::X509_Pointer cert;
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)) {
// The certificate changed (renewed or other reason).
// Generete a new one with the updated fields.
cert.reset(NULL);
pkey.reset(NULL);
db.purgeCert(cert_subject);
}
}
if (!cert || !pkey) {
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.");
Ssl::CrtdMessage response_message(Ssl::CrtdMessage::REPLY);
response_message.setCode("OK");
response_message.setBody(bufferToWrite);
// Use the '\1' char as end-of-message character
std::cout << response_message.compose() << '\1' << std::flush;
return true;
}
/**
\ingroup ssl_crtd
* This is the external ssl_crtd process.
*/
int main(int argc, char *argv[])
{
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
