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@

3 files changed, 54 insertions, 31 deletions

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(&param->id->email, &param->id->emaillen,
-            email, emaillen);
+        if (x509_param_set1_internal(&param->id->email, &param->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 **)&param->id->ip, &param->id->iplen,
-            (char *)ip, iplen);
+        if (iplen != 4 && iplen != 16)
+                goto err;
+        if (x509_param_set1_internal((char **)&param->id->ip, &param->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);
 }