/pidgin/main: 72bdcc0f7267: Clean up unused ssl/NSS code and wri...

[Tomasz Wasilczyk]

Changeset: 72bdcc0f726799cc6d4048f4003aa1db1f5efd1c
Author:	 Tomasz Wasilczyk <twasilczyk at pidgin.im>
Date:	 2014-01-03 20:48 +0100
Branch:	 release-2.x.y
URL: https://hg.pidgin.im/pidgin/main/rev/72bdcc0f7267

Description:

Clean up unused ssl/NSS code and write up some comments to resolve any doubts. Refs #15308

diffstat:

 libpurple/plugins/ssl/ssl-nss.c |  96 +++++++++-------------------------------
 1 files changed, 23 insertions(+), 73 deletions(-)

diffs (124 lines):

diff --git a/libpurple/plugins/ssl/ssl-nss.c b/libpurple/plugins/ssl/ssl-nss.c
--- a/libpurple/plugins/ssl/ssl-nss.c
+++ b/libpurple/plugins/ssl/ssl-nss.c
@@ -155,75 +155,25 @@ ssl_nss_init_nss(void)
 }
 
 static SECStatus
-ssl_auth_cert(void *arg, PRFileDesc *socket, PRBool checksig,
-			  PRBool is_server)
+ssl_auth_cert(void *arg, PRFileDesc *socket, PRBool checksig, PRBool is_server)
 {
+	/* We just skip cert verification here, and will verify the whole chain
+	 * in ssl_nss_handshake_cb, after the handshake is complete.
+	 *
+	 * The problem is, purple_certificate_verify is asynchronous and
+	 * ssl_auth_cert should return the result synchronously (it may ask the
+	 * user, if an unknown certificate should be trusted or not).
+	 *
+	 * Ideally, SSL_AuthCertificateHook/ssl_auth_cert should decide
+	 * immediately, if the certificate chain is already trusted and possibly
+	 * SSL_BadCertHook to deal with unknown certificates.
+	 *
+	 * Current implementation may not be ideal, but is no less secure in
+	 * terms of MITM attack.
+	 */
 	return SECSuccess;
-
-#if 0
-	CERTCertificate *cert;
-	void *pinArg;
-	SECStatus status;
-
-	cert = SSL_PeerCertificate(socket);
-	pinArg = SSL_RevealPinArg(socket);
-
-	status = CERT_VerifyCertNow((CERTCertDBHandle *)arg, cert, checksig,
-								certUsageSSLClient, pinArg);
-
-	if (status != SECSuccess) {
-		purple_debug_error("nss", "CERT_VerifyCertNow failed\n");
-		CERT_DestroyCertificate(cert);
-		return status;
-	}
-
-	CERT_DestroyCertificate(cert);
-	return SECSuccess;
-#endif
 }
 
-#if 0
-static SECStatus
-ssl_bad_cert(void *arg, PRFileDesc *socket)
-{
-	SECStatus status = SECFailure;
-	PRErrorCode err;
-
-	if (arg == NULL)
-		return status;
-
-	*(PRErrorCode *)arg = err = PORT_GetError();
-
-	switch (err)
-	{
-		case SEC_ERROR_INVALID_AVA:
-		case SEC_ERROR_INVALID_TIME:
-		case SEC_ERROR_BAD_SIGNATURE:
-		case SEC_ERROR_EXPIRED_CERTIFICATE:
-		case SEC_ERROR_UNKNOWN_ISSUER:
-		case SEC_ERROR_UNTRUSTED_CERT:
-		case SEC_ERROR_CERT_VALID:
-		case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
-		case SEC_ERROR_CRL_EXPIRED:
-		case SEC_ERROR_CRL_BAD_SIGNATURE:
-		case SEC_ERROR_EXTENSION_VALUE_INVALID:
-		case SEC_ERROR_CA_CERT_INVALID:
-		case SEC_ERROR_CERT_USAGES_INVALID:
-		case SEC_ERROR_UNKNOWN_CRITICAL_EXTENSION:
-			status = SECSuccess;
-			break;
-
-		default:
-			status = SECFailure;
-			break;
-	}
-
-	purple_debug_error("nss", "Bad certificate: %d\n", err);
-
-	return status;
-}
-#endif
-
 static gboolean
 ssl_nss_init(void)
 {
@@ -362,7 +312,10 @@ ssl_nss_handshake_cb(gpointer data, int 
 		purple_certificate_destroy_list(peers);
 	} else {
 		/* Otherwise, just call the "connection complete"
-		   callback */
+		 * callback. The verification was already done with
+		 * SSL_AuthCertificate, the default verifier
+		 * (SSL_AuthCertificateHook was not called in ssl_nss_connect).
+		 */
 		gsc->connect_cb(gsc->connect_cb_data, gsc, cond);
 	}
 }
@@ -427,13 +380,10 @@ ssl_nss_connect(PurpleSslConnection *gsc
 	SSL_OptionSet(nss_data->in, SSL_SECURITY,            PR_TRUE);
 	SSL_OptionSet(nss_data->in, SSL_HANDSHAKE_AS_CLIENT, PR_TRUE);
 
-	SSL_AuthCertificateHook(nss_data->in,
-							(SSLAuthCertificate)ssl_auth_cert,
-							(void *)CERT_GetDefaultCertDB());
-#if 0
-	/* No point in hooking BadCert, since ssl_auth_cert always succeeds */
-	SSL_BadCertHook(nss_data->in, (SSLBadCertHandler)ssl_bad_cert, NULL);
-#endif
+	/* If we have our internal verifier set up, use it. Otherwise,
+	 * use default. */
+	if (gsc->verifier != NULL)
+		SSL_AuthCertificateHook(nss_data->in, ssl_auth_cert, NULL);
 
 	if(gsc->host)
 		SSL_SetURL(nss_data->in, gsc->host);