URL: https://github.com/SSSD/sssd/pull/5867 Author: ikerexxe Title: #5867: usertools: force local user for sssd process user Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5867/head:pr5867 git checkout pr5867
From 119fa3c2bb0f2e1a6f10fcb32e18512d9cf150b6 Mon Sep 17 00:00:00 2001 From: Iker Pedrosa <ipedr...@redhat.com> Date: Mon, 23 Aug 2021 12:04:42 +0200 Subject: [PATCH 1/2] usertools: force local user for sssd process user System hardening by forcing the sssd user to be loaded from a local database (/etc/passwd) instead of using any remote user. This could happen in very special conditions and might change the owner of the sssd databases and generate a denial of service. Signed-off-by: Iker Pedrosa <ipedr...@redhat.com> --- Makefile.am | 3 + src/tests/cwrap/Makefile.am | 8 ++- src/tests/cwrap/common_mock_nss_dl_load.c | 71 +++++++++++++++++++++++ src/tests/cwrap/common_mock_nss_dl_load.h | 30 ++++++++++ src/tests/cwrap/test_responder_common.c | 7 +++ src/tests/cwrap/test_usertools.c | 6 ++ src/util/nss_dl_load.c | 11 +++- src/util/nss_dl_load.h | 3 + src/util/nss_dl_load_extra.c | 51 ++++++++++++++++ src/util/usertools.c | 32 +++++++--- 10 files changed, 211 insertions(+), 11 deletions(-) create mode 100644 src/tests/cwrap/common_mock_nss_dl_load.c create mode 100644 src/tests/cwrap/common_mock_nss_dl_load.h create mode 100644 src/util/nss_dl_load_extra.c diff --git a/Makefile.am b/Makefile.am index f6bc9414d0..ebd92afd55 100644 --- a/Makefile.am +++ b/Makefile.am @@ -884,6 +884,7 @@ dist_noinst_HEADERS = \ src/tests/cmocka/test_expire_common.h \ src/tests/cmocka/test_sdap_access.h \ src/tests/cmocka/data_provider/mock_dp.h \ + src/tests/cwrap/common_mock_nss_dl_load.h \ src/sss_client/pam_message.h \ src/sss_client/ssh/sss_ssh_client.h \ src/sss_client/sudo/sss_sudo.h \ @@ -1265,6 +1266,8 @@ libsss_util_la_SOURCES = \ src/util/selinux.c \ src/util/sss_regexp.c \ src/util/sss_chain_id.c \ + src/util/nss_dl_load.c \ + src/util/nss_dl_load_extra.c \ $(NULL) libsss_util_la_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/src/tests/cwrap/Makefile.am b/src/tests/cwrap/Makefile.am index 6d3dc45e57..24e6c4a7c1 100644 --- a/src/tests/cwrap/Makefile.am +++ b/src/tests/cwrap/Makefile.am @@ -141,15 +141,18 @@ endif usertools_tests_SOURCES = \ test_usertools.c \ + common_mock_nss_dl_load.c \ + ../../../src/util/usertools.c \ + ../../../src/util/nss_dl_load.c \ $(NULL) usertools_tests_CFLAGS = \ $(AM_CFLAGS) \ $(NULL) usertools_tests_LDADD = \ + $(LIBADD_DL) \ $(CMOCKA_LIBS) \ $(POPT_LIBS) \ $(TALLOC_LIBS) \ - $(abs_top_builddir)/libsss_util.la \ $(abs_top_builddir)/libsss_debug.la \ $(abs_top_builddir)/libsss_test_common.la \ $(NULL) @@ -159,9 +162,11 @@ endif responder_common_tests_SOURCES =\ test_responder_common.c \ + common_mock_nss_dl_load.c \ $(SSSD_RESPONDER_IFACE_OBJ) \ ../../../src/responder/common/negcache_files.c \ ../../../src/util/nss_dl_load.c \ + ../../../src/util/usertools.c \ ../../../src/responder/common/negcache.c \ ../../../src/responder/common/responder_common.c \ ../../../src/responder/common/responder_packet.c \ @@ -179,7 +184,6 @@ responder_common_tests_LDADD = \ $(SSSD_LIBS) \ $(SELINUX_LIBS) \ $(SYSTEMD_DAEMON_LIBS) \ - $(abs_top_builddir)/libsss_util.la \ $(abs_top_builddir)/libsss_debug.la \ $(abs_top_builddir)/libsss_test_common.la \ $(abs_top_builddir)/libsss_iface.la \ diff --git a/src/tests/cwrap/common_mock_nss_dl_load.c b/src/tests/cwrap/common_mock_nss_dl_load.c new file mode 100644 index 0000000000..f42c5be383 --- /dev/null +++ b/src/tests/cwrap/common_mock_nss_dl_load.c @@ -0,0 +1,71 @@ +/* + Authors: + Iker Pedrosa <ipedr...@redhat.com> + + Copyright (C) 2021 Red Hat + + SSSD tests: Fake nss dl load + + 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 <http://www.gnu.org/licenses/>. +*/ + +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <dlfcn.h> + +#include "common_mock_nss_dl_load.h" + + +static enum nss_status +mock_getpwnam_r(const char *name, struct passwd *result, + char *buffer, size_t buflen, int *errnop) +{ + void *pwd_pointer = NULL; + int rc; + + rc = getpwnam_r(name, result, buffer, buflen, (struct passwd **)&pwd_pointer); + if (rc == 0 && pwd_pointer == result) { + return NSS_STATUS_SUCCESS; + } else if (rc == 0 && (result == NULL)) { + return NSS_STATUS_NOTFOUND; + } else { + return NSS_STATUS_UNAVAIL; + } +} + +static enum nss_status +mock_getpwuid_r(uid_t uid, struct passwd *result, + char *buffer, size_t buflen, int *errnop) +{ + void *pwd_pointer = NULL; + int rc; + + rc = getpwuid_r(uid, result, buffer, buflen, (struct passwd **)&pwd_pointer); + if (rc == 0 && pwd_pointer == result) { + return NSS_STATUS_SUCCESS; + } else if (rc == 0 && (result == NULL)) { + return NSS_STATUS_NOTFOUND; + } else { + return NSS_STATUS_UNAVAIL; + } +} + +errno_t mock_sss_load_nss_pw_symbols(struct sss_nss_ops *ops) +{ + ops->getpwnam_r = mock_getpwnam_r; + ops->getpwuid_r = mock_getpwuid_r; + + return EOK; +} diff --git a/src/tests/cwrap/common_mock_nss_dl_load.h b/src/tests/cwrap/common_mock_nss_dl_load.h new file mode 100644 index 0000000000..6db4114508 --- /dev/null +++ b/src/tests/cwrap/common_mock_nss_dl_load.h @@ -0,0 +1,30 @@ +/* + Authors: + Iker Pedrosa <ipedr...@redhat.com> + + Copyright (C) 2021 Red Hat + + SSSD tests: Fake nss dl load + + 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 <http://www.gnu.org/licenses/>. +*/ + +#ifndef __COMMON_MOCK_NSS_DL_LOAD_H_ +#define __COMMON_MOCK_NSS_DL_LOAD_H_ + +#include "util/nss_dl_load.h" + +errno_t mock_sss_load_nss_pw_symbols(struct sss_nss_ops *ops); + +#endif /* __COMMON_MOCK_NSS_DL_LOAD_H_ */ diff --git a/src/tests/cwrap/test_responder_common.c b/src/tests/cwrap/test_responder_common.c index 11cc3abd8d..571e95d36c 100644 --- a/src/tests/cwrap/test_responder_common.c +++ b/src/tests/cwrap/test_responder_common.c @@ -29,6 +29,13 @@ #include "util/util.h" #include "responder/common/responder.h" #include "tests/cmocka/common_mock.h" +#include "tests/cwrap/common_mock_nss_dl_load.h" + + +errno_t sss_load_nss_pw_symbols(struct sss_nss_ops *ops) +{ + return mock_sss_load_nss_pw_symbols(ops); +} /* Just to satisfy dependencies */ struct cli_protocol_version *register_cli_protocol_version(void) diff --git a/src/tests/cwrap/test_usertools.c b/src/tests/cwrap/test_usertools.c index f61ae83e20..eb30a540cb 100644 --- a/src/tests/cwrap/test_usertools.c +++ b/src/tests/cwrap/test_usertools.c @@ -27,6 +27,12 @@ #include <popt.h> #include "util/util.h" #include "tests/cmocka/common_mock.h" +#include "tests/cwrap/common_mock_nss_dl_load.h" + +errno_t sss_load_nss_pw_symbols(struct sss_nss_ops *ops) +{ + return mock_sss_load_nss_pw_symbols(ops); +} void test_get_user_num(void **state) { diff --git a/src/util/nss_dl_load.c b/src/util/nss_dl_load.c index 4421083077..2de3572582 100644 --- a/src/util/nss_dl_load.c +++ b/src/util/nss_dl_load.c @@ -48,6 +48,14 @@ static void *proxy_dlsym(void *handle, return funcptr; } +static void sss_close_handle(struct sss_nss_ops *ops, const char *libname) +{ + if (dlclose(ops->dl_handle) != 0) { + DEBUG(SSSDBG_OP_FAILURE, + "Error closing the handle for the '%s' library, error: %s.\n", + libname, dlerror()); + } +} errno_t sss_load_nss_symbols(struct sss_nss_ops *ops, const char *libname, struct sss_nss_symbols *syms, size_t nsyms) @@ -72,7 +80,7 @@ errno_t sss_load_nss_symbols(struct sss_nss_ops *ops, const char *libname, for (i = 0; i < nsyms; i++) { *(syms[i].fptr) = proxy_dlsym(ops->dl_handle, syms[i].fname, - libname); + libname); if (*(syms[i].fptr) == NULL) { if (syms[i].mandatory) { @@ -80,6 +88,7 @@ errno_t sss_load_nss_symbols(struct sss_nss_ops *ops, const char *libname, "mandatory symbol '%s', error: %s.\n", libpath, syms[i].fname, dlerror()); ret = ELIBBAD; + sss_close_handle(ops, libname); goto out; } else { DEBUG(SSSDBG_OP_FAILURE, "Library '%s' did not provide " diff --git a/src/util/nss_dl_load.h b/src/util/nss_dl_load.h index f1e882b969..07c04e0916 100644 --- a/src/util/nss_dl_load.h +++ b/src/util/nss_dl_load.h @@ -23,6 +23,8 @@ #include <pwd.h> #include <grp.h> #include <netdb.h> +#include <stdbool.h> + #include "util/util_errors.h" #include "sss_client/nss_compat.h" @@ -118,5 +120,6 @@ struct sss_nss_symbols { errno_t sss_load_nss_symbols(struct sss_nss_ops *ops, const char *libname, struct sss_nss_symbols *syms, size_t nsyms); +errno_t sss_load_nss_pw_symbols(struct sss_nss_ops *ops); #endif /* __SSSD_NSS_DL_LOAD_H__ */ diff --git a/src/util/nss_dl_load_extra.c b/src/util/nss_dl_load_extra.c new file mode 100644 index 0000000000..49eb24d62f --- /dev/null +++ b/src/util/nss_dl_load_extra.c @@ -0,0 +1,51 @@ +/* + SSSD + + nss_dl_load_extra.c + + Authors: + Sumit Bose <sb...@redhat.com> + Iker Pedrosa <ipedr...@redhat.com> + + Copyright (C) 2021 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 <http://www.gnu.org/licenses/>. +*/ + +#include <pwd.h> +#include <errno.h> +#include <talloc.h> +#include <grp.h> + +#include "db/sysdb.h" +#include "confdb/confdb.h" +#include "util/nss_dl_load.h" +#include "util/safe-format-string.h" +#include "util/strtonum.h" +#include "util/util.h" +#include "responder/common/responder.h" + +errno_t sss_load_nss_pw_symbols(struct sss_nss_ops *ops) +{ + errno_t ret; + struct sss_nss_symbols syms[] = { + {(void*)&ops->getpwnam_r, true, "getpwnam_r" }, + {(void*)&ops->getpwuid_r, true, "getpwuid_r" } + }; + size_t nsyms = sizeof(syms) / sizeof(struct sss_nss_symbols); + + ret = sss_load_nss_symbols(ops, "files", syms, nsyms); + + return ret; +} diff --git a/src/util/usertools.c b/src/util/usertools.c index 370a98b417..5c3af08bb0 100644 --- a/src/util/usertools.c +++ b/src/util/usertools.c @@ -27,12 +27,14 @@ #include "db/sysdb.h" #include "confdb/confdb.h" +#include "util/nss_dl_load.h" #include "util/strtonum.h" #include "util/util.h" #include "util/safe-format-string.h" #include "responder/common/responder.h" #define NAME_DOMAIN_PATTERN_OPTIONS (SSS_REGEXP_DUPNAMES | SSS_REGEXP_EXTENDED) +#define NSS_BUFFER_SIZE 16384 /* Function returns given realm name as new uppercase string */ char *get_uppercase_realm(TALLOC_CTX *memctx, const char *name) @@ -572,10 +574,23 @@ sss_fqname(char *str, size_t size, struct sss_names_ctx *nctx, errno_t sss_user_by_name_or_uid(const char *input, uid_t *_uid, gid_t *_gid) { + static struct sss_nss_ops nss_ops; uid_t uid; errno_t ret; char *endptr; - struct passwd *pwd; + struct passwd pwd = { 0 }; + int errnop = 0; + enum nss_status status; + static char s_nss_buffer[NSS_BUFFER_SIZE]; + + if (!nss_ops.dl_handle) { + ret = sss_load_nss_pw_symbols(&nss_ops); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "Unable to load NSS symbols [%d]: %s\n", + ret, sss_strerror(ret)); + return ret; + } + } /* Try if it's an ID first */ uid = strtouint32(input, &endptr, 10); @@ -587,26 +602,27 @@ errno_t sss_user_by_name_or_uid(const char *input, uid_t *_uid, gid_t *_gid) return ret; } - /* Nope, maybe a username? */ - pwd = getpwnam(input); + status = nss_ops.getpwnam_r(input, &pwd, s_nss_buffer, NSS_BUFFER_SIZE, &errnop); } else { - pwd = getpwuid(uid); + status = nss_ops.getpwuid_r(uid, &pwd, s_nss_buffer, NSS_BUFFER_SIZE, &errnop); } - if (pwd == NULL) { + if (status != NSS_STATUS_SUCCESS) { DEBUG(SSSDBG_OP_FAILURE, "[%s] is neither a valid UID nor a user name which could be " - "resolved by getpwnam().\n", input); + "resolved by getpwnam() [%d][%s]. status returned [%d]\n", + input, errnop, strerror(errnop), status); return EINVAL; } if (_uid) { - *_uid = pwd->pw_uid; + *_uid = pwd.pw_uid; } if (_gid) { - *_gid = pwd->pw_gid; + *_gid = pwd.pw_gid; } + return EOK; } From 3486fca9acc594b0616838504511f8875c47e640 Mon Sep 17 00:00:00 2001 From: Iker Pedrosa <ipedr...@redhat.com> Date: Wed, 1 Sep 2021 09:55:12 +0200 Subject: [PATCH 2/2] man: sssd.conf and sssd-ifp clarify user option user and allowed_uids options should be accessible via the files service of nsswitch.conf. Signed-off-by: Iker Pedrosa <ipedr...@redhat.com> --- src/man/sssd-ifp.5.xml | 5 +++++ src/man/sssd.conf.5.xml | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/man/sssd-ifp.5.xml b/src/man/sssd-ifp.5.xml index acb3e341eb..c787ef9d4d 100644 --- a/src/man/sssd-ifp.5.xml +++ b/src/man/sssd-ifp.5.xml @@ -55,6 +55,11 @@ responder. User names are resolved to UIDs at startup. </para> + <para> + Local user names are required, i.e. accessible via + <quote>files</quote> service of + <filename>nsswitch.conf</filename>. + </para> <para> Default: 0 (only the root user is allowed to access the InfoPipe responder) diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 8ca267b253..34bfe8bfcd 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -423,6 +423,12 @@ responder. </phrase> </para> + <para> + Both a user name and a uid can be used but the + user should be a local one, i.e. accessible via + <quote>files</quote> service of + <filename>nsswitch.conf</filename>. + </para> <para> Default: not set, process will run as root </para> @@ -2162,6 +2168,11 @@ pam_gssapi_indicators_map = sudo:pkinit, sudo-i:pkinit responder. User names are resolved to UIDs at startup. </para> + <para> + Local user names are required, i.e. accessible via + <quote>files</quote> service of + <filename>nsswitch.conf</filename>. + </para> <para> Default: 0 (only the root user is allowed to access the PAC responder)
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure