On Wed, 10 Nov 2010 17:05:16 +0200, Tsantilas Christos <[email protected]> wrote: > Hi all, > > This patch implements dynamic SSL certificate generartion in > Squid.When used with SSL Bump, the feature allows Squid to dynamically > generate (using a configurable CA certificate) and cache SSL > certificates for the proxied hosts. > > A description for this feature can be found at: > http://wiki.squid-cache.org/Features/DynamicSslCert > > A first version of the patch posted by Alex, some months before: > http://www.squid-cache.org/mail-archive/squid-dev/201003/0201.html > > Some words about the patch: > > * ssl related source files moved under the src/ssl directory > > * Introduce the TidyPointer class similar to std::auto_ptr, which > implements a pointer that deletes the object it points to when the > pointer's owner or context is gone. It is designed to avoid memory > leaks in the presence of exceptions and processing short cuts.
So whats different about this new pointer type that the existing std:auto_ptr, RefCount::Pointer and CbcPointer are all insufficient? >From what I've heard of the SSL problems they are all cause by it not using RefCount on the contexts themselves. > > * Implements ssl context cache to use with generated ssl contexts. > The Ssl::LocalContextStorage class stores the hostname/ssl context pairs > for a local listening address/port. The Ssl::GlobalContextStorage class > used to store Ssl::LocalContextStorages per local listening address and > handles squid shutdown/configure/reconfigure > > * Ssl::Helper class implements the squid part of the ssl_crtd helpers. > > * The ssl_crtd helper implemented in ssl_crtd.cc and > certificate_db.* files > > * The Ssl::CertificateDb class (certificate_db.* files) implements > a database of certificates on disk files. It is used by ssl_crtd > helper to manipulate generated certificates. > > * The ssl related files included in the libraries libsslutil.a which > contains common classes and functions and the libsquidssl.a which has > squid related ssl objects and functions > > * Use the Ssl namespace for new ssl code > > > Authors: Alex Rousskov, Andrew Balabohin, Christos Tsantilas > This is a Measurement Factory Project. Thanks Christos, configure.in: * why is the ./configure option disabled by default? * please polish up to new configure.in style: ** use SQUID_YESNO, SQUID_DEFINE_BOOL helper macros ** use ENABLE_* for the automake conditional (the ident-lookups block just above the new lines should serve as a demo. If a warning is needed the internal-dns block demos that) src/Makefile.am: * why is WIN32_ALL_SOURCE being altered by this patch? * Instead of adding a new /var/ssl_db which the distros are guaranteed to patch elsewhere... Possibly $(DEFAULT_SWAP_DIR)/ssl_crtd/ (aka $(localstatedir)/cache/squid/ssl_crtd/) ** data may be erased at any time or $(localstatedir)/lib/ssl_crtd/ ** data may be erased but must persist while process running please see http://www.pathname.com/fhs/pub/fhs-2.3.pdf for the options and criteria. src/ProtoPort.cc: * missing required wrapping around <> includes. ** possibly also missing <climits> include for other g++ versions and alternative compilers. same in other .cc files. src/cf.data.pre: (http_port?) - grammar: "If this option is enable[d], cert and key options [are] use[d] to sign generated certificate." "When enabled, the cert and key options are used to sign generated certificates." - "This option is enabled by default". hopefully that is not true. missing qualifier "when SslBump is used"? (sslcrtd_program) - grammar: "SHOULD receive -s and -m options to correct run" "requires the -s and -M parameters." - grammar: "The maximum number of processes spawn to service ssl server" "The maximum number of processes to spawn for SSL certificate generation" src/ssl/Makefile.am: * are you able to wrap ALL of the ssl_crtd lines inside the conditional ENABLE_SSL_CRTD ? with just the source files added to EXTRA_DIST in the else case? * please use $(COMPAT_LIB) instead of linking explicitly. This will build fail since libcompat-squid has sub-dependencies. NP: all of the new .cc and .h files lack <> include wrappers. src/ssl/certificate_db.cc and src/ssl/certificate_db.h: * sys/types.h is included by config.h maybe some of the others as well. * use "#if _SQUID_MSWIN_" not #ifdef. * Ssl::CertificateDb:: size(), readSize() and getSNString() seem to be missing const; * please refer to SSL (uppercase) in the documentation. src/ssl/context_storage.cc and src/ssl/helper.cc: * missing config.h first include src/ssl/crtd_message.h: * CrtdMessage::parse() - please use \retval for doxygen syntax with multiple return values. * no need to use \brief if there is only one line of paragraph documentation. Just stick the text on the /** line. src/ssl/gadgets.h and src/ssl/gadgets.cc: * missing #endif \n #if HAVE_OPENSSL_TXT_DB_H between openssl includes. (and around <string>) * please try and move config.h include from .h to .cc src/ssl/ssl_crtd.cc: * please add -d option for debug tracing. * please use helpers/defines.h macros for communication back to Squid, at least for the debug tracing messages and the IO buffer sizes. src/structs.h: * can the new config options at least be placed in a new src/ssl/Config.* location? Amos
