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 &TxtKeyForCertificateProperties(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 > &certProperties, SBuf &key); > - 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