Bug 63810 (CVE-2007-6746)

(Deliberately not under embargo because this has been fairly obvious in the source code since 2007.)

Created attachment 78338 [details] [review]
Include config.h in each source file

Among other effects, this makes GLIB_VERSION_MIN_REQUIRED effective.

---

Not related to this bug, but without this patch, I can't compile Idle with its default warnings setup.

Created attachment 78339 [details] [review]
messages/invalid-utf8.py: amend test-case to work under  GLib 2.36

---

Again, this is unrelated to this bug, but without this patch, Idle fails tests on my development machine.

Created attachment 78341 [details] [review]
IdleServerConnection: check certificates properly,  except in the tests

---

I deliberately didn't add an ignore-ssl-errors boolean parameter, because there doesn't seem much point, other than regression testing: all public IRC services listen on a non-SSL port, and SSL vs. non-SSL cannot be auto-detected on IRC (there is no STARTTLS support), so we only use SSL if the user has explicitly set use-ssl to TRUE. So, users who would otherwise use ignore-ssl-errors can just turn off use-ssl.

Meanwhile, private SSL-only IRC servers (e.g. a company's internal IRC) can and should have a proper certificate, either from a company CA or from Startcom or something.

Comment on attachment 78338 [details] [review]
Include config.h in each source file

Review of attachment 78338 [details] [review]:
-----------------------------------------------------------------

++

Comment on attachment 78339 [details] [review]
messages/invalid-utf8.py: amend test-case to work under  GLib 2.36

Review of attachment 78339 [details] [review]:
-----------------------------------------------------------------

++

Comment on attachment 78341 [details] [review]
IdleServerConnection: check certificates properly,  except in the tests

Review of attachment 78341 [details] [review]:
-----------------------------------------------------------------

++

Fixed in 0.1.15.

The minimal patch for distributors of Idle 0.1.11-0.1.14 is something more like this:

diff --git a/src/idle-server-connection.c b/src/idle-server-connection.c
index 5b8629c..8c8eeff 100644
--- a/src/idle-server-connection.c
+++ b/src/idle-server-connection.c
@@ -469,9 +469,4 @@ IdleServerConnectionState idle_server_connection_get_state(IdleServerConnection
 void idle_server_connection_set_tls(IdleServerConnection *conn, gboolean tls) {
 	IdleServerConnectionPrivate *priv = IDLE_SERVER_CONNECTION_GET_PRIVATE(conn);
 	g_socket_client_set_tls(priv->socket_client, tls);
-	g_socket_client_set_tls_validation_flags(priv->socket_client,
-		G_TLS_CERTIFICATE_VALIDATE_ALL
-		& ~G_TLS_CERTIFICATE_UNKNOWN_CA
-		& ~G_TLS_CERTIFICATE_BAD_IDENTITY
-		& ~G_TLS_CERTIFICATE_EXPIRED);
 }

(which breaks the regression tests, but is minimal). That's what I plan to do for Debian 7.

I don't plan to implement certificate verification for 0.1.10 or older. If a distributor wants to do so, they're welcome to share patches via this bug.

(In reply to comment #4)
> Created attachment 78341 [details] [review] [review]
> IdleServerConnection: check certificates properly,  except in the tests
> 
> ---
> 
> I deliberately didn't add an ignore-ssl-errors boolean parameter, because
> there doesn't seem much point, other than regression testing: all public IRC
> services listen on a non-SSL port, and SSL vs. non-SSL cannot be
> auto-detected on IRC (there is no STARTTLS support), so we only use SSL if
> the user has explicitly set use-ssl to TRUE. So, users who would otherwise
> use ignore-ssl-errors can just turn off use-ssl.

This implies you seem to think encrypted, but unverified connections are not valuable. Which is not practically true.

> Meanwhile, private SSL-only IRC servers (e.g. a company's internal IRC) can
> and should have a proper certificate, either from a company CA or from
> Startcom or something.

They indeed should, but if they don't that doesn't mean their users can force that change to happen. Those users, without an option to turn of certificate checking (or until my interactive tls patch are merged) won't be able to use idle for IRC which is somewhat sad.

I totally agree with Comment #9. Just removing the option leaves users with no option to use telepathy-idle. They will probably use other irc clients that have the unverified connection option available.

(In reply to comment #10)
> I totally agree with Comment #9. Just removing the option leaves users with
> no option to use telepathy-idle. They will probably use other irc clients
> that have the unverified connection option available.

We should probably have updated this bug: 0.1.16, released two days after 0.1.15, implements the Telepathy API for interactive certificate verification, so you can use servers with sketchy certificates again!

Release notes for 0.1.16: http://lists.freedesktop.org/archives/telepathy/2013-May/006434.html