Re: [PATCH] certificate_db/ssl_crtd fixes

From: Tsantilas Christos <chtsanti_at_users.sourceforge.net>
Date: Tue, 13 Nov 2012 17:43:01 +0200

On 11/13/2012 09:48 AM, Amos Jeffries wrote:
> On 11/11/2012 10:41 p.m., Tsantilas Christos wrote:
>> On 11/10/2012 06:25 AM, Amos Jeffries wrote:
>>> Christos,
>>> which of these bugs (if either) are closed by this patch?
>>> http://bugs.squid-cache.org/show_bug.cgi?id=3405
>>> http://bugs.squid-cache.org/show_bug.cgi?id=3436
>> Looks that it fixes the 3405 bug. But maybe we still need to make some
>> improvements.
>
> Great. Ported back to 3.3 citing bug 3405.

OK.

>
> There is at least one clear conflict which makes a semantic change I'm
> not confident enough about to resolve properly for the port back to 3.2:
> Ssl::CertificateDb::addCertAndPrivateKey() condition "if (rrow != NULL)"
> return result changed from false in 3.2 to true in 3.3 as a result of
> the stable cert feature update. I am quite uncomfortable just back
> porting this false->true change and not sure that the false is correct
> for this patch either. If you wish to supply a patch against 3.2 I
> would be happy to have it applied but IMO it is not urgent.

Amos I posted a patch for squid3.2 too with the trunk/3.3 patch...
It is the file certificate_db_fixes-squid-32-v2.patch included in my
first mail in this mail thread.

About the condition "if (rrow != NULL)" changed with the "SslBump:
Support bump-ssl-server-first and mimic SSL server certificates" patch.
This check examines if a certificate with a same serial number is
already stored in database. If the rrow is NULL then there is not any.

In squid-3.2 the latest serial number stored in a file, and any new
generated certificate has as serial this number increment by one (and
updates the serial file). It is a fatal error for the certificates db
to have a certificate in database with a serial number same as a new
generated certificate. This is why it returns false in squid-3.2.

After the bump-ssl-server-first/mimic patch the serial number is a hash
of the generated certificate (or something like that), and if we found a
certificate with the same serial number in db means 1) hash conflict
(not possible) or 2) the certificate is already stored (because an other
squid kid add it for example). On both cases there is not any problem,
we just do not store the already stored certificate.

So the return value for "if (rrow != NULL)" condition must not change.
If you have any problem with the certificate_db_fixes-squid-32-v2.patch
tell me, I will build a new one...

>>
>> We need to make more work to fix the 3436 bug. Currently the ssl_crtd
>> helper will crash for every simple failure. For example if it fails to
>> add to certificate db a certificate, because the TXT_DB_insert openSSL
>> function fails.
>> We should not let ssl_crtd daemon crash on every single error, but just
>> trying to not leave garbage on DB on failures.
>> This patch make a step, because try to clean up the database after
>> failures. But the ssl_crtd still crashes on failures....
>>
>
> BTW, I am starting out on adding GnuTLS and other SSL library support by
> Squid.

Are you planning to use GnuTLS openssl wrappers?
In this case I am not sure it is easy. I am not sure that this GnuTLS
openssl wrappers interface includes all of the OpenSSL functions we are
using. It will not be easy, and requires some of the openSSL parts
implemented inside squid. I think TXT_DB is one of them ...

Which are the other SSL libraries?

> That db_TXT_DB_insert function was a good step. If there are any other
> pieces of SSL code you find which can be shuffled into operational-step
> pieces like that it would be a great help if it were done. I am having
> to identify what the core operation for each SSL call and usage is about
> and find the matching feature usage in each library. The more modular we
> can make Squid code usages the better.

OK. But I do not believe that it is enough just replacing openSSL
functions with GnuTLS equivalent. Whether we like it or not the
architecture affected by the SDKs and libraries we are using.

I am worrying because we are still continue adding new SSL features in
squid. For example we are ready to submit patches to squid-dev which add
support for shared SSL sessions and other SSL related features.
It is not easy to stop the development, there are still thinks to do in
SSL subsystem...

However with a very quick research looks that there are equivalent
interfaces in GnuTLS for the openSSL interfaces we are using for shared
sessions ...

>
> Amos
>
Received on Tue Nov 13 2012 - 15:43:33 MST

This archive was generated by hypermail 2.2.0 : Wed Nov 14 2012 - 12:00:07 MST