diff options
author | bcook <> | 2014-12-07 16:56:17 +0000 |
---|---|---|
committer | bcook <> | 2014-12-07 16:56:17 +0000 |
commit | df159794178eaab75bdfe6f692e876f2da10c396 (patch) | |
tree | 5c66d957212c1e16825ea92d01779555b1a78024 | |
parent | 6095fdcf15afb6c9da4fd123ad558a7abb9e2524 (diff) | |
download | openbsd-df159794178eaab75bdfe6f692e876f2da10c396.tar.gz openbsd-df159794178eaab75bdfe6f692e876f2da10c396.tar.bz2 openbsd-df159794178eaab75bdfe6f692e876f2da10c396.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); |