git: cb350ba7bf7c - main - kerberos: Fix numerous segfaults when using weak crypto

Reply: Jessica Clarke : "Re: git: cb350ba7bf7c - main - kerberos: Fix numerous segfaults when using weak crypto"
Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Cy Schubert <cy_at_FreeBSD.org>
Date: Thu, 11 Jan 2024 13:31:35 UTC
The branch main has been updated by cy:

URL: https://cgit.FreeBSD.org/src/commit/?id=cb350ba7bf7ca7c4cb97ed2c20ab45af60382cfb

commit cb350ba7bf7ca7c4cb97ed2c20ab45af60382cfb
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2023-12-06 15:30:05 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2024-01-11 13:26:42 +0000

    kerberos: Fix numerous segfaults when using weak crypto
    
    Weak crypto is provided by the openssl legacy provider which is
    not load by default. Load the legacy providers as needed.
    
    When the legacy provider is loaded into the default context the default
    provider will no longer be automatically loaded. Without the default
    provider the various kerberos applicaions and functions will abort().
    
    PR:                     272835
    MFC after:              3 days
    Differential Revision:  https://reviews.freebsd.org/D43009
    Tested by:              netchild, Joerg Pulz <Joerg.Pulz@frm2.tum.de>
---
 Makefile.inc1                                    |  7 ++--
 crypto/heimdal/lib/kadm5/create_s.c              |  4 +++
 crypto/heimdal/lib/kadm5/kadm5_locl.h            |  1 +
 crypto/heimdal/lib/krb5/context.c                |  4 +++
 crypto/heimdal/lib/krb5/crypto.c                 |  3 ++
 crypto/heimdal/lib/krb5/salt.c                   |  5 +++
 crypto/heimdal/lib/roken/version-script.map      |  1 +
 kerberos5/include/crypto-headers.h               |  4 +++
 kerberos5/include/fbsd_ossl_provider.h           |  4 +++
 kerberos5/lib/libroken/Makefile                  | 11 +++++--
 kerberos5/lib/libroken/fbsd_ossl_provider_load.c | 41 ++++++++++++++++++++++++
 kerberos5/libexec/kdc/Makefile                   |  2 +-
 share/mk/src.libnames.mk                         |  2 +-
 13 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/Makefile.inc1 b/Makefile.inc1
index 34b49ee319ec..864b5151692f 100644
--- a/Makefile.inc1
+++ b/Makefile.inc1
@@ -2635,9 +2635,10 @@ ${_bt}-usr.bin/grep: ${_bt}-lib/libbz2
 _other_bootstrap_tools+=lib/libz
 ${_bt}-lib/libdwarf: ${_bt}-lib/libz
 
-# libroken depends on libcrypt
+# libroken depends on libcrypt and libcrypto
 _other_bootstrap_tools+=lib/libcrypt
-${_bt}-lib/libroken: ${_bt}-lib/libcrypt
+_other_bootstrap_tools+=secure/lib/libcrypto
+${_bt}-lib/libroken: ${_bt}-lib/libcrypt ${_bt}-lib/libcrypto
 .else
 # All tools in _basic_bootstrap_tools have the same name as the subdirectory
 # so we can use :T to get the name of the symlinks that we need to create.
@@ -3283,7 +3284,7 @@ kerberos5/lib/libkrb5__L: kerberos5/lib/libasn1__L lib/libcom_err__L \
     lib/libcrypt__L secure/lib/libcrypto__L kerberos5/lib/libhx509__L \
     kerberos5/lib/libroken__L kerberos5/lib/libwind__L \
     kerberos5/lib/libheimbase__L kerberos5/lib/libheimipcc__L
-kerberos5/lib/libroken__L: lib/libcrypt__L
+kerberos5/lib/libroken__L: lib/libcrypt__L secure/lib/libcrypto__L
 kerberos5/lib/libwind__L: kerberos5/lib/libroken__L lib/libcom_err__L
 kerberos5/lib/libheimbase__L: lib/libthr__L
 kerberos5/lib/libheimipcc__L: kerberos5/lib/libroken__L kerberos5/lib/libheimbase__L lib/libthr__L
diff --git a/crypto/heimdal/lib/kadm5/create_s.c b/crypto/heimdal/lib/kadm5/create_s.c
index 1033ca103239..267e9bbda2a0 100644
--- a/crypto/heimdal/lib/kadm5/create_s.c
+++ b/crypto/heimdal/lib/kadm5/create_s.c
@@ -169,6 +169,10 @@ kadm5_s_create_principal(void *server_handle,
     ent.entry.keys.len = 0;
     ent.entry.keys.val = NULL;
 
+    ret = fbsd_ossl_provider_load();
+    if (ret)
+	goto out;
+
     ret = _kadm5_set_keys(context, &ent.entry, password);
     if (ret)
 	goto out;
diff --git a/crypto/heimdal/lib/kadm5/kadm5_locl.h b/crypto/heimdal/lib/kadm5/kadm5_locl.h
index 68b6a5ebf024..63b367ab7e21 100644
--- a/crypto/heimdal/lib/kadm5/kadm5_locl.h
+++ b/crypto/heimdal/lib/kadm5/kadm5_locl.h
@@ -79,5 +79,6 @@
 #include <der.h>
 #include <parse_units.h>
 #include "private.h"
+#include "fbsd_ossl_provider.h"
 
 #endif /* __KADM5_LOCL_H__ */
diff --git a/crypto/heimdal/lib/krb5/context.c b/crypto/heimdal/lib/krb5/context.c
index 86bfe539b974..681bc9a0982f 100644
--- a/crypto/heimdal/lib/krb5/context.c
+++ b/crypto/heimdal/lib/krb5/context.c
@@ -392,6 +392,10 @@ krb5_init_context(krb5_context *context)
     }
     HEIMDAL_MUTEX_init(p->mutex);
 
+    ret = fbsd_ossl_provider_load();
+    if(ret)
+	goto out;
+
     p->flags |= KRB5_CTX_F_HOMEDIR_ACCESS;
 
     ret = krb5_get_default_config_files(&files);
diff --git a/crypto/heimdal/lib/krb5/crypto.c b/crypto/heimdal/lib/krb5/crypto.c
index 67ecef62e875..6ee22609a4d5 100644
--- a/crypto/heimdal/lib/krb5/crypto.c
+++ b/crypto/heimdal/lib/krb5/crypto.c
@@ -2054,6 +2054,9 @@ krb5_crypto_init(krb5_context context,
 	*crypto = NULL;
 	return ret;
     }
+    ret = fbsd_ossl_provider_load();
+    if (ret)
+	return ret;
     (*crypto)->key.schedule = NULL;
     (*crypto)->num_key_usage = 0;
     (*crypto)->key_usage = NULL;
diff --git a/crypto/heimdal/lib/krb5/salt.c b/crypto/heimdal/lib/krb5/salt.c
index 5e4c8a1c8572..2b1fbee80ab6 100644
--- a/crypto/heimdal/lib/krb5/salt.c
+++ b/crypto/heimdal/lib/krb5/salt.c
@@ -43,6 +43,8 @@ krb5_salttype_to_string (krb5_context context,
     struct _krb5_encryption_type *e;
     struct salt_type *st;
 
+    (void) fbsd_ossl_provider_load();
+
     e = _krb5_find_enctype (etype);
     if (e == NULL) {
 	krb5_set_error_message(context, KRB5_PROG_ETYPE_NOSUPP,
@@ -75,6 +77,8 @@ krb5_string_to_salttype (krb5_context context,
     struct _krb5_encryption_type *e;
     struct salt_type *st;
 
+    (void) fbsd_ossl_provider_load();
+
     e = _krb5_find_enctype (etype);
     if (e == NULL) {
 	krb5_set_error_message(context, KRB5_PROG_ETYPE_NOSUPP,
@@ -196,6 +200,7 @@ krb5_string_to_key_data_salt_opaque (krb5_context context,
 			       enctype);
 	return KRB5_PROG_ETYPE_NOSUPP;
     }
+    (void) fbsd_ossl_provider_load();
     for(st = et->keytype->string_to_key; st && st->type; st++)
 	if(st->type == salt.salttype)
 	    return (*st->string_to_key)(context, enctype, password,
diff --git a/crypto/heimdal/lib/roken/version-script.map b/crypto/heimdal/lib/roken/version-script.map
index 72d2ea7e4f7c..bb2139ed74cc 100644
--- a/crypto/heimdal/lib/roken/version-script.map
+++ b/crypto/heimdal/lib/roken/version-script.map
@@ -13,6 +13,7 @@ HEIMDAL_ROKEN_1.0 {
 		ct_memcmp;
 		err;
 		errx;
+		fbsd_ossl_provider_load;
 		free_getarg_strings;
 		get_default_username;
 		get_window_size;
diff --git a/kerberos5/include/crypto-headers.h b/kerberos5/include/crypto-headers.h
index 3ae0d9624ffd..2cc870642964 100644
--- a/kerberos5/include/crypto-headers.h
+++ b/kerberos5/include/crypto-headers.h
@@ -17,5 +17,9 @@
 #include <openssl/ec.h>
 #include <openssl/ecdsa.h>
 #include <openssl/ecdh.h>
+#if defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+#include <openssl/provider.h>
+#include "fbsd_ossl_provider.h"
+#endif
 
 #endif /* __crypto_headers_h__ */
diff --git a/kerberos5/include/fbsd_ossl_provider.h b/kerberos5/include/fbsd_ossl_provider.h
new file mode 100644
index 000000000000..013983ca9f83
--- /dev/null
+++ b/kerberos5/include/fbsd_ossl_provider.h
@@ -0,0 +1,4 @@
+#ifndef __fbsd_ossl_provider_h
+#define __fbsd_ossl_provider_h
+int  fbsd_ossl_provider_load(void);
+#endif
diff --git a/kerberos5/lib/libroken/Makefile b/kerberos5/lib/libroken/Makefile
index 0c46ba6c4cb5..24dc3a5b2c45 100644
--- a/kerberos5/lib/libroken/Makefile
+++ b/kerberos5/lib/libroken/Makefile
@@ -2,7 +2,7 @@
 PACKAGE=	kerberos-lib
 
 LIB=	roken
-LIBADD=	crypt
+LIBADD=	crypt crypto
 VERSION_MAP=	${KRB5DIR}/lib/roken/version-script.map
 INCS=	roken.h \
 	roken-common.h \
@@ -74,15 +74,20 @@ SRCS=	base64.c \
 	vis.c \
 	warnerr.c \
 	write_pid.c \
-	xfree.c
+	xfree.c \
+	fbsd_ossl_provider_load.c
 
-CFLAGS+=-I${KRB5DIR}/lib/roken -I.
+CFLAGS+=-I${KRB5DIR}/lib/roken \
+	-I${SRCTOP}/kerberos5/include \
+	-I${KRB5DIR}/lib/krb5 \
+	-I${SRCTOP}/crypto/openssl/include -I.
 
 CLEANFILES= roken.h
 
 roken.h:
 	${MAKE_ROKEN} > ${.TARGET}
 
+
 .include <bsd.lib.mk>
 
 .PATH: ${KRB5DIR}/lib/roken
diff --git a/kerberos5/lib/libroken/fbsd_ossl_provider_load.c b/kerberos5/lib/libroken/fbsd_ossl_provider_load.c
new file mode 100644
index 000000000000..f49c8746c9e4
--- /dev/null
+++ b/kerberos5/lib/libroken/fbsd_ossl_provider_load.c
@@ -0,0 +1,41 @@
+#include <errno.h>
+#include <krb5_locl.h>
+
+static void fbsd_ossl_provider_unload(void);
+
+static OSSL_PROVIDER *legacy;
+static OSSL_PROVIDER *deflt;
+static int providers_loaded = 0;
+
+int
+fbsd_ossl_provider_load(void)
+{
+#if defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+	if (providers_loaded == 0) {
+		if ((legacy = OSSL_PROVIDER_load(NULL, "legacy")) == NULL)
+			return (EINVAL);
+		if ((deflt = OSSL_PROVIDER_load(NULL, "default")) == NULL) {
+			OSSL_PROVIDER_unload(legacy);
+			return (EINVAL);
+		}
+		if (atexit(fbsd_ossl_provider_unload)) {
+			fbsd_ossl_provider_unload();
+			return (errno);
+		}
+		providers_loaded = 1;
+	}
+#endif
+	return (0);
+}
+
+static void
+fbsd_ossl_provider_unload(void)
+{
+#if defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >= 3)
+	if (providers_loaded == 1) {
+		OSSL_PROVIDER_unload(legacy);
+		OSSL_PROVIDER_unload(deflt);
+		providers_loaded = 0;
+	}
+#endif
+}
diff --git a/kerberos5/libexec/kdc/Makefile b/kerberos5/libexec/kdc/Makefile
index 41fde9115c00..211f4f379054 100644
--- a/kerberos5/libexec/kdc/Makefile
+++ b/kerberos5/libexec/kdc/Makefile
@@ -11,7 +11,7 @@ SRCS=	config.c \
 
 CFLAGS+=-I${KRB5DIR}/lib/krb5 -I${KRB5DIR}/lib/asn1 -I${KRB5DIR}/lib/roken \
 	-I${KRB5DIR}/kdc -I${SRCTOP}/contrib/com_err ${LDAPCFLAGS}
-LIBADD=	kdc hdb krb5 roken crypt vers
+LIBADD=	kdc hdb krb5 roken crypt vers crypto
 LDFLAGS=${LDAPLDFLAGS}
 
 .include <bsd.prog.mk>
diff --git a/share/mk/src.libnames.mk b/share/mk/src.libnames.mk
index 33b8507a9eb9..e4fd3270a3d3 100644
--- a/share/mk/src.libnames.mk
+++ b/share/mk/src.libnames.mk
@@ -367,7 +367,7 @@ _DP_pam+=	ssh
 .if ${MK_NIS} != "no"
 _DP_pam+=	ypclnt
 .endif
-_DP_roken=	crypt
+_DP_roken=	crypt crypto
 _DP_kadm5clnt=	com_err krb5 roken
 _DP_kadm5srv=	com_err hdb krb5 roken
 _DP_heimntlm=	crypto com_err krb5 roken