From df159794178eaab75bdfe6f692e876f2da10c396 Mon Sep 17 00:00:00 2001 From: bcook <> Date: Sun, 7 Dec 2014 16:56:17 +0000 Subject: 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(). ok jsing@ --- src/lib/libtls/tls_client.c | 9 ++++---- src/lib/libtls/tls_internal.h | 4 ++-- src/lib/libtls/tls_verify.c | 35 +++++++++++++++++------------- src/regress/lib/libtls/verify/verifytest.c | 10 ++++++--- 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/lib/libtls/tls_client.c b/src/lib/libtls/tls_client.c index b851a6ecd0..43819cf0b6 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.4 2014/12/07 15:48:02 bcook Exp $ */ +/* $OpenBSD: tls_client.c,v 1.5 2014/12/07 16:56:17 bcook Exp $ */ /* * Copyright (c) 2014 Joel Sing * @@ -209,9 +209,10 @@ 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); + if ((ret = tls_check_hostname(ctx, cert, hostname)) != 0) { + if (ret != -2) + 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 a23e63f7af..bfd7146d7d 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.3 2014/12/07 15:48:02 bcook Exp $ */ +/* $OpenBSD: tls_internal.h,v 1.4 2014/12/07 16:56:17 bcook Exp $ */ /* * Copyright (c) 2014 Jeremie Courreges-Anglas * Copyright (c) 2014 Joel Sing @@ -62,7 +62,7 @@ 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); diff --git a/src/lib/libtls/tls_verify.c b/src/lib/libtls/tls_verify.c index ddc403fb10..697432c429 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.4 2014/12/07 16:01:03 jsing Exp $ */ +/* $OpenBSD: tls_verify.c,v 1.5 2014/12/07 16:56:17 bcook Exp $ */ /* * Copyright (c) 2014 Jeremie Courreges-Anglas * @@ -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,13 @@ tls_check_subject_altname(X509 *cert, const char *host) rv = 0; break; } - } else + } else { +#ifdef DEBUG fprintf(stdout, "%s: unhandled subjectAltName " "dNSName encoding (%d)\n", getprogname(), format); +#endif + } } else if (type == GEN_IPADD) { unsigned char *data; @@ -160,7 +164,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 +190,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 +218,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); } diff --git a/src/regress/lib/libtls/verify/verifytest.c b/src/regress/lib/libtls/verify/verifytest.c index bb8b372014..81dcb90a67 100644 --- a/src/regress/lib/libtls/verify/verifytest.c +++ b/src/regress/lib/libtls/verify/verifytest.c @@ -1,4 +1,4 @@ -/* $OpenBSD: verifytest.c,v 1.1 2014/11/01 11:55:27 jsing Exp $ */ +/* $OpenBSD: verifytest.c,v 1.2 2014/12/07 16:56:17 bcook Exp $ */ /* * Copyright (c) 2014 Joel Sing * @@ -20,8 +20,9 @@ #include #include +#include -extern int tls_check_hostname(X509 *cert, const char *host); +extern int tls_check_hostname(struct tls *ctx, X509 *cert, const char *host); struct verify_test { const char common_name[128]; @@ -162,6 +163,7 @@ do_verify_test(int test_no, struct verify_test *vt) GENERAL_NAME *alt_name; X509_NAME *name; X509 *cert; + struct tls *tls; /* Build certificate structure. */ if ((cert = X509_new()) == NULL) @@ -174,6 +176,8 @@ do_verify_test(int test_no, struct verify_test *vt) if (X509_set_subject_name(cert, name) == 0) errx(1, "failed to set subject name"); X509_NAME_free(name); + if ((tls = tls_client()) == NULL) + errx(1, "failed to malloc tls_client"); if (vt->alt_name_type != 0) { if ((alt_name_stack = sk_GENERAL_NAME_new_null()) == NULL) @@ -209,7 +213,7 @@ do_verify_test(int test_no, struct verify_test *vt) sk_GENERAL_NAME_pop_free(alt_name_stack, GENERAL_NAME_free); } - if (tls_check_hostname(cert, vt->hostname) != vt->want) { + if (tls_check_hostname(tls, cert, vt->hostname) != vt->want) { fprintf(stderr, "FAIL: test %i failed with common name " "'%s', alt name '%s' and hostname '%s'\n", test_no, vt->common_name, vt->alt_name, vt->hostname); -- cgit v1.2.3-55-g6feb