Merge r1505, r1512 from trunk:

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.


* src/ne_gnutls.c (provide_client_cert): Set ssl_cc_requested flag
  if no client cert is provided.
  (ne__negotiate_ssl): Give useful error message if handshake fails
  and ssl_cc_requested flag is set.


git-svn-id: http://svn.webdav.org/repos/projects/neon/branches/0.28.x@1516 61a7d7f5-40b7-0310-9c16-bb0ea8cb1845

6 files changed, 99 insertions, 64 deletions

diff --git a/BUGS b/BUGS
index 734cbe8..761d643 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_gnutls.c b/src/ne_gnutls.c
index 1f1be93..3e0495f 100644
--- a/src/ne_gnutls.c
+++ b/src/ne_gnutls.c
@@ -325,27 +325,6 @@ void ne_ssl_cert_validity_time(const ne_ssl_certificate *cert,
     }
 }
 
-/* Return non-zero if hostname from certificate (cn) matches hostname
- * used for session (hostname).  (Wildcard matching is no longer
- * mandated by RFC3280, but certs are deployed which use wildcards) */
-static int match_hostname(char *cn, const char *hostname)
-{
-    const char *dot;
-    dot = strchr(hostname, '.');
-    if (dot == NULL) {
-	char *pnt = strchr(cn, '.');
-	/* hostname is not fully-qualified; unqualify the cn. */
-	if (pnt != NULL) {
-	    *pnt = '\0';
-	}
-    }
-    else if (strncmp(cn, "*.", 2) == 0) {
-	hostname = dot + 1;
-	cn += 2;
-    }
-    return !ne_strcasecmp(cn, hostname);
-}
-
 /* Check certificate identity.  Returns zero if identity matches; 1 if
  * identity does not match, or <0 if the certificate had no identity.
  * If 'identity' is non-NULL, store the malloc-allocated identity in
@@ -371,7 +350,7 @@ static int check_identity(const ne_uri *server, gnutls_x509_crt cert,
         case GNUTLS_SAN_DNSNAME:
             name[len] = '\0';
             if (identity && !found) *identity = ne_strdup(name);
-            match = match_hostname(name, hostname);
+            match = ne__ssl_match_hostname(name, hostname);
             found = 1;
             break;
         case GNUTLS_SAN_IPADDRESS: {
@@ -440,7 +419,7 @@ static int check_identity(const ne_uri *server, gnutls_x509_crt cert,
                                                 seq, 0, name, &len);
             if (ret == 0) {
                 if (identity) *identity = ne_strdup(name);
-                match = match_hostname(name, hostname);
+                match = ne__ssl_match_hostname(name, hostname);
             }
         } else {
             return -1;
@@ -575,6 +554,7 @@ static int provide_client_cert(gnutls_session session,
         }
     } else {
         NE_DEBUG(NE_DBG_SSL, "No client certificate supplied.\n");
+        sess->ssl_cc_requested = 1;
         return GNUTLS_E_NO_CERTIFICATE_FOUND;
     }
 
@@ -733,9 +713,16 @@ int ne__negotiate_ssl(ne_session *sess)
         sess->flags[NE_SESSFLAG_TLS_SNI] ? sess->server.hostname : NULL;
 
     if (ne_sock_connect_ssl(sess->socket, ctx, sess)) {
-	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;
     }
 
     sock = ne__sock_sslsock(sess->socket);
diff --git a/src/ne_openssl.c b/src/ne_openssl.c
index 31b3e48..a69a240 100644
--- a/src/ne_openssl.c
+++ b/src/ne_openssl.c
@@ -212,29 +212,6 @@ void ne_ssl_cert_validity_time(const ne_ssl_certificate *cert,
     }
 }
 
-/* Return non-zero if hostname from certificate (cn) matches hostname
- * used for session (hostname).  Doesn't implement complete RFC 2818
- * logic; omits "f*.example.com" support for simplicity. */
-static int match_hostname(char *cn, const char *hostname)
-{
-    const char *dot;
-
-    dot = strchr(hostname, '.');
-    if (dot == NULL) {
-	char *pnt = strchr(cn, '.');
-	/* hostname is not fully-qualified; unqualify the cn. */
-	if (pnt != NULL) {
-	    *pnt = '\0';
-	}
-    }
-    else if (strncmp(cn, "*.", 2) == 0) {
-	hostname = dot + 1;
-	cn += 2;
-    }
-
-    return !ne_strcasecmp(cn, hostname);
-}
-
 /* Check certificate identity.  Returns zero if identity matches; 1 if
  * identity does not match, or <0 if the certificate had no identity.
  * If 'identity' is non-NULL, store the malloc-allocated identity in
@@ -259,7 +236,7 @@ static int check_identity(const ne_uri *server, X509 *cert, char **identity)
 	    if (nm->type == GEN_DNS) {
 		char *name = dup_ia5string(nm->d.ia5);
                 if (identity && !found) *identity = ne_strdup(name);
-		match = match_hostname(name, hostname);
+		match = ne__ssl_match_hostname(name, hostname);
 		ne_free(name);
 		found = 1;
             } 
@@ -343,7 +320,7 @@ static int check_identity(const ne_uri *server, X509 *cert, char **identity)
             return -1;
         }
         if (identity) *identity = ne_strdup(cname->data);
-        match = match_hostname(cname->data, hostname);
+        match = ne__ssl_match_hostname(cname->data, hostname);
         ne_buffer_destroy(cname);
     }
 
@@ -526,6 +503,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 +607,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..9ebfa6b 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: */
@@ -124,4 +127,9 @@ int ne__negotiate_ssl(ne_session *sess);
 /* Set the session error appropriate for SSL verification failures. */
 void ne__ssl_set_verify_err(ne_session *sess, int failures);
 
+/* Return non-zero if hostname from certificate (cn) matches hostname
+ * used for session (hostname); follows RFC2818 logic.  cn is modified
+ * in-place. */
+int ne__ssl_match_hostname(char *cn, const char *hostname);
+
 #endif /* HTTP_PRIVATE_H */
diff --git a/src/ne_session.c b/src/ne_session.c
index f286606..9630ea9 100644
--- a/src/ne_session.c
+++ b/src/ne_session.c
@@ -400,7 +400,30 @@ void ne__ssl_set_verify_err(ne_session *sess, int failures)
 	}
     }
 }
-#endif
+
+/* This doesn't actually implement complete RFC 2818 logic; omits
+ * "f*.example.com" support for simplicity. */
+int ne__ssl_match_hostname(char *cn, const char *hostname)
+{
+    const char *dot;
+
+    dot = strchr(hostname, '.');
+    if (dot == NULL) {
+	char *pnt = strchr(cn, '.');
+	/* hostname is not fully-qualified; unqualify the cn. */
+	if (pnt != NULL) {
+	    *pnt = '\0';
+	}
+    }
+    else if (strncmp(cn, "*.", 2) == 0) {
+	hostname = dot + 1;
+	cn += 2;
+    }
+
+    return !ne_strcasecmp(cn, hostname);
+}
+
+#endif /* NE_HAVE_SSL */
 
 typedef void (*void_fn)(void);
 
diff --git a/test/ssl.c b/test/ssl.c
index d104750..649ff51 100644
--- a/test/ssl.c
+++ b/test/ssl.c
@@ -73,6 +73,7 @@ struct ssl_server_args {
     int require_cc; /* require a client cert if non-NULL */
     const char *ca_list; /* file of CA certs to verify client cert against */
     const char *send_ca; /* file of CA certs to send in client cert request */
+    int fail_silently; /* exit with success if handshake fails */
     
     /* session caching: */
     int cache; /* use the session cache if non-zero */
@@ -115,8 +116,11 @@ static int ssl_server(ne_socket *sock, void *userdata)
     ne_ssl_context_set_verify(ctx, args->require_cc, args->send_ca,
                               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++;
 
@@ -971,6 +975,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; 
 
@@ -1617,6 +1653,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),