Re: [ANNOUNCE] perdition 2.1

Daniel Kahn Gillmor <[email protected]> Fri, 07 Feb 2014 00:39:40 -0500
Newsgroups	gmane.mail.perdition.user
Message-ID	<[email protected]>
On 02/06/2014 11:01 PM, Simon Horman wrote:
> On Thu, Feb 06, 2014 at 08:41:44PM -0500, Daniel Kahn Gillmor wrote:
>> any thoughts about how to address the PFS/DHE situation?
> 
> Sorry, I completely missed that.
> 
> I think the patch looks good, though I'd like to take a closer look
> at it before applying it before applying it.

that sounds reasonable :)  If you have any feedback or suggestions about
it, i'd be happy to talk it over.

> My preference is to wait a bit before releasing 2.2, but if its
> urgent I can make a release more quickly.

it's not urgent for me personally; i've been running it patched in
production on a few servers for months now.  But i do think it's a
relevant feature for other perdition users.

>> Also, what are the plans for uploading 2.1 to debian?  Sorry to ask for
>> pony after pony :)
> 
> Yes, I plan to do it, addressing Debian bugs #609654 and #692721 at the
> same time. If you would like to help out please feel free to do so :)

hmm, looking at them:

 https://bugs.debian.org/609654

I definitely think there could be something funny with client-side
certificates, but i haven't done enough work to pin it down.  I know
that even for perdition instances that have no expectation of using
client-side certificates, a thunderbird instance that knows about a
client-side cert tries to offer it to the server.  this is a little
weird, and is probably worth digging deeper into perdition's use of the
openssl stack.

I'm seeing a few things that you might find useful:

Looking at the code and documentation around ssl_no_cert_verify and
ssl_no_client_cert_verify, i'm not convinced it's correct, actually.  In
perdition.8, both ssl_no_cert_verify and ssl_no_cert_client_verify are
declared as being relevant for outbound connections only.  should one of
them be for incoming connections?  i think ssl_no_cert_client_verify
should be documented as for incoming connections.

Aside from the options.c parsing code, the only places those two
variables are used are on three lines of ssl.c.  line 585 is clearly for
outgoing (PERDITION_SSL_CLIENT), but line 610 is for incoming
(PERDITION_SSL_SERVER) and line 972 appears only during the certificate
check itself, which presumably won't be called if the client isn't
prompted for a certificate.

incidentally, the comment block above __perdition_ssl_log_certificate
appears to name the wrong function in line 888.

perdition_ssl_ctx() also seems to have some odd logic in it around line
585: it skips loading the CA information for outbound connections that
will not verify a cert, but it doesn't bother to skip this info for
incoming connections that won't verify a cert. why not skip it in both
client and server cases (presumably based off of a different option)?

Another concern: while ca_chain_file is documented as the list of
intermediate certificates, it appears to be used as a set of certificate
authorities for verification (via SSL_CTX_load_verify_locations())
instead of its stated purpose.  ironically, the certificate file itself
is loaded with SSL_CTX_use_certificate_chain_file(), presumably because
of OpenSSL's silly API that doesn't make it simple to separate the EE
cert from the intermediate certs.  treating the ca_chain_file as a list
of legitimate CAs seems problematic; it's certainly possible for an IMAP
frontend to offer one CA for its clients to use (e.g. an intermediate
cert that chains back to a member of the well-known CA cartel) while
using in-house certificate authorities for the backhaul (outbound)
links.  By loading the chain_file into the list of verifiers for the
backhaul links, anyone who compromises the CA used for the frontend
connections can now compromise the backhaul without perdition noticing.

So i think there might be significant cert-handling cleanup to do, both
in documentation and code.  I'm happy to offer patches to make it "Do
the Right Thing" in the cases i'm thinking about, but those changes
might actually affect the crypto semantics for existing users with
existing configs, and i'm not sure how you want to address that.  What
do you think?

 https://bugs.debian.org/692721

This one looks like it might be unreproducible.  do you have a test
suite (or a fuzzing environment or something) that's able to cause these
asserts to happen?

	--dkg

______________________________________________
Perdition-users mailing list
[email protected]
http://lists.vergenet.net/listinfo/perdition-users
signature.asc (application/pgp-signature, 1010 B)