Fail with a useful error message in the case where a client cert is

requested during handshake, none can be provided, and the handshake
fails:

* src/ne_private.h (struct ne_session_s): Add ssl_cc_requested field.

* src/ne_openssl.c (provide_client_cert): Set ssl_cc_requested if
  no cert is provided.
  (ne__negotiate_ssl): Clear ssl_cc_requested before handshake.
  Use different, more useful error message if handshake fails and flag
  is now set.

* test/ssl.c (struct ssl_server_args): Add fail_silently flag.
  (ssl_server): Exit with success if handshake fails and above flag set.
  (no_client_cert): New test case.


git-svn-id: http://svn.webdav.org/repos/projects/neon/trunk@1505 61a7d7f5-40b7-0310-9c16-bb0ea8cb1845

4 files changed, 55 insertions, 12 deletions

diff --git a/BUGS b/BUGS
index f19adfc..68f1065 100644
--- a/BUGS
+++ b/BUGS
@@ -18,13 +18,6 @@ Known problems/bugs in neon                                      -*- text -*-
  only cache on shutdown, since the SSL_SESSION may change during
  an ne_session?
 
-* It would be nice to fail with a friendly error message if a client
-cert is requested by the srever but one is not provided.  Currently,
-returning -1 from the provide_client_cert function would allow that
-(as it forces the SSL handshake to fail), but that would prevent
-opportunistic use of client certificates, of the "SSLVerifyClient
-optional" variety.
-
 * perhaps allow a per-Server-header hack for "Darwin Streaming Server
 4.0" which doesn't terminate the response headers:
 http://bugzilla.gnome.org/show_bug.cgi?id=366331
diff --git a/src/ne_openssl.c b/src/ne_openssl.c
index 5b63bd5..5785373 100644
--- a/src/ne_openssl.c
+++ b/src/ne_openssl.c
@@ -526,6 +526,7 @@ static int provide_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey)
 	*pkey = cc->pkey;
 	return 1;
     } else {
+        sess->ssl_cc_requested = 1;
 	NE_DEBUG(NE_DBG_SSL, "No client certificate supplied.\n");
 	return 0;
     }
@@ -629,15 +630,24 @@ int ne__negotiate_ssl(ne_session *sess)
     ctx->hostname = 
         sess->flags[NE_SESSFLAG_TLS_SNI] ? sess->server.hostname : NULL;
 
+    sess->ssl_cc_requested = 0;
+
     if (ne_sock_connect_ssl(sess->socket, ctx, sess)) {
 	if (ctx->sess) {
 	    /* remove cached session. */
 	    SSL_SESSION_free(ctx->sess);
 	    ctx->sess = NULL;
 	}
-	ne_set_error(sess, _("SSL negotiation failed: %s"),
-		     ne_sock_error(sess->socket));
-	return NE_ERROR;
+        if (sess->ssl_cc_requested) {
+            ne_set_error(sess, _("SSL negotiation failed, "
+                                 "client certificate was requested: %s"),
+                         ne_sock_error(sess->socket));
+        }
+        else {
+            ne_set_error(sess, _("SSL negotiation failed: %s"),
+                         ne_sock_error(sess->socket));
+        }
+        return NE_ERROR;
     }	
     
     ssl = ne__sock_sslsock(sess->socket);
diff --git a/src/ne_private.h b/src/ne_private.h
index 5124cea..55f09e0 100644
--- a/src/ne_private.h
+++ b/src/ne_private.h
@@ -99,6 +99,9 @@ struct ne_session_s {
     ne_ssl_client_cert *client_cert;
     ne_ssl_certificate *server_cert;
     ne_ssl_context *ssl_context;
+    int ssl_cc_requested; /* set to non-zero if a client cert was
+                           * requested during initial handshake, but
+                           * none could be provided. */
 #endif
 
     /* Server cert verification callback: */
diff --git a/test/ssl.c b/test/ssl.c
index 619af1a..ea0ee0b 100644
--- a/test/ssl.c
+++ b/test/ssl.c
@@ -75,6 +75,7 @@ struct ssl_server_args {
     /* client cert handling: */
     int require_cc; /* require a client cert if non-NULL */
     const char *ca_list; /* file of CA certs to verify client cert against */
+    int fail_silently; /* exit with success if handshake fails */
     
     /* session caching: */
     int cache; /* use the session cache if non-zero */
@@ -117,8 +118,11 @@ static int ssl_server(ne_socket *sock, void *userdata)
     ne_ssl_context_set_verify(ctx, args->require_cc, 
                               args->ca_list, args->ca_list);
 
-    ONV(ne_sock_accept_ssl(sock, ctx),
-        ("SSL accept failed: %s", ne_sock_error(sock)));
+    ret = ne_sock_accept_ssl(sock, ctx);
+    if (ret && args->fail_silently) {
+        return 0;
+    }
+    ONV(ret, ("SSL accept failed: %s", ne_sock_error(sock)));
 
     args->count++;
 
@@ -997,6 +1001,38 @@ static int ccert_unencrypted(void)
     return OK;
 }
 
+#define NOCERT_MESSAGE "client certificate was requested"
+
+/* Tests for useful error message if a handshake fails where a client
+ * cert was requested. */
+static int no_client_cert(void)
+{
+    ne_session *sess = DEFSESS;
+    struct ssl_server_args args = {SERVER_CERT, NULL};
+    int ret;
+
+    args.require_cc = 1;
+    args.fail_silently = 1;
+
+    ne_ssl_trust_cert(sess, def_ca_cert);
+
+    CALL(spawn_server(7777, ssl_server, &args));
+    
+    ret = any_request(sess, "/failme");
+
+    ONV(ret != NE_ERROR,
+        ("unexpected result %d: %s", ret, ne_get_error(sess)));
+
+    ONV(strstr(ne_get_error(sess), NOCERT_MESSAGE) == NULL,
+        ("error message was '%s', missing '%s'", 
+         ne_get_error(sess), NOCERT_MESSAGE));
+    
+    reap_server();
+
+    ne_session_destroy(sess);    
+    return OK;
+}
+
 /* non-zero if a server auth header was received */
 static int got_server_auth; 
 
@@ -1651,6 +1687,7 @@ ne_test tests[] = {
     T(ccert_unencrypted),
     T(client_cert_provided),
     T(cc_provided_dnames),
+    T(no_client_cert),
 
     T(parse_cert),
     T(parse_chain),