diff --git a/src/client_side.cc b/src/client_side.cc index 3c78c62e5c2..8fcf59e29e3 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2358,16 +2358,9 @@ clientNegotiateSSL(int fd, void *data) conn->clientConnection->tlsNegotiations()->retrieveNegotiatedInfo(session); #if USE_OPENSSL - X509 *client_cert = SSL_get_peer_certificate(session.get()); - - if (client_cert) { - debugs(83, 3, "FD " << fd << " client certificate: subject: " << - Security::SubjectName(*client_cert)); - - debugs(83, 3, "FD " << fd << " client certificate: issuer: " << - Security::IssuerName(*client_cert)); - - X509_free(client_cert); + if (const auto clientCert = Security::CertPointer(SSL_get_peer_certificate(session.get()))) { + debugs(83, 3, "FD " << fd << " client certificate: subject: " << *clientCert); + debugs(83, 3, "FD " << fd << " client certificate: issuer: " << Security::IssuerName(*clientCert)); } else { debugs(83, 5, "FD " << fd << " has no client certificate."); } diff --git a/src/security/Certificate.h b/src/security/Certificate.h index 4d1971e6d3a..0bf6064489d 100644 --- a/src/security/Certificate.h +++ b/src/security/Certificate.h @@ -18,6 +18,20 @@ namespace Security { +/// Content of a X.509 file before parsing. +#if USE_OPENSSL +using CertFileRawPointer = std::unique_ptr>; +#elif HAVE_LIBGNUTLS +inline void gnutls_datum_free(gnutls_datum_t *p) { + if (p && p->size > 0) + gnutls_free(p->data); + delete p; +} +using CertFileRawPointer = std::unique_ptr>; +#else +using CertFileRawPointer = std::unique_ptr; +#endif + /// The SubjectName field of the given certificate (if found) or an empty SBuf. SBuf SubjectName(Certificate &); diff --git a/src/security/ErrorDetail.cc b/src/security/ErrorDetail.cc index afe9fd2e0eb..217ed543988 100644 --- a/src/security/ErrorDetail.cc +++ b/src/security/ErrorDetail.cc @@ -14,7 +14,6 @@ #include "sbuf/Stream.h" #include "security/Certificate.h" #include "security/ErrorDetail.h" -#include "security/forward.h" #include "security/Io.h" #include "util.h" diff --git a/src/security/KeyData.cc b/src/security/KeyData.cc index 94261326ae0..b5046bf4307 100644 --- a/src/security/KeyData.cc +++ b/src/security/KeyData.cc @@ -9,34 +9,65 @@ #include "squid.h" #include "anyp/PortCfg.h" #include "fatal.h" -#include "security/Certificate.h" +#include "sbuf/Stream.h" #include "security/KeyData.h" #include "SquidConfig.h" #include "ssl/bio.h" #include "ssl/gadgets.h" -#include - -/// load the signing certificate and its chain, if any, from certFile -/// \return true if the signing certificate was obtained +/** + * Read certificate from file. + * See also: Ssl::ReadX509Certificate function, gadgets.cc file + */ bool Security::KeyData::loadCertificates() { debugs(83, 2, "from " << certFile); cert.reset(); // paranoid: ensure cert is unset + // Open the .PEM certificate file #if USE_OPENSSL - const char *certFilename = certFile.c_str(); - Ssl::BIO_Pointer bio(BIO_new(BIO_s_file())); - if (!bio || !BIO_read_filename(bio.get(), certFilename)) { + CertFileRawPointer bio(BIO_new(BIO_s_file())); + if (!bio || !BIO_read_filename(bio.get(), certFile.c_str())) { const auto x = ERR_get_error(); debugs(83, DBG_IMPORTANT, "ERROR: unable to load certificate file '" << certFile << "': " << ErrorString(x)); return false; } +#elif HAVE_LIBGNUTLS + CertFileRawPointer bio(new gnutls_datum_t({.data=nullptr, .size=0})); + Security::LibErrorCode x = gnutls_load_file(certFile.c_str(), bio.get()); + if (x != GNUTLS_E_SUCCESS) { + debugs(83, DBG_IMPORTANT, "ERROR: unable to load certificate file '" << certFile << "': " << ErrorString(x)); + return false; + } +#else + CertFileRawPointer bio; // unused +#endif + // Read the client/server/proxy leaf certificate try { +#if USE_OPENSSL cert = Ssl::ReadCertificate(bio); debugs(83, DBG_PARSE_NOTE(2), "Loaded signing certificate: " << *cert); +#elif HAVE_LIBGNUTLS + gnutls_pcert_st pcrt; + x = gnutls_pcert_import_x509_raw(&pcrt, bio.get(), GNUTLS_X509_FMT_PEM, 0); + if (x != GNUTLS_E_SUCCESS) + throw TextException(ToSBuf("failed to import certificate due to ", ErrorString(x)), Here()); + + gnutls_x509_crt_t certificate; + x = gnutls_pcert_export_x509(&pcrt, &certificate); + if (x != GNUTLS_E_SUCCESS) + throw TextException(ToSBuf("unable to X.509 convert certificate due to ", ErrorString(x)), Here()); + + if (certificate) { + cert = Security::CertPointer(certificate, [](gnutls_x509_crt_t p) { + debugs(83, 5, "gnutls_x509_crt_deinit cert=" << (void*)p); + gnutls_x509_crt_deinit(p); + }); + } +#endif + Assure(cert != nullptr); } catch (...) { // TODO: Convert the rest of this method to throw on errors instead. @@ -46,90 +77,96 @@ Security::KeyData::loadCertificates() } try { - // Squid sends `cert` (loaded above) followed by certificates in `chain` - // (formed below by loading and sorting the remaining certificates). - - // load all the remaining configured certificates - CertList candidates; - while (const auto c = Ssl::ReadOptionalCertificate(bio)) - candidates.emplace_back(c); - - // Push certificates into `chain` in on-the-wire order, as defined by - // RFC 8446 Section 4.4.2: "Each following certificate SHOULD directly - // certify the one immediately preceding it." - while (!candidates.empty()) { - const auto precedingCert = chain.empty() ? cert : chain.back(); - - // We cannot chain any certificate after a self-signed certificate. - // This check also protects the IssuedBy() search below from adding - // duplicated (i.e. listed multiple times) self-signed certificates. - if (SelfSigned(*precedingCert)) - break; - - const auto issuerPos = std::find_if(candidates.begin(), candidates.end(), [&](const CertPointer &i) { - return IssuedBy(*precedingCert, *i); - }); - if (issuerPos == candidates.end()) - break; - - const auto &issuer = *issuerPos; - debugs(83, DBG_PARSE_NOTE(3), "Adding CA certificate: " << *issuer); - chain.emplace_back(issuer); - candidates.erase(issuerPos); - } - - for (const auto &c: candidates) - debugs(83, DBG_IMPORTANT, "WARNING: Ignoring certificate that does not extend the chain: " << *c); - } - catch (...) { + if (SelfSigned(*cert)) + debugs(83, DBG_PARSE_NOTE(2), "Certificate is self-signed, will not be chained: " << *cert); + else + loadX509ChainFromFile(bio); + } catch (...) { // TODO: Reject configs with malformed intermediate certs instead. debugs(83, DBG_IMPORTANT, "ERROR: Failure while loading intermediate certificate(s) from '" << certFile << "':" << Debug::Extra << "problem: " << CurrentException); } -#elif HAVE_LIBGNUTLS - const char *certFilename = certFile.c_str(); - gnutls_datum_t data; - Security::LibErrorCode x = gnutls_load_file(certFilename, &data); - if (x != GNUTLS_E_SUCCESS) { - debugs(83, DBG_IMPORTANT, "ERROR: unable to load certificate file '" << certFile << "': " << ErrorString(x)); - return false; - } + return bool(cert); +} - gnutls_pcert_st pcrt; - x = gnutls_pcert_import_x509_raw(&pcrt, &data, GNUTLS_X509_FMT_PEM, 0); - if (x != GNUTLS_E_SUCCESS) { - debugs(83, DBG_IMPORTANT, "ERROR: unable to import certificate from '" << certFile << "': " << ErrorString(x)); - return false; - } - gnutls_free(data.data); +/** + * Read certificate chain from BIO object. + * See also: Ssl::ReadX509Certificate function, gadgets.cc file + */ +void +Security::KeyData::loadX509ChainFromFile(CertFileRawPointer &bio) +{ + if (!bio) + return; - gnutls_x509_crt_t certificate; - x = gnutls_pcert_export_x509(&pcrt, &certificate); - if (x != GNUTLS_E_SUCCESS) { - debugs(83, DBG_IMPORTANT, "ERROR: unable to X.509 convert certificate from '" << certFile << "': " << ErrorString(x)); - return false; - } + debugs(83, DBG_PARSE_NOTE(2), "Using certificate chain in " << certFile); - if (certificate) { - cert = Security::CertPointer(certificate, [](gnutls_x509_crt_t p) { +#if USE_OPENSSL && !TLS_CHAIN_NO_SELFSIGNED + const bool dropSelfSigned = false; +#else + const bool dropSelfSigned = true; +#endif + + debugs(83, DBG_PARSE_NOTE(3), "NOTICE: will " << (dropSelfSigned?"drop":"send") << " self-signed CA (if any) in the chain"); + + // Squid sends `cert` (loaded above) followed by certificates in `chain` + // (formed below by loading and sorting the remaining certificates). + + // load all the remaining configured certificates + CertList candidates; +#if USE_OPENSSL + while (const auto c = Ssl::ReadOptionalCertificate(bio)) + candidates.emplace_back(c); +#elif HAVE_LIBGNUTLS + unsigned int loadedCertCount = 0; + gnutls_x509_crt_t *loadedCerts = nullptr; + const auto x = gnutls_x509_crt_list_import2(&loadedCerts, &loadedCertCount, bio.get(), GNUTLS_X509_FMT_PEM, 0); + if (x != GNUTLS_E_SUCCESS) + throw TextException(ToSBuf(ErrorString(x)), Here()); + + for (unsigned int i = 0; i < loadedCertCount; ++i) { + const auto ca = CertPointer(loadedCerts[i], [](const gnutls_x509_crt_t p) { debugs(83, 5, "gnutls_x509_crt_deinit cert=" << (void*)p); gnutls_x509_crt_deinit(p); }); + + // abuse fact that gnutls_x509_crt_t is actually a pointer in libgnutls + loadedCerts[i] = nullptr; // memory this points to is managed by 'ca' now + candidates.emplace_back(ca); } + // certs in loadedCerts are now either part of the chain, or destroyed by 'ca' + // we just have to free the loadedCerts array itself. + gnutls_free(loadedCerts); +#endif - // XXX: implement chain loading - debugs(83, 2, "Loading certificate chain from PEM files not implemented in this Squid."); + // Push certificates into `chain` in on-the-wire order, as defined by + // RFC 8446 Section 4.4.2: "Each following certificate SHOULD directly + // certify the one immediately preceding it." + while (!candidates.empty()) { + const auto precedingCert = chain.empty() ? cert : chain.back(); -#else - // do nothing. -#endif + // We cannot chain any certificate after a self-signed certificate. + // This check also protects the IssuedBy() search below from adding + // duplicated (i.e. listed multiple times) self-signed certificates. + if (dropSelfSigned && SelfSigned(*precedingCert)) + break; + + const auto issuerPos = std::find_if(candidates.begin(), candidates.end(), [&](const CertPointer &i) { + return IssuedBy(*precedingCert, *i); + }); + if (issuerPos == candidates.end()) + break; - if (!cert) { - debugs(83, DBG_IMPORTANT, "ERROR: unable to load certificate from '" << certFile << "'"); + const auto &issuer = *issuerPos; + debugs(83, DBG_PARSE_NOTE(3), "Adding CA certificate: " << *issuer); + chain.emplace_back(issuer); + candidates.erase(issuerPos); } - return bool(cert); + for (const auto &c: candidates) { + debugs(83, DBG_IMPORTANT, "WARNING: Ignoring certificate that does not extend the chain: " << *c); + } } /** @@ -154,11 +191,11 @@ Security::KeyData::loadX509PrivateKeyFromFile() #elif HAVE_LIBGNUTLS const char *keyFilename = privateKeyFile.c_str(); - gnutls_datum_t data; - if (gnutls_load_file(keyFilename, &data) == GNUTLS_E_SUCCESS) { + gnutls_datum_t rawFileContent; + if (gnutls_load_file(keyFilename, &rawFileContent) == GNUTLS_E_SUCCESS) { gnutls_privkey_t key; (void)gnutls_privkey_init(&key); - Security::ErrorCode x = gnutls_privkey_import_x509_raw(key, &data, GNUTLS_X509_FMT_PEM, nullptr, 0); + Security::ErrorCode x = gnutls_privkey_import_x509_raw(key, &rawFileContent, GNUTLS_X509_FMT_PEM, nullptr, 0); if (x == GNUTLS_E_SUCCESS) { gnutls_x509_privkey_t xkey; gnutls_privkey_export_x509(key, &xkey); @@ -169,7 +206,7 @@ Security::KeyData::loadX509PrivateKeyFromFile() }); } } - gnutls_free(data.data); + gnutls_free(rawFileContent.data); #else // nothing to do. diff --git a/src/security/KeyData.h b/src/security/KeyData.h index d7b84f8f091..77f6f66082d 100644 --- a/src/security/KeyData.h +++ b/src/security/KeyData.h @@ -11,7 +11,7 @@ #include "anyp/forward.h" #include "sbuf/SBuf.h" -#include "security/forward.h" +#include "security/Certificate.h" namespace Security { @@ -37,6 +37,7 @@ class KeyData private: bool loadCertificates(); bool loadX509PrivateKeyFromFile(); + void loadX509ChainFromFile(CertFileRawPointer &); }; } // namespace Security diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index 811d264d6d7..14bb67c7c3e 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -646,17 +646,17 @@ Security::PeerConnector::certDownloadingDone(DownloaderAnswer &downloaderAnswer) // be able to accept collection of certificates. // TODO: support collection of certificates auto raw = reinterpret_cast(downloaderAnswer.resource.rawContent()); - if (auto cert = d2i_X509(nullptr, &raw, downloaderAnswer.resource.length())) { + if (auto cert = CertPointer(d2i_X509(nullptr, &raw, downloaderAnswer.resource.length()))) { debugs(81, 5, "Retrieved certificate: " << *cert); if (!downloadedCerts) downloadedCerts.reset(sk_X509_new_null()); - sk_X509_push(downloadedCerts.get(), cert); + sk_X509_push(downloadedCerts.get(), cert.get()); const auto ctx = peerContext()->raw; const auto certsList = SSL_get_peer_cert_chain(&sconn); - if (!Ssl::findIssuerCertificate(cert, certsList, ctx)) { - if (const auto issuerUri = Ssl::findIssuerUri(cert)) { + if (!Ssl::findIssuerCertificate(cert.get(), certsList, ctx)) { + if (const auto issuerUri = Ssl::findIssuerUri(cert.get())) { debugs(81, 5, "certificate " << *cert << " points to its missing issuer certificate at " << issuerUri); urlsOfMissingCerts.push(SBuf(issuerUri)); diff --git a/src/security/ServerOptions.cc b/src/security/ServerOptions.cc index 8039663660c..2bce4b69d1b 100644 --- a/src/security/ServerOptions.cc +++ b/src/security/ServerOptions.cc @@ -239,7 +239,7 @@ Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &) return false; } - for (auto cert : keys.chain) { + for (const auto &cert : keys.chain) { if (SSL_CTX_add_extra_chain_cert(t.get(), cert.get())) { // increase the certificate lock X509_up_ref(cert.get()); @@ -251,9 +251,20 @@ Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &) #elif HAVE_LIBGNUTLS for (auto &keys : certs) { - gnutls_x509_crt_t crt = keys.cert.get(); gnutls_x509_privkey_t xkey = keys.pkey.get(); - const auto x = gnutls_certificate_set_x509_key(t.get(), &crt, 1, xkey); + + const auto certCount = 1 + keys.chain.size(); + auto crt = new gnutls_x509_crt_t[certCount]; + crt[0] = keys.cert.get(); + if (certCount > 1) { + int i = 1; + for (const auto &cert : keys.chain) { + crt[i++] = cert.get(); // gnutls_x509_crt_t is a raw-pointer + } + } + + const auto x = gnutls_certificate_set_x509_key(t.get(), crt, certCount, xkey); + delete[] crt; if (x != GNUTLS_E_SUCCESS) { SBuf whichFile = keys.certFile; if (keys.certFile != keys.privateKeyFile) { @@ -263,7 +274,6 @@ Security::ServerOptions::createStaticServerContext(AnyP::PortCfg &) debugs(83, DBG_CRITICAL, "ERROR: Failed to initialize server context with keys from " << whichFile << ": " << Security::ErrorString(x)); return false; } - // XXX: add cert chain to the context } #endif diff --git a/src/security/forward.h b/src/security/forward.h index 14e02e25aa4..167f6b1a20a 100644 --- a/src/security/forward.h +++ b/src/security/forward.h @@ -33,8 +33,8 @@ #if HAVE_OPENSSL_RSA_H #include #endif -#if HAVE_OPENSSL_X509_H -#include +#if HAVE_OPENSSL_X509V3_H +#include #endif #endif /* USE_OPENSSL */ #include diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 6d7c61ab4a9..4da241c4a9b 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -1234,6 +1234,7 @@ Ssl::loadCerts(const char *certsFile, Ssl::CertsIndexedList &list) static X509 * findCertIssuerFast(Ssl::CertsIndexedList &list, X509 *cert) { + assert(cert); // TODO: Convert cert parameter to a reference instead. const auto name = Security::IssuerName(*cert); if (name.isEmpty()) return nullptr;