Re: [squid-dev] [PATCH] Fix SSL certificate cache refresh and collision handling.
Seems like a very required patch. I was wondering about another semi-related issue from the past: Certificate DB directory become unusable, Was it resolved on 3.5 or 4? Thanks, Eliezer Eliezer Croitoru Linux System Administrator Mobile: +972-5-28704261 Email: elie...@ngtech.co.il -Original Message- From: squid-dev [mailto:squid-dev-boun...@lists.squid-cache.org] On Behalf Of Christos Tsantilas Sent: Friday, July 14, 2017 18:19 To: Squid Developers <squid-...@squid-cache.org> Subject: [squid-dev] [PATCH] Fix SSL certificate cache refresh and collision handling. SslBump was ignoring origin server certificate changes and using the previously cached fake certificate (mimicking now-stale properties). Also, Squid was not detecting key collisions inside certificate caches. On-disk certificate cache fixes: - Use the original certificate signature instead of the certificate subject as part of the key. Using signatures reduces certificate key collisions to deliberate attacks and woefully misconfigured origins, and makes any mishandled attacks a lot less dangerous because the attacking origin server certificate cannot by trusted by a properly configured Squid and cannot be used for encryption by an attacker. We have considered using certificate digests instead of signatures. Digests would further reduce the attack surface to copies of public certificates (as if the origin server was woefully misconfigured). However, unlike the origin-supplied signatures, digests require (expensive) computation in Squid, and implemented collision handling should make any signature-based attacks unappealing. Signatures won on performance grounds. Other key components remain the same: NotValidAfter, NotValidBefore, forced common name, non-default signing algorithm, and signing hash. - Store the original server certificate in the cache (together with the generated certificate) for reliable key collision detection. - Upon detecting key collisions, ignore and replace the existing cache entry with a freshly computed one. This change is required to prevent an attacker from tricking Squid into hitting a cached impersonating certificate when talking to a legitimate origin. In-memory SSL context cache fixes: - Use the original server certificate (in ASN.1 form) as a part of the cache key, to completely eliminate cache key collisions. Other related improvements: - Make the LruMap keys template parameters. - Polish Ssl::CertificateDb class member names to match Squid coding style. Rename some functions parameters to better match their meaning. - Replace Ssl::CertificateProperties::dbKey() with: * Ssl::TxtKeyForCertificateProperties() in ssl/gadgets.cc for on-disk key generation by the ssl_crtd helper; * Ssl::UniqueKeyForCertificateProperties() in ssl/support.cc for in-memory binary keys generation by the SSL context memory cache. - Optimization: Added Ssl::BIO_new_SBuf(SBuf*) for OpenSSL to write directly into SBuf objects. This is a Measurement Factory project. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] [PATCH] Fix SSL certificate cache refresh and collision handling.
On 07/14/2017 09:18 AM, Christos Tsantilas wrote: > SslBump was ignoring origin server certificate changes and using the > previously cached fake certificate (mimicking now-stale properties). I suggest replacing the above claim with a more accurate one (based on our off-list discussions): """ SslBump was ignoring some origin server certificate changes or differences, incorrectly using the previously cached fake certificate (mimicking now-stale properties or properties of a slightly different certificate). """ > +/// \ingroup SslCrtdSslAPI > +/// \returns certificate database key" > +std::string (const CertificateProperties &); An extra trailing '"'. > +/** > + \ingroup ServerProtocolSSLAPI > + * Generates a unique key based on CertificateProperties object and store > it to key > + */ > +void UniqueKeyForCertificateProperties(const Ssl::CertificateProperties > , SBuf ); > - Replace Ssl::CertificateProperties::dbKey() with: > * Ssl::TxtKeyForCertificateProperties() in ssl/gadgets.cc for >on-disk key generation by the ssl_crtd helper; > * Ssl::UniqueKeyForCertificateProperties() in ssl/support.cc for >in-memory binary keys generation by the SSL context memory cache. I think we need to do a better job distinguishing these two functions. I understand that they make keys (with different sets of features) for two different databases and that they have to be in different files due to ssl_crtd linking requirements. I suggest: * Ssl::OnDiskCertificateDbKey() and Ssl::InRamCertificateDbKey() or even * Ssl::DiskDbKey() and Ssl::RamDbKey() This suggestion also removes the current "unique key" tautology. I realize that the suggested names do not describe the features of each key generation method, but neither does the old dbKey() or the new names in the patch. Those features are too complex to be described in a few words and, more importantly, are largely irrelevant to the caller. The above polishing touches can be done during commit. I cannot track all the low-level changes in the patch, but I agree that it solves a serious problem, and support the described solution (but I am biased because I helped design it). Thank you, Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
[squid-dev] [PATCH] Fix SSL certificate cache refresh and collision handling.
SslBump was ignoring origin server certificate changes and using the previously cached fake certificate (mimicking now-stale properties). Also, Squid was not detecting key collisions inside certificate caches. On-disk certificate cache fixes: - Use the original certificate signature instead of the certificate subject as part of the key. Using signatures reduces certificate key collisions to deliberate attacks and woefully misconfigured origins, and makes any mishandled attacks a lot less dangerous because the attacking origin server certificate cannot by trusted by a properly configured Squid and cannot be used for encryption by an attacker. We have considered using certificate digests instead of signatures. Digests would further reduce the attack surface to copies of public certificates (as if the origin server was woefully misconfigured). However, unlike the origin-supplied signatures, digests require (expensive) computation in Squid, and implemented collision handling should make any signature-based attacks unappealing. Signatures won on performance grounds. Other key components remain the same: NotValidAfter, NotValidBefore, forced common name, non-default signing algorithm, and signing hash. - Store the original server certificate in the cache (together with the generated certificate) for reliable key collision detection. - Upon detecting key collisions, ignore and replace the existing cache entry with a freshly computed one. This change is required to prevent an attacker from tricking Squid into hitting a cached impersonating certificate when talking to a legitimate origin. In-memory SSL context cache fixes: - Use the original server certificate (in ASN.1 form) as a part of the cache key, to completely eliminate cache key collisions. Other related improvements: - Make the LruMap keys template parameters. - Polish Ssl::CertificateDb class member names to match Squid coding style. Rename some functions parameters to better match their meaning. - Replace Ssl::CertificateProperties::dbKey() with: * Ssl::TxtKeyForCertificateProperties() in ssl/gadgets.cc for on-disk key generation by the ssl_crtd helper; * Ssl::UniqueKeyForCertificateProperties() in ssl/support.cc for in-memory binary keys generation by the SSL context memory cache. - Optimization: Added Ssl::BIO_new_SBuf(SBuf*) for OpenSSL to write directly into SBuf objects. This is a Measurement Factory project. Fix SSL certificate cache refresh and collision handling. SslBump was ignoring origin server certificate changes and using the previously cached fake certificate (mimicking now-stale properties). Also, Squid was not detecting key collisions inside certificate caches. On-disk certificate cache fixes: - Use the original certificate signature instead of the certificate subject as part of the key. Using signatures reduces certificate key collisions to deliberate attacks and woefully misconfigured origins, and makes any mishandled attacks a lot less dangerous because the attacking origin server certificate cannot by trusted by a properly configured Squid and cannot be used for encryption by an attacker. We have considered using certificate digests instead of signatures. Digests would further reduce the attack surface to copies of public certificates (as if the origin server was woefully misconfigured). However, unlike the origin-supplied signatures, digests require (expensive) computation in Squid, and implemented collision handling should make any signature-based attacks unappealing. Signatures won on performance grounds. Other key components remain the same: NotValidAfter, NotValidBefore, forced common name, non-default signing algorithm, and signing hash. - Store the original server certificate in the cache (together with the generated certificate) for reliable key collision detection. - Upon detecting key collisions, ignore and replace the existing cache entry with a freshly computed one. This change is required to prevent an attacker from tricking Squid into hitting a cached impersonating certificate when talking to a legitimate origin. In-memory SSL context cache fixes: - Use the original server certificate (in ASN.1 form) as a part of the cache key, to completely eliminate cache key collisions. Other related improvements: - Make the LruMap keys template parameters. - Polish Ssl::CertificateDb class member names to match Squid coding style. Rename some functions parameters to better match their meaning. - Replace Ssl::CertificateProperties::dbKey() with: * Ssl::TxtKeyForCertificateProperties() in ssl/gadgets.cc for on-disk key generation by the ssl_crtd helper; * Ssl::UniqueKeyForCertificateProperties() in ssl/support.cc for in-memory binary keys generation by the SSL
