Merge tag 'upstream/2.4.3'

Upstream version 2.4.3

1 files changed, 154 insertions, 77 deletions

diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index e9692a0..468b495 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -17,10 +17,9 @@
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
  *
- *  You should have received a copy of the GNU General Public License
- *  along with this program (see the file COPYING included with this
- *  distribution); if not, write to the Free Software Foundation, Inc.,
- *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
 /**
@@ -43,6 +42,7 @@
 #include "ssl_openssl.h"
 #include "ssl_verify.h"
 #include "ssl_verify_backend.h"
+#include "openssl_compat.h"
 
 #include <openssl/x509v3.h>
 #include <openssl/err.h>
@@ -61,14 +61,15 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
     session = (struct tls_session *) SSL_get_ex_data(ssl, mydata_index);
     ASSERT(session);
 
-    struct buffer cert_hash = x509_get_sha256_fingerprint(ctx->current_cert, &gc);
-    cert_hash_remember(session, ctx->error_depth, &cert_hash);
+    X509 *current_cert = X509_STORE_CTX_get_current_cert(ctx);
+    struct buffer cert_hash = x509_get_sha256_fingerprint(current_cert, &gc);
+    cert_hash_remember(session, X509_STORE_CTX_get_error_depth(ctx), &cert_hash);
 
     /* did peer present cert which was signed by our root cert? */
     if (!preverify_ok)
     {
         /* get the X509 name */
-        char *subject = x509_get_subject(ctx->current_cert, &gc);
+        char *subject = x509_get_subject(current_cert, &gc);
 
         if (!subject)
         {
@@ -76,11 +77,11 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
         }
 
         /* Log and ignore missing CRL errors */
-        if (ctx->error == X509_V_ERR_UNABLE_TO_GET_CRL)
+        if (X509_STORE_CTX_get_error(ctx) == X509_V_ERR_UNABLE_TO_GET_CRL)
         {
             msg(D_TLS_DEBUG_LOW, "VERIFY WARNING: depth=%d, %s: %s",
-                ctx->error_depth,
-                X509_verify_cert_error_string(ctx->error),
+                X509_STORE_CTX_get_error_depth(ctx),
+                X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)),
                 subject);
             ret = 1;
             goto cleanup;
@@ -88,8 +89,8 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
 
         /* Remote site specified a certificate, but it's not correct */
         msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s",
-            ctx->error_depth,
-            X509_verify_cert_error_string(ctx->error),
+            X509_STORE_CTX_get_error_depth(ctx),
+            X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)),
             subject);
 
         ERR_clear_error();
@@ -98,7 +99,7 @@ verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
         goto cleanup;
     }
 
-    if (SUCCESS != verify_cert(session, ctx->current_cert, ctx->error_depth))
+    if (SUCCESS != verify_cert(session, current_cert, X509_STORE_CTX_get_error_depth(ctx)))
     {
         goto cleanup;
     }
@@ -112,16 +113,29 @@ cleanup:
 }
 
 #ifdef ENABLE_X509ALTUSERNAME
+bool x509_username_field_ext_supported(const char *fieldname)
+{
+    int nid = OBJ_txt2nid(fieldname);
+    return nid == NID_subject_alt_name || nid == NID_issuer_alt_name;
+}
+
 static
 bool
 extract_x509_extension(X509 *cert, char *fieldname, char *out, int size)
 {
     bool retval = false;
     char *buf = 0;
-    GENERAL_NAMES *extensions;
-    int nid = OBJ_txt2nid(fieldname);
 
-    extensions = (GENERAL_NAMES *)X509_get_ext_d2i(cert, nid, NULL, NULL);
+    if (!x509_username_field_ext_supported(fieldname))
+    {
+        msg(D_TLS_ERRORS,
+            "ERROR: --x509-alt-username field 'ext:%s' not supported",
+            fieldname);
+        return false;
+    }
+
+    int nid = OBJ_txt2nid(fieldname);
+    GENERAL_NAMES *extensions = X509_get_ext_d2i(cert, nid, NULL, NULL);
     if (extensions)
     {
         int numalts;
@@ -142,7 +156,10 @@ extract_x509_extension(X509 *cert, char *fieldname, char *out, int size)
             switch (name->type)
             {
                 case GEN_EMAIL:
-                    ASN1_STRING_to_UTF8((unsigned char **)&buf, name->d.ia5);
+                    if (ASN1_STRING_to_UTF8((unsigned char **)&buf, name->d.ia5) < 0)
+                    {
+                        continue;
+                    }
                     if (strlen(buf) != name->d.ia5->length)
                     {
                         msg(D_TLS_ERRORS, "ASN1 ERROR: string contained terminating zero");
@@ -162,7 +179,7 @@ extract_x509_extension(X509 *cert, char *fieldname, char *out, int size)
                     break;
             }
         }
-        sk_GENERAL_NAME_free(extensions);
+        GENERAL_NAMES_free(extensions);
     }
     return retval;
 }
@@ -189,15 +206,24 @@ extract_x509_field_ssl(X509_NAME *x509, const char *field_name, char *out,
     X509_NAME_ENTRY *x509ne = 0;
     ASN1_STRING *asn1 = 0;
     unsigned char *buf = NULL;
-    int nid = OBJ_txt2nid(field_name);
+    ASN1_OBJECT *field_name_obj = OBJ_txt2obj(field_name, 0);
+
+    if (field_name_obj == NULL)
+    {
+        msg(D_TLS_ERRORS, "Invalid X509 attribute name '%s'", field_name);
+        return FAILURE;
+    }
 
     ASSERT(size > 0);
     *out = '\0';
-    do {
+    do
+    {
         lastpos = tmp;
-        tmp = X509_NAME_get_index_by_NID(x509, nid, lastpos);
+        tmp = X509_NAME_get_index_by_OBJ(x509, field_name_obj, lastpos);
     } while (tmp > -1);
 
+    ASN1_OBJECT_free(field_name_obj);
+
     /* Nothing found */
     if (lastpos == -1)
     {
@@ -215,8 +241,7 @@ extract_x509_field_ssl(X509_NAME *x509, const char *field_name, char *out,
     {
         return FAILURE;
     }
-    tmp = ASN1_STRING_to_UTF8(&buf, asn1);
-    if (tmp <= 0)
+    if (ASN1_STRING_to_UTF8(&buf, asn1) < 0)
     {
         return FAILURE;
     }
@@ -283,18 +308,20 @@ backend_x509_get_serial_hex(openvpn_x509_cert_t *cert, struct gc_arena *gc)
 struct buffer
 x509_get_sha1_fingerprint(X509 *cert, struct gc_arena *gc)
 {
-    struct buffer hash = alloc_buf_gc(sizeof(cert->sha1_hash), gc);
-    memcpy(BPTR(&hash), cert->sha1_hash, sizeof(cert->sha1_hash));
-    ASSERT(buf_inc_len(&hash, sizeof(cert->sha1_hash)));
+    const EVP_MD *sha1 = EVP_sha1();
+    struct buffer hash = alloc_buf_gc(EVP_MD_size(sha1), gc);
+    X509_digest(cert, EVP_sha1(), BPTR(&hash), NULL);
+    ASSERT(buf_inc_len(&hash, EVP_MD_size(sha1)));
     return hash;
 }
 
 struct buffer
 x509_get_sha256_fingerprint(X509 *cert, struct gc_arena *gc)
 {
-    struct buffer hash = alloc_buf_gc((EVP_sha256())->md_size, gc);
+    const EVP_MD *sha256 = EVP_sha256();
+    struct buffer hash = alloc_buf_gc(EVP_MD_size(sha256), gc);
     X509_digest(cert, EVP_sha256(), BPTR(&hash), NULL);
-    ASSERT(buf_inc_len(&hash, (EVP_sha256())->md_size));
+    ASSERT(buf_inc_len(&hash, EVP_MD_size(sha256)));
     return hash;
 }
 
@@ -304,7 +331,6 @@ x509_get_subject(X509 *cert, struct gc_arena *gc)
     BIO *subject_bio = NULL;
     BUF_MEM *subject_mem;
     char *subject = NULL;
-    int maxlen = 0;
 
     /*
      * Generate the subject string in OpenSSL proprietary format,
@@ -335,11 +361,10 @@ x509_get_subject(X509 *cert, struct gc_arena *gc)
 
     BIO_get_mem_ptr(subject_bio, &subject_mem);
 
-    maxlen = subject_mem->length + 1;
-    subject = gc_malloc(maxlen, false, gc);
+    subject = gc_malloc(subject_mem->length + 1, false, gc);
 
-    memcpy(subject, subject_mem->data, maxlen);
-    subject[maxlen - 1] = '\0';
+    memcpy(subject, subject_mem->data, subject_mem->length);
+    subject[subject_mem->length] = '\0';
 
 err:
     if (subject_bio)
@@ -457,7 +482,7 @@ x509_setenv_track(const struct x509_track *xt, struct env_set *es, const int dep
                             ASN1_STRING *val = X509_NAME_ENTRY_get_data(ent);
                             unsigned char *buf;
                             buf = (unsigned char *)1; /* bug in OpenSSL 0.9.6b ASN1_STRING_to_UTF8 requires this workaround */
-                            if (ASN1_STRING_to_UTF8(&buf, val) > 0)
+                            if (ASN1_STRING_to_UTF8(&buf, val) >= 0)
                             {
                                 do_setenv_x509(es, xt->name, (char *)buf, depth);
                                 OPENSSL_free(buf);
@@ -545,7 +570,7 @@ x509_setenv(struct env_set *es, int cert_depth, openvpn_x509_cert_t *peer_cert)
             continue;
         }
         buf = (unsigned char *)1; /* bug in OpenSSL 0.9.6b ASN1_STRING_to_UTF8 requires this workaround */
-        if (ASN1_STRING_to_UTF8(&buf, val) <= 0)
+        if (ASN1_STRING_to_UTF8(&buf, val) < 0)
         {
             continue;
         }
@@ -563,7 +588,7 @@ x509_setenv(struct env_set *es, int cert_depth, openvpn_x509_cert_t *peer_cert)
 }
 
 result_t
-x509_verify_ns_cert_type(const openvpn_x509_cert_t *peer_cert, const int usage)
+x509_verify_ns_cert_type(openvpn_x509_cert_t *peer_cert, const int usage)
 {
     if (usage == NS_CERT_CHECK_NONE)
     {
@@ -571,13 +596,59 @@ x509_verify_ns_cert_type(const openvpn_x509_cert_t *peer_cert, const int usage)
     }
     if (usage == NS_CERT_CHECK_CLIENT)
     {
-        return ((peer_cert->ex_flags & EXFLAG_NSCERT)
-                && (peer_cert->ex_nscert & NS_SSL_CLIENT)) ? SUCCESS : FAILURE;
+        /*
+         * Unfortunately, X509_check_purpose() does some weird thing that
+         * prevent it to take a const argument
+         */
+        result_t result = X509_check_purpose(peer_cert, X509_PURPOSE_SSL_CLIENT, 0) ?
+	       SUCCESS : FAILURE;
+
+        /*
+         * old versions of OpenSSL allow us to make the less strict check we used to
+         * do. If this less strict check pass, warn user that this might not be the
+         * case when its distribution will update to OpenSSL 1.1
+         */
+        if (result == FAILURE)
+        {
+            ASN1_BIT_STRING *ns;
+            ns = X509_get_ext_d2i(peer_cert, NID_netscape_cert_type, NULL, NULL);
+            result = (ns && ns->length > 0 && (ns->data[0] & NS_SSL_CLIENT)) ? SUCCESS : FAILURE;
+            if (result == SUCCESS)
+            {
+                msg(M_WARN, "X509: Certificate is a client certificate yet it's purpose "
+                    "cannot be verified (check may fail in the future)");
+            }
+            ASN1_BIT_STRING_free(ns);
+        }
+        return result;
     }
     if (usage == NS_CERT_CHECK_SERVER)
     {
-        return ((peer_cert->ex_flags & EXFLAG_NSCERT)
-                && (peer_cert->ex_nscert & NS_SSL_SERVER))  ? SUCCESS : FAILURE;
+        /*
+         * Unfortunately, X509_check_purpose() does some weird thing that
+         * prevent it to take a const argument
+         */
+        result_t result = X509_check_purpose(peer_cert, X509_PURPOSE_SSL_SERVER, 0) ?
+	       SUCCESS : FAILURE;
+
+        /*
+         * old versions of OpenSSL allow us to make the less strict check we used to
+         * do. If this less strict check pass, warn user that this might not be the
+         * case when its distribution will update to OpenSSL 1.1
+         */
+        if (result == FAILURE)
+        {
+            ASN1_BIT_STRING *ns;
+            ns = X509_get_ext_d2i(peer_cert, NID_netscape_cert_type, NULL, NULL);
+            result = (ns && ns->length > 0 && (ns->data[0] & NS_SSL_SERVER)) ? SUCCESS : FAILURE;
+            if (result == SUCCESS)
+            {
+                msg(M_WARN, "X509: Certificate is a server certificate yet it's purpose "
+                    "cannot be verified (check may fail in the future)");
+            }
+            ASN1_BIT_STRING_free(ns);
+        }
+        return result;
     }
 
     return FAILURE;
@@ -587,55 +658,60 @@ result_t
 x509_verify_cert_ku(X509 *x509, const unsigned *const expected_ku,
                     int expected_len)
 {
-    ASN1_BIT_STRING *ku = NULL;
-    result_t fFound = FAILURE;
+    ASN1_BIT_STRING *ku = X509_get_ext_d2i(x509, NID_key_usage, NULL, NULL);
 
-    if ((ku = (ASN1_BIT_STRING *) X509_get_ext_d2i(x509, NID_key_usage, NULL,
-                                                   NULL)) == NULL)
+    if (ku == NULL)
     {
-        msg(D_HANDSHAKE, "Certificate does not have key usage extension");
+        msg(D_TLS_ERRORS, "Certificate does not have key usage extension");
+        return FAILURE;
     }
-    else
+
+    if (expected_ku[0] == OPENVPN_KU_REQUIRED)
     {
-        unsigned nku = 0;
-        int i;
-        for (i = 0; i < 8; i++)
-        {
-            if (ASN1_BIT_STRING_get_bit(ku, i))
-            {
-                nku |= 1 << (7 - i);
-            }
-        }
+        /* Extension required, value checked by TLS library */
+        ASN1_BIT_STRING_free(ku);
+        return SUCCESS;
+    }
 
-        /*
-         * Fixup if no LSB bits
-         */
-        if ((nku & 0xff) == 0)
+    unsigned nku = 0;
+    for (size_t i = 0; i < 8; i++)
+    {
+        if (ASN1_BIT_STRING_get_bit(ku, i))
         {
-            nku >>= 8;
+            nku |= 1 << (7 - i);
         }
+    }
 
-        msg(D_HANDSHAKE, "Validating certificate key usage");
-        for (i = 0; fFound != SUCCESS && i < expected_len; i++)
-        {
-            if (expected_ku[i] != 0)
-            {
-                msg(D_HANDSHAKE, "++ Certificate has key usage  %04x, expects "
-                    "%04x", nku, expected_ku[i]);
+    /*
+     * Fixup if no LSB bits
+     */
+    if ((nku & 0xff) == 0)
+    {
+        nku >>= 8;
+    }
 
-                if (nku == expected_ku[i])
-                {
-                    fFound = SUCCESS;
-                }
-            }
+    msg(D_HANDSHAKE, "Validating certificate key usage");
+    result_t fFound = FAILURE;
+    for (size_t i = 0; fFound != SUCCESS && i < expected_len; i++)
+    {
+        if (expected_ku[i] != 0 && (nku & expected_ku[i]) == expected_ku[i])
+        {
+            fFound = SUCCESS;
         }
     }
 
-    if (ku != NULL)
+    if (fFound != SUCCESS)
     {
-        ASN1_BIT_STRING_free(ku);
+        msg(D_TLS_ERRORS,
+            "ERROR: Certificate has key usage %04x, expected one of:", nku);
+        for (size_t i = 0; i < expected_len && expected_ku[i]; i++)
+        {
+            msg(D_TLS_ERRORS, " * %04x", expected_ku[i]);
+        }
     }
 
+    ASN1_BIT_STRING_free(ku);
+
     return fFound;
 }
 
@@ -714,11 +790,12 @@ tls_verify_crl_missing(const struct tls_options *opt)
         crypto_msg(M_FATAL, "Cannot get certificate store");
     }
 
-    for (int i = 0; i < sk_X509_OBJECT_num(store->objs); i++)
+    STACK_OF(X509_OBJECT) *objs = X509_STORE_get0_objects(store);
+    for (int i = 0; i < sk_X509_OBJECT_num(objs); i++)
     {
-        X509_OBJECT *obj = sk_X509_OBJECT_value(store->objs, i);
+        X509_OBJECT *obj = sk_X509_OBJECT_value(objs, i);
         ASSERT(obj);
-        if (obj->type == X509_LU_CRL)
+        if (X509_OBJECT_get_type(obj) == X509_LU_CRL)
         {
             return false;
         }