From Mon Sep 17 00:00:00 2001
From: Matt McCutchen <matt@mattmccutchen.net>
Date: Tue, 5 Jan 2010 23:46:03 -0500

Change the Camel certdb to look up certificates by expected hostname (bug 606181).

This way, accepting a bad certificate for one mail server does not give
it a free pass to impersonate the user's other mail servers.  Storing a
second bad certificate for the same server will replace the first, but
that should be OK (Mozilla PSM works the same way).

The camel-cert.db format is unchanged except that it can now contain
multiple entries for the same certificate with different hostnames, and
if it contains multiple certificates for the same hostname, all but the
last will be dropped (becoming permanent the next time the certdb is
saved).

Users who were taking advantage of evolution-data-server's previous,
vulenrable behavior of accepting a certificate for a hostname other than
the originally user-approved one will get bad certificate dialogs and
will need to re-approve the certificate for the desired hostname(s).
---
 camel/camel-certdb.c         |   36 +++++++++++++++++++++++++-----------
 camel/camel-certdb.h         |   13 ++++++++++---
 camel/camel-tcp-stream-ssl.c |   41 +++++++++++++++++++++++++----------------
 3 files changed, 60 insertions(+), 30 deletions(-)

diff --git a/camel/camel-certdb.c b/camel/camel-certdb.c
index cce0fd6..480c65d 100644
--- a/camel/camel-certdb.c
+++ b/camel/camel-certdb.c
@@ -265,7 +265,13 @@ camel_certdb_load (CamelCertDB *certdb)
 		if (cert == NULL)
 			goto error;
 
-		camel_certdb_add (certdb, cert);
+		/* NOTE: If we are upgrading from an evolution-data-server version
+		 * prior to the change to look up certs by hostname (bug 606181),
+		 * this "put" will result in duplicate entries for the same
+		 * hostname being dropped.  The change will become permanent on
+		 * disk the next time the certdb is dirtied for some reason and
+		 * has to be saved. */
+		camel_certdb_put (certdb, cert);
 	}
 
 	CAMEL_CERTDB_UNLOCK (certdb, io_lock);
@@ -411,7 +417,7 @@ camel_certdb_touch (CamelCertDB *certdb)
 }
 
 CamelCert *
-camel_certdb_get_cert (CamelCertDB *certdb, const gchar *fingerprint)
+camel_certdb_get_host (CamelCertDB *certdb, const gchar *hostname)
 {
 	CamelCert *cert;
 
@@ -419,7 +425,7 @@ camel_certdb_get_cert (CamelCertDB *certdb, const gchar *fingerprint)
 
 	CAMEL_CERTDB_LOCK (certdb, db_lock);
 
-	cert = g_hash_table_lookup (certdb->cert_hash, fingerprint);
+	cert = g_hash_table_lookup (certdb->cert_hash, hostname);
 	if (cert)
 		camel_certdb_cert_ref (certdb, cert);
 
@@ -429,20 +435,25 @@ camel_certdb_get_cert (CamelCertDB *certdb, const gchar *fingerprint)
 }
 
 void
-camel_certdb_add (CamelCertDB *certdb, CamelCert *cert)
+camel_certdb_put (CamelCertDB *certdb, CamelCert *cert)
 {
+	CamelCert *old_cert;
+
 	g_return_if_fail (CAMEL_IS_CERTDB (certdb));
 
 	CAMEL_CERTDB_LOCK (certdb, db_lock);
 
-	if (g_hash_table_lookup (certdb->cert_hash, cert->fingerprint)) {
-		CAMEL_CERTDB_UNLOCK (certdb, db_lock);
-		return;
+	/* Replace an existing entry with the same hostname. */
+	old_cert = g_hash_table_lookup (certdb->cert_hash, cert->hostname);
+	if (old_cert) {
+		g_hash_table_remove (certdb->cert_hash, cert->hostname);
+		g_ptr_array_remove (certdb->certs, old_cert);
+		camel_certdb_cert_unref (certdb, old_cert);
 	}
 
 	camel_certdb_cert_ref (certdb, cert);
 	g_ptr_array_add (certdb->certs, cert);
-	g_hash_table_insert (certdb->cert_hash, cert->fingerprint, cert);
+	g_hash_table_insert (certdb->cert_hash, cert->hostname, cert);
 
 	certdb->flags |= CAMEL_CERTDB_DIRTY;
 
@@ -450,14 +461,17 @@ camel_certdb_add (CamelCertDB *certdb, CamelCert *cert)
 }
 
 void
-camel_certdb_remove (CamelCertDB *certdb, CamelCert *cert)
+camel_certdb_remove_host (CamelCertDB *certdb, const gchar *hostname)
 {
+	CamelCert *cert;
+
 	g_return_if_fail (CAMEL_IS_CERTDB (certdb));
 
 	CAMEL_CERTDB_LOCK (certdb, db_lock);
 
-	if (g_hash_table_lookup (certdb->cert_hash, cert->fingerprint)) {
-		g_hash_table_remove (certdb->cert_hash, cert->fingerprint);
+	cert = g_hash_table_lookup (certdb->cert_hash, hostname);
+	if (cert) {
+		g_hash_table_remove (certdb->cert_hash, hostname);
 		g_ptr_array_remove (certdb->certs, cert);
 		camel_certdb_cert_unref (certdb, cert);
 
diff --git a/camel/camel-certdb.h b/camel/camel-certdb.h
index 21b6168..9c5e055 100644
--- a/camel/camel-certdb.h
+++ b/camel/camel-certdb.h
@@ -114,10 +114,17 @@ gint camel_certdb_save (CamelCertDB *certdb);
 
 void camel_certdb_touch (CamelCertDB *certdb);
 
-CamelCert *camel_certdb_get_cert (CamelCertDB *certdb, const gchar *fingerprint);
+/* The lookup key was changed from fingerprint to hostname to fix bug 606181. */
 
-void camel_certdb_add (CamelCertDB *certdb, CamelCert *cert);
-void camel_certdb_remove (CamelCertDB *certdb, CamelCert *cert);
+/* Get the certificate for the given hostname, if any. */
+CamelCert *camel_certdb_get_host (CamelCertDB *certdb, const gchar *hostname);
+
+/* Store cert for cert->hostname, replacing any existing certificate for the
+ * same hostname. */
+void camel_certdb_put (CamelCertDB *certdb, CamelCert *cert);
+
+/* Remove any user-accepted certificate for the given hostname. */
+void camel_certdb_remove_host (CamelCertDB *certdb, const gchar *hostname);
 
 CamelCert *camel_certdb_cert_new (CamelCertDB *certdb);
 void camel_certdb_cert_ref (CamelCertDB *certdb, CamelCert *cert);
diff --git a/camel/camel-tcp-stream-ssl.c b/camel/camel-tcp-stream-ssl.c
index 62290c4..fe8a048 100644
--- a/camel/camel-tcp-stream-ssl.c
+++ b/camel/camel-tcp-stream-ssl.c
@@ -640,8 +640,8 @@ ssl_auth_cert (gpointer data, PRFileDesc *sockfd, PRBool checksig, PRBool is_ser
 }
 #endif
 
-CamelCert *camel_certdb_nss_cert_get(CamelCertDB *certdb, CERTCertificate *cert);
-CamelCert *camel_certdb_nss_cert_add(CamelCertDB *certdb, CERTCertificate *cert);
+CamelCert *camel_certdb_nss_cert_get(CamelCertDB *certdb, CERTCertificate *cert, const gchar *hostname);
+CamelCert *camel_certdb_nss_cert_convert(CamelCertDB *certdb, CERTCertificate *cert);
 void camel_certdb_nss_cert_set(CamelCertDB *certdb, CamelCert *ccert, CERTCertificate *cert);
 
 static gchar *
@@ -684,16 +684,20 @@ cert_fingerprint(CERTCertificate *cert)
 
 /* lookup a cert uses fingerprint to index an on-disk file */
 CamelCert *
-camel_certdb_nss_cert_get(CamelCertDB *certdb, CERTCertificate *cert)
+camel_certdb_nss_cert_get(CamelCertDB *certdb, CERTCertificate *cert, const gchar *hostname)
 {
 	gchar *fingerprint;
 	CamelCert *ccert;
 
+	ccert = camel_certdb_get_host (certdb, hostname);
+	if (ccert == NULL)
+		return NULL;
+
 	fingerprint = cert_fingerprint (cert);
-	ccert = camel_certdb_get_cert (certdb, fingerprint);
-	if (ccert == NULL) {
+	if (strcmp(fingerprint, ccert->fingerprint) != 0) {
+		/* The saved certificate is not the one we wanted. */
 		g_free (fingerprint);
-		return ccert;
+		return NULL;
 	}
 
 	if (ccert->rawcert == NULL) {
@@ -740,9 +744,9 @@ camel_certdb_nss_cert_get(CamelCertDB *certdb, CERTCertificate *cert)
 	return ccert;
 }
 
-/* add a cert to the certdb */
+/* Create a CamelCert corresponding to the NSS cert, with unknown trust. */
 CamelCert *
-camel_certdb_nss_cert_add(CamelCertDB *certdb, CERTCertificate *cert)
+camel_certdb_nss_cert_convert(CamelCertDB *certdb, CERTCertificate *cert)
 {
 	CamelCert *ccert;
 	gchar *fingerprint;
@@ -758,10 +762,6 @@ camel_certdb_nss_cert_add(CamelCertDB *certdb, CERTCertificate *cert)
 	camel_cert_set_trust(certdb, ccert, CAMEL_CERT_TRUST_UNKNOWN);
 	g_free(fingerprint);
 
-	camel_certdb_nss_cert_set(certdb, ccert, cert);
-
-	camel_certdb_add(certdb, ccert);
-
 	return ccert;
 }
 
@@ -843,6 +843,7 @@ ssl_bad_cert (gpointer data, PRFileDesc *sockfd)
 	gboolean accept;
 	CamelCertDB *certdb = NULL;
 	CamelCert *ccert = NULL;
+	gboolean ccert_is_new = FALSE;
 	gchar *prompt, *cert_str, *fingerprint;
 	CamelTcpStreamSSL *ssl;
 	CERTCertificate *cert;
@@ -858,10 +859,14 @@ ssl_bad_cert (gpointer data, PRFileDesc *sockfd)
 		return SECFailure;
 
 	certdb = camel_certdb_get_default();
-	ccert = camel_certdb_nss_cert_get(certdb, cert);
+	ccert = camel_certdb_nss_cert_get(certdb, cert, ssl->priv->expected_host);
 	if (ccert == NULL) {
-		ccert = camel_certdb_nss_cert_add(certdb, cert);
+		ccert = camel_certdb_nss_cert_convert(certdb, cert);
 		camel_cert_set_hostname(certdb, ccert, ssl->priv->expected_host);
+		/* Don't put in the certdb yet.  Since we can only store one
+		 * entry per hostname, we'd rather not ruin any existing entry
+		 * for this hostname if the user rejects the new certificate. */
+		ccert_is_new = TRUE;
 	}
 
 	if (ccert->trust == CAMEL_CERT_TRUST_UNKNOWN) {
@@ -885,9 +890,13 @@ ssl_bad_cert (gpointer data, PRFileDesc *sockfd)
 		accept = camel_session_alert_user (ssl->priv->session, CAMEL_SESSION_ALERT_WARNING, prompt, TRUE);
 		g_free(prompt);
 		if (accept) {
-			camel_certdb_nss_cert_set(certdb, ccert, cert);
 			camel_cert_set_trust(certdb, ccert, CAMEL_CERT_TRUST_FULLY);
-			camel_certdb_touch(certdb);
+			if (ccert_is_new) {
+				camel_certdb_nss_cert_set(certdb, ccert, cert);
+				camel_certdb_put(certdb, ccert);
+			} else {
+				camel_certdb_touch(certdb);
+			}
 		}
 	} else {
 		accept = ccert->trust != CAMEL_CERT_TRUST_NEVER;
