SSL certificate chain validation issues

[Mark Doliner]

On Sat, Apr 19, 2014 at 7:21 AM, Daniel Atallah <datallah at pidgin.im> wrote:
> it allows it to be delegated to
> the SSL plugin, which I think is where the validation should occur -
> we shouldn't be implementing that ourselves.

I agree with this so, so, so, much. I had never looked at our
certificate code before this. Now that I'm looking at it I'm ashamed
we've been doing it ourselves for so long.

> I only implemented it for NSS, but hopefully it shouldn't be too
> tricky to implement for gnutls and whatever else stuff like adium
> uses.

I didn't look at your NSS-specific code, but the changes to
libpurple/certificate.c look pretty good to me. I'm wondering if it
would be better to move the peer_crt->scheme->verify_cert() call from
x509_tls_cached_unknown_peer() to x509_tls_cached_start_verify().

> I've only tested this a small amount so far, and haven't yet reviewed
> if there are other code paths that would need to be changed too.

I think this change covers all code paths (assuming we implement the
verify_cert callback for gnutls).

All ssl verifications go through purple_certificate_verify(). This
uses our x509_singleuse_start_verify() function for all our
purple_ssl_connect* functions (unless Adium or Instantbird or some
other client registers their own x509 tls_cached verifier by calling
purple_certificate_register_verifier(), which seems unlikely.

> I'm not sure what the best thing to do with the existing
> implementation is - perhaps the best thing to do would be to cripple
> it and have it fail unless the server certificate is explicitly
> trusted

I'm totally ok with this.