[Dovecot] mandatory client certificates and crl check in ssl-proxy-openssl.c

[HenkJan Wolthuis]

hello,

I made a modification to ssl-proxy-openssl.c (patch attached) zo that it
a) disconnects when no client certificate is presented
b) checks the client certificate against the crl for our root cert. (so 
you can't use a revoked client cert.)
c) returns the CommonName from the client cert. in 
ssl_proxy_get_peer_name (this way it's easier to use dovecot as 
imap-proxy with a passwd-like userdb, ssl_require_client_cert and 
ssl_username_from_cert, it "binds" the emailuser to the 
clientcertificate, a clientcert. can access only the account from the 
userdb)

in order to use it, the CAfile must be a file which contains the 
CAcertificate (pem format) followed by the CRL (also in pem format). 
(servercert and the clientcerts are signed with a self-signed rootcert)

there are some issues with the patch:
a) it needs openssl > 0.9.7 for the CRL checking
b) ssl_verify_client_cert now returns 0 in case of an invalid cert. was 
there a reason why it always returned 1?
c) i'm not too happy with the commonname extraction code, is it secure??
d) i've no experience with programming openssl or dovecot
e) i haven't programmed in C for at least 8 years......

does anyone here have more issues, corrections, comments on the patch?
can/should this functionality be implemented in dovecot? (conf-file option?)

-- 

groeten,

HenkJan Wolthuis

-------------- next part --------------
--- ssl-proxy-openssl.c.orig	2006-04-04 10:32:58.000000000 +0200
+++ ssl-proxy-openssl.c	2006-05-11 14:08:02.000000000 +0200
@@ -11,6 +11,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/stat.h>
+#include <string.h>
 
 #ifdef HAVE_OPENSSL
 
@@ -508,9 +509,42 @@
 	if (x509 == NULL)
 		return NULL; /* we should have had it.. */
 
+	
+	// ORIG: X509_NAME_oneline(X509_get_subject_name(x509), buf, sizeof(buf));
+	// ORIG: name = t_strndup(buf, sizeof(buf));
+	// ORIG: X509_free(x509);
+	
+	/* HJHJ get CommonName from peer-certificate */
 	X509_NAME_oneline(X509_get_subject_name(x509), buf, sizeof(buf));
-	name = t_strndup(buf, sizeof(buf));
+	
+	char *tmp;
+	char *start;
+	start = buf;
+	tmp = strstr( buf, "CN=" );
+	if( NULL != tmp )
+		{
+		if( strlen(tmp) > 3 ) /* CN= + something */
+			{
+			start = tmp + 3;
+			tmp = strstr( start, "/" );
+			if( NULL != tmp )		/* tmp == NULL: CN is last element in buf */
+				{ *tmp = '\0'; }
+			}
+		else	/* empty CN= */
+			{ 
+			*start = '\0'; 
+			i_warning("get_peer_name empty CN=");
+			}
+		}
+	else
+		{ 
+		*start = '\0'; 
+		i_warning("get_peer_name NO CN=");
+		} /* CN= not found (no certificate or certificate without commonname??) */
+		
+	name = t_strndup(start, strlen(start) ); 
 	X509_free(x509);
+	/* HJHJ */
 
 	return *name == '\0' ? NULL : name;
 }
@@ -582,10 +616,26 @@
 	proxy = SSL_get_ex_data(ssl, extdata_index);
 
 	proxy->cert_received = TRUE;
+	// ORIG: if (!preverify_ok)
+	// ORIG: 	proxy->cert_broken = TRUE;
+
+	// ORIG: return 1;
+	
+	/* HJHJ */
+	char buf[1024];
+	X509_NAME_oneline( X509_get_subject_name(ctx->current_cert),buf,sizeof(buf));
+
 	if (!preverify_ok)
+		{
 		proxy->cert_broken = TRUE;
+		i_warning("BAD CERT %s: %s",X509_verify_cert_error_string(ctx->error),buf);
+		}
+	else
+		{ i_warning("CERT: %s",buf); }
 
-	return 1;
+	return preverify_ok;
+	/* HJHJ */
+	
 }
 
 static int
@@ -666,10 +716,20 @@
 
 	if (getenv("SSL_VERIFY_CLIENT_CERT") != NULL) {
 		SSL_CTX_set_verify(ssl_ctx, SSL_VERIFY_PEER |
-				   SSL_VERIFY_CLIENT_ONCE,
+				   SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
 				   ssl_verify_client_cert);
 	}
 
+	/* HJHJ */
+#if OPENSSL_VERSION_NUMBER >= 0x00907000L
+	X509_STORE *store;
+	if( (store=SSL_CTX_get_cert_store(ssl_ctx)) != NULL )
+		{ X509_STORE_set_flags( store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); }
+	else
+		{ i_warning("X509 get cert store failed..."); }	
+#endif
+	/* HJHJ */
+
 	/* PRNG initialization might want to use /dev/urandom, make sure it
 	   does it before chrooting. We might not have enough entropy at
 	   the first try, so this function may fail. It's still been