From 80c64be80f2bffdcf5d2432e1e59d633fd68d516 Mon Sep 17 00:00:00 2001 From: Grace Chin Date: Mon, 13 Jun 2022 09:02:32 -0400 Subject: [PATCH 1/4] Add pcmk__is_user_in_group() --- lib/common/crmcommon_private.h | 3 +++ lib/common/utils.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/lib/common/crmcommon_private.h b/lib/common/crmcommon_private.h index 6b7be9c68..c2fcb0adf 100644 --- a/lib/common/crmcommon_private.h +++ b/lib/common/crmcommon_private.h @@ -96,6 +96,9 @@ void pcmk__free_acls(GList *acls); G_GNUC_INTERNAL void pcmk__unpack_acl(xmlNode *source, xmlNode *target, const char *user); +G_GNUC_INTERNAL +bool pcmk__is_user_in_group(const char *user, const char *group); + G_GNUC_INTERNAL void pcmk__apply_acl(xmlNode *xml); diff --git a/lib/common/utils.c b/lib/common/utils.c index 2dfbef278..f23583acb 100644 --- a/lib/common/utils.c +++ b/lib/common/utils.c @@ -27,6 +27,7 @@ #include #include #include +#include #include @@ -53,6 +54,38 @@ gboolean crm_config_error = FALSE; gboolean crm_config_warning = FALSE; char *crm_system_name = NULL; +bool +pcmk__is_user_in_group(const char *user, const char *group) +{ + struct group *grent; + char **gr_mem; + + if (user == NULL || group == NULL) { + return false; + } + + setgrent(); + while ((grent = getgrent()) != NULL) { + if (grent->gr_mem == NULL) { + continue; + } + + if(strcmp(group, grent->gr_name) != 0) { + continue; + } + + gr_mem = grent->gr_mem; + while (*gr_mem != NULL) { + if (!strcmp(user, *gr_mem++)) { + endgrent(); + return true; + } + } + } + endgrent(); + return false; +} + int crm_user_lookup(const char *name, uid_t * uid, gid_t * gid) { -- 2.31.1 From 5fbe5c310de00390fb36d866823a7745ba4812e3 Mon Sep 17 00:00:00 2001 From: Grace Chin Date: Mon, 13 Jun 2022 09:04:57 -0400 Subject: [PATCH 2/4] Add unit test for pcmk__is_user_in_group() --- lib/common/Makefile.am | 2 +- lib/common/mock.c | 31 +++++-- lib/common/mock_private.h | 11 +++ lib/common/tests/acl/Makefile.am | 11 ++- .../tests/acl/pcmk__is_user_in_group_test.c | 92 +++++++++++++++++++ 5 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 lib/common/tests/acl/pcmk__is_user_in_group_test.c diff --git a/lib/common/Makefile.am b/lib/common/Makefile.am index d7aae53bf..04d56dc3c 100644 --- a/lib/common/Makefile.am +++ b/lib/common/Makefile.am @@ -94,7 +94,7 @@ libcrmcommon_la_SOURCES += watchdog.c libcrmcommon_la_SOURCES += xml.c libcrmcommon_la_SOURCES += xpath.c -WRAPPED = calloc getenv getpwnam_r uname +WRAPPED = calloc getenv getpwnam_r uname setgrent getgrent endgrent WRAPPED_FLAGS = $(foreach fn,$(WRAPPED),-Wl,--wrap=$(fn)) libcrmcommon_test_la_SOURCES = $(libcrmcommon_la_SOURCES) diff --git a/lib/common/mock.c b/lib/common/mock.c index 55812ddbc..fa9431e6d 100644 --- a/lib/common/mock.c +++ b/lib/common/mock.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "mock_private.h" @@ -18,13 +19,13 @@ * libcrmcommon_test.a, not into libcrmcommon.so. It is used to support * constructing mock versions of library functions for unit testing. * - * Each unit test will only ever want to use a mocked version of one or two - * library functions. However, we need to mark all the mocked functions as - * wrapped (with -Wl,--wrap= in the LDFLAGS) in libcrmcommon_test.a so that - * all those unit tests can share the same special test library. The unit - * test then defines its own wrapped function. Because a unit test won't - * define every single wrapped function, there will be undefined references - * at link time. + * Each unit test will only ever want to use a mocked version of a few + * library functions (i.e. not all of them). However, we need to mark all + * the mocked functions as wrapped (with -Wl,--wrap= in the LDFLAGS) in + * libcrmcommon_test.a so that all those unit tests can share the same + * special test library. The unit test then defines its own wrapped + * function. Because a unit test won't define every single wrapped + * function, there will be undefined references at link time. * * This file takes care of those undefined references. It defines a * wrapped version of every function that simply calls the real libc @@ -74,3 +75,19 @@ int __attribute__((weak)) __wrap_uname(struct utsname *buf) { return __real_uname(buf); } + +void __attribute__((weak)) +__wrap_setgrent(void) { + __real_setgrent(); +} + +struct group * __attribute__((weak)) +__wrap_getgrent(void) { + return __real_getgrent(); +} + +void __attribute__((weak)) +__wrap_endgrent(void) { + __real_endgrent(); +} + diff --git a/lib/common/mock_private.h b/lib/common/mock_private.h index 3df7c9839..0c1134cc3 100644 --- a/lib/common/mock_private.h +++ b/lib/common/mock_private.h @@ -14,6 +14,7 @@ #include #include #include +#include /* This header is for the sole use of libcrmcommon_test. */ @@ -31,4 +32,14 @@ int __wrap_getpwnam_r(const char *name, struct passwd *pwd, int __real_uname(struct utsname *buf); int __wrap_uname(struct utsname *buf); +void __real_setgrent(void); +void __wrap_setgrent(void); + +struct group *__real_getgrent(void); +struct group *__wrap_getgrent(void); + +void __real_endgrent(void); +void __wrap_endgrent(void); + + #endif // MOCK_PRIVATE__H diff --git a/lib/common/tests/acl/Makefile.am b/lib/common/tests/acl/Makefile.am index 679c9cb8e..a73fc354c 100644 --- a/lib/common/tests/acl/Makefile.am +++ b/lib/common/tests/acl/Makefile.am @@ -1,19 +1,26 @@ # -# Copyright 2021 the Pacemaker project contributors +# Copyright 2021-2022 the Pacemaker project contributors # # The version control history for this file may have further details. # # This source code is licensed under the GNU General Public License version 2 # or later (GPLv2+) WITHOUT ANY WARRANTY. # -AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include +AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include -I$(top_srcdir)/lib/common LDADD = $(top_builddir)/lib/common/libcrmcommon.la -lcmocka +pcmk__is_user_in_group_test_LDADD = $(top_builddir)/lib/common/libcrmcommon_test.la -lcmocka +pcmk__is_user_in_group_test_LDFLAGS = \ + -Wl,--wrap=setgrent \ + -Wl,--wrap=getgrent \ + -Wl,--wrap=endgrent + include $(top_srcdir)/mk/tap.mk # Add "_test" to the end of all test program names to simplify .gitignore. check_PROGRAMS = \ + pcmk__is_user_in_group_test \ pcmk_acl_required_test \ xml_acl_denied_test \ xml_acl_enabled_test diff --git a/lib/common/tests/acl/pcmk__is_user_in_group_test.c b/lib/common/tests/acl/pcmk__is_user_in_group_test.c new file mode 100644 index 000000000..67b8c2c7c --- /dev/null +++ b/lib/common/tests/acl/pcmk__is_user_in_group_test.c @@ -0,0 +1,92 @@ +/* + * Copyright 2020-2022 the Pacemaker project contributors + * + * The version control history for this file may have further details. + * + * This source code is licensed under the GNU Lesser General Public License + * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY. + */ + +#include +#include +#include "../../crmcommon_private.h" + +#include "mock_private.h" + +#include +#include +#include +#include +#include + +// THe index of the group that is going to be returned next from "get group entry" (getgrent) +static int group_idx = 0; + +// Data used for testing +static const char* grp0_members[] = { + "user0", "user1", NULL +}; + +static const char* grp1_members[] = { + "user1", NULL +}; + +static const char* grp2_members[] = { + "user2", "user1", NULL +}; + +// an array of "groups" (a struct from grp.h), the members of the groups are initalized here to some testing data. +// Casting away the consts to make the compiler happy and simplify initialization. +// We never actually change these variables during the test! +// string literal = const char* (cannot be changed b/c ? ) vs. char* (its getting casted to this) +static const int NUM_GROUPS = 3; +static struct group groups[] = { + {(char*)"grp0", (char*)"", 0, (char**)grp0_members}, + {(char*)"grp1", (char*)"", 1, (char**)grp1_members}, + {(char*)"grp2", (char*)"", 2, (char**)grp2_members}, +}; + +// This function resets the group_idx to 0. +void +__wrap_setgrent(void) { + group_idx = 0; +} + +// This function returns the next group entry in the list of groups, or +// NULL if there aren't any left. +// group_idx is a global variable which keeps track of where you are in the list +struct group * +__wrap_getgrent(void) { + if(group_idx >= NUM_GROUPS) return NULL; + return &groups[group_idx++]; +} + +void +__wrap_endgrent(void) { +} + +static void +is_pcmk__is_user_in_group(void **state) +{ + // null user + assert_false(pcmk__is_user_in_group(NULL, "grp0")); + // null group + assert_false(pcmk__is_user_in_group("user0", NULL)); + // nonexistent group + assert_false(pcmk__is_user_in_group("user0", "nonexistent_group")); + // user is in group + assert_true(pcmk__is_user_in_group("user0", "grp0")); + // user is not in group + assert_false(pcmk__is_user_in_group("user2", "grp0")); +} + +int +main(int argc, char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(is_pcmk__is_user_in_group) + }; + + cmocka_set_message_output(CM_OUTPUT_TAP); + return cmocka_run_group_tests(tests, NULL, NULL); +} -- 2.31.1 From 1bb7fda60f5b8547d7457f20543b7e50089cf06b Mon Sep 17 00:00:00 2001 From: Grace Chin Date: Mon, 13 Jun 2022 09:17:36 -0400 Subject: [PATCH 3/4] Add ACL group support closes T61 --- lib/common/acl.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/common/acl.c b/lib/common/acl.c index f68069bbd..d7f8469b1 100644 --- a/lib/common/acl.c +++ b/lib/common/acl.c @@ -320,6 +320,13 @@ pcmk__unpack_acl(xmlNode *source, xmlNode *target, const char *user) crm_debug("Unpacking ACLs for user '%s'", id); p->acls = parse_acl_entry(acls, child, p->acls); } + } else if (!strcmp(tag, XML_ACL_TAG_GROUP)) { + const char *id = crm_element_value(child, XML_ATTR_ID); + + if (id && pcmk__is_user_in_group(user,id)) { + crm_debug("Unpacking ACLs for group '%s'", id); + p->acls = parse_acl_entry(acls, child, p->acls); + } } } } -- 2.31.1 From f4efd55d9424d34908ba3e2bcffe16c00b2cf660 Mon Sep 17 00:00:00 2001 From: Grace Chin Date: Mon, 13 Jun 2022 09:20:36 -0400 Subject: [PATCH 4/4] Allow acl_target and acl_group elements to take a 'name' attribute to use a name different from 'id' closes T60 --- include/crm/msg_xml.h | 1 + lib/common/acl.c | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/include/crm/msg_xml.h b/include/crm/msg_xml.h index b36dcf060..6470520b1 100644 --- a/include/crm/msg_xml.h +++ b/include/crm/msg_xml.h @@ -133,6 +133,7 @@ extern "C" { # define XML_ATTR_VERSION "version" # define XML_ATTR_DESC "description" # define XML_ATTR_ID "id" +# define XML_ATTR_NAME "name" # define XML_ATTR_IDREF "id-ref" # define XML_ATTR_ID_LONG "long-id" # define XML_ATTR_TYPE "type" diff --git a/lib/common/acl.c b/lib/common/acl.c index d7f8469b1..b9f7472ee 100644 --- a/lib/common/acl.c +++ b/lib/common/acl.c @@ -278,8 +278,13 @@ pcmk__apply_acl(xmlNode *xml) /*! * \internal - * \brief Unpack ACLs for a given user - * + * \brief Unpack ACLs for a given user into the + * metadata of the target XML tree + * + * Taking the description of ACLs from the source XML tree and + * marking up the target XML tree with access information for the + * given user by tacking it onto the relevant nodes + * * \param[in] source XML with ACL definitions * \param[in,out] target XML that ACLs will be applied to * \param[in] user Username whose ACLs need to be unpacked @@ -314,14 +319,22 @@ pcmk__unpack_acl(xmlNode *source, xmlNode *target, const char *user) if (!strcmp(tag, XML_ACL_TAG_USER) || !strcmp(tag, XML_ACL_TAG_USERv1)) { - const char *id = crm_element_value(child, XML_ATTR_ID); + const char *id = crm_element_value(child, XML_ATTR_NAME); + + if (id == NULL) { + id = crm_element_value(child, XML_ATTR_ID); + } if (id && strcmp(id, user) == 0) { crm_debug("Unpacking ACLs for user '%s'", id); p->acls = parse_acl_entry(acls, child, p->acls); } } else if (!strcmp(tag, XML_ACL_TAG_GROUP)) { - const char *id = crm_element_value(child, XML_ATTR_ID); + const char *id = crm_element_value(child, XML_ATTR_NAME); + + if (id == NULL) { + id = crm_element_value(child, XML_ATTR_ID); + } if (id && pcmk__is_user_in_group(user,id)) { crm_debug("Unpacking ACLs for group '%s'", id); -- 2.31.1