diff options
| author | bcook <> | 2014-12-07 16:56:17 +0000 |
|---|---|---|
| committer | bcook <> | 2014-12-07 16:56:17 +0000 |
| commit | 26bb73d6664efd39d99f8477df943507a7f03b5e (patch) | |
| tree | 5c66d957212c1e16825ea92d01779555b1a78024 | |
| parent | ee67fae33e31b0a137774a402a100e614c3cbc9d (diff) | |
| download | openbsd-26bb73d6664efd39d99f8477df943507a7f03b5e.tar.gz openbsd-26bb73d6664efd39d99f8477df943507a7f03b5e.tar.bz2 openbsd-26bb73d6664efd39d99f8477df943507a7f03b5e.zip | |
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@
| -rw-r--r-- | src/lib/libtls/tls_client.c | 9 | ||||
| -rw-r--r-- | src/lib/libtls/tls_internal.h | 4 | ||||
| -rw-r--r-- | src/lib/libtls/tls_verify.c | 35 | ||||
| -rw-r--r-- | 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 @@ | |||
| 1 | /* $OpenBSD: tls_client.c,v 1.4 2014/12/07 15:48:02 bcook Exp $ */ | 1 | /* $OpenBSD: tls_client.c,v 1.5 2014/12/07 16:56:17 bcook Exp $ */ |
| 2 | /* | 2 | /* |
| 3 | * Copyright (c) 2014 Joel Sing <jsing@openbsd.org> | 3 | * Copyright (c) 2014 Joel Sing <jsing@openbsd.org> |
| 4 | * | 4 | * |
| @@ -209,9 +209,10 @@ tls_connect_fds(struct tls *ctx, int fd_read, int fd_write, | |||
| 209 | tls_set_error(ctx, "no server certificate"); | 209 | tls_set_error(ctx, "no server certificate"); |
| 210 | goto err; | 210 | goto err; |
| 211 | } | 211 | } |
| 212 | if (tls_check_hostname(cert, hostname) != 0) { | 212 | if ((ret = tls_check_hostname(ctx, cert, hostname)) != 0) { |
| 213 | tls_set_error(ctx, "host `%s' not present in" | 213 | if (ret != -2) |
| 214 | " server certificate", hostname); | 214 | tls_set_error(ctx, "host `%s' not present in" |
| 215 | " server certificate", hostname); | ||
| 215 | goto err; | 216 | goto err; |
| 216 | } | 217 | } |
| 217 | } | 218 | } |
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 @@ | |||
| 1 | /* $OpenBSD: tls_internal.h,v 1.3 2014/12/07 15:48:02 bcook Exp $ */ | 1 | /* $OpenBSD: tls_internal.h,v 1.4 2014/12/07 16:56:17 bcook Exp $ */ |
| 2 | /* | 2 | /* |
| 3 | * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org> | 3 | * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org> |
| 4 | * Copyright (c) 2014 Joel Sing <jsing@openbsd.org> | 4 | * Copyright (c) 2014 Joel Sing <jsing@openbsd.org> |
| @@ -62,7 +62,7 @@ struct tls { | |||
| 62 | struct tls *tls_new(void); | 62 | struct tls *tls_new(void); |
| 63 | struct tls *tls_server_conn(struct tls *ctx); | 63 | struct tls *tls_server_conn(struct tls *ctx); |
| 64 | 64 | ||
| 65 | int tls_check_hostname(X509 *cert, const char *host); | 65 | int tls_check_hostname(struct tls *ctx, X509 *cert, const char *host); |
| 66 | int tls_configure_keypair(struct tls *ctx); | 66 | int tls_configure_keypair(struct tls *ctx); |
| 67 | int tls_configure_server(struct tls *ctx); | 67 | int tls_configure_server(struct tls *ctx); |
| 68 | int tls_configure_ssl(struct tls *ctx); | 68 | 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 @@ | |||
| 1 | /* $OpenBSD: tls_verify.c,v 1.4 2014/12/07 16:01:03 jsing Exp $ */ | 1 | /* $OpenBSD: tls_verify.c,v 1.5 2014/12/07 16:56:17 bcook Exp $ */ |
| 2 | /* | 2 | /* |
| 3 | * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org> | 3 | * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org> |
| 4 | * | 4 | * |
| @@ -27,8 +27,8 @@ | |||
| 27 | #include "tls_internal.h" | 27 | #include "tls_internal.h" |
| 28 | 28 | ||
| 29 | int tls_match_hostname(const char *cert_hostname, const char *hostname); | 29 | int tls_match_hostname(const char *cert_hostname, const char *hostname); |
| 30 | int tls_check_subject_altname(X509 *cert, const char *host); | 30 | int tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *host); |
| 31 | int tls_check_common_name(X509 *cert, const char *host); | 31 | int tls_check_common_name(struct tls *ctx, X509 *cert, const char *host); |
| 32 | 32 | ||
| 33 | int | 33 | int |
| 34 | tls_match_hostname(const char *cert_hostname, const char *hostname) | 34 | tls_match_hostname(const char *cert_hostname, const char *hostname) |
| @@ -80,7 +80,7 @@ tls_match_hostname(const char *cert_hostname, const char *hostname) | |||
| 80 | } | 80 | } |
| 81 | 81 | ||
| 82 | int | 82 | int |
| 83 | tls_check_subject_altname(X509 *cert, const char *host) | 83 | tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *host) |
| 84 | { | 84 | { |
| 85 | STACK_OF(GENERAL_NAME) *altname_stack = NULL; | 85 | STACK_OF(GENERAL_NAME) *altname_stack = NULL; |
| 86 | union { struct in_addr ip4; struct in6_addr ip6; } addrbuf; | 86 | union { struct in_addr ip4; struct in6_addr ip6; } addrbuf; |
| @@ -123,10 +123,11 @@ tls_check_subject_altname(X509 *cert, const char *host) | |||
| 123 | 123 | ||
| 124 | if (ASN1_STRING_length(altname->d.dNSName) != | 124 | if (ASN1_STRING_length(altname->d.dNSName) != |
| 125 | (int)strlen(data)) { | 125 | (int)strlen(data)) { |
| 126 | fprintf(stdout, "%s: NUL byte in " | 126 | tls_set_error(ctx, |
| 127 | "subjectAltName, probably a " | 127 | "error verifying host '%s': " |
| 128 | "malicious certificate.\n", | 128 | "NUL byte in subjectAltName, " |
| 129 | getprogname()); | 129 | "probably a malicious certificate", |
| 130 | host); | ||
| 130 | rv = -2; | 131 | rv = -2; |
| 131 | break; | 132 | break; |
| 132 | } | 133 | } |
| @@ -135,10 +136,13 @@ tls_check_subject_altname(X509 *cert, const char *host) | |||
| 135 | rv = 0; | 136 | rv = 0; |
| 136 | break; | 137 | break; |
| 137 | } | 138 | } |
| 138 | } else | 139 | } else { |
| 140 | #ifdef DEBUG | ||
| 139 | fprintf(stdout, "%s: unhandled subjectAltName " | 141 | fprintf(stdout, "%s: unhandled subjectAltName " |
| 140 | "dNSName encoding (%d)\n", getprogname(), | 142 | "dNSName encoding (%d)\n", getprogname(), |
| 141 | format); | 143 | format); |
| 144 | #endif | ||
| 145 | } | ||
| 142 | 146 | ||
| 143 | } else if (type == GEN_IPADD) { | 147 | } else if (type == GEN_IPADD) { |
| 144 | unsigned char *data; | 148 | unsigned char *data; |
| @@ -160,7 +164,7 @@ tls_check_subject_altname(X509 *cert, const char *host) | |||
| 160 | } | 164 | } |
| 161 | 165 | ||
| 162 | int | 166 | int |
| 163 | tls_check_common_name(X509 *cert, const char *host) | 167 | tls_check_common_name(struct tls *ctx, X509 *cert, const char *host) |
| 164 | { | 168 | { |
| 165 | X509_NAME *name; | 169 | X509_NAME *name; |
| 166 | char *common_name = NULL; | 170 | char *common_name = NULL; |
| @@ -186,8 +190,9 @@ tls_check_common_name(X509 *cert, const char *host) | |||
| 186 | 190 | ||
| 187 | /* NUL bytes in CN? */ | 191 | /* NUL bytes in CN? */ |
| 188 | if (common_name_len != (int)strlen(common_name)) { | 192 | if (common_name_len != (int)strlen(common_name)) { |
| 189 | fprintf(stdout, "%s: NUL byte in Common Name field, " | 193 | tls_set_error(ctx, "error verifying host '%s': " |
| 190 | "probably a malicious certificate.\n", getprogname()); | 194 | "NUL byte in Common Name field, " |
| 195 | "probably a malicious certificate.", host); | ||
| 191 | rv = -2; | 196 | rv = -2; |
| 192 | goto out; | 197 | goto out; |
| 193 | } | 198 | } |
| @@ -213,13 +218,13 @@ out: | |||
| 213 | } | 218 | } |
| 214 | 219 | ||
| 215 | int | 220 | int |
| 216 | tls_check_hostname(X509 *cert, const char *host) | 221 | tls_check_hostname(struct tls *ctx, X509 *cert, const char *host) |
| 217 | { | 222 | { |
| 218 | int rv; | 223 | int rv; |
| 219 | 224 | ||
| 220 | rv = tls_check_subject_altname(cert, host); | 225 | rv = tls_check_subject_altname(ctx, cert, host); |
| 221 | if (rv == 0 || rv == -2) | 226 | if (rv == 0 || rv == -2) |
| 222 | return rv; | 227 | return rv; |
| 223 | 228 | ||
| 224 | return tls_check_common_name(cert, host); | 229 | return tls_check_common_name(ctx, cert, host); |
| 225 | } | 230 | } |
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 @@ | |||
| 1 | /* $OpenBSD: verifytest.c,v 1.1 2014/11/01 11:55:27 jsing Exp $ */ | 1 | /* $OpenBSD: verifytest.c,v 1.2 2014/12/07 16:56:17 bcook Exp $ */ |
| 2 | /* | 2 | /* |
| 3 | * Copyright (c) 2014 Joel Sing <jsing@openbsd.org> | 3 | * Copyright (c) 2014 Joel Sing <jsing@openbsd.org> |
| 4 | * | 4 | * |
| @@ -20,8 +20,9 @@ | |||
| 20 | #include <stdlib.h> | 20 | #include <stdlib.h> |
| 21 | 21 | ||
| 22 | #include <openssl/x509v3.h> | 22 | #include <openssl/x509v3.h> |
| 23 | #include <tls.h> | ||
| 23 | 24 | ||
| 24 | extern int tls_check_hostname(X509 *cert, const char *host); | 25 | extern int tls_check_hostname(struct tls *ctx, X509 *cert, const char *host); |
| 25 | 26 | ||
| 26 | struct verify_test { | 27 | struct verify_test { |
| 27 | const char common_name[128]; | 28 | const char common_name[128]; |
| @@ -162,6 +163,7 @@ do_verify_test(int test_no, struct verify_test *vt) | |||
| 162 | GENERAL_NAME *alt_name; | 163 | GENERAL_NAME *alt_name; |
| 163 | X509_NAME *name; | 164 | X509_NAME *name; |
| 164 | X509 *cert; | 165 | X509 *cert; |
| 166 | struct tls *tls; | ||
| 165 | 167 | ||
| 166 | /* Build certificate structure. */ | 168 | /* Build certificate structure. */ |
| 167 | if ((cert = X509_new()) == NULL) | 169 | if ((cert = X509_new()) == NULL) |
| @@ -174,6 +176,8 @@ do_verify_test(int test_no, struct verify_test *vt) | |||
| 174 | if (X509_set_subject_name(cert, name) == 0) | 176 | if (X509_set_subject_name(cert, name) == 0) |
| 175 | errx(1, "failed to set subject name"); | 177 | errx(1, "failed to set subject name"); |
| 176 | X509_NAME_free(name); | 178 | X509_NAME_free(name); |
| 179 | if ((tls = tls_client()) == NULL) | ||
| 180 | errx(1, "failed to malloc tls_client"); | ||
| 177 | 181 | ||
| 178 | if (vt->alt_name_type != 0) { | 182 | if (vt->alt_name_type != 0) { |
| 179 | if ((alt_name_stack = sk_GENERAL_NAME_new_null()) == NULL) | 183 | if ((alt_name_stack = sk_GENERAL_NAME_new_null()) == NULL) |
| @@ -209,7 +213,7 @@ do_verify_test(int test_no, struct verify_test *vt) | |||
| 209 | sk_GENERAL_NAME_pop_free(alt_name_stack, GENERAL_NAME_free); | 213 | sk_GENERAL_NAME_pop_free(alt_name_stack, GENERAL_NAME_free); |
| 210 | } | 214 | } |
| 211 | 215 | ||
| 212 | if (tls_check_hostname(cert, vt->hostname) != vt->want) { | 216 | if (tls_check_hostname(tls, cert, vt->hostname) != vt->want) { |
| 213 | fprintf(stderr, "FAIL: test %i failed with common name " | 217 | fprintf(stderr, "FAIL: test %i failed with common name " |
| 214 | "'%s', alt name '%s' and hostname '%s'\n", test_no, | 218 | "'%s', alt name '%s' and hostname '%s'\n", test_no, |
| 215 | vt->common_name, vt->alt_name, vt->hostname); | 219 | vt->common_name, vt->alt_name, vt->hostname); |
