[Bug 38749] Accept extra certificate identities without relying on an external channel handler

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Tue Jun 28 18:52:18 CEST 2011


https://bugs.freedesktop.org/show_bug.cgi?id=38749

--- Comment #2 from Will Thompson <will.thompson at collabora.co.uk> 2011-06-28 09:52:15 PDT ---
Wōcky review:

> In this case the
> XMPP server (rightly) provides a certificate for talk.google.com, but
> the JID's domain part is the Google apps domain.

It’s not really “rightly”—the server is not really meant to behave like this.
More like, “Rather than providing a certificate for the JID’s domain part, the
server provides a certificate for talk.google.com; if the user has explicitly
configured a ‘Google Talk’ account, it’s reasonable to accept certificates for
this domain.”

Regarding the comment: I think the only part which was inaccurate before your
changes was the use of “When lenient” and “When 'strict'” rather than “When
ignore_ssl_errors is TRUE” and “Otherwise”. The rest is accurate (though
confusingly-worded as if there were four cases rather than two orthogonal pairs
of cases): when using legacy TLS you expect the certificate of the domain you
connected to (because the server has to present a certificate before you tell
it what your domain is); when using STARTTLS your initial, unencrypted
<stream:stream> opening includes the domain of your JID so when you do
<starttls/> the server knows which domain it should present a cert for (but of
course Google ignores this information).

So I think it could do with a bit of rewording in any case, and of course needs
updating in the face of these changes: if the user/application has explicitly
specified other possible identities, then those are acceptable too.

+      peers = gnutls_certificate_get_peers (session->session, &cls);

'cls' is not a great name for the that variable, compared to something like
‘n_peers’. (“Certificate List Size” is the only expansion I can come up with
that makes sense.) This is existing code, so I'm not going to demand it be
fixed. (On closer inspection, it's unused in this function—only the bizzare
construction ‘&peers[0]’ is used—but GnuTLS, that shining jewel of a library,
doesn't let you pass NULL instead …)

-              DEBUG ("gnutls_openpgp_crt_check_hostname: %s ->
%d",peername,rval);
+              DEBUG ("gnutls_openpgp_crt_check_hostname: %s -> %d",peername,
rval);

I guess this is to fix the coding style by adding a space … could you add
another space?

+                  rval = gnutls_openpgp_crt_check_hostname(opgp, peername);

Style: space before the paren plz.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.


More information about the telepathy-bugs mailing list