From nobody Thu Jan 18 08:03:10 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4TFwH10C5Cz57m7W for ; Thu, 18 Jan 2024 08:03:25 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4TFwH056YXz41np for ; Thu, 18 Jan 2024 08:03:24 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-wm1-f47.google.com with SMTP id 5b1f17b1804b1-40e958cd226so109455e9.2 for ; Thu, 18 Jan 2024 00:03:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705565002; x=1706169802; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=aqRe+TolKcjGqYGOim8ehbHUp7FOUnM+4p7X8FD4gDc=; b=mpLe0nnoaom6TWWnzzq8vbNHTrd2HMoz6zBXTJK77ng0Oy2eOPTw12D9cSW5iq6NwM 3C0dPJCzq/f2YZ6Mpb78xfU1Ki/r8SUj7vY+CgZG0IAwaMkQZVJ4e514p09XZ3RiUoDy HPCfSEr9AMVlqDc5EYzEGdjNIz+5IafZmrgG5YsNNuEbQjonoGjSmGtQ4/2bGz71V9J8 r4wKBO3hXzQoQBhgD9mjDTnkNg8zEoeGEgfd9dZ9xldeFGh9JNS/IbqpUsQdVgmk3aEn VBBWLI/cCcjovQe0FhHKMg7w1CX78VHL/vR/L5JXNBLpUVjCPYaGv7HipZaLXDZFasUB oU9A== X-Gm-Message-State: AOJu0YzU12oQFWzlyuGvsQwoF6QRMlqkZ7QkM6jn3/zXaju7010CGIoW QYUgDd3jecyuq6u28TOjx4zow+f3qsbIogbex+34RXiac7hSiqj91CPvlV4qGYV+Z6s5bOwbjL/ I X-Google-Smtp-Source: AGHT+IGT1OtKbOl6mIkpwTRCwrnAT2t3yABAjEgB4y4M95uIHJh9jlCRWs9gogoBiFJ/gFB7OFyE1Q== X-Received: by 2002:a1c:7919:0:b0:40d:4e28:43e8 with SMTP id l25-20020a1c7919000000b0040d4e2843e8mr243931wme.167.1705565001772; Thu, 18 Jan 2024 00:03:21 -0800 (PST) Received: from smtpclient.apple ([131.111.5.246]) by smtp.gmail.com with ESMTPSA id q8-20020a7bce88000000b0040c11fbe581sm24431581wmj.27.2024.01.18.00.03.21 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 18 Jan 2024 00:03:21 -0800 (PST) Content-Type: text/plain; charset=utf-8 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.200.91.1.1\)) Subject: Re: git: 476d63e091c2 - main - kerberos: Fix numerous segfaults when using weak crypto From: Jessica Clarke In-Reply-To: <202401180748.40I7mt58008173@gitrepo.freebsd.org> Date: Thu, 18 Jan 2024 08:03:10 +0000 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: References: <202401180748.40I7mt58008173@gitrepo.freebsd.org> To: Cy Schubert X-Mailer: Apple Mail (2.3774.200.91.1.1) X-Rspamd-Queue-Id: 4TFwH056YXz41np X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] On 18 Jan 2024, at 07:48, Cy Schubert wrote: >=20 > The branch main has been updated by cy: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D476d63e091c2e663b51d18acf6acb282= e1f22bbc >=20 > commit 476d63e091c2e663b51d18acf6acb282e1f22bbc > Author: Cy Schubert > AuthorDate: 2023-12-06 15:30:05 +0000 > Commit: Cy Schubert > CommitDate: 2024-01-18 07:46:57 +0000 >=20 > kerberos: Fix numerous segfaults when using weak crypto >=20 > Weak crypto is provided by the openssl legacy provider which is > not load by default. Load the legacy providers as needed. >=20 > 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(). >=20 > This is the second attempt at this patch. Instead of linking > secure/lib/libcrypto at build time we now link it at runtime, = avoiding > buildworld failures under Linux and MacOS. This is because > TARGET_ENDIANNESS is undefined at pre-build time. >=20 > PR: 272835 > MFC after: 3 days > X-MFC: only to stable/14 > Tested by: netchild > Joerg Pulz (previous = version) > --- > 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 | 8 ++- > kerberos5/lib/libroken/fbsd_ossl_provider_load.c | 77 = ++++++++++++++++++++++++ > 10 files changed, 109 insertions(+), 2 deletions(-) >=20 > 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 =3D 0; > ent.entry.keys.val =3D NULL; >=20 > + ret =3D fbsd_ossl_provider_load(); > + if (ret) > + goto out; > + > ret =3D _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 > #include > #include "private.h" > +#include "fbsd_ossl_provider.h" >=20 > #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); >=20 > + ret =3D fbsd_ossl_provider_load(); > + if(ret) > + goto out; > + > p->flags |=3D KRB5_CTX_F_HOMEDIR_ACCESS; >=20 > ret =3D 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 =3D NULL; > return ret; > } > + ret =3D fbsd_ossl_provider_load(); > + if (ret) > + return ret; > (*crypto)->key.schedule =3D NULL; > (*crypto)->num_key_usage =3D 0; > (*crypto)->key_usage =3D 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; >=20 > + (void) fbsd_ossl_provider_load(); > + > e =3D _krb5_find_enctype (etype); > if (e =3D=3D 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; >=20 > + (void) fbsd_ossl_provider_load(); > + > e =3D _krb5_find_enctype (etype); > if (e =3D=3D 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 =3D et->keytype->string_to_key; st && st->type; st++) > if(st->type =3D=3D 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 > #include > #include > +#if defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >=3D 3) > +#include > +#include "fbsd_ossl_provider.h" > +#endif >=20 > #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..ca6d090e64f0 100644 > --- a/kerberos5/lib/libroken/Makefile > +++ b/kerberos5/lib/libroken/Makefile > @@ -74,9 +74,13 @@ SRCS=3D base64.c \ > vis.c \ > warnerr.c \ > write_pid.c \ > - xfree.c > + xfree.c \ > + fbsd_ossl_provider_load.c >=20 > -CFLAGS+=3D-I${KRB5DIR}/lib/roken -I. > +CFLAGS+=3D-I${KRB5DIR}/lib/roken \ > + -I${SRCTOP}/kerberos5/include \ > + -I${KRB5DIR}/lib/krb5 \ > + -I${SRCTOP}/crypto/openssl/include -I. >=20 > CLEANFILES=3D roken.h >=20 > 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..497b32124f96 > --- /dev/null > +++ b/kerberos5/lib/libroken/fbsd_ossl_provider_load.c > @@ -0,0 +1,77 @@ > +#include > +#include > +#include > +#include > +#include > + > +#if defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >=3D 3) > +static void fbsd_ossl_provider_unload(void); > +static void print_dlerror(char *); > +static OSSL_PROVIDER *legacy; > +static OSSL_PROVIDER *deflt; > +static int providers_loaded =3D 0; > +static OSSL_PROVIDER * (*ossl_provider_load)(OSSL_LIB_CTX *, const = char*) =3D NULL; > +static int (*ossl_provider_unload)(OSSL_PROVIDER *) =3D NULL; > +static void *crypto_lib_handle =3D NULL; > + > +static void > +fbsd_ossl_provider_unload(void) > +{ > + if (ossl_provider_unload =3D=3D NULL) { > + if (!(ossl_provider_unload =3D (int (*)(OSSL_PROVIDER*)) = dlsym(crypto_lib_handle, "OSSL_PROVIDER_unload"))) { > + print_dlerror("Unable to link OSSL_PROVIDER_unload"); > + return; > + } > + } > + if (providers_loaded =3D=3D 1) { > + (*ossl_provider_unload)(legacy); > + (*ossl_provider_unload)(deflt); > + providers_loaded =3D 0; > + } > +} > + > +static void > +print_dlerror(char *message) > +{ > + char *errstr; > + > + if ((errstr =3D dlerror()) !=3D NULL) > + fprintf(stderr, "%s: %s\n", > + message, errstr); > +} > +#endif > + > +int > +fbsd_ossl_provider_load(void) > +{ > +#if defined(OPENSSL_VERSION_MAJOR) && (OPENSSL_VERSION_MAJOR >=3D 3) > + if (crypto_lib_handle =3D=3D NULL) { > + if (!(crypto_lib_handle =3D dlopen("/usr/lib/libcrypto.so", Doesn=E2=80=99t this need to account for libcompat? And probably best to = use the full soname here so old binaries don=E2=80=99t use newer = incompatible libraries (or new use old)? At which point can you not just drop the path and use the file name on its own? That is, I would expect this to be just "libcrypto.so.30". Also you can direct link just fine so long as bootstrapping turns it off, so you can simplify this unless there are other motivating reasons for using dlopen. Jess > + RTLD_LAZY|RTLD_GLOBAL))) { > + print_dlerror("Unable to load libcrypto.so"); > + return (EINVAL); > + } > + } > + if (ossl_provider_load =3D=3D NULL) { > + if (!(ossl_provider_load =3D (OSSL_PROVIDER * (*)(OSSL_LIB_CTX*, = const char *)) dlsym(crypto_lib_handle, "OSSL_PROVIDER_load"))) { > + print_dlerror("Unable to link OSSL_PROVIDER_load"); > + return(ENOENT); > + } > + } > + > + if (providers_loaded =3D=3D 0) { > + if ((legacy =3D (*ossl_provider_load)(NULL, "legacy")) =3D=3D NULL) > + return (EINVAL); > + if ((deflt =3D (*ossl_provider_load)(NULL, "default")) =3D=3D NULL) = { > + (*ossl_provider_unload)(legacy); > + return (EINVAL); > + } > + if (atexit(fbsd_ossl_provider_unload)) { > + fbsd_ossl_provider_unload(); > + return (errno); > + } > + providers_loaded =3D 1; > + } > +#endif > + return (0); > +}