/pidgin/main: e256e8133f37: Manual merge changes from the releas...

Changeset: e256e8133f37c7573dee9e93df3b3f139cc9ce58
Author:	 Mark Doliner <[email protected]>
Date:	 2014-07-07 23:08 -0700
Branch:	 default
URL: https://hg.pidgin.im/pidgin/main/rev/e256e8133f37

Description:

Manual merge changes from the release-2.x.y branch into default.

- Easy change in certificate.h (conflict with new doc style).
- Easy change in ssl-gnutls.c (keep my huge debug statement and comment
  from 2.x.y)
- I didn't copy any changes from de.po. I always assume translation changes
  should not be propagated between the release-2.x.y branch and default.
  Translators are responsible for updating both of their translation files
  as desired.
- I completely dropped datallah's change to certificate.c from
  https://hg.pidgin.im/pidgin/main/rev/2948449ffd12
  The purple_certificate_display_x509() function no longer exists in default.
  It was changed in https://hg.pidgin.im/pidgin/main/rev/d65261e7426e
  and then removed in https://hg.pidgin.im/pidgin/main/rev/cf4c54e487bc
  I might check whether we should make datallah's change in the new code in
  default.

diffstat:

 ChangeLog                          |   3 ++
 libpurple/certificate.h            |   3 +-
 libpurple/plugins/ssl/ssl-gnutls.c |  52 ++++++++++++++++++++++++++++++++++++-
 libpurple/protocols/jabber/iq.c    |  12 +++++++-
 4 files changed, 65 insertions(+), 5 deletions(-)

diffs (129 lines):

diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -90,6 +90,9 @@ version 2.10.10 (?/?/?):
 	XMPP:
 	* Fix Facebook XMPP roster quirks. (#15041, #15957)
 
+	Yahoo:
+	* Fix login when using the GnuTLS library for TLS connections. (#16172)
+
 version 2.10.9 (2/2/2014):
 	XMPP:
 	* Fix problems logging into some servers including jabber.org and
diff --git a/libpurple/certificate.h b/libpurple/certificate.h
--- a/libpurple/certificate.h
+++ b/libpurple/certificate.h
@@ -697,7 +697,8 @@ purple_certificate_pool_contains(PurpleC
  *
  * Retrieve a certificate from a pool.
  *
- * Returns: Retrieved certificate, or NULL if it wasn't there
+ * Returns: Retrieved certificate (to be freed with
+ *         purple_certificate_destroy), or NULL if it wasn't there
  */
 PurpleCertificate *
 purple_certificate_pool_retrieve(PurpleCertificatePool *pool, const gchar *id);
diff --git a/libpurple/plugins/ssl/ssl-gnutls.c b/libpurple/plugins/ssl/ssl-gnutls.c
--- a/libpurple/plugins/ssl/ssl-gnutls.c
+++ b/libpurple/plugins/ssl/ssl-gnutls.c
@@ -473,11 +473,59 @@ ssl_gnutls_read(PurpleSslConnection *gsc
 	if(s == GNUTLS_E_AGAIN || s == GNUTLS_E_INTERRUPTED) {
 		s = -1;
 		errno = EAGAIN;
+
 #ifdef GNUTLS_E_PREMATURE_TERMINATION
 	} else if (s == GNUTLS_E_PREMATURE_TERMINATION) {
-		purple_debug_warning("gnutls", "premature termination\n");
+		purple_debug_warning("gnutls", "Received a FIN on the TCP socket "
+				"for %s. This either means that the remote server closed "
+				"the socket without sending us a Close Notify alert or a "
+				"man-in-the-middle injected a FIN into the TCP stream. "
+				"Assuming it's the former.\n", gsc->host);
+#else
+	} else if (s == GNUTLS_E_UNEXPECTED_PACKET_LENGTH) {
+		purple_debug_warning("gnutls", "Received packet of unexpected "
+				"length on the TCP socket for %s. Among other "
+				"possibilities this might mean that the remote server "
+				"closed the socket without sending us a Close Notify alert. "
+				"Assuming that's the case for compatibility, however, note "
+				"that it's quite possible that we're incorrectly ignoing "
+				"a real error.\n", gsc->host);
+#endif
+		/*
+		 * Summary:
+		 * Always treat a closed TCP connection as if the remote server cleanly
+		 * terminated the SSL session.
+		 *
+		 * Background:
+		 * Most TLS servers send a Close Notify alert before sending TCP FIN
+		 * when closing a session. This informs us at the TLS layer that the
+		 * connection is being cleanly closed. Without this it's more
+		 * difficult for us to determine whether the session was closed
+		 * cleanly (we would need to resort to having the application layer
+		 * perform this check, e.g. by looking at the Content-Length HTTP
+		 * header for HTTP connections).
+		 *
+		 * There ARE servers that don't send Close Notify and we want to be
+		 * compatible with them. And so we don't require Close Notify. This
+		 * seems to match the behavior of libnss. This is a slightly
+		 * unfortunate situation. It means a malicious MITM can inject a FIN
+		 * into our TCP stream and cause our encrypted session to termiate
+		 * and we won't indicate any problem to the user.
+		 *
+		 * GnuTLS < 3.0.0 returned the UNEXPECTED_PACKET_LENGTH error on EOF.
+		 * GnuTLS >= 3.0.0 added the PREMATURE_TERMINATION error to allow us
+		 * to detect the problem more specifically.
+		 *
+		 * For historical discussion see:
+		 * https://developer.pidgin.im/ticket/16172
+		 * http://trac.adiumx.com/intertrac/ticket%3A16678
+		 * https://bugzilla.mozilla.org/show_bug.cgi?id=508698#c4
+		 * http://lists.gnu.org/archive/html/gnutls-devel/2008-03/msg00058.html
+		 * Or search for GNUTLS_E_UNEXPECTED_PACKET_LENGTH or
+		 * GNUTLS_E_PREMATURE_TERMINATION
+		 */
 		s = 0;
-#endif
+
 	} else if(s < 0) {
 		purple_debug_error("gnutls", "receive failed: %s\n",
 				gnutls_strerror(s));
diff --git a/libpurple/protocols/jabber/iq.c b/libpurple/protocols/jabber/iq.c
--- a/libpurple/protocols/jabber/iq.c
+++ b/libpurple/protocols/jabber/iq.c
@@ -290,17 +290,19 @@ void jabber_iq_remove_callback_by_id(Jab
  * be a valid match if any of the following is true:
  * - Request 'to' matches reply 'from' (including the case where
  *   neither are set).
- * - Request 'to' was empty and reply 'from' is server JID.
+ * - Request 'to' was my JID (bare or full) and reply 'from' is empty.
  * - Request 'to' was empty and reply 'from' is my JID. The spec says
  *   we should only allow bare JID, but we also allow full JID for
  *   compatibility with some servers.
+ * - Request 'to' was empty and reply 'from' is server JID. Not allowed by
+ *   any spec, but for compatibility with some servers.
  *
  * These rules should allow valid IQ replies while preventing spoofed
  * ones.
  *
  * For more discussion see the "Spoofing of iq ids and misbehaving
  * servers" email thread from January 2014 on the jdev and security
- * mailing lists.
+ * mailing lists. Also see https://developer.pidgin.im/ticket/15879
  *
  * @return TRUE if this reply is valid for the given request.
  */
@@ -311,6 +313,12 @@ static gboolean does_reply_from_match_re
 		return TRUE;
 	}
 
+	if (!from && purple_strequal(to->node, js->user->node)
+			&& purple_strequal(to->domain, js->user->domain)) {
+		/* Request 'to' was my JID (bare or full) and reply 'from' is empty */
+		return TRUE;
+	}
+
 	if (!to && purple_strequal(from->domain, js->user->domain)) {
 		/* Request 'to' is empty and reply 'from' domain matches our domain */