[PATCH] UTIL: Add a function to convert id_t from a number or a name

[Jakub Hrozek]

Hi,

attached is patch that implements a utility function to convert a number
or a string into a uid_t. It must be attached atop my previous patches
for server_setup.

[Jakub Hrozek]

On Tue, Oct 14, 2014 at 01:53:02PM +0200, Jakub Hrozek wrote:
> Hi,
>
> attached is patch that implements a utility function to convert a number
> or a string into a uid_t. It must be attached atop my previous patches
> for server_setup.

Sorry, the respoder_common test relied on a patch that was after it in
my tree, so it didn't pass on its own. A new patch is attached.

[Pavel Reichl]

On 10/15/2014 09:35 PM, Jakub Hrozek wrote:
> -errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> +errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string,
>                                   bool allow_sss_loop,
>                                   size_t *_uid_count, uid_t **_uids);

Could you please educate me and explain to me what does 'cvs' and 'csv' 
stands for?

Thanks!

[Pavel Reichl]

On 10/16/2014 02:12 PM, Pavel Reichl wrote:
> On 10/15/2014 09:35 PM, Jakub Hrozek wrote:
> > -errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char 
> > *cvs_string,
> > +errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char 
> > *csv_string,
> >                                   bool allow_sss_loop,
> >                                   size_t *_uid_count, uid_t **_uids);
>
> Could you please educate me and explain to me what does 'cvs' and 
> 'csv' stands for?

comma separated list, I see now!
> Thanks!
> _______________________________________________
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Pavel Reichl]

On 10/15/2014 09:35 PM, Jakub Hrozek wrote:
> 0001-UTIL-Add-a-function-to-convert-id_t-from-a-number-or.patch
>
> From 9ef055cef1d109e0e932ddb0c43e6a59b9b29e0f Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek
> Date: Tue, 23 Sep 2014 16:27:23 +0200
> Subject: [PATCH] UTIL: Add a function to convert id_t from a number or a name
>
> We need a custom function that would convert a numeric or string input
> into uid_t. The function will be used to drop privileges in servers and
> also in the PAC and IFP responders.
>
> Includes a unit test to test all code that changed as well as a fix for
> a misnamed attribute in the csv_to_uid_list function synopsis.
>
> src/responder/common/responder.h        |   2 +-
>   src/responder/common/responder_common.c |  13 +--
>   src/tests/cwrap/Makefile.am             |  53 ++++++++++++
>   src/tests/cwrap/passwd                  |   1 +
>   src/tests/cwrap/test_responder_common.c | 143 ++++++++++++++++++++++++++++++++
>   src/tests/cwrap/test_usertools.c        | 108 ++++++++++++++++++++++++
>   src/util/usertools.c                    |  64 ++++++++++++++
>   src/util/util.c                         |   1 +
>   src/util/util.h                         |   3 +
>   9 files changed, 378 insertions(+), 10 deletions(-)
>   create mode 100644 src/tests/cwrap/test_responder_common.c
>   create mode 100644 src/tests/cwrap/test_usertools.c
>
> diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
> index 3674d13f2303d0ce248f765a638aaa83d0c16cf3..97552ec472c5baa285b41cc48b51149f3ef6adb5 100644
> --- a/src/responder/common/responder.h
> +++ b/src/responder/common/responder.h
> @@ -308,7 +308,7 @@ errno_t schedule_get_domains_task(TALLOC_CTX *mem_ctx,
>                                     struct tevent_context *ev,
>                                     struct resp_ctx *rctx);
>
> -errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> +errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string,
>                                   bool allow_sss_loop,
>                                   size_t *_uid_count, uid_t **_uids);
>
> diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
> index b7331ac8ab1de51839937d117968e92062af76d7..8c185dce00c28d99d4f10252cd7b9748663fb08a 100644
> --- a/src/responder/common/responder_common.c
> +++ b/src/responder/common/responder_common.c
> @@ -159,7 +159,7 @@ errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count,
>       return EACCES;
>   }
>
> -errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> +errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string,
>                                   bool allow_sss_loop,
>                                   size_t *_uid_count, uid_t **_uids)
>   {
> @@ -169,9 +169,8 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
>       int list_size;
>       uid_t *uids = NULL;
>       char *endptr;
>
> struct passwd *pwd;
>
> ret = split_on_separator(mem_ctx, cvs_string, ',', true, false,
> ret = split_on_separator(mem_ctx, csv_string, ',', true, false,
>                           &list, &list_size);
>  if (ret != EOK) {
>      DEBUG(SSSDBG_OP_FAILURE, "split_on_separator failed [%d][%s].\n",
>
> @@ -211,17 +210,13 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
>                   goto done;
>               }
>        errno = 0;
>        pwd = getpwnam(list[c]);
>        if (pwd == NULL) {
>        ret = sss_user_from_string(list[c], &uids[c]);
>        if (ret != EOK) {
>             DEBUG(SSSDBG_OP_FAILURE, "List item [%s] is neither a valid "
>                                       "UID nor a user name which cloud be "

s/cloud/could
>                                         "resolved by getpwnam().\n", list[c]);
>            ret = EINVAL;
>             goto done;
>         }
>        uids[c] = pwd->pw_uid;
>     }
> }
>
> diff --git a/src/tests/cwrap/Makefile.am b/src/tests/cwrap/Makefile.am
> index 3fb30b28c0e94ac9a447a92bfaf1bfcc1013fd57..6f41a4e3c30fa735c7bc285ad0cc3547635d9056 100644
> --- a/src/tests/cwrap/Makefile.am
> +++ b/src/tests/cwrap/Makefile.am
> @@ -45,6 +45,8 @@ if HAVE_UID_WRAPPER
>   check_PROGRAMS = \
>    become_user-tests \
>    server-tests \
> usertools-tests \
> responder_common-tests \
> $(NULL)
> endif # HAVE_UID_WRAPPER
> endif # HAVE_NSS_WRAPPER
>
> @@ -105,4 +107,55 @@ server_tests_LDADD = \
>       $(abs_top_builddir)/libsss_test_common.la \
>       $(NULL)
>
> +usertools_tests_SOURCES = \
> test_usertools.c \
> ../../../src/util/domain_info_utils.c \
> ../../../src/util/safe-format-string.c \
> ../../../src/util/usertools.c \
> ../../../src/util/strtonum.c \
> ../../../src/util/backup_file.c \
> ../../../src/util/atomic_io.c \
> ../../../src/util/util.c \
> ../../../src/util/util_errors.c \
> ../../../src/util/sss_tc_utf8.c \
> ../../../src/util/sss_utf8.c \
> ../../../src/confdb/confdb.c \
> ../../../src/db/sysdb.c \
> ../../../src/db/sysdb_upgrade.c \
> ../../../src/db/sysdb_autofs.c \
> ../../../src/db/sysdb_search.c \
> ../../../src/db/sysdb_services.c \
> ../../../src/db/sysdb_ops.c \
> $(NULL)
>
> +usertools_tests_CFLAGS = \
> $(AM_CFLAGS) \
> $(NULL)
>
> +usertools_tests_LDADD = \
> $(CMOCKA_LIBS) \
> $(UNICODE_LIBS) \
> $(SSSD_LIBS) \
> $(abs_top_builddir)/libsss_debug.la \
> $(abs_top_builddir)/libsss_crypt.la \
> $(abs_top_builddir)/libsss_test_common.la \
> $(NULL)
>
> +responder_common_tests_SOURCES =\
> test_responder_common.c \
> ../../../src/responder/common/responder_common.c \
> ../../../src/responder/common/responder_packet.c \
> ../../../src/responder/common/responder_cmd.c \
> $(NULL)
>
> +responder_common_tests_CFLAGS = \
> $(AM_CFLAGS) \
> $(NULL)
>
> +responder_common_tests_LDADD = \
> $(CMOCKA_LIBS) \
> $(UNICODE_LIBS) \
> $(SSSD_LIBS) \
> $(abs_top_builddir)/libsss_debug.la \
> $(abs_top_builddir)/libsss_crypt.la \
> $(abs_top_builddir)/libsss_util.la \
> $(abs_top_builddir)/libsss_test_common.la \
> $(NULL)
> tests: $(check_PROGRAMS)
>
> diff --git a/src/tests/cwrap/passwd b/src/tests/cwrap/passwd
> index aa0a97db5259172c0b4ab47c7c2346fa5c2aa88e..d1316f41a3b7cfa41a08dbd8d297af7eec5e22f7 100644
> --- a/src/tests/cwrap/passwd
> +++ b/src/tests/cwrap/passwd
> @@ -1 +1,2 @@
>   sssd:x:123:123:sssd unprivileged user:/:/sbin/nologin
> +foobar:x:10001:10001:User for SSSD testing:/home/foobar:/bin/bash
> diff --git a/src/tests/cwrap/test_responder_common.c b/src/tests/cwrap/test_responder_common.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..7de6714cb85bb3735afb643e40e365f032528692
> --- /dev/null
> +++ b/src/tests/cwrap/test_responder_common.c
> @@ -0,0 +1,143 @@
> +/*
> Authors:
>    Jakub Hrozek<[e-mail]>
> Copyright (C) 2014 Red Hat
> SSSD tests: User utilities
> 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, seehttp://www.gnu.org/licenses/.
>
> +*/
>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
>
> +#include <popt.h>
> +#include "util/util.h"
> +#include "responder/common/responder.h"
> +#include "tests/cmocka/common_mock.h"
>
> +/* Just to satisfy dependencies */
> +struct cli_protocol_version *register_cli_protocol_version(void)
> +{
> static struct cli_protocol_version responder_test_cli_protocol_version[] = {
>    {0, NULL, NULL}
> };
> return responder_test_cli_protocol_version;
>
> +}
>
> +void test_uid_csv_to_uid_list(void **state)
> +{
> TALLOC_CTX *tmp_ctx;
> errno_t ret;
> size_t count;
> uid_t *list;
> tmp_ctx = talloc_new(global_talloc_context);
> assert_non_null(tmp_ctx);
> check_leaks_push(tmp_ctx);
> ret = csv_string_to_uid_array(tmp_ctx, "1, 2, 3", false, &count, &list);
> assert_int_equal(ret, EOK);
> assert_int_equal(count, 3);
> assert_int_equal(list[0], 1);
> assert_int_equal(list[1], 2);
> assert_int_equal(list[2], 3);
> talloc_free(list);
> check_leaks_pop(tmp_ctx);
> talloc_free(tmp_ctx);
>
> +}
>
> +void test_name_csv_to_uid_list(void **state)
> +{
> TALLOC_CTX *tmp_ctx;
> errno_t ret;
> size_t count;
> uid_t *list;
> tmp_ctx = talloc_new(global_talloc_context);
> assert_non_null(tmp_ctx);
> check_leaks_push(tmp_ctx);
> ret = csv_string_to_uid_array(tmp_ctx, "sssd, foobar", true, &count, &list);
> assert_int_equal(ret, EOK);
> assert_int_equal(count, 2);
> assert_int_equal(list[0], 123);
> assert_int_equal(list[1], 10001);
> talloc_free(list);
> check_leaks_pop(tmp_ctx);
> talloc_free(tmp_ctx);
>
> +}
>
> +void test_csv_to_uid_list_neg(void **state)
> +{
> TALLOC_CTX *tmp_ctx;
> errno_t ret;
> size_t count;
> uid_t *list = NULL;
> tmp_ctx = talloc_new(global_talloc_context);
> assert_non_null(tmp_ctx);
> check_leaks_push(tmp_ctx);
> ret = csv_string_to_uid_array(tmp_ctx, "nosuchuser", true, &count, &list);
> assert_int_not_equal(ret, EOK);
> check_leaks_pop(tmp_ctx);
> talloc_free(tmp_ctx);
>
> +}

This test in not added to tests.
> +int main(int argc, const char *argv[])
> +{
> poptContext pc;
> int opt;
> struct poptOption long_options[] = {
>    POPT_AUTOHELP
>    SSSD_DEBUG_OPTS
>    POPT_TABLEEND
> };
> const UnitTest tests[] = {
>    unit_test(test_uid_csv_to_uid_list),
>    unit_test(test_name_csv_to_uid_list),
> };
> /* Set debug level to invalid value so we can deside if -d 0 was used. */
> debug_level = SSSDBG_INVALID;
> pc = poptGetContext(argv[0], argc, argv, long_options, 0);
> while((opt = poptGetNextOpt(pc)) != -1) {
>    switch(opt) {
>    default:
>        fprintf(stderr, "\nInvalid option %s: %s\n\n",
>                poptBadOption(pc, 0), poptStrerror(opt));
>        poptPrintUsage(pc, stderr, 0);
>        return 1;
>    }
> }
> poptFreeContext(pc);
> DEBUG_CLI_INIT(debug_level);
> tests_set_cwd();
> return run_tests(tests);
>
> +}
> diff --git a/src/tests/cwrap/test_usertools.c b/src/tests/cwrap/test_usertools.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..4358618dec0ce2e3e80b609a793ae386c819ad74
> --- /dev/null
> +++ b/src/tests/cwrap/test_usertools.c
> @@ -0,0 +1,108 @@
> +/*
> Authors:
>    Jakub Hrozek<[e-mail]>
> Copyright (C) 2014 Red Hat
> SSSD tests: User utilities
> 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, seehttp://www.gnu.org/licenses/.
>
> +*/
>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
>
> +#include <popt.h>
> +#include "util/util.h"
> +#include "tests/cmocka/common_mock.h"
>
> +void test_get_user_num(void **state)
> +{
> uid_t uid;
> errno_t ret;
> ret = sss_user_from_string("123", &uid);
> assert_int_equal(ret, 0);

In previous test you used EOK instead of 0.
> assert_int_equal(uid, 123);
>
> +}
>
> +void test_get_user_str(void **state)
> +{
> uid_t uid;
> errno_t ret;
> ret = sss_user_from_string("sssd", &uid);
> assert_int_equal(ret, 0);

same here
> assert_int_equal(uid, 123);
>
> +}
>
> +void test_get_group_num(void **state)
> +{
> gid_t gid;
> errno_t ret;
> ret = sss_group_from_string("123", &gid);
> assert_int_equal(ret, 0);

same here
> assert_int_equal(gid, 123);
>
> +}
>
> +void test_get_group_str(void **state)
> +{
> gid_t gid;
> errno_t ret;
> ret = sss_group_from_string("sssd", &gid);
> assert_int_equal(ret, 0);

same here
> assert_int_equal(gid, 123);
>
> +}
>
> +int main(int argc, const char *argv[])
> +{
> poptContext pc;
> int opt;
> struct poptOption long_options[] = {
>    POPT_AUTOHELP
>    SSSD_DEBUG_OPTS
>    POPT_TABLEEND
> };
> const UnitTest tests[] = {
>    unit_test(test_get_user_num),
>    unit_test(test_get_user_str),
>    unit_test(test_get_group_num),
>    unit_test(test_get_group_str),
> };
> /* Set debug level to invalid value so we can deside if -d 0 was used. */
> debug_level = SSSDBG_INVALID;
> pc = poptGetContext(argv[0], argc, argv, long_options, 0);
> while((opt = poptGetNextOpt(pc)) != -1) {
>    switch(opt) {
>    default:
>        fprintf(stderr, "\nInvalid option %s: %s\n\n",
>                poptBadOption(pc, 0), poptStrerror(opt));
>        poptPrintUsage(pc, stderr, 0);
>        return 1;
>    }
> }
> poptFreeContext(pc);
> DEBUG_CLI_INIT(debug_level);
> tests_set_cwd();
> return run_tests(tests);
>
> +}
> diff --git a/src/util/usertools.c b/src/util/usertools.c
> index 809b42d67c7b1cdfa0729c3a7e835fab37297596..a26faea75ab6417eceaccca010841365913e0e0f 100644
> --- a/src/util/usertools.c
> +++ b/src/util/usertools.c
> @@ -23,8 +23,11 @@
>   #include <pcre.h>
>   #include <errno.h>
>   #include <talloc.h>
> +#include <pwd.h>
> +#include <grp.h>
>
> #include "confdb/confdb.h"
> +#include "util/strtonum.h"
>   #include "util/util.h"
>   #include "util/safe-format-string.h"
>   #include "responder/common/responder.h"
> @@ -659,3 +662,64 @@ sss_get_domain_name(TALLOC_CTX *mem_ctx,
>   return user_name;
>
> }
>
> +static errno_t sss_user_or_group_from_string(const char *input,
>                                         bool is_user,
>                                         id_t *_id)
>
> +{

I prefer to use something like 'enum target{USER, GROUP}' instead of 
just bool variable in these cases but I don't insist.
> id_t id;
> errno_t ret;
> char *endptr;
> struct passwd *pwd;
> struct group *grp;
> /* Try if it's an ID first */
> id = strtouint32(input, &endptr, 10);
> if (errno != 0 || *endptr != '\0') {
>    ret = errno;
>    if (ret == ERANGE) {
>        DEBUG(SSSDBG_OP_FAILURE,
>              "List item [%s] is out of range.\n", input);
>        return ret;
>    }
>    if (is_user) {
>        /* Nope, maybe a username? */
>        pwd = getpwnam(input);
>        if (pwd == NULL) {
>            DEBUG(SSSDBG_OP_FAILURE,
>                  "List item [%s] is neither a valid "
>                  "UID nor a user name which cloud be "

s/cloud/could
>                  "resolved by getpwnam().\n", input);
>            return EINVAL;
>        }
>        id = pwd->pw_uid;
>    } else {
>        /* Nope, maybe a username? */
>        grp = getgrnam(input);
>        if (grp == NULL) {
>            DEBUG(SSSDBG_OP_FAILURE,
>                  "List item [%s] is neither a valid "
>                  "UID nor a user name which cloud be "

GID..group name
s/cloud/could
>                  "resolved by getpwnam().\n", input);

getgrnam
>            return EINVAL;
>        }
>        id = grp->gr_gid;
>    }
> }
> *_id = id;
> return EOK;
>
> +}
>
> +errno_t sss_user_from_string(const char *input, uid_t *_uid)
> +{
> return sss_user_or_group_from_string(input, true, _uid);
>
> +}
>
> +errno_t sss_group_from_string(const char *input, gid_t *_gid)
> +{
> return sss_user_or_group_from_string(input, false, _gid);
>
> +}
> diff --git a/src/util/util.c b/src/util/util.c
> index 7f80771ecd9868feaf43e34cbd61e44dd8ae5f3a..d78d37d975e6591bca6ac3f2fa36b5b9f4659a29 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -21,6 +21,7 @@
>   #include <ctype.h>
>   #include <netdb.h>
>   #include <poll.h>
> +#include <sys/types.h>
>   #include <sys/socket.h>
>   #include <arpa/inet.h>
>   #include <talloc.h>
> diff --git a/src/util/util.h b/src/util/util.h
> index 0d7a2daa3bbcc9b8a86681c119bec8d556fcde49..d608ecc37b839640e997344a24de0c0c41e24a34 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -403,6 +403,9 @@ bool check_ipv6_addr(struct in6_addr *addr, uint8_t check);
>
> const char * const * get_known_services(void);
>
> +errno_t sss_user_from_string(const char *input, uid_t *_uid);
> +errno_t sss_group_from_string(const char *input, gid_t *_gid);
> int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
>                        const char sep, bool trim, bool skip_empty,
>                        char ***_list, int *size);
>
> -- 1.9.3

[Jakub Hrozek]

On Thu, Oct 16, 2014 at 04:37:25PM +0200, Pavel Reichl wrote:
> On 10/15/2014 09:35 PM, Jakub Hrozek wrote:
> > 0001-UTIL-Add-a-function-to-convert-id_t-from-a-number-or.patch
> >
> > From 9ef055cef1d109e0e932ddb0c43e6a59b9b29e0f Mon Sep 17 00:00:00 2001
> > From: Jakub Hrozek
> > Date: Tue, 23 Sep 2014 16:27:23 +0200
> > Subject: [PATCH] UTIL: Add a function to convert id_t from a number or a name
> >
> > We need a custom function that would convert a numeric or string input
> > into uid_t. The function will be used to drop privileges in servers and
> > also in the PAC and IFP responders.
> >
> > Includes a unit test to test all code that changed as well as a fix for
> > a misnamed attribute in the csv_to_uid_list function synopsis.
> >
> > src/responder/common/responder.h        |   2 +-
> >  src/responder/common/responder_common.c |  13 +--
> >  src/tests/cwrap/Makefile.am             |  53 ++++++++++++
> >  src/tests/cwrap/passwd                  |   1 +
> >  src/tests/cwrap/test_responder_common.c | 143 ++++++++++++++++++++++++++++++++
> >  src/tests/cwrap/test_usertools.c        | 108 ++++++++++++++++++++++++
> >  src/util/usertools.c                    |  64 ++++++++++++++
> >  src/util/util.c                         |   1 +
> >  src/util/util.h                         |   3 +
> >  9 files changed, 378 insertions(+), 10 deletions(-)
> >  create mode 100644 src/tests/cwrap/test_responder_common.c
> >  create mode 100644 src/tests/cwrap/test_usertools.c
> >
> > diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
> > index 3674d13f2303d0ce248f765a638aaa83d0c16cf3..97552ec472c5baa285b41cc48b51149f3ef6adb5 100644
> > --- a/src/responder/common/responder.h
> > +++ b/src/responder/common/responder.h
> > @@ -308,7 +308,7 @@ errno_t schedule_get_domains_task(TALLOC_CTX *mem_ctx,
> >                                    struct tevent_context *ev,
> >                                    struct resp_ctx *rctx);
> > -errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> > +errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string,
> >                                  bool allow_sss_loop,
> >                                  size_t *_uid_count, uid_t **_uids);
> > diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
> > index b7331ac8ab1de51839937d117968e92062af76d7..8c185dce00c28d99d4f10252cd7b9748663fb08a 100644
> > --- a/src/responder/common/responder_common.c
> > +++ b/src/responder/common/responder_common.c
> > @@ -159,7 +159,7 @@ errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count,
> >      return EACCES;
> >  }
> > -errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> > +errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string,
> >                                  bool allow_sss_loop,
> >                                  size_t *_uid_count, uid_t **_uids)
> >  {
> > @@ -169,9 +169,8 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> >      int list_size;
> >      uid_t *uids = NULL;
> >      char *endptr;
> > struct passwd *pwd;
> > ret = split_on_separator(mem_ctx, cvs_string, ',', true, false,
> > ret = split_on_separator(mem_ctx, csv_string, ',', true, false,
> >                          &list, &list_size);
> > if (ret != EOK) {
> >     DEBUG(SSSDBG_OP_FAILURE, "split_on_separator failed [%d][%s].\n",
> >
> > @@ -211,17 +210,13 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> >                  goto done;
> >              }
> >        errno = 0;
> >        pwd = getpwnam(list[c]);
> >        if (pwd == NULL) {
> >        ret = sss_user_from_string(list[c], &uids[c]);
> >        if (ret != EOK) {
> >            DEBUG(SSSDBG_OP_FAILURE, "List item [%s] is neither a valid "
> >                                      "UID nor a user name which cloud be "
>
> s/cloud/could

[...]

I fixed this error and the others you pointed out, but after discussion
with Simo in the following patchset, I removed the functions to retrieve
groups completely -- as sssd.conf would only allow specifiying the user
to run as, not groups.

A new patch is attached.

As Sumit and Pavel are doing great progress with the views feature, I
hope we can start pushing the already acked patches and avoid several
dispersed threads..

[Jakub Hrozek]

On Fri, Oct 17, 2014 at 02:17:21PM +0200, Jakub Hrozek wrote:
> On Thu, Oct 16, 2014 at 04:37:25PM +0200, Pavel Reichl wrote:
> > On 10/15/2014 09:35 PM, Jakub Hrozek wrote:
> > > 0001-UTIL-Add-a-function-to-convert-id_t-from-a-number-or.patch
> > >
> > > From 9ef055cef1d109e0e932ddb0c43e6a59b9b29e0f Mon Sep 17 00:00:00 2001
> > > From: Jakub Hrozek
> > > Date: Tue, 23 Sep 2014 16:27:23 +0200
> > > Subject: [PATCH] UTIL: Add a function to convert id_t from a number or a name
> > >
> > > We need a custom function that would convert a numeric or string input
> > > into uid_t. The function will be used to drop privileges in servers and
> > > also in the PAC and IFP responders.
> > >
> > > Includes a unit test to test all code that changed as well as a fix for
> > > a misnamed attribute in the csv_to_uid_list function synopsis.
> > >
> > > src/responder/common/responder.h        |   2 +-
> > >  src/responder/common/responder_common.c |  13 +--
> > >  src/tests/cwrap/Makefile.am             |  53 ++++++++++++
> > >  src/tests/cwrap/passwd                  |   1 +
> > >  src/tests/cwrap/test_responder_common.c | 143 ++++++++++++++++++++++++++++++++
> > >  src/tests/cwrap/test_usertools.c        | 108 ++++++++++++++++++++++++
> > >  src/util/usertools.c                    |  64 ++++++++++++++
> > >  src/util/util.c                         |   1 +
> > >  src/util/util.h                         |   3 +
> > >  9 files changed, 378 insertions(+), 10 deletions(-)
> > >  create mode 100644 src/tests/cwrap/test_responder_common.c
> > >  create mode 100644 src/tests/cwrap/test_usertools.c
> > >
> > > diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
> > > index 3674d13f2303d0ce248f765a638aaa83d0c16cf3..97552ec472c5baa285b41cc48b51149f3ef6adb5 100644
> > > --- a/src/responder/common/responder.h
> > > +++ b/src/responder/common/responder.h
> > > @@ -308,7 +308,7 @@ errno_t schedule_get_domains_task(TALLOC_CTX *mem_ctx,
> > >                                    struct tevent_context *ev,
> > >                                    struct resp_ctx *rctx);
> > > -errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> > > +errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string,
> > >                                  bool allow_sss_loop,
> > >                                  size_t *_uid_count, uid_t **_uids);
> > > diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
> > > index b7331ac8ab1de51839937d117968e92062af76d7..8c185dce00c28d99d4f10252cd7b9748663fb08a 100644
> > > --- a/src/responder/common/responder_common.c
> > > +++ b/src/responder/common/responder_common.c
> > > @@ -159,7 +159,7 @@ errno_t check_allowed_uids(uid_t uid, size_t allowed_uids_count,
> > >      return EACCES;
> > >  }
> > > -errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> > > +errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *csv_string,
> > >                                  bool allow_sss_loop,
> > >                                  size_t *_uid_count, uid_t **_uids)
> > >  {
> > > @@ -169,9 +169,8 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> > >      int list_size;
> > >      uid_t *uids = NULL;
> > >      char *endptr;
> > > struct passwd *pwd;
> > > ret = split_on_separator(mem_ctx, cvs_string, ',', true, false,
> > > ret = split_on_separator(mem_ctx, csv_string, ',', true, false,
> > >                          &list, &list_size);
> > > if (ret != EOK) {
> > >     DEBUG(SSSDBG_OP_FAILURE, "split_on_separator failed [%d][%s].\n",
> > >
> > > @@ -211,17 +210,13 @@ errno_t csv_string_to_uid_array(TALLOC_CTX *mem_ctx, const char *cvs_string,
> > >                  goto done;
> > >              }
> > >        errno = 0;
> > >        pwd = getpwnam(list[c]);
> > >        if (pwd == NULL) {
> > >        ret = sss_user_from_string(list[c], &uids[c]);
> > >        if (ret != EOK) {
> > >            DEBUG(SSSDBG_OP_FAILURE, "List item [%s] is neither a valid "
> > >                                      "UID nor a user name which cloud be "
> >
> > s/cloud/could
>
> [...]
>
> I fixed this error and the others you pointed out, but after discussion
> with Simo in the following patchset, I removed the functions to retrieve
> groups completely -- as sssd.conf would only allow specifiying the user
> to run as, not groups.
>
> A new patch is attached.
>
> As Sumit and Pavel are doing great progress with the views feature, I
> hope we can start pushing the already acked patches and avoid several
> dispersed threads..

It makes more sense to review the patches together with the ones that
depend on the packaging. This thread is retired in favor of "Monitor and
sbus changes for running SSSD as a non-privileged user"

Sorry for the noise.