After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 645119 - Use ServerTLSConnection.ReferenceIdentities to check cert identity.
Use ServerTLSConnection.ReferenceIdentities to check cert identity.
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Auth client
unspecified
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
Depends on: 645115
Blocks: 645133
 
 
Reported: 2011-03-18 11:04 UTC by Stef Walter
Modified: 2011-08-29 10:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
attaching diff for easier review (8.80 KB, patch)
2011-03-18 13:24 UTC, Guillaume Desmottes
reviewed Details | Review
Updated patch with style changes (9.05 KB, patch)
2011-03-18 14:04 UTC, Stef Walter
reviewed Details | Review

Description Stef Walter 2011-03-18 11:04:23 UTC
ServerTLSConnection.ReferenceIdentities [1] is a way for a connection manager to
indicate that there can be more than one expected identity for a certificate on
the other side of a TLS connection.

This patch implements support for reference identities in empathy-auth-client for checking the identity of server certificates. The connection manager (eg: gabble [2]) supplies the various identities that the certificate can match.

Use Case
========

 * Fry follows google's instructions [3] when setting up his XMPP
   client with google talk. The instructions ask him to override the
   server name with 'talk.google.com'.
 * Fry gets a scary certificate warning that there's someone trying
   to screw with his encrypted connection.
 * Fry gets used to certificate warnings, and sees them as an expected
   part of using his computer.
 * Alternatively Fry uses pidgin or other XMPP clients which don't
   produce a warning in this situation.

Obviously we should never use reference identities that were not specified by
the user either through direct configuration or a choice of some sort. The reference identities supplied by the connection manager represent user input or choices, and not things like DNS lookups.

Depends on the telepathy bits [1] [2], but falls back and behaves correctly when they're not available or unimplemented by a connection manager:

[1] https://bugs.freedesktop.org/show_bug.cgi?id=35408

[2] https://bugs.freedesktop.org/show_bug.cgi?id=35410

[3] http://www.google.com/support/talk/bin/topic.py?topic=1415
Comment 2 Guillaume Desmottes 2011-03-18 13:24:42 UTC
Created attachment 183713 [details] [review]
attaching diff for easier review
Comment 3 Guillaume Desmottes 2011-03-18 13:38:38 UTC
Review of attachment 183713 [details] [review]:

Looks good from a code pov; some coding style issues though.

I'd prefer to merge this once we have a Gabble release with the corresponding bits in so I'll document in release notes that distro should update to this version.

::: libempathy/empathy-server-tls-handler.c
@@ +106,3 @@
   TpDBusDaemon *dbus;
   GError *error = NULL;
+  gchar *default_identities[2];

Please add a small comment explaining the semantic of this variable. Atm it's not clear why it has a size of 2.

@@ +123,3 @@
 
+  identities = tp_asv_get_strv (properties,
+      EMP_IFACE_CHANNEL_TYPE_SERVER_TLS_CONNECTION ".ReferenceIdentities");

Ideally we should use the TP_PROP_* constant here. But that needs a spec and tp-glib release.

@@ +129,3 @@
+   * then fallback to the hostname.
+   */
+  if (!identities)

if (identities == NULL)

@@ +131,3 @@
+  if (!identities)
+    {
+   * If the channel doesn't implement the ReferenceIdentities parameter

(gchar *) hostname;

@@ +133,3 @@
+      default_identities[0] = (gchar*)hostname;
+      default_identities[1] = NULL;
+   * then fallback to the hostname.

(const gchar **) default_..

@@ +144,3 @@
+  }
+
+      default_identities[1] = NULL;

((gchar **) identities);

::: libempathy/empathy-tls-verifier.c
@@ +301,3 @@
 
+  /* now check if the certificate matches one of the reference identities. */
+  for (i = priv->reference_identities, matched = FALSE; i && *i; ++i)

Pointer arithmetic scares me. I'd prefer

for (i =0; priv->reference_identities[i] != NULL; i++)

::: src/empathy-auth-client.c
@@ +210,3 @@
 
+  verifier = empathy_tls_verifier_new (certificate, hostname,
+      (const gchar**)reference_identities);

(const gchar **) reference_
Comment 4 Stef Walter 2011-03-18 13:59:21 UTC
Done. Made those changes in my reference-identities branch, except for:

> Ideally we should use the TP_PROP_* constant here. But that needs a spec and tp-glib release.
Comment 5 Stef Walter 2011-03-18 14:04:09 UTC
Created attachment 183720 [details] [review]
Updated patch with style changes
Comment 6 Guillaume Desmottes 2011-03-18 14:11:23 UTC
Review of attachment 183720 [details] [review]:

Looks good. Feel free to merge once the equivalent Gabble patch has been merged.

::: libempathy/empathy-server-tls-handler.c
@@ +283,3 @@
+      G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
+  g_object_class_install_property (oclass, PROP_REFERENCE_IDENTITIES, pspec);
+

Useless \n here.
Comment 7 Guillaume Desmottes 2011-03-22 13:56:33 UTC
(In reply to comment #4)
> Done. Made those changes in my reference-identities branch, except for:
> 
> > Ideally we should use the TP_PROP_* constant here. But that needs a spec and tp-glib release.

tp-glib 0.14.1 now has this property and Empathy already depends on it so you can use it.
Comment 8 Stef Walter 2011-03-22 14:44:21 UTC
Merged into master. Will attach gnome-2-34 branch patch.