On 14.11.2012 04:43, Tsantilas Christos wrote:
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?
Not the official ones no. They still have known issues when linking
both OpenSSL and GnuTLS against the same binary - which we will have to
allow for a while. I am going for implementing Squid custom wrappers as
appropriate. I was playing with your SSL_CTX_Pointer and related
typedefs last night and I think we should move to using them exclusively
and only have to change them with #if..#else'd definitions for each
library. there will be some functional operations to re-write from
scratch too I'm fairly sure.
Which are the other SSL libraries?
GnuTLS (Debian, Gentoo, Ubuntu), libNSS (Fedora/RHEL), CryptoAPI
(MacOS) are the ones I'm getting pressure about. There are others on a
"nice to have" list if they are easy to do - but not worth any extra
work.
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.
Understood and I agree. Which is why I'm requesting modularity along
the lines of operations rather than OpenSSL API calls.
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...
I'm not asking you to stop or even to alter what you have lined up.
Just to keep an eye out on the layering for me as you go forward from
today - you have the most visible experience with current SSL
infrastructure in Squid-3 so seem to me a good person to help decide
when something is, or not, appropriate to be abstracted into a unit
behind one function/class/typedef/#define name.
However with a very quick research looks that there are equivalent
interfaces in GnuTLS for the openSSL interfaces we are using for
shared
sessions ...
I am fully expecting that there will be features we cannot support in
all SSL libraries. That is fine, user demand will drive any re-working
that needs to be done on the more difficult features. For now the demand
is simply to get the basic old HTTPS connectivity working without
OpenSSl dependency.
Amos
