PR#27: Instrument with USDT probes related to SHA-1 deprecation - rpms/openssl

rpms / openssl

#27 Instrument with USDT probes related to SHA-1 deprecation

Merged 2 years ago by clang. Opened 2 years ago by asosedkin.

rpms/ asosedkin/openssl sha1-usdt into rawhide

Instrument with USDT probes related to SHA-1 deprecation

Alexander Sosedkin • 2 years ago

8f08128

0053-Add-SHA1-probes.patch

file added

+235

		`@@ -0,0 +1,235 @@`
		`+ From a1905af412163cf971107f51a33dff8b416ab690 Mon Sep 17 00:00:00 2001`
		`+ From: Clemens Lang <cllang@redhat.com>`
		`+ Date: Mon, 25 Apr 2022 15:21:46 +0200`
		`+ Subject: [PATCH] Instrument SHA-1 signatures with USDT probes`
		`+`
		`+ In order to discover remaining uses of SHA-1 in signatures without`
		`+ forcefully breaking the code paths, add USDT probes that can be queried`
		`+ with systemtap at runtime.`
		`+`
		`+ This should allow identifying components that still use SHA-1 signatures`
		`+ in production so that they can be transitioned to more modern hash`
		`+ algorithms.`
		`+ ---`
		`+ crypto/evp/m_sigver.c \| 13 +++++++++----`
		`+ crypto/evp/pmeth_lib.c \| 13 +++++++++----`
		`+ crypto/x509/x509_vfy.c \| 6 +++++-`
		`+ providers/common/securitycheck.c \| 22 +++++++++++++++-------`
		`+ providers/common/securitycheck_default.c \| 13 +++++++++++--`
		`+ ssl/t1_lib.c \| 8 +++++++-`
		`+ 6 files changed, 56 insertions(+), 19 deletions(-)`
		`+`
		`+ diff --git a/crypto/evp/m_sigver.c b/crypto/evp/m_sigver.c`
		`+ index 8da2183ce0..c17cdfa5d5 100644`
		`+ --- a/crypto/evp/m_sigver.c`
		`+ +++ b/crypto/evp/m_sigver.c`
		`+ @@ -16,6 +16,8 @@`
		`+ #include "internal/numbers.h" /* includes SIZE_MAX */`
		`+ #include "evp_local.h"`
		`+`
		`+ +#include <sys/sdt.h>`
		`+ +`
		`+ typedef struct ossl_legacy_digest_signatures_st {`
		`+ int allowed;`
		`+ } OSSL_LEGACY_DIGEST_SIGNATURES;`
		`+ @@ -336,10 +338,13 @@ static int do_sigver_init(EVP_MD_CTX ctx, EVP_PKEY_CTX *pctx,`
		`+ && !EVP_PKEY_is_a(locpctx->pkey, SN_tls1_prf)`
		`+ && !EVP_PKEY_is_a(locpctx->pkey, SN_hkdf)) {`
		`+ int mdnid = EVP_MD_nid(ctx->reqdigest);`
		`+ - if (!ossl_ctx_legacy_digest_signatures_allowed(locpctx->libctx, 0)`
		`+ - && (mdnid == NID_sha1 \|\| mdnid == NID_md5_sha1)) {`
		`+ - ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_DIGEST);`
		`+ - goto err;`
		`+ + if (mdnid == NID_sha1 \|\| mdnid == NID_md5_sha1) {`
		`+ + if (!ossl_ctx_legacy_digest_signatures_allowed(locpctx->libctx, 0)) {`
		`+ + ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_DIGEST);`
		`+ + goto err;`
		`+ + } else {`
		`+ + DTRACE_PROBE1(libcrypto, fedora_do_sigver_init_1, mdnid);`
		`+ + }`
		`+ }`
		`+ }`
		`+`
		`+ diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c`
		`+ index 3c5a1e6f5d..589a352974 100644`
		`+ --- a/crypto/evp/pmeth_lib.c`
		`+ +++ b/crypto/evp/pmeth_lib.c`
		`+ @@ -36,6 +36,8 @@`
		`+ #include "internal/sslconf.h"`
		`+ #include "evp_local.h"`
		`+`
		`+ +#include <sys/sdt.h>`
		`+ +`
		`+ #ifndef FIPS_MODULE`
		`+`
		`+ static int evp_pkey_ctx_store_cached_data(EVP_PKEY_CTX *ctx,`
		`+ @@ -954,10 +956,13 @@ static int evp_pkey_ctx_set_md(EVP_PKEY_CTX ctx, const EVP_MD md,`
		`+ && !EVP_PKEY_is_a(ctx->pkey, SN_tls1_prf)`
		`+ && !EVP_PKEY_is_a(ctx->pkey, SN_hkdf)) {`
		`+ int mdnid = EVP_MD_nid(md);`
		`+ - if ((mdnid == NID_sha1 \|\| mdnid == NID_md5_sha1)`
		`+ - && !ossl_ctx_legacy_digest_signatures_allowed(ctx->libctx, 0)) {`
		`+ - ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_DIGEST);`
		`+ - return -1;`
		`+ + if (mdnid == NID_sha1 \|\| mdnid == NID_md5_sha1) {`
		`+ + if (!ossl_ctx_legacy_digest_signatures_allowed(ctx->libctx, 0)) {`
		`+ + ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_DIGEST);`
		`+ + return -1;`
		`+ + } else {`
		`+ + DTRACE_PROBE1(libcrypto, fedora_evp_pkey_ctx_set_md_1, mdnid);`
		`+ + }`
		`+ }`
		`+ }`
		`+`
		`+ diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c`
		`+ index 60aa26f552..d054acd5a7 100644`
		`+ --- a/crypto/x509/x509_vfy.c`
		`+ +++ b/crypto/x509/x509_vfy.c`
		`+ @@ -29,6 +29,8 @@`
		`+ #include "crypto/x509.h"`
		`+ #include "x509_local.h"`
		`+`
		`+ +#include <sys/sdt.h>`
		`+ +`
		`+ /* CRL score values */`
		`+`
		`+ #define CRL_SCORE_NOCRITICAL 0x100 /* No unhandled critical extensions */`
		`+ @@ -3462,10 +3464,12 @@ static int check_sig_level(X509_STORE_CTX ctx, X509 cert)`
		`+`
		`+ if (nid == NID_sha1`
		`+ && ossl_ctx_legacy_digest_signatures_allowed(libctx, 0)`
		`+ - && ctx->param->auth_level < 2)`
		`+ + && ctx->param->auth_level < 2) {`
		`+ + DTRACE_PROBE1(libcrypto, fedora_check_sig_level_1, nid);`
		`+ /* When rh-allow-sha1-signatures = yes and security level <= 1,`
		`+ * explicitly allow SHA1 for backwards compatibility. */`
		`+ return 1;`
		`+ + }`
		`+`
		`+ return secbits >= minbits_table[level - 1];`
		`+ }`
		`+ diff --git a/providers/common/securitycheck.c b/providers/common/securitycheck.c`
		`+ index e534ad0a5f..bf496450cf 100644`
		`+ --- a/providers/common/securitycheck.c`
		`+ +++ b/providers/common/securitycheck.c`
		`+ @@ -21,6 +21,8 @@`
		`+ #include "prov/securitycheck.h"`
		`+ #include "internal/sslconf.h"`
		`+`
		`+ +#include <sys/sdt.h>`
		`+ +`
		`+ /*`
		`+ * FIPS requires a minimum security strength of 112 bits (for encryption or`
		`+ * signing), and for legacy purposes 80 bits (for decryption or verifying).`
		`+ @@ -238,11 +240,14 @@ int ossl_digest_get_approved_nid_with_sha1(OSSL_LIB_CTX ctx, const EVP_MD md,`
		`+ # endif /* OPENSSL_NO_FIPS_SECURITYCHECKS */`
		`+`
		`+ #ifndef FIPS_MODULE`
		`+ - if (!ossl_ctx_legacy_digest_signatures_allowed(ctx, 0))`
		`+ - /* SHA1 is globally disabled, check whether we want to locally allow`
		`+ - * it. */`
		`+ - if (mdnid == NID_sha1 && !sha1_allowed)`
		`+ + if (mdnid == NID_sha1 && !sha1_allowed) {`
		`+ + if (!ossl_ctx_legacy_digest_signatures_allowed(ctx, 0))`
		`+ + /* SHA1 is globally disabled, check whether we want to locally allow`
		`+ + * it. */`
		`+ mdnid = -1;`
		`+ + else`
		`+ + DTRACE_PROBE1(libcrypto, fedora_ossl_digest_get_approved_nid_with_sha1_1, mdnid);`
		`+ + }`
		`+ #endif`
		`+`
		`+ return mdnid;`
		`+ @@ -258,9 +263,12 @@ int ossl_digest_is_allowed(OSSL_LIB_CTX ctx, const EVP_MD md)`
		`+ #ifndef FIPS_MODULE`
		`+ {`
		`+ int mdnid = EVP_MD_nid(md);`
		`+ - if ((mdnid == NID_sha1 \|\| mdnid == NID_md5_sha1)`
		`+ - && !ossl_ctx_legacy_digest_signatures_allowed(ctx, 0))`
		`+ - return 0;`
		`+ + if (mdnid == NID_sha1 \|\| mdnid == NID_md5_sha1) {`
		`+ + if (!ossl_ctx_legacy_digest_signatures_allowed(ctx, 0))`
		`+ + return 0;`
		`+ + else`
		`+ + DTRACE_PROBE1(libcrypto, fedora_ossl_digest_is_allowed_1, mdnid);`
		`+ + }`
		`+ }`
		`+ #endif`
		`+`
		`+ diff --git a/providers/common/securitycheck_default.c b/providers/common/securitycheck_default.c`
		`+ index ce54a94fbc..ecb3a9d4b6 100644`
		`+ --- a/providers/common/securitycheck_default.c`
		`+ +++ b/providers/common/securitycheck_default.c`
		`+ @@ -17,6 +17,8 @@`
		`+ #include "internal/nelem.h"`
		`+ #include "internal/sslconf.h"`
		`+`
		`+ +#include <sys/sdt.h>`
		`+ +`
		`+ /* Disable the security checks in the default provider */`
		`+ int ossl_securitycheck_enabled(OSSL_LIB_CTX *libctx)`
		`+ {`
		`+ @@ -40,9 +42,16 @@ int ossl_digest_rsa_sign_get_md_nid(OSSL_LIB_CTX ctx, const EVP_MD md,`
		`+`
		`+ ldsigs_allowed = ossl_ctx_legacy_digest_signatures_allowed(ctx, 0);`
		`+ mdnid = ossl_digest_get_approved_nid_with_sha1(ctx, md, sha1_allowed \|\| ldsigs_allowed);`
		`+ + if (mdnid == NID_sha1)`
		`+ + /* This will only happen if SHA1 is allowed, otherwise mdnid is -1. */`
		`+ + DTRACE_PROBE1(libcrypto, fedora_ossl_digest_rsa_sign_get_md_nid_1, mdnid);`
		`+ if (mdnid == NID_undef)`
		`+ mdnid = ossl_digest_md_to_nid(md, name_to_nid, OSSL_NELEM(name_to_nid));`
		`+ - if (mdnid == NID_md5_sha1 && !ldsigs_allowed)`
		`+ - mdnid = -1;`
		`+ + if (mdnid == NID_md5_sha1) {`
		`+ + if (ldsigs_allowed)`
		`+ + DTRACE_PROBE1(libcrypto, fedora_ossl_digest_rsa_sign_get_md_nid_2, mdnid);`
		`+ + else`
		`+ + mdnid = -1;`
		`+ + }`
		`+ return mdnid;`
		`+ }`
		`+ diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c`
		`+ index e47ddf56f1..a4b9ff749e 100644`
		`+ --- a/ssl/t1_lib.c`
		`+ +++ b/ssl/t1_lib.c`
		`+ @@ -28,6 +28,8 @@`
		`+ #include "ssl_local.h"`
		`+ #include <openssl/ct.h>`
		`+`
		`+ +#include <sys/sdt.h>`
		`+ +`
		`+ static const SIGALG_LOOKUP find_sig_alg(SSL s, X509 x, EVP_PKEY pkey);`
		`+ static int tls12_sigalg_allowed(const SSL s, int op, const SIGALG_LOOKUP lu);`
		`+`
		`+ @@ -1568,6 +1570,7 @@ int tls12_check_peer_sigalg(SSL s, uint16_t sig, EVP_PKEY pkey)`
		`+ && SSL_get_security_level(s) < 2) {`
		`+ /* when rh-allow-sha1-signatures = yes and security level <= 1,`
		`+ * explicitly allow SHA1 for backwards compatibility */`
		`+ + DTRACE_PROBE1(libssl, fedora_tls12_check_peer_sigalg_1, lu->hash);`
		`+ } else {`
		`+ /*`
		`+ * Make sure security callback allows algorithm. For historical`
		`+ @@ -2120,6 +2123,7 @@ static int tls12_sigalg_allowed(const SSL s, int op, const SIGALG_LOOKUP lu)`
		`+ && SSL_get_security_level(s) < 2) {`
		`+ /* when rh-allow-sha1-signatures = yes and security level <= 1,`
		`+ * explicitly allow SHA1 for backwards compatibility */`
		`+ + DTRACE_PROBE1(libssl, fedora_tls12_sigalg_allowed_1, lu->hash);`
		`+ return 1;`
		`+ }`
		`+`
		`+ @@ -3018,10 +3022,12 @@ static int ssl_security_cert_sig(SSL s, SSL_CTX ctx, X509 *x, int op)`
		`+ && ossl_ctx_legacy_digest_signatures_allowed(libctx, 0)`
		`+ && ((s != NULL && SSL_get_security_level(s) < 2)`
		`+ \|\| (ctx != NULL && SSL_CTX_get_security_level(ctx) < 2)`
		`+ - ))`
		`+ + )) {`
		`+ /* When rh-allow-sha1-signatures = yes and security level <= 1,`
		`+ * explicitly allow SHA1 for backwards compatibility. */`
		`+ + DTRACE_PROBE1(libssl, fedora_ssl_security_cert_sig_1, nid);`
		`+ return 1;`
		`+ + }`
		`+`
		`+ if (s)`
		`+ return ssl_security(s, op, secbits, nid, x);`
		`+ --`
		`+ 2.35.1`

openssl.spec

file modified

+11 -1

		`@@ -15,7 +15,7 @@`
		`Summary: Utilities from the general purpose cryptography library with TLS implementation`
		`Name: openssl`
		`Version: 3.0.2`
		`- Release: 3%{?dist}`
		`+ Release: 4%{?dist}`
		`Epoch: 1`
		`# We have to remove certain patented algorithms from the openssl source`
		`# tarball with the hobble-openssl script which is included below.`
		`@@ -73,6 +73,12 @@`
		`# Support SHA1 in TLS in LEGACY crypto-policy (which is SECLEVEL=1)`
		`Patch52: 0052-Allow-SHA1-in-seclevel-1-if-rh-allow-sha1-signatures.patch`
		`%endif`
		`+ %if 0%{?rhel}`
		`+ # no USDT probe instrumentation required`
		`+ %else`
		`+ # Instrument with USDT probes related to SHA-1 deprecation`
		`+ Patch53: 0053-Add-SHA1-probes.patch`
		`+ %endif`

		`License: ASL 2.0`
		`URL: http://www.openssl.org/`
		`@@ -87,6 +93,7 @@`
		`BuildRequires: perl(Time::HiRes), perl(IPC::Cmd), perl(Pod::Html), perl(Digest::SHA)`
		`BuildRequires: perl(FindBin), perl(lib), perl(File::Compare), perl(File::Copy), perl(bigint)`
		`BuildRequires: git-core`
		`+ BuildRequires: systemtap-sdt-devel`
		`Requires: coreutils`
		`Requires: %{name}-libs%{?_isa} = %{epoch}:%{version}-%{release}`

		`@@ -403,6 +410,9 @@`
		`%ldconfig_scriptlets libs`

		`%changelog`
		`+ * Tue Apr 26 2022 Alexander Sosedkin <asosedkin@redhat.com> - 1:3.0.2-4`
		`+ - Instrument with USDT probes related to SHA-1 deprecation`
		`+`
		`* Wed Apr 20 2022 Clemens Lang <cllang@redhat.com> - 1:3.0.2-3`
		`- Disable SHA-1 by default in ELN using the patches from CentOS`
		`- Fix a FIXME in the openssl.cnf(5) manpage`

asosedkin commented 2 years ago

Here's an attempt to add probes to have a "logging" approach to report SHA-1 deprecation bugs for Fedora 37.

Tested to work using https://gitlab.com/redhat-crypto/fedora-sha1sig-tracer

asosedkin commented 2 years ago

(relaying Simo's note): downstream probes should be prefixed or something

zuul commented 2 years ago

Build succeeded.

check-for-arches : SUCCESS in 1m 31s
rpm-scratch-build : SUCCESS in 22m 45s
rpm-scratch-build-s390x : SUCCESS in 1h 11m 25s (non-voting)
rpm-scratch-build-ppc64le : SUCCESS in 29m 38s (non-voting)
rpm-scratch-build-i686 : SUCCESS in 18m 09s (non-voting)
rpm-scratch-build-aarch64 : SUCCESS in 44m 32s (non-voting)
rpm-linter : SUCCESS in 4m 09s
rpm-rpminspect : SUCCESS in 6m 30s (non-voting)
check-for-tests : SUCCESS in 46s
check-for-sti-tests : SUCCESS in 45s
check-for-fmf-tests : SUCCESS in 46s
rpm-install-test : SUCCESS in 2m 18s
rpm-test : SUCCESS in 4m 27s
rpm-tmt-test : SKIPPED
rpm-sti-test : SUCCESS in 11m 19s

rebased onto e386aed259c9f4f910243fd1ccde8f1c6ebec807

2 years ago

asosedkin commented 2 years ago

Good thing is, tracing works on Fedora 36 + bpftrace-0.14.0-1 + bpftrace with, e.g. the following program:

usdt:/usr/lib64/libcrypto.so.3:fedora_openssl_do_sigver_init { printf("do_sigver_init\n"); }
usdt:/usr/lib64/libcrypto.so.3:fedora_openssl_evp_pkey_ctx_set_md { printf("openssl_evp_pkey_ctx_set_md\n"); }                                                                                              usdt:/usr/lib64/libcrypto.so.3:fedora_ossl_digest_get_approved_nid_with_sha1 { printf("ossl_digest_get_approved_nid_with_sha1\n"); }
usdt:/usr/lib64/libcrypto.so.3:fedora_ossl_digest_rsa_sign_get_md_nid { printf("openssl_ossl_digest_rsa_sign_get_md_nid\n"); }
usdt:/usr/lib64/libcrypto.so.3:fedora_ossl_digest_is_allowed { printf("ossl_digest_is_allowed\n"); }
usdt:/usr/lib64/libcrypto.so.3:fedora_openssl_rsa_set_ctx_params { printf("openssl_rsa_set_ctx_params\n"); }
usdt:/usr/lib64/libcrypto.so.3:fedora_openssl_ssl_setup_sig_algs { printf("openssl_ssl_setup_sig_algs\n"); }

Bad thing is, I think I've managed to place'em wrong, so I'm misreading the disabling patches or something? I'll need help placing them in the places worth logging, which are code paths that would be blocked with rh-allow-sha1-signatures = no but currently proceed with a DEFAULT's yes.

zuul commented 2 years ago

Build succeeded.