From: Christos Tsantilas Date: Wed, 22 Feb 2012 14:03:43 +0000 (+0200) Subject: stable certificates part3 X-Git-Tag: BumpSslServerFirst.take05~16 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4ece76b2dfa1ec0f9c5510b1f394bf9f425411a7;p=thirdparty%2Fsquid.git stable certificates part3 - Handle the case the signing certificate changed. The ssl_crtd daemon must drop cached certificates which has signed with older signing certificates and generate new one using the current signing certificate - We need to generate certificates with different serial numbers, if the signing certificate has changes in any way, even if the certificates has exactly the same fields. To achieve this we are firstly generating a temporary fake certificate with serial number the hash digest of signing certificates public key. The digest of the temporary fake certificate used as serial key to the final certificate - Bug fix: A cached certificate which has adaptated with one or more algorithms (setNotAfter, setNotBefore, setCommonName etc) did not used and always a new certificate generated. This patch fixes this bug. Notes: - Ssl::ssl_match_certificates replaced with Ssl::certificateMatchesProperties function which checks if a given certificate matches given properties: Checks if the certificate signed with current signing certificate, and check if mimicked certificate matches the given certificate. - The Ssl::CertificateDb::purgeCert method added to delete a certificate from database. --- diff --git a/src/client_side.cc b/src/client_side.cc index 8bcdaba2db..bfb52111e1 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -3738,7 +3738,7 @@ ConnStateData::getSslContextStart() SSL_CTX * dynCtx = ssl_ctx_cache.find(sslBumpCertKey.termedBuf()); if (dynCtx) { debugs(33, 5, HERE << "SSL certificate for " << sslBumpCertKey << " have found in cache"); - if (Ssl::verifySslCertificate(dynCtx, bumpServerCert.get())) { + if (Ssl::verifySslCertificate(dynCtx, certProperties)) { debugs(33, 5, HERE << "Cached SSL certificate for " << sslBumpCertKey << " is valid"); getSslContextDone(dynCtx); return; diff --git a/src/ssl/certificate_db.cc b/src/ssl/certificate_db.cc index 88e70cceb5..114ae6f2e5 100644 --- a/src/ssl/certificate_db.cc +++ b/src/ssl/certificate_db.cc @@ -200,6 +200,20 @@ bool Ssl::CertificateDb::find(std::string const & host_name, Ssl::X509_Pointer & return pure_find(host_name, cert, pkey); } +bool Ssl::CertificateDb::purgeCert(std::string const & key) +{ + const Locker locker(dbLock, Here); + load(); + if (!db) + return false; + + if (!deleteByHostname(key)) + return false; + + save(); + return true; +} + bool Ssl::CertificateDb::addCertAndPrivateKey(Ssl::X509_Pointer & cert, Ssl::EVP_PKEY_Pointer & pkey, std::string const & useName) { const Locker locker(dbLock, Here); diff --git a/src/ssl/certificate_db.h b/src/ssl/certificate_db.h index 2a31d20b7e..88d7fc1612 100644 --- a/src/ssl/certificate_db.h +++ b/src/ssl/certificate_db.h @@ -94,6 +94,8 @@ public: CertificateDb(std::string const & db_path, size_t aMax_db_size, size_t aFs_block_size); /// Find certificate and private key for host name bool find(std::string const & host_name, Ssl::X509_Pointer & cert, Ssl::EVP_PKEY_Pointer & pkey); + /// Delete a certificate from database + bool purgeCert(std::string const & key); /// Save certificate to disk. bool addCertAndPrivateKey(Ssl::X509_Pointer & cert, Ssl::EVP_PKEY_Pointer & pkey, std::string const & useName); /// Create and initialize a database under the db_path diff --git a/src/ssl/gadgets.cc b/src/ssl/gadgets.cc index d44fc3c3c3..8608da568b 100644 --- a/src/ssl/gadgets.cc +++ b/src/ssl/gadgets.cc @@ -338,9 +338,33 @@ static bool generateFakeSslCertificate(Ssl::X509_Pointer & certToStore, Ssl::EVP return true; } +static BIGNUM *createCertSerial(unsigned char *md, unsigned int n) +{ + + assert(n == 20); //for sha1 n is 20 (for md5 n is 16) + + BIGNUM *serial = NULL; + serial = BN_bin2bn(md, n, NULL); + + // if the serial is "0" set it to '1' + if (BN_is_zero(serial)) + BN_one(serial); + + // According the RFC 5280, serial is an 20 bytes ASN.1 INTEGER (a signed big integer) + // and the maximum value for X.509 certificate serial number is 2^159-1 and + // the minimum 0. If the first bit of the serial is '1' ( eg 2^160-1), + // will result to a negative integer. + // To handle this, if the produced serial is greater than 2^159-1 + // truncate the last bit + if (BN_is_bit_set(serial, 159)) + BN_clear_bit(serial, 159); + + return serial; +} + /// Return the SHA1 digest of the DER encoded version of the certificate /// stored in a BIGNUM -static BIGNUM *x509Fingerprint(Ssl::X509_Pointer const & cert) +static BIGNUM *x509Digest(Ssl::X509_Pointer const & cert) { unsigned int n; unsigned char md[EVP_MAX_MD_SIZE]; @@ -348,11 +372,18 @@ static BIGNUM *x509Fingerprint(Ssl::X509_Pointer const & cert) if (!X509_digest(cert.get(),EVP_sha1(),md,&n)) return NULL; - assert(n == 20); //for sha1 n is 20 (for md5 n is 16) + return createCertSerial(md, n); +} + +static BIGNUM *x509Pubkeydigest(Ssl::X509_Pointer const & cert) +{ + unsigned int n; + unsigned char md[EVP_MAX_MD_SIZE]; + + if (!X509_pubkey_digest(cert.get(),EVP_sha1(),md,&n)) + return NULL; - BIGNUM *r = NULL; - r = BN_bin2bn(md, n, NULL); - return r; + return createCertSerial(md, n); } /// Generate a unique serial number based on a Ssl::CertificateProperties object @@ -362,30 +393,17 @@ static bool createSerial(Ssl::BIGNUM_Pointer &serial, Ssl::CertificateProperties Ssl::EVP_PKEY_Pointer fakePkey; Ssl::X509_Pointer fakeCert; - serial.reset(BN_new()); - BN_zero(serial.get()); + serial.reset(x509Pubkeydigest(properties.signWithX509)); + if (!generateFakeSslCertificate(fakeCert, fakePkey, properties, serial)) return false; // The x509Fingerprint return an SHA1 hash. // both SHA1 hash and maximum serial number size are 20 bytes. - BIGNUM *r = x509Fingerprint(fakeCert); + BIGNUM *r = x509Digest(fakeCert); if (!r) return false; - // if the serial is "0" set it to '1' - if (BN_is_zero(r)) - BN_one(r); - - // According the RFC 5280, serial is an 20 bytes ASN.1 INTEGER (a signed big integer) - // and the maximum value for X.509 certificate serial number is 2^159-1 and - // the minimum 0. If the first bit of the serial is '1' ( eg 2^160-1), - // will result to a negative integer. - // To handle this, if the produced serial is greater than 2^159-1 - // truncate the last bit - if (BN_is_bit_set(r, 159)) - BN_clear_bit(r, 159); - serial.reset(r); return true; } @@ -495,27 +513,55 @@ static int asn1time_cmp(ASN1_TIME *asnTime1, ASN1_TIME *asnTime2) return strcmp(strTime1, strTime2); } -bool Ssl::ssl_match_certificates(X509 *cert1, X509 *cert2) +bool Ssl::certificateMatchesProperties(X509 *cert, CertificateProperties const &properties) { - assert(cert1 && cert2); - X509_NAME *cert1_name = X509_get_subject_name(cert1); - X509_NAME *cert2_name = X509_get_subject_name(cert2); - if (X509_NAME_cmp(cert1_name, cert2_name) != 0) - return false; + assert(cert); + + // For non self-signed certificates we have to check if the signing certificate changed + if (properties.signAlgorithm != Ssl::algSignSelf) { + if (X509_check_issued(properties.signWithX509.get(), cert) != X509_V_OK) + return false; + } + + X509 *cert2 = properties.mimicCert.get(); + // If there is not certificate to mimic stop here + if (!cert2) + return true; + + if (!properties.setCommonName) { + X509_NAME *cert1_name = X509_get_subject_name(cert); + X509_NAME *cert2_name = X509_get_subject_name(cert2); + if (X509_NAME_cmp(cert1_name, cert2_name) != 0) + return false; + } + /* else { + if (properties.commonName != Ssl::CommonHostName(cert)) + return false; + This function normaly called to verify a cached certificate matches the + specifications given by properties parameter. + The cached certificate retrieved from the cache using a key which has + as part the properties.commonName. This is enough to assume that the + cached cert has in its subject the properties.commonName as cn field. + } + */ - ASN1_TIME *aTime = X509_get_notBefore(cert1); - ASN1_TIME *bTime = X509_get_notBefore(cert2); - if (asn1time_cmp(aTime, bTime) != 0) - return false; + if (!properties.setValidBefore) { + ASN1_TIME *aTime = X509_get_notBefore(cert); + ASN1_TIME *bTime = X509_get_notBefore(cert2); + if (asn1time_cmp(aTime, bTime) != 0) + return false; + } - aTime = X509_get_notAfter(cert1); - bTime = X509_get_notAfter(cert2); - if (asn1time_cmp(aTime, bTime) != 0) - return false; + if (!properties.setValidAfter) { + ASN1_TIME *aTime = X509_get_notAfter(cert); + ASN1_TIME *bTime = X509_get_notAfter(cert2); + if (asn1time_cmp(aTime, bTime) != 0) + return false; + } char *alStr1; int alLen; - alStr1 = (char *)X509_alias_get0(cert1, &alLen); + alStr1 = (char *)X509_alias_get0(cert, &alLen); char *alStr2 = (char *)X509_alias_get0(cert2, &alLen); if ((!alStr1 && alStr2) || (alStr1 && !alStr2) || (alStr1 && alStr2 && strcmp(alStr1, alStr2)) != 0) @@ -523,7 +569,7 @@ bool Ssl::ssl_match_certificates(X509 *cert1, X509 *cert2) // Compare subjectAltName extension STACK_OF(GENERAL_NAME) * cert1_altnames; - cert1_altnames = (STACK_OF(GENERAL_NAME)*)X509_get_ext_d2i(cert1, NID_subject_alt_name, NULL, NULL); + cert1_altnames = (STACK_OF(GENERAL_NAME)*)X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL); STACK_OF(GENERAL_NAME) * cert2_altnames; cert2_altnames = (STACK_OF(GENERAL_NAME)*)X509_get_ext_d2i(cert2, NID_subject_alt_name, NULL, NULL); bool match = true; diff --git a/src/ssl/gadgets.h b/src/ssl/gadgets.h index 1d099ea6a8..cc0008bed6 100644 --- a/src/ssl/gadgets.h +++ b/src/ssl/gadgets.h @@ -249,9 +249,10 @@ bool sslDateIsInTheFuture(char const * date); /** \ingroup SslCrtdSslAPI - * Check if the major (mimicked) fields of the two certificates matches + * Check if the major fields of a certificates matches the properties given by + * a CertficateProperties object \return true if the certificates matches false otherwise. */ -bool ssl_match_certificates(X509 *peer_cert, X509 *peeked_cert); +bool certificateMatchesProperties(X509 *peer_cert, CertificateProperties const &properties); } // namespace Ssl #endif // SQUID_SSL_GADGETS_H diff --git a/src/ssl/ssl_crtd.cc b/src/ssl/ssl_crtd.cc index 9ef7a2dba8..59f0c93f77 100644 --- a/src/ssl/ssl_crtd.cc +++ b/src/ssl/ssl_crtd.cc @@ -213,12 +213,13 @@ static bool proccessNewRequest(Ssl::CrtdMessage & request_message, std::string c db.find(cert_subject, cert, pkey); - if (cert.get() && certProperties.mimicCert.get()) { - if (!Ssl::ssl_match_certificates(cert.get(), certProperties.mimicCert.get())) { + 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); } } diff --git a/src/ssl/support.cc b/src/ssl/support.cc index e17119ccd0..06e3e04071 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -1276,7 +1276,7 @@ SSL_CTX * Ssl::generateSslContext(CertificateProperties const &properties) return createSSLContext(cert, pkey); } -bool Ssl::verifySslCertificate(SSL_CTX * sslContext, X509 *checkCert) +bool Ssl::verifySslCertificate(SSL_CTX * sslContext, CertificateProperties const &properties) { // Temporary ssl for getting X509 certificate from SSL_CTX. Ssl::SSL_Pointer ssl(SSL_new(sslContext)); @@ -1287,10 +1287,7 @@ bool Ssl::verifySslCertificate(SSL_CTX * sslContext, X509 *checkCert) if (!ret) return false; - if (checkCert) - return ssl_match_certificates(cert, checkCert); - - return true; + return certificateMatchesProperties(cert, properties); } bool diff --git a/src/ssl/support.h b/src/ssl/support.h index 5584a74bdc..9eaf8dd2a4 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -125,10 +125,10 @@ SSL_CTX * generateSslContext(CertificateProperties const &properties); \ingroup ServerProtocolSSLAPI * Check if the certificate of the given context is still valid \param sslContext The context to check - \param checkCert Also check if the context certificate matches this certificate + \param properties Check if the context certificate matches the given properties \return true if the contexts certificate is valid, false otherwise */ -bool verifySslCertificate(SSL_CTX * sslContext, X509 *checkCert = NULL); +bool verifySslCertificate(SSL_CTX * sslContext, CertificateProperties const &properties); /** \ingroup ServerProtocolSSLAPI