From 19fde46942f550580d4c67831d627584bfa2aada Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Mon, 23 Sep 2024 17:27:01 +0200 Subject: [PATCH 1/2] CLIENT:PAM: replace deprecated `_pam_overwrite` with `sss_erase_mem_securely()` Resolves: https://github.com/SSSD/sssd/issues/7606 --- Makefile.am | 7 ++++++ src/sss_client/pam_sss.c | 38 +++++++++++++++-------------- src/sss_client/sss_pam_macros.h | 29 ----------------------- src/util/memory.c | 27 --------------------- src/util/memory_erase.c | 42 +++++++++++++++++++++++++++++++++ src/util/memory_erase.h | 25 ++++++++++++++++++++ src/util/util.h | 2 +- 7 files changed, 95 insertions(+), 75 deletions(-) create mode 100644 src/util/memory_erase.c create mode 100644 src/util/memory_erase.h diff --git a/Makefile.am b/Makefile.am index 627593f2aa5..2c3ff5f0da0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -703,6 +703,7 @@ dist_noinst_HEADERS = \ src/util/cert.h \ src/util/dlinklist.h \ src/util/debug.h \ + src/util/memory_erase.h \ src/util/util.h \ src/util/util_errors.h \ src/util/safe-format-string.h \ @@ -985,6 +986,7 @@ SSS_CRYPT_SOURCES = src/util/crypto/libcrypto/crypto_base64.c \ src/util/crypto/libcrypto/crypto_prng.c \ src/util/atomic_io.c \ src/util/memory.c \ + src/util/memory_erase.c \ $(NULL) SSS_CRYPT_CFLAGS = $(CRYPTO_CFLAGS) SSS_CRYPT_LIBS = $(CRYPTO_LIBS) @@ -1264,6 +1266,7 @@ libsss_util_la_SOURCES = \ src/util/util_ext.c \ src/util/util_preauth.c \ src/util/memory.c \ + src/util/memory_erase.c \ src/util/safe-format-string.c \ src/util/server.c \ src/util/signal.c \ @@ -4168,6 +4171,7 @@ pam_sss_la_SOURCES = \ src/sss_client/sss_cli.h \ src/util/atomic_io.c \ src/util/authtok-utils.c \ + src/util/memory_erase.c \ src/sss_client/sss_pam_macros.h \ src/sss_client/sss_pam_compat.h @@ -4692,6 +4696,7 @@ krb5_child_SOURCES = \ src/util/find_uid.c \ src/util/atomic_io.c \ src/util/memory.c \ + src/util/memory_erase.c \ src/util/authtok.c \ src/util/authtok-utils.c \ src/util/util.c \ @@ -4736,6 +4741,7 @@ ldap_child_SOURCES = \ src/util/sss_iobuf.c \ src/util/atomic_io.c \ src/util/memory.c \ + src/util/memory_erase.c \ src/util/authtok.c \ src/util/authtok-utils.c \ src/util/util.c \ @@ -4885,6 +4891,7 @@ oidc_child_SOURCES = \ src/oidc_child/oidc_child_json.c \ src/util/atomic_io.c \ src/util/memory.c \ + src/util/memory_erase.c \ src/util/strtonum.c \ $(NULL) oidc_child_CFLAGS = \ diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c index d1101e16c69..edac3591f3c 100644 --- a/src/sss_client/pam_sss.c +++ b/src/sss_client/pam_sss.c @@ -49,6 +49,7 @@ #include "util/atomic_io.h" #include "util/authtok-utils.h" #include "util/dlinklist.h" +#include "util/memory_erase.h" #include #define _(STRING) dgettext (PACKAGE, STRING) @@ -171,19 +172,19 @@ static void free_cert_list(struct cert_auth_info *list) static void overwrite_and_free_authtoks(struct pam_items *pi) { if (pi->pam_authtok != NULL) { - _pam_overwrite_n((void *)pi->pam_authtok, pi->pam_authtok_size); + sss_erase_mem_securely((void *)pi->pam_authtok, pi->pam_authtok_size); free((void *)pi->pam_authtok); pi->pam_authtok = NULL; } if (pi->pam_newauthtok != NULL) { - _pam_overwrite_n((void *)pi->pam_newauthtok, pi->pam_newauthtok_size); + sss_erase_mem_securely((void *)pi->pam_newauthtok, pi->pam_newauthtok_size); free((void *)pi->pam_newauthtok); pi->pam_newauthtok = NULL; } if (pi->first_factor != NULL) { - _pam_overwrite_n((void *)pi->first_factor, strlen(pi->first_factor)); + sss_erase_mem_securely((void *)pi->first_factor, strlen(pi->first_factor)); free((void *)pi->first_factor); pi->first_factor = NULL; } @@ -304,10 +305,10 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style, if (state == SSS_PAM_CONV_REENTER) { if (null_strcmp(answer, resp[0].resp) != 0) { logger(pamh, LOG_NOTICE, "Passwords do not match."); - _pam_overwrite((void *)resp[0].resp); + sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); free(resp[0].resp); if (answer != NULL) { - _pam_overwrite((void *) answer); + sss_erase_mem_securely((void *) answer, strlen(answer)); free(answer); answer = NULL; } @@ -322,7 +323,7 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style, ret = PAM_CRED_ERR; goto failed; } - _pam_overwrite((void *)resp[0].resp); + sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); free(resp[0].resp); } else { if (resp[0].resp == NULL) { @@ -330,7 +331,7 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style, answer = NULL; } else { answer = strndup(resp[0].resp, MAX_AUTHTOK_SIZE); - _pam_overwrite((void *)resp[0].resp); + sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); free(resp[0].resp); if(answer == NULL) { D(("strndup failed")); @@ -1616,7 +1617,7 @@ static int send_and_receive(pam_handle_t *pamh, struct pam_items *pi, done: if (buf != NULL ) { - _pam_overwrite_n((void *)buf, rd.len); + sss_erase_mem_securely((void *)buf, rd.len); free(buf); } free(repbuf); @@ -1642,7 +1643,7 @@ static int prompt_password(pam_handle_t *pamh, struct pam_items *pi, pi->pam_authtok_size=0; } else { pi->pam_authtok = strdup(answer); - _pam_overwrite((void *)answer); + sss_erase_mem_securely((void *)answer, strlen(answer)); free(answer); answer=NULL; if (pi->pam_authtok == NULL) { @@ -1781,11 +1782,11 @@ static int prompt_2fa(pam_handle_t *pamh, struct pam_items *pi, done: if (resp != NULL) { if (resp[0].resp != NULL) { - _pam_overwrite((void *)resp[0].resp); + sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); free(resp[0].resp); } if (resp[1].resp != NULL) { - _pam_overwrite((void *)resp[1].resp); + sss_erase_mem_securely((void *)resp[1].resp, strlen(resp[1].resp)); free(resp[1].resp); } @@ -1814,7 +1815,7 @@ static int prompt_2fa_single(pam_handle_t *pamh, struct pam_items *pi, pi->pam_authtok_size=0; } else { pi->pam_authtok = strdup(answer); - _pam_overwrite((void *)answer); + sss_erase_mem_securely((void *)answer, strlen(answer)); free(answer); answer=NULL; if (pi->pam_authtok == NULL) { @@ -1995,7 +1996,8 @@ static int prompt_passkey(pam_handle_t *pamh, struct pam_items *pi, done: if (resp != NULL) { if (resp[pin_idx].resp != NULL) { - _pam_overwrite((void *)resp[pin_idx].resp); + sss_erase_mem_securely((void *)resp[pin_idx].resp, + strlen(resp[pin_idx].resp)); free(resp[pin_idx].resp); } @@ -2278,7 +2280,7 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) } answer = strndup(resp[0].resp, MAX_AUTHTOK_SIZE); - _pam_overwrite((void *)resp[0].resp); + sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); free(resp[0].resp); resp[0].resp = NULL; if (answer == NULL) { @@ -2368,17 +2370,17 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) ret = PAM_SUCCESS; done: - _pam_overwrite((void *)answer); + sss_erase_mem_securely((void *)answer, strlen(answer)); free(answer); answer=NULL; if (resp != NULL) { if (resp[0].resp != NULL) { - _pam_overwrite((void *)resp[0].resp); + sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); free(resp[0].resp); } if (resp[1].resp != NULL) { - _pam_overwrite((void *)resp[1].resp); + sss_erase_mem_securely((void *)resp[1].resp, strlen(resp[1].resp)); free(resp[1].resp); } @@ -2408,7 +2410,7 @@ static int prompt_new_password(pam_handle_t *pamh, struct pam_items *pi) pi->pam_newauthtok_size=0; } else { pi->pam_newauthtok = strdup(answer); - _pam_overwrite((void *)answer); + sss_erase_mem_securely((void *)answer, strlen(answer)); free(answer); answer=NULL; if (pi->pam_newauthtok == NULL) { diff --git a/src/sss_client/sss_pam_macros.h b/src/sss_client/sss_pam_macros.h index 0a7e266010a..0832cf14a39 100644 --- a/src/sss_client/sss_pam_macros.h +++ b/src/sss_client/sss_pam_macros.h @@ -25,35 +25,6 @@ #ifndef _SSS_PAM_MACROS_H #define _SSS_PAM_MACROS_H -/* Older versions of the pam development headers do not include the - * _pam_overwrite_n(n,x) macro. This implementation is copied from - * the Fedora 11 _pam_macros.h. - */ -#ifdef HAVE_SECURITY__PAM_MACROS_H -# include -#endif /* HAVE_SECURITY__PAM_MACROS_H */ - -#ifndef _pam_overwrite -#define _pam_overwrite(x) \ -do { \ - register char *__xx__; \ - if ((__xx__=(x))) \ - while (*__xx__) \ - *__xx__++ = '\0'; \ -} while (0) -#endif /* _pam_overwrite */ - -#ifndef _pam_overwrite_n -#define _pam_overwrite_n(x,n) \ -do { \ - register char *__xx__; \ - register unsigned int __i__ = 0; \ - if ((__xx__=(x))) \ - for (;__i__ - -#else - -typedef void *(*_sss_memset_t)(void *, int, size_t); - -static volatile _sss_memset_t memset_func = memset; - -static void explicit_bzero(void *s, size_t n) -{ - memset_func(s, 0, n); -} - -#endif - - void sss_erase_krb5_data_securely(krb5_data *data) { if (data != NULL) { @@ -72,15 +54,6 @@ int sss_erase_talloc_mem_securely(void *p) return 0; } -void sss_erase_mem_securely(void *p, size_t size) -{ - if ((p == NULL) || (size == 0)) { - return; - } - - explicit_bzero(p, size); -} - struct mem_holder { void *mem; diff --git a/src/util/memory_erase.c b/src/util/memory_erase.c new file mode 100644 index 00000000000..d10d9ea042a --- /dev/null +++ b/src/util/memory_erase.c @@ -0,0 +1,42 @@ +/* + Copyright (C) 2024 Red Hat + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + 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. If not, see . +*/ + +#include "config.h" +#include + +#ifndef HAVE_EXPLICIT_BZERO + +typedef void *(*_sss_memset_t)(void *, int, size_t); + +static volatile _sss_memset_t memset_func = memset; + +static void explicit_bzero(void *s, size_t n) +{ + memset_func(s, 0, n); +} + +#endif + + +void sss_erase_mem_securely(void *p, size_t size) +{ + if ((p == NULL) || (size == 0)) { + return; + } + + explicit_bzero(p, size); +} diff --git a/src/util/memory_erase.h b/src/util/memory_erase.h new file mode 100644 index 00000000000..b75b5ff3e27 --- /dev/null +++ b/src/util/memory_erase.h @@ -0,0 +1,25 @@ +/* + Copyright (C) 2024 Red Hat + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + 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. If not, see . +*/ + +#ifndef __SSSD_MEMORY_ERASE_H__ +#define __SSSD_MEMORY_ERASE_H__ + +#include + +void sss_erase_mem_securely(void *p, size_t size); + +#endif /* __SSSD_MEMORY_ERASE_H__ */ diff --git a/src/util/util.h b/src/util/util.h index a3d297513bf..406c4178589 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -49,6 +49,7 @@ #include "util/sss_format.h" #include "util/sss_regexp.h" #include "util/debug.h" +#include "util/memory_erase.h" /* name of the monitor server instance */ #define SSSD_MONITOR_NAME "sssd" @@ -237,7 +238,6 @@ int sss_mem_attach(TALLOC_CTX *mem_ctx, void *ptr, void_destructor_fn_t *fn); * to make it possible to use it as talloc destructor. */ int sss_erase_talloc_mem_securely(void *p); -void sss_erase_mem_securely(void *p, size_t size); void sss_erase_krb5_data_securely(krb5_data *data); void sss_erase_krb5_creds_securely(krb5_creds *cred); From 40c74d819071d27c2bf1ae50eaec8aa6d155d248 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Wed, 18 Sep 2024 16:41:04 +0200 Subject: [PATCH 2/2] Revert "ci: allow deprecated functions during build" This reverts commit ef014b8b293ca7859dc8c30db4cdcfa343c3c477. --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 494784be292..aa27cb00d94 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,14 +29,14 @@ jobs: working-directory: x86_64 run: | source ../contrib/fedora/bashrc_sssd - make CFLAGS+="$SSS_WARNINGS -Werror -Wno-error=deprecated-declarations" + make CFLAGS+="$SSS_WARNINGS -Werror" - name: make check shell: bash working-directory: x86_64 run: | source ../contrib/fedora/bashrc_sssd - make CFLAGS+="$SSS_WARNINGS -Werror -Wno-error=deprecated-declarations" check + make CFLAGS+="$SSS_WARNINGS -Werror" check - name: make distcheck shell: bash