Allow specific libtls hostname validation errors to propagate.

Remove direct calls to printf from the tls_check_hostname() path. This allows
NUL byte error messages to bubble up to the caller, to be logged in a
program-appropriate way. It also removes non-portable calls to getprogname().

The semantics of tls_error() are changed slightly: the last error message is
not necessarily preserved between subsequent calls into the library.
When the previous call to libtls succeeds, client programs should treat the
return value of tls_error() as undefined.

ok tedu@

4 files changed, 36 insertions, 27 deletions

diff --git a/src/lib/libtls/tls.c b/src/lib/libtls/tls.c
index a7f612e40b..d3bb79b3fe 100644
--- a/src/lib/libtls/tls.c
+++ b/src/lib/libtls/tls.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls.c,v 1.1 2014/10/31 13:46:17 jsing Exp $ */
+/* $OpenBSD: tls.c,v 1.2 2014/12/07 15:00:32 bcook Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -56,15 +56,22 @@ tls_error(struct tls *ctx)
         return ctx->errmsg;
 }
 
+void
+tls_clear_error(struct tls *ctx)
+{
+        ctx->err = 0;
+        free(ctx->errmsg);
+        ctx->errmsg = NULL;
+}
+
 int
 tls_set_error(struct tls *ctx, char *fmt, ...)
 {
         va_list ap;
         int rv;
 
+        tls_clear_error(ctx);
         ctx->err = errno;
-        free(ctx->errmsg);
-        ctx->errmsg = NULL;
 
         va_start(ap, fmt);
         rv = vasprintf(&ctx->errmsg, fmt, ap);
diff --git a/src/lib/libtls/tls_client.c b/src/lib/libtls/tls_client.c
index a4528b9b87..c5849a6897 100644
--- a/src/lib/libtls/tls_client.c
+++ b/src/lib/libtls/tls_client.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_client.c,v 1.2 2014/11/02 14:45:05 jsing Exp $ */
+/* $OpenBSD: tls_client.c,v 1.3 2014/12/07 15:00:32 bcook Exp $ */
 /*
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
  *
@@ -209,9 +209,11 @@ tls_connect_fds(struct tls *ctx, int fd_read, int fd_write,
                         tls_set_error(ctx, "no server certificate");
                         goto err;
                 }
-                if (tls_check_hostname(cert, hostname) != 0) {
-                        tls_set_error(ctx, "host `%s' not present in"
-                            " server certificate", hostname);
+                tls_clear_error(ctx);
+                if (tls_check_hostname(ctx, cert, hostname) != 0) {
+                        if (tls_error(ctx) == NULL)
+                                tls_set_error(ctx, "host `%s' not present in"
+                                    " server certificate", hostname);
                         goto err;
                 }
         }
diff --git a/src/lib/libtls/tls_internal.h b/src/lib/libtls/tls_internal.h
index da696e228d..e6f2d4ac71 100644
--- a/src/lib/libtls/tls_internal.h
+++ b/src/lib/libtls/tls_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_internal.h,v 1.1 2014/10/31 13:46:17 jsing Exp $ */
+/* $OpenBSD: tls_internal.h,v 1.2 2014/12/07 15:00:32 bcook Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  * Copyright (c) 2014 Joel Sing <jsing@openbsd.org>
@@ -62,11 +62,12 @@ struct tls {
 struct tls *tls_new(void);
 struct tls *tls_server_conn(struct tls *ctx);
 
-int tls_check_hostname(X509 *cert, const char *host);
+int tls_check_hostname(struct tls *ctx, X509 *cert, const char *host);
 int tls_configure_keypair(struct tls *ctx);
 int tls_configure_server(struct tls *ctx);
 int tls_configure_ssl(struct tls *ctx);
 int tls_host_port(const char *hostport, char **host, char **port);
+void tls_clear_error(struct tls *ctx);
 int tls_set_error(struct tls *ctx, char *fmt, ...);
 
 #endif /* HEADER_TLS_INTERNAL_H */
diff --git a/src/lib/libtls/tls_verify.c b/src/lib/libtls/tls_verify.c
index fa0010922f..0252e20575 100644
--- a/src/lib/libtls/tls_verify.c
+++ b/src/lib/libtls/tls_verify.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_verify.c,v 1.1 2014/10/31 13:46:17 jsing Exp $ */
+/* $OpenBSD: tls_verify.c,v 1.2 2014/12/07 15:00:32 bcook Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  *
@@ -27,8 +27,8 @@
 #include "tls_internal.h"
 
 int tls_match_hostname(const char *cert_hostname, const char *hostname);
-int tls_check_subject_altname(X509 *cert, const char *host);
-int tls_check_common_name(X509 *cert, const char *host);
+int tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *host);
+int tls_check_common_name(struct tls *ctx, X509 *cert, const char *host);
 
 int
 tls_match_hostname(const char *cert_hostname, const char *hostname)
@@ -80,7 +80,7 @@ tls_match_hostname(const char *cert_hostname, const char *hostname)
 }
 
 int
-tls_check_subject_altname(X509 *cert, const char *host)
+tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *host)
 {
         STACK_OF(GENERAL_NAME) *altname_stack = NULL;
         union { struct in_addr ip4; struct in6_addr ip6; } addrbuf;
@@ -123,10 +123,11 @@ tls_check_subject_altname(X509 *cert, const char *host)
 
                                 if (ASN1_STRING_length(altname->d.dNSName) !=
                                     (int)strlen(data)) {
-                                        fprintf(stdout, "%s: NUL byte in "
-                                            "subjectAltName, probably a "
-                                            "malicious certificate.\n",
-                                            getprogname());
+                                        tls_set_error(ctx,
+                                            "error verifying host '%s': "
+                                            "NUL byte in subjectAltName, "
+                                            "probably a malicious certificate",
+                                            host);
                                         rv = -2;
                                         break;
                                 }
@@ -135,10 +136,7 @@ tls_check_subject_altname(X509 *cert, const char *host)
                                         rv = 0;
                                         break;
                                 }
-                        } else
-                                fprintf(stdout, "%s: unhandled subjectAltName "
-                                    "dNSName encoding (%d)\n", getprogname(),
-                                    format);
+                        }
 
                 } else if (type == GEN_IPADD) {
                         unsigned char   *data;
@@ -160,7 +158,7 @@ tls_check_subject_altname(X509 *cert, const char *host)
 }
 
 int
-tls_check_common_name(X509 *cert, const char *host)
+tls_check_common_name(struct tls *ctx, X509 *cert, const char *host)
 {
         X509_NAME *name;
         char *common_name = NULL;
@@ -186,8 +184,9 @@ tls_check_common_name(X509 *cert, const char *host)
 
         /* NUL bytes in CN? */
         if (common_name_len != (int)strlen(common_name)) {
-                fprintf(stdout, "%s: NUL byte in Common Name field, "
-                    "probably a malicious certificate.\n", getprogname());
+                tls_set_error(ctx, "error verifying host '%s': "
+                    "NUL byte in Common Name field, "
+                    "probably a malicious certificate.", host);
                 rv = -2;
                 goto out;
         }
@@ -213,13 +212,13 @@ out:
 }
 
 int
-tls_check_hostname(X509 *cert, const char *host)
+tls_check_hostname(struct tls *ctx, X509 *cert, const char *host)
 {
         int     rv;
 
-        rv = tls_check_subject_altname(cert, host);
+        rv = tls_check_subject_altname(ctx, cert, host);
         if (rv == 0 || rv == -2)
                 return rv;
 
-        return tls_check_common_name(cert, host);
+        return tls_check_common_name(ctx, cert, host);
 }