From cbd1d6a8808038e6f357e956a343f70ecaf110f4 Mon Sep 17 00:00:00 2001 From: beck <> Date: Fri, 6 Apr 2018 07:08:20 +0000 Subject: poison for X509_VERIFY_PARAM's Tighten up checks for various X509_VERIFY_PARAM functions, and allow for the verify param to be poisoned (preculding future successful cert validation) if the setting of host, ip, or email for certificate validation fails. (since many callers do not check the return code in the wild and blunder along anyway) Inspired by some discussions with Adam Langley. ok jsing@ --- .../libcrypto/man/X509_VERIFY_PARAM_set_flags.3 | 67 ++++++++++++++++----- src/lib/libcrypto/x509/vpm_int.h | 3 +- src/lib/libcrypto/x509/x509_vfy.c | 13 +++- src/lib/libcrypto/x509/x509_vpm.c | 69 +++++++++++++--------- 4 files changed, 107 insertions(+), 45 deletions(-) diff --git a/src/lib/libcrypto/man/X509_VERIFY_PARAM_set_flags.3 b/src/lib/libcrypto/man/X509_VERIFY_PARAM_set_flags.3 index 4f3261c975..9c0150700d 100644 --- a/src/lib/libcrypto/man/X509_VERIFY_PARAM_set_flags.3 +++ b/src/lib/libcrypto/man/X509_VERIFY_PARAM_set_flags.3 @@ -1,4 +1,4 @@ -.\" $OpenBSD: X509_VERIFY_PARAM_set_flags.3,v 1.12 2018/03/23 14:26:40 schwarze Exp $ +.\" $OpenBSD: X509_VERIFY_PARAM_set_flags.3,v 1.13 2018/04/06 07:08:20 beck Exp $ .\" full merge up to: OpenSSL d33def66 Feb 9 14:17:13 2016 -0500 .\" selective merge up to: OpenSSL 48e5119a Jan 19 10:49:22 2018 +0100 .\" @@ -68,7 +68,7 @@ .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED .\" OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.Dd $Mdocdate: March 23 2018 $ +.Dd $Mdocdate: April 6 2018 $ .Dt X509_VERIFY_PARAM_SET_FLAGS 3 .Os .Sh NAME @@ -344,14 +344,14 @@ is .Dv NULL or empty, the list of hostnames is cleared, and name checks are not performed on the peer certificate. -If +.Fa namelen +should be set to the length of +.Fa name . +For historical compatibility, if .Fa name is NUL-terminated, .Fa namelen -may be zero, otherwise -.Fa namelen -must be set to the length of -.Fa name . +may be specified as zero. When a hostname is specified, certificate verification automatically invokes .Xr X509_check_host 3 @@ -360,6 +360,10 @@ with flags equal to the argument given to .Fn X509_VERIFY_PARAM_set_hostflags (default zero). +.Fn X509_VERIFY_PARAM_set1_host +will fail if +.Fa name +contains any embedded 0 bytes. .Pp .Fn X509_VERIFY_PARAM_add1_host adds @@ -376,6 +380,18 @@ No change is made if is .Dv NULL or empty. +.Fa namelen +should be set to the length of +.Fa name . +For historical compatibility, if +.Fa name +is NUL-terminated, +.Fa namelen +may be specified as zero. +.Fn X509_VERIFY_PARAM_add1_host +will fail if +.Fa name +contains any embedded 0 bytes. When multiple names are configured, the peer is considered verified when any name matches. .Pp @@ -390,14 +406,18 @@ identifier respectively. .Fn X509_VERIFY_PARAM_set1_email sets the expected RFC822 email address to .Fa email . -If +.Fa emaillen +should be set to the length of +.Fa email . +For historical compatibility, if .Fa email is NUL-terminated, .Fa emaillen -may be zero, otherwise -.Fa emaillen -must be set to the length of -.Fa email . +may be specified as zero, +.Fn X509_VERIFY_PARAM_set1_email +will fail if +.Fa email +is NULL, an empty string, or contains embedded 0 bytes. When an email address is specified, certificate verification automatically invokes .Xr X509_check_email 3 . @@ -410,6 +430,12 @@ The argument is in binary format, in network byte-order, and .Fa iplen must be set to 4 for IPv4 and 16 for IPv6. +.Fn X509_VERIFY_PARAM_set1_ip +will fail if +.Fa ip +is NULL or if +.Fa iplen +is not 4 or 16. When an IP address is specified, certificate verification automatically invokes .Xr X509_check_ip 3 . @@ -422,6 +448,10 @@ The argument is a NUL-terminal ASCII string: dotted decimal quad for IPv4 and colon-separated hexadecimal for IPv6. The condensed "::" notation is supported for IPv6 addresses. +.Fn X509_VERIFY_PARAM_set1_ip_asc +will fail if +.Fa ipasc +is unparsable. .Pp .Fn X509_VERIFY_PARAM_add0_table adds @@ -476,14 +506,23 @@ on allocation failure. .Fn X509_VERIFY_PARAM_set_trust , .Fn X509_VERIFY_PARAM_add0_policy , .Fn X509_VERIFY_PARAM_set1_policies , +and +.Fn X509_VERIFY_PARAM_add0_table +return 1 for success or 0 for failure. +.Pp .Fn X509_VERIFY_PARAM_set1_host , .Fn X509_VERIFY_PARAM_add1_host , .Fn X509_VERIFY_PARAM_set1_email , .Fn X509_VERIFY_PARAM_set1_ip , -.Fn X509_VERIFY_PARAM_set1_ip_asc , and -.Fn X509_VERIFY_PARAM_add0_table +.Fn X509_VERIFY_PARAM_set1_ip_asc , return 1 for success or 0 for failure. +A failure from these routines will poison +the +.Vt X509_VERIFY_PARAM +object so that future calls to +.Xr X509_verify_cert +using the poisoned object will fail. .Pp .Fn X509_VERIFY_PARAM_get_flags returns the current verification flags. diff --git a/src/lib/libcrypto/x509/vpm_int.h b/src/lib/libcrypto/x509/vpm_int.h index 6c8061c847..7fc9fef761 100644 --- a/src/lib/libcrypto/x509/vpm_int.h +++ b/src/lib/libcrypto/x509/vpm_int.h @@ -1,4 +1,4 @@ -/* $OpenBSD: vpm_int.h,v 1.3 2016/12/21 15:49:29 jsing Exp $ */ +/* $OpenBSD: vpm_int.h,v 1.4 2018/04/06 07:08:20 beck Exp $ */ /* * Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL project * 2013. @@ -69,6 +69,7 @@ struct X509_VERIFY_PARAM_ID_st { size_t emaillen; unsigned char *ip; /* If not NULL IP address to match */ size_t iplen; /* Length of IP address */ + int poisoned; }; __END_HIDDEN_DECLS diff --git a/src/lib/libcrypto/x509/x509_vfy.c b/src/lib/libcrypto/x509/x509_vfy.c index c8ccae5029..8392f509e7 100644 --- a/src/lib/libcrypto/x509/x509_vfy.c +++ b/src/lib/libcrypto/x509/x509_vfy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vfy.c,v 1.68 2018/02/22 17:11:30 jsing Exp $ */ +/* $OpenBSD: x509_vfy.c,v 1.69 2018/04/06 07:08:20 beck Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -182,10 +182,13 @@ check_id_error(X509_STORE_CTX *ctx, int errcode) static int check_hosts(X509 *x, X509_VERIFY_PARAM_ID *id) { - size_t i; - size_t n = sk_OPENSSL_STRING_num(id->hosts); + size_t i, n; char *name; + if (id->poisoned) + return 0; + + n = sk_OPENSSL_STRING_num(id->hosts); free(id->peername); id->peername = NULL; @@ -205,6 +208,10 @@ check_id(X509_STORE_CTX *ctx) X509_VERIFY_PARAM_ID *id = vpm->id; X509 *x = ctx->cert; + if (id->poisoned) + if (!check_id_error(ctx, X509_V_ERR_INVALID_CALL)) + return 0; + if (id->hosts && check_hosts(x, id) <= 0) { if (!check_id_error(ctx, X509_V_ERR_HOSTNAME_MISMATCH)) return 0; diff --git a/src/lib/libcrypto/x509/x509_vpm.c b/src/lib/libcrypto/x509/x509_vpm.c index 0897137697..baebcf7bca 100644 --- a/src/lib/libcrypto/x509/x509_vpm.c +++ b/src/lib/libcrypto/x509/x509_vpm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vpm.c,v 1.17 2018/03/22 15:54:46 beck Exp $ */ +/* $OpenBSD: x509_vpm.c,v 1.18 2018/04/06 07:08:20 beck Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2004. */ @@ -125,7 +125,7 @@ sk_deep_copy(void *sk_void, void *copy_func_void, void *free_func_void) } static int -int_x509_param_set_hosts(X509_VERIFY_PARAM_ID *id, int mode, +x509_param_set_hosts_internal(X509_VERIFY_PARAM_ID *id, int mode, const char *name, size_t namelen) { char *copy; @@ -134,7 +134,6 @@ int_x509_param_set_hosts(X509_VERIFY_PARAM_ID *id, int mode, namelen = strlen(name); /* * Refuse names with embedded NUL bytes. - * XXX: Do we need to push an error onto the error stack? */ if (name && memchr(name, '\0', namelen)) return 0; @@ -197,6 +196,7 @@ x509_verify_param_zero(X509_VERIFY_PARAM *param) free(paramid->ip); paramid->ip = NULL; paramid->iplen = 0; + paramid->poisoned = 0; } X509_VERIFY_PARAM * @@ -367,24 +367,28 @@ X509_VERIFY_PARAM_set1(X509_VERIFY_PARAM *to, const X509_VERIFY_PARAM *from) } static int -int_x509_param_set1(char **pdest, size_t *pdestlen, const char *src, - size_t srclen) +x509_param_set1_internal(char **pdest, size_t *pdestlen, const char *src, + size_t srclen, int nonul) { char *tmp; - if (src) { - if (srclen == 0) { - if ((tmp = strdup(src)) == NULL) - return 0; - srclen = strlen(src); - } else { - if ((tmp = malloc(srclen)) == NULL) - return 0; - memcpy(tmp, src, srclen); - } + + if (src == NULL) + return 0; + + if (srclen == 0) { + srclen = strlen(src); + if (srclen == 0) + return 0; + if ((tmp = strdup(src)) == NULL) + return 0; } else { - tmp = NULL; - srclen = 0; + if (nonul && memchr(src, '\0', srclen)) + return 0; + if ((tmp = malloc(srclen)) == NULL) + return 0; + memcpy(tmp, src, srclen); } + if (*pdest) free(*pdest); *pdest = tmp; @@ -505,14 +509,20 @@ int X509_VERIFY_PARAM_set1_host(X509_VERIFY_PARAM *param, const char *name, size_t namelen) { - return int_x509_param_set_hosts(param->id, SET_HOST, name, namelen); + if (x509_param_set_hosts_internal(param->id, SET_HOST, name, namelen)) + return 1; + param->id->poisoned = 1; + return 0; } int X509_VERIFY_PARAM_add1_host(X509_VERIFY_PARAM *param, const char *name, size_t namelen) { - return int_x509_param_set_hosts(param->id, ADD_HOST, name, namelen); + if (x509_param_set_hosts_internal(param->id, ADD_HOST, name, namelen)) + return 1; + param->id->poisoned = 1; + return 0; } void @@ -531,18 +541,25 @@ int X509_VERIFY_PARAM_set1_email(X509_VERIFY_PARAM *param, const char *email, size_t emaillen) { - return int_x509_param_set1(¶m->id->email, ¶m->id->emaillen, - email, emaillen); + if (x509_param_set1_internal(¶m->id->email, ¶m->id->emaillen, + email, emaillen, 1)) + return 1; + param->id->poisoned = 1; + return 0; } int X509_VERIFY_PARAM_set1_ip(X509_VERIFY_PARAM *param, const unsigned char *ip, size_t iplen) { - if (iplen != 0 && iplen != 4 && iplen != 16) - return 0; - return int_x509_param_set1((char **)¶m->id->ip, ¶m->id->iplen, - (char *)ip, iplen); + if (iplen != 4 && iplen != 16) + goto err; + if (x509_param_set1_internal((char **)¶m->id->ip, ¶m->id->iplen, + (char *)ip, iplen, 0)) + return 1; + err: + param->id->poisoned = 1; + return 0; } int @@ -552,8 +569,6 @@ X509_VERIFY_PARAM_set1_ip_asc(X509_VERIFY_PARAM *param, const char *ipasc) size_t iplen; iplen = (size_t)a2i_ipadd(ipout, ipasc); - if (iplen == 0) - return 0; return X509_VERIFY_PARAM_set1_ip(param, ipout, iplen); } -- cgit v1.2.3-55-g6feb