changeset 27999:99baf778e0b9

Fix GnuTLS validation of the CACert Chain. Closes #4458. If certificate validation fails partway through, check the last validated certificate and, if it's in the CA store, consider the chain validated. This allows GnuTLS to validate the CAcert Class 3 intermediate without requiring us to accept MD5 signatures anywhere.
author Paul Aurich <paul@darkrain42.org>
date Wed, 22 Jul 2009 07:31:40 +0000
parents 31905a0d1c9d
children 05331a8eafb3 19d283331b9d
files ChangeLog.API libpurple/certificate.c libpurple/certificate.h
diffstat 3 files changed, 184 insertions(+), 98 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog.API	Wed Jul 22 06:12:13 2009 +0000
+++ b/ChangeLog.API	Wed Jul 22 07:31:40 2009 +0000
@@ -26,6 +26,7 @@
 		* purple_blist_set_ui_data
 		* purple_blist_node_get_ui_data
 		* purple_blist_node_set_ui_data
+		* purple_certificate_check_signature_chain_with_failing
 		* purple_chat_destroy
 		* purple_connection_get_protocol_data
 		* purple_connection_set_protocol_data
@@ -92,6 +93,7 @@
 		* purple_blist_destroy
 		* purple_blist_new
 		* purple_buddy_get_local_alias
+		* purple_certificate_check_signature_chain
 		* purple_ip_address_is_valid
 		* purple_notify_user_info_remove_entry
 		* purple_set_blist
--- a/libpurple/certificate.c	Wed Jul 22 06:12:13 2009 +0000
+++ b/libpurple/certificate.c	Wed Jul 22 07:31:40 2009 +0000
@@ -190,7 +190,8 @@
 }
 
 gboolean
-purple_certificate_check_signature_chain(GList *chain)
+purple_certificate_check_signature_chain_with_failing(GList *chain,
+                                                      PurpleCertificate **failing)
 {
 	GList *cur;
 	PurpleCertificate *crt, *issuer;
@@ -200,6 +201,9 @@
 
 	g_return_val_if_fail(chain, FALSE);
 
+	if (failing)
+		*failing = NULL;
+
 	uid = purple_certificate_get_unique_id((PurpleCertificate *) chain->data);
 	purple_debug_info("certificate",
 			  "Checking signature chain for uid=%s\n",
@@ -239,6 +243,9 @@
 						"...Not-yet-activated issuer %s will be valid at %s\n"
 						"Chain is INVALID\n", uid, ctime(&activation));
 
+			if (failing)
+				*failing = crt;
+
 			g_free(uid);
 			return FALSE;
 		}
@@ -250,6 +257,9 @@
 					  uid);
 			g_free(uid);
 
+			if (failing)
+				*failing = crt;
+
 			return FALSE;
 		}
 
@@ -268,6 +278,12 @@
 	return TRUE;
 }
 
+gboolean
+purple_certificate_check_signature_chain(GList *chain)
+{
+	return purple_certificate_check_signature_chain_with_failing(chain, NULL);
+}
+
 PurpleCertificate *
 purple_certificate_import(PurpleCertificateScheme *scheme, const gchar *filename)
 {
@@ -1312,11 +1328,10 @@
 x509_tls_cached_unknown_peer(PurpleCertificateVerificationRequest *vrq)
 {
 	PurpleCertificatePool *ca, *tls_peers;
-	PurpleCertificate *end_crt, *ca_crt, *peer_crt;
+	PurpleCertificate *peer_crt;
+	PurpleCertificate *failing_crt;
 	GList *chain = vrq->cert_chain;
-	GList *last;
-	gchar *ca_id;
-	GByteArray *last_fingerprint, *ca_fingerprint;
+	gboolean chain_validated = FALSE;
 
 	peer_crt = (PurpleCertificate *) chain->data;
 
@@ -1342,35 +1357,74 @@
 		return;
 	} /* if (self signed) */
 
-	/* Next, check that the certificate chain is valid */
-	if ( ! purple_certificate_check_signature_chain(chain) ) {
-		/* TODO: Tell the user where the chain broke? */
-		/* TODO: This error will hopelessly confuse any
-		   non-elite user. */
-		gchar *secondary;
-
-		secondary = g_strdup_printf(_("The certificate chain presented"
-					      " for %s is not valid."),
-					    vrq->subject_name);
-
-		/* TODO: Make this error either block the ensuing SSL
-		   connection error until the user dismisses this one, or
-		   stifle it. */
-		purple_notify_error(NULL, /* TODO: Probably wrong. */
-				    _("SSL Certificate Error"),
-				    _("Invalid certificate chain"),
-				    secondary );
-		g_free(secondary);
-
-		/* Okay, we're done here */
-		purple_certificate_verify_complete(vrq,
-						   PURPLE_CERTIFICATE_INVALID);
-		return;
-	} /* if (signature chain not good) */
-
 	/* Next, attempt to verify the last certificate against a CA */
 	ca = purple_certificate_find_pool(x509_tls_cached.scheme_name, "ca");
 
+	/* Next, check that the certificate chain is valid */
+	if (purple_certificate_check_signature_chain_with_failing(chain,
+	                                                          &failing_crt))
+		chain_validated = TRUE;
+	else {
+		/*
+		 * Check if the failing certificate is in the CA store. If it is, then
+		 * consider this fully validated. This works around issues with some
+		 * prominent intermediate CAs whose signature is md5WithRSAEncryption.
+		 * I'm looking at CACert Class 3 here. See #4458 for details.
+		 */
+		if (ca) {
+			gchar *uid = purple_certificate_get_unique_id(failing_crt);
+			PurpleCertificate *ca_crt = purple_certificate_pool_retrieve(ca, uid);
+			if (ca_crt != NULL) {
+				GByteArray *failing_fpr;
+				GByteArray *ca_fpr;
+				failing_fpr = purple_certificate_get_fingerprint_sha1(failing_crt);
+				ca_fpr = purple_certificate_get_fingerprint_sha1(ca_crt);
+				if (byte_arrays_equal(failing_fpr, ca_fpr)) {
+					purple_debug_info("certificate/x509/tls_cached",
+							"Full chain verification failed (probably a bad "
+							"signature algorithm), but found the last "
+							"certificate %s in the CA pool.\n", uid);
+					chain_validated = TRUE;
+				}
+
+				g_byte_array_free(failing_fpr, TRUE);
+				g_byte_array_free(ca_fpr, TRUE);
+			}
+
+			purple_certificate_destroy(ca_crt);
+			g_free(uid);
+		}
+
+		/*
+		 * If we get here, either the cert matched the stuff right above
+		 * or it didn't, in which case we give up and complain to the user.
+		 */
+		if (!chain_validated) {
+			/* TODO: Tell the user where the chain broke? */
+			/* TODO: This error will hopelessly confuse any
+			   non-elite user. */
+			gchar *secondary;
+
+			secondary = g_strdup_printf(_("The certificate chain presented"
+						      " for %s is not valid."),
+						    vrq->subject_name);
+
+			/* TODO: Make this error either block the ensuing SSL
+			   connection error until the user dismisses this one, or
+			   stifle it. */
+			purple_notify_error(NULL, /* TODO: Probably wrong. */
+					    _("SSL Certificate Error"),
+					    _("Invalid certificate chain"),
+					    secondary );
+			g_free(secondary);
+
+			/* Okay, we're done here */
+			purple_certificate_verify_complete(vrq,
+							   PURPLE_CERTIFICATE_INVALID);
+			return;
+		}
+	} /* if (signature chain not good) */
+
 	/* If, for whatever reason, there is no Certificate Authority pool
 	   loaded, we will simply present it to the user for checking. */
 	if ( !ca ) {
@@ -1386,82 +1440,88 @@
 		return;
 	}
 
-	last = g_list_last(chain);
-	end_crt = (PurpleCertificate *) last->data;
+	if (!chain_validated) {
+		GByteArray *last_fpr, *ca_fpr;
+		PurpleCertificate *ca_crt, *end_crt;
+		gchar *ca_id;
+
+		end_crt = g_list_last(chain)->data;
 
-	/* Attempt to look up the last certificate's issuer */
-	ca_id = purple_certificate_get_issuer_unique_id(end_crt);
-	purple_debug_info("certificate/x509/tls_cached",
-			  "Checking for a CA with DN=%s\n",
-			  ca_id);
-	ca_crt = purple_certificate_pool_retrieve(ca, ca_id);
-	if ( NULL == ca_crt ) {
-		purple_debug_warning("certificate/x509/tls_cached",
-				  "Certificate Authority with DN='%s' not "
-				  "found. I'll prompt the user, I guess.\n",
+		/* Attempt to look up the last certificate's issuer */
+		ca_id = purple_certificate_get_issuer_unique_id(end_crt);
+		purple_debug_info("certificate/x509/tls_cached",
+				  "Checking for a CA with DN=%s\n",
 				  ca_id);
+		ca_crt = purple_certificate_pool_retrieve(ca, ca_id);
+		if ( NULL == ca_crt ) {
+			purple_debug_warning("certificate/x509/tls_cached",
+					  "Certificate Authority with DN='%s' not "
+					  "found. I'll prompt the user, I guess.\n",
+					  ca_id);
+			g_free(ca_id);
+			/* vrq will be completed by user_auth */
+			x509_tls_cached_user_auth(vrq,_("The root certificate this "
+							"one claims to be issued by "
+							"is unknown to Pidgin."));
+			return;
+		}
+
 		g_free(ca_id);
-		/* vrq will be completed by user_auth */
-		x509_tls_cached_user_auth(vrq,_("The root certificate this "
-						"one claims to be issued by "
-						"is unknown to Pidgin."));
-		return;
-	}
-
-	g_free(ca_id);
 
-	/*
-	 * Check the fingerprints; if they match, then this certificate *is* one
-	 * of the designated "trusted roots", and we don't need to verify the
-	 * signature. This is good because some of the older roots are self-signed
-	 * with bad hash algorithms that we don't want to allow in any other
-	 * circumstances (one of Verisign's root CAs is self-signed with MD2).
-	 *
-	 * If the fingerprints don't match, we'll fall back to checking the
-	 * signature.
-	 *
-	 * GnuTLS doesn't seem to include the final root in the verification
-	 * list, so this check will never succeed.  NSS *does* include it in
-	 * the list, so here we are.
-	 */
-	last_fingerprint = purple_certificate_get_fingerprint_sha1(end_crt);
-	ca_fingerprint   = purple_certificate_get_fingerprint_sha1(ca_crt);
+		/*
+		 * Check the fingerprints; if they match, then this certificate *is* one
+		 * of the designated "trusted roots", and we don't need to verify the
+		 * signature. This is good because some of the older roots are self-signed
+		 * with bad hash algorithms that we don't want to allow in any other
+		 * circumstances (one of Verisign's root CAs is self-signed with MD2).
+		 *
+		 * If the fingerprints don't match, we'll fall back to checking the
+		 * signature.
+		 *
+		 * GnuTLS doesn't seem to include the final root in the verification
+		 * list, so this check will never succeed.  NSS *does* include it in
+		 * the list, so here we are.
+		 */
+		last_fpr = purple_certificate_get_fingerprint_sha1(end_crt);
+		ca_fpr   = purple_certificate_get_fingerprint_sha1(ca_crt);
 
-	if ( !byte_arrays_equal(last_fingerprint, ca_fingerprint) &&
-			!purple_certificate_signed_by(end_crt, ca_crt) )
-	{
-		/* TODO: If signed_by ever returns a reason, maybe mention
-		   that, too. */
-		/* TODO: Also mention the CA involved. While I could do this
-		   now, a full DN is a little much with which to assault the
-		   user's poor, leaky eyes. */
-		/* TODO: This error message makes my eyes cross, and I wrote it */
-		gchar * secondary =
-			g_strdup_printf(_("The certificate chain presented by "
-					  "%s does not have a valid digital "
-					  "signature from the Certificate "
-					  "Authority from which it claims to "
-					  "have a signature."),
-					vrq->subject_name);
+		if ( !byte_arrays_equal(last_fpr, ca_fpr) &&
+				!purple_certificate_signed_by(end_crt, ca_crt) )
+		{
+			/* TODO: If signed_by ever returns a reason, maybe mention
+			   that, too. */
+			/* TODO: Also mention the CA involved. While I could do this
+			   now, a full DN is a little much with which to assault the
+			   user's poor, leaky eyes. */
+			/* TODO: This error message makes my eyes cross, and I wrote it */
+			gchar * secondary =
+				g_strdup_printf(_("The certificate chain presented by "
+						  "%s does not have a valid digital "
+						  "signature from the Certificate "
+						  "Authority from which it claims to "
+						  "have a signature."),
+						vrq->subject_name);
 
-		purple_notify_error(NULL, /* TODO: Probably wrong */
-				    _("SSL Certificate Error"),
-				    _("Invalid certificate authority"
-				      " signature"),
-				    secondary);
-		g_free(secondary);
+			purple_notify_error(NULL, /* TODO: Probably wrong */
+					    _("SSL Certificate Error"),
+					    _("Invalid certificate authority"
+					      " signature"),
+					    secondary);
+			g_free(secondary);
 
-		/* Signal "bad cert" */
-		purple_certificate_verify_complete(vrq,
-						   PURPLE_CERTIFICATE_INVALID);
+			/* Signal "bad cert" */
+			purple_certificate_verify_complete(vrq,
+							   PURPLE_CERTIFICATE_INVALID);
 
-		g_byte_array_free(ca_fingerprint, TRUE);
-		g_byte_array_free(last_fingerprint, TRUE);
-		return;
-	} /* if (CA signature not good) */
+			purple_certificate_destroy(ca_crt);
+			g_byte_array_free(ca_fpr, TRUE);
+			g_byte_array_free(last_fpr, TRUE);
+			return;
+		} /* if (CA signature not good) */
 
-	g_byte_array_free(ca_fingerprint, TRUE);
-	g_byte_array_free(last_fingerprint, TRUE);
+		g_byte_array_free(ca_fpr, TRUE);
+		g_byte_array_free(last_fpr, TRUE);
+	}
 
 	/* Last, check that the hostname matches */
 	if ( ! purple_certificate_check_subject_name(peer_crt,
--- a/libpurple/certificate.h	Wed Jul 22 06:12:13 2009 +0000
+++ b/libpurple/certificate.h	Wed Jul 22 07:31:40 2009 +0000
@@ -443,6 +443,28 @@
 purple_certificate_signed_by(PurpleCertificate *crt, PurpleCertificate *issuer);
 
 /**
+ * Check that a certificate chain is valid and, if not, the failing certificate.
+ *
+ * Uses purple_certificate_signed_by() to verify that each PurpleCertificate
+ * in the chain carries a valid signature from the next. A single-certificate
+ * chain is considered to be valid.
+ *
+ * @param chain      List of PurpleCertificate instances comprising the chain,
+ *                   in the order certificate, issuer, issuer's issuer, etc.
+ * @param failing    A pointer to a PurpleCertificate*. If not NULL, if the
+ *                   chain fails to validate, this will be set to the
+ *                   certificate whose signature could not be validated.
+ * @return TRUE if the chain is valid. See description.
+ *
+ * @since 2.6.0
+ * @deprecated  This function will become
+ *              purple_certificate_check_signature_chain in 3.0.0
+ */
+gboolean
+purple_certificate_check_signature_chain_with_failing(GList *chain,
+		PurpleCertificate **failing);
+
+/**
  * Check that a certificate chain is valid
  *
  * Uses purple_certificate_signed_by() to verify that each PurpleCertificate
@@ -453,6 +475,8 @@
  *                   in the order certificate, issuer, issuer's issuer, etc.
  * @return TRUE if the chain is valid. See description.
  * @todo Specify which certificate in the chain caused a failure
+ * @deprecated  This function will be removed in 3.0.0 and replaced with
+ *              purple_certificate_check_signature_chain_with_failing
  */
 gboolean
 purple_certificate_check_signature_chain(GList *chain);