Improve future OpenSSL compatibility

Avoid calling deprecated OpenSSL functions when compiling against
versions of OpenSSL where they are deprecated.  Add
-DOPENSSL_NO_DEPRECATED to the linux-clang-openssl CI build to help
detect calls to deprecated functions in the future.

Use const pointer variables to hold values retrieved by accessors
which will return const pointers in OpenSSL 4.0.  Define macros to
make certain functions accept these const pointers in versions of
OpenSSL where they don't already do so.

Use accessor functions instead of direct field access for ASN1_STRING
values, as the type will become opaque in OpenSSL 4.0.

The PKINIT code is written to assume that DHX support was not present
until OpenSSL 1.1, but it was added in release 1.0.2, causing a
compilation error against 1.0.2 from a double definition of
EVP_PKEY_DHX.  Minimally fix the compilation error.  (The custom DHX
marshalling code to support 1.0.x could be removed, as 1.0.2 is the
minimum version after commit f5bbfa4821cf590a4748f96d0e016bc0485e95c4,
but the plan is to remove 1.0.x compatibility shortly.)

Contains work by Frederik Wedel-Heinen
<frederik.wedel-heinen@dencrypt.dk> and Dimitri John Ledkov
<dimitri.ledkov@surgut.co.uk>.

[ghudson@mit.edu: combined numerous commits; added compatibility
macros; rewrote commit message; fixed 1.0.2 compatibility issue]

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index f12422d19a3b8ee3b1316bf9d63644027cb660de..97f1082c59a89dea7b564864ad74f256ff5db688 100644 (file)
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -16,17 +16,17 @@ jobs:
                     - name: linux-clang
                       os: ubuntu-latest
                       compiler: clang
-                      makevars: CPPFLAGS=-Werror
+                      flags: -Werror
                       configureopts: --enable-asan
                     - name: linux-clang-openssl
                       os: ubuntu-latest
                       compiler: clang
-                      makevars: CPPFLAGS=-Werror
+                      flags: -Werror -DOPENSSL_NO_DEPRECATED
                       configureopts: --with-crypto-impl=openssl
                     - name: linux-gcc
                       os: ubuntu-latest
                       compiler: gcc
-                      makevars: CPPFLAGS=-D_FORTIFY_SOURCE=3
+                      flags: -D_FORTIFY_SOURCE=3
         steps:
             - name: Checkout repository
               uses: actions/checkout@v1
@@ -39,7 +39,7 @@ jobs:
             - name: Build
               env:
                 CC: ${{ matrix.compiler }}
-                MAKEVARS: ${{ matrix.makevars }}
+                FLAGS: ${{ matrix.flags }}
                 CONFIGURE_OPTS:  ${{ matrix.configureopts }}
               run: |
                 # For the ksu tests, allow homedir access from other users.
@@ -49,7 +49,7 @@ jobs:
                 cd src
                 autoreconf
                 ./configure --enable-maintainer-mode --with-ldap $CONFIGURE_OPTS --prefix=$HOME/inst
-                make $MAKEVARS
+                make CPPFLAGS="$FLAGS"
                 make check
                 make install
                 (cd clients/ksu && make check-ksu)

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index d1fe18e5abfef240ddc8f9d78485bd7898d08d6f..aa969aa37cd2509249ae9b1942210053f639f7f9 100644 (file)
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -209,13 +209,20 @@ create_identifiers_from_stack(STACK_OF(X509) *sk,
 #define EVP_MD_CTX_new EVP_MD_CTX_create
 #define EVP_MD_CTX_free EVP_MD_CTX_destroy
 #define ASN1_STRING_get0_data ASN1_STRING_data
+#define X509_STORE_CTX_set0_trusted_stack X509_STORE_CTX_trusted_stack
 
 /*
- * 1.1 adds DHX support, which uses the RFC 3279 DomainParameters encoding we
+ * 1.0.2 adds DHX support, which uses the RFC 3279 DomainParameters encoding we
  * need for PKINIT.  For 1.0 we must use the original DH type when creating
  * EVP_PKEY objects.
  */
+#ifndef EVP_PKEY_DHX
 #define EVP_PKEY_DHX EVP_PKEY_DH
+#endif
+
+/* Make X509_NAME_print_ex() accept a const name pointer by adding a cast. */
+#define X509_NAME_print_ex(a, b, c, d)          \
+    X509_NAME_print_ex(a, (X509_NAME *)b, c, d)
 
 /* 1.1 makes many handle types opaque and adds accessors.  Add compatibility
  * versions of the new accessors we use for pre-1.1. */
@@ -295,6 +302,10 @@ compat_ECDSA_SIG_set0(ECDSA_SIG *sig, BIGNUM *r, BIGNUM *s)
 #define EVP_PKEY_get_bits EVP_PKEY_bits
 #define EVP_PKEY_get_base_id EVP_PKEY_base_id
 
+/* Make X509_dup() accept a const pointer by adding a cast. */
+#define X509_dup(a) X509_dup((X509 *)a)
+#define i2d_X509_NAME(a, b) i2d_X509_NAME((X509_NAME *)a, b)
+
 /*
  * Convert *dh to an EVP_PKEY object, taking ownership of *dh and setting it to
  * NULL.  On error, return NULL and do not take ownership of or change *dh.
@@ -318,6 +329,11 @@ dh_to_pkey(DH **dh)
 }
 #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
 
+#if OPENSSL_VERSION_NUMBER < 0x40000000L
+/* Make X509V3_EXT_d2i() accept a const pointer by adding a cast. */
+#define X509V3_EXT_d2i(a) X509V3_EXT_d2i((X509_EXTENSION *)a)
+#endif
+
 /* Encode a bignum as an ASN.1 integer in DER. */
 static int
 encode_bn_der(const BIGNUM *bn, uint8_t **der_out, int *len_out)
@@ -1134,6 +1150,13 @@ oerr_cert(krb5_context context, krb5_error_code code, X509_STORE_CTX *certctx,
     return oerr(context, code, _("%s (depth %d): %s"), msg, depth, errstr);
 }
 
+/* Convert an OpenSSL ASN.1 string value to krb5_data, without copying. */
+static inline krb5_data
+asn1string_to_data(ASN1_STRING *s)
+{
+    return make_data((char *)ASN1_STRING_get0_data(s), ASN1_STRING_length(s));
+}
+
 krb5_error_code
 pkinit_init_plg_crypto(krb5_context context,
                        pkinit_plg_crypto_context *cryptoctx)
@@ -1775,7 +1798,7 @@ cms_signeddata_create(krb5_context context,
             goto cleanup;
         X509_STORE_CTX_init(certctx, certstore, id_cryptoctx->my_cert,
                             id_cryptoctx->intermediateCAs);
-        X509_STORE_CTX_trusted_stack(certctx, id_cryptoctx->trustedCAs);
+        X509_STORE_CTX_set0_trusted_stack(certctx, id_cryptoctx->trustedCAs);
         if (!X509_verify_cert(certctx)) {
             retval = oerr_cert(context, 0, certctx,
                                _("Failed to verify own certificate"));
@@ -2002,7 +2025,7 @@ cms_signeddata_verify(krb5_context context,
         unsigned char *d;
         *is_signed = 0;
         octets = CMS_get0_content(cms);
-        if (!octets || ((*octets)->type != V_ASN1_OCTET_STRING)) {
+        if (!octets || (ASN1_STRING_type(*octets) != V_ASN1_OCTET_STRING)) {
             retval = KRB5KDC_ERR_PREAUTH_FAILED;
             krb5_set_error_message(context, retval,
                                    _("Invalid pkinit packet: octet string "
@@ -2058,7 +2081,8 @@ cms_signeddata_verify(krb5_context context,
         /* We cannot use CMS_dataInit because there may be no digest */
         octets = CMS_get0_content(cms);
         if (octets)
-            out = BIO_new_mem_buf((*octets)->data, (*octets)->length);
+            out = BIO_new_mem_buf(ASN1_STRING_get0_data(*octets),
+                                  ASN1_STRING_length(*octets));
         if (out == NULL)
             goto cleanup;
     } else {
@@ -2118,7 +2142,7 @@ cms_signeddata_verify(krb5_context context,
 
         /* add trusted CAs certificates for cert verification */
         if (idctx->trustedCAs != NULL)
-            X509_STORE_CTX_trusted_stack(cert_ctx, idctx->trustedCAs);
+            X509_STORE_CTX_set0_trusted_stack(cert_ctx, idctx->trustedCAs);
         else {
             pkiDebug("unable to find any trusted CAs\n");
             goto cleanup;
@@ -2156,7 +2180,7 @@ cms_signeddata_verify(krb5_context context,
         i = X509_verify_cert(cert_ctx);
         if (i <= 0) {
             int j = X509_STORE_CTX_get_error(cert_ctx);
-            X509 *cert;
+            const X509 *cert;
 
             cert = X509_STORE_CTX_get_current_cert(cert_ctx);
             reqctx->received_cert = X509_dup(cert);
@@ -2316,7 +2340,7 @@ crypto_retrieve_X509_sans(krb5_context context,
     krb5_principal *princs = NULL;
     char **upns = NULL;
     unsigned char **dnss = NULL;
-    X509_EXTENSION *ext = NULL;
+    const X509_EXTENSION *ext = NULL;
     GENERAL_NAMES *ialt = NULL;
     GENERAL_NAME *gen = NULL;
 
@@ -2379,8 +2403,7 @@ crypto_retrieve_X509_sans(krb5_context context,
         gen = sk_GENERAL_NAME_value(ialt, i);
         switch (gen->type) {
         case GEN_OTHERNAME:
-            name.length = gen->d.otherName->value->value.sequence->length;
-            name.data = (char *)gen->d.otherName->value->value.sequence->data;
+            name = asn1string_to_data(gen->d.otherName->value->value.sequence);
             if (princs != NULL &&
                 OBJ_cmp(plgctx->id_pkinit_san,
                         gen->d.otherName->type_id) == 0) {
@@ -2414,12 +2437,13 @@ crypto_retrieve_X509_sans(krb5_context context,
         case GEN_DNS:
             if (dnss != NULL) {
                 /* Prevent abuse of embedded null characters. */
-                if (memchr(gen->d.dNSName->data, '\0', gen->d.dNSName->length))
+                if (memchr(ASN1_STRING_get0_data(gen->d.dNSName), '\0',
+                           ASN1_STRING_length(gen->d.dNSName)))
                     break;
                 pkiDebug("%s: found dns name = %s\n", __FUNCTION__,
-                         gen->d.dNSName->data);
+                         ASN1_STRING_get0_data(gen->d.dNSName));
                 dnss[d] = (unsigned char *)
-                    strdup((char *)gen->d.dNSName->data);
+                    strdup((char *)ASN1_STRING_get0_data(gen->d.dNSName));
                 if (dnss[d] == NULL) {
                     pkiDebug("%s: failed to duplicate dns name\n",
                              __FUNCTION__);
@@ -3094,8 +3118,10 @@ int
 pkinit_openssl_init(void)
 {
     /* Initialize OpenSSL. */
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
     ERR_load_crypto_strings();
     OpenSSL_add_all_algorithms();
+#endif
     return 0;
 }
 
@@ -4766,7 +4792,7 @@ out:
 }
 
 static krb5_error_code
-rfc2253_name(X509_NAME *name, char **str_out)
+rfc2253_name(const X509_NAME *name, char **str_out)
 {
     BIO *b = NULL;
     char *str;
@@ -5227,7 +5253,7 @@ create_identifiers_from_stack(STACK_OF(X509) *sk,
     int i = 0, sk_size = sk_X509_num(sk);
     krb5_external_principal_identifier **krb5_cas = NULL;
     X509 *x = NULL;
-    X509_NAME *xn = NULL;
+    const X509_NAME *xn = NULL;
     unsigned char *p = NULL;
     int len = 0;
     PKCS7_ISSUER_AND_SERIAL *is = NULL;

diff --git a/src/plugins/tls/k5tls/openssl.c b/src/plugins/tls/k5tls/openssl.c
index 42d72dc9ec2ec278966db351e1b45ae06926f0f8..7763327b7a8b16fbb960da4acdd2a3da6a116160 100644 (file)
--- a/src/plugins/tls/k5tls/openssl.c
+++ b/src/plugins/tls/k5tls/openssl.c
@@ -38,6 +38,24 @@
 #include <openssl/x509v3.h>
 #include <dirent.h>
 
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+/* Make X509_get_subject_name() accept a const pointer by adding a cast. */
+#define X509_get_subject_name(a) X509_get_subject_name((X509 *)a)
+
+/* OpenSSL 1.0 did not have TLS_client_method(); use the best alternative. */
+#define TLS_client_method() SSLv23_client_method()
+#endif
+
+#if OPENSSL_VERSION_NUMBER < 0x40000000L
+/*
+ * OpenSSL 4.0 constifies the result of X509_STORE_CTX_get_current_cert() and
+ * the input of X509_check_host() and X509_check_ip_asc().  For prior versions,
+ * make the latter two functions accept const pointers via a cast.
+ */
+#define X509_check_host(a, b, c, d, e) X509_check_host((X509 *)a, b, c, d, e)
+#define X509_check_ip_asc(a, b, c) X509_check_ip_asc((X509 *)a, b, c)
+#endif
+
 struct k5_tls_handle_st {
     SSL *ssl;
     char *servername;
@@ -51,9 +69,11 @@ MAKE_INIT_FUNCTION(init_openssl);
 int
 init_openssl(void)
 {
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
     SSL_library_init();
     SSL_load_error_strings();
     OpenSSL_add_all_algorithms();
+#endif
     ex_context_id = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL);
     ex_handle_id = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL);
     return 0;
@@ -72,7 +92,7 @@ flush_errors(krb5_context context)
 }
 
 static krb5_boolean
-check_cert_name_or_ip(X509 *x, const char *expected_name)
+check_cert_name_or_ip(const X509 *x, const char *expected_name)
 {
     struct in_addr in;
     struct in6_addr in6;
@@ -89,7 +109,7 @@ check_cert_name_or_ip(X509 *x, const char *expected_name)
 static int
 verify_callback(int preverify_ok, X509_STORE_CTX *store_ctx)
 {
-    X509 *x;
+    const X509 *x;
     SSL *ssl;
     BIO *bio;
     krb5_context context;
@@ -246,7 +266,7 @@ setup(krb5_context context, SOCKET fd, const char *servername,
         return KRB5_PLUGIN_OP_NOTSUPP;
 
     /* Do general SSL library setup. */
-    ctx = SSL_CTX_new(SSLv23_client_method());
+    ctx = SSL_CTX_new(TLS_client_method());
     if (ctx == NULL)
         goto error;