If there is not any objections I will commit this patch to the trunk.

Regards,
  Christos

On 11/16/2010 10:47 AM, Amos Jeffries wrote:
On 16/11/10 20:27, Tsantilas Christos wrote:
This version of the patch addresses the Amos comments and suggestions.
Please see below


On 11/11/2010 03:11 AM, Amos Jeffries wrote:
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,
<snip>

src/Makefile.am:
* why is WIN32_ALL_SOURCE being altered by this patch?
It is not altered.


In EXTRA_squid_SOURCES you had/have:

- $(WIN32_ALL_SOURCE) \

Seems like it may be useful at some point but not related to 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.

I put it to $(localstatedir)/lib/.
I am not sure it is correct. Maybe the best is under the
$(localstatedir)/cache/ but here exist the main squid cache.


Okay.

** possibly also missing<climits> include for other g++ versions and
alternative compilers.
same in other .cc files.

Is it needed? We are using "limits.h"


I think not when <limits.h> is used, but when the C++ <limits> is used
like ProtoPort.cc there have been problems.



Okay. Looks good. +1

Amos

Reply via email to