diff options
| -rw-r--r-- | Makefile.am | 2 | ||||
| -rw-r--r-- | src/confdb/confdb.h | 1 | ||||
| -rw-r--r-- | src/config/SSSDConfig/__init__.py.in | 1 | ||||
| -rw-r--r-- | src/config/etc/sssd.api.conf | 1 | ||||
| -rw-r--r-- | src/man/sssd-ifp.5.xml | 62 | ||||
| -rw-r--r-- | src/responder/ifp/ifp_private.h | 3 | ||||
| -rw-r--r-- | src/responder/ifp/ifpsrv.c | 17 | ||||
| -rw-r--r-- | src/responder/ifp/ifpsrv_cmd.c | 30 | ||||
| -rw-r--r-- | src/responder/ifp/ifpsrv_util.c | 146 | ||||
| -rw-r--r-- | src/tests/cmocka/test_ifp.c | 101 |
10 files changed, 361 insertions, 3 deletions
diff --git a/Makefile.am b/Makefile.am index a60c97e0f..ff95518a5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1638,7 +1638,7 @@ ifp_tests_LDADD = \ $(SSSD_LIBS) \ $(SSSD_INTERNAL_LTLIBS) \ libsss_test_common.la -endif +endif # BUILD_IFP endif # HAVE_CMOCKA diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 465b41db0..0f70671c6 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -136,6 +136,7 @@ /* InfoPipe */ #define CONFDB_IFP_CONF_ENTRY "config/ifp" +#define CONFDB_IFP_USER_ATTR_LIST "user_attributes" /* Domains */ #define CONFDB_DOMAIN_PATH_TMPL "config/domain/%s" diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 7070b88a1..a4a5770db 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -96,6 +96,7 @@ option_strings = { # [ifp] 'allowed_uids': _('List of UIDs or user names allowed to access the InfoPipe responder'), + 'user_attributes': _('List of user attributes the InfoPipe is allowed to publish'), # [provider] 'id_provider' : _('Identity provider'), diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index d6f2d6b45..85253aad5 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -71,6 +71,7 @@ ssh_known_hosts_timeout = int, None, false [pac] # PAC responder allowed_uids = str, None, false +user_attributes = str, None, false [ifp] # InfoPipe responder diff --git a/src/man/sssd-ifp.5.xml b/src/man/sssd-ifp.5.xml index 7e2ea7cfc..867c117ed 100644 --- a/src/man/sssd-ifp.5.xml +++ b/src/man/sssd-ifp.5.xml @@ -69,6 +69,68 @@ </para> </listitem> </varlistentry> + + <varlistentry> + <term>user_attributes (string)</term> + <listitem> + <para> + Specifies the comma-separated list of white + or blacklisted attributes. + </para> + <para> + By default, the InfoPipe responder only + allows the default set of POSIX attributes to + be requested. This set is the same as returned by + <citerefentry> + <refentrytitle>getpwnam</refentrytitle> + <manvolnum>3</manvolnum> + </citerefentry> + and includes: + <variablelist> + <varlistentry> + <term>name</term> + <listitem><para>user's login name</para></listitem> + </varlistentry> + <varlistentry> + <term>uidNumber</term> + <listitem><para>user ID</para></listitem> + </varlistentry> + <varlistentry> + <term>gidNumber</term> + <listitem><para>primary group ID</para></listitem> + </varlistentry> + <varlistentry> + <term>gecos</term> + <listitem><para>user information, typically full name</para></listitem> + </varlistentry> + <varlistentry> + <term>homeDirectory</term> + <listitem><para>home directory</para></listitem> + </varlistentry> + <varlistentry> + <term>loginShell</term> + <listitem><para>user shell</para></listitem> + </varlistentry> + </variablelist> + </para> + <para> + It is possible to add another attribute to + this set by using <quote>+attr_name</quote> + or explicitly remove an attribute using + <quote>-attr_name</quote>. For example, to + allow <quote>telephoneNumber</quote> but deny + <quote>loginShell</quote>, you would use the + following configuration: + <programlisting> +user_attributes = +telephoneNumber, -loginShell + </programlisting> + </para> + <para> + Default: not set. Only the default set of + POSIX attributes is allowed. + </para> + </listitem> + </varlistentry> </variablelist> </refsect1> diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h index 52c480bb4..c03cf6ccc 100644 --- a/src/responder/ifp/ifp_private.h +++ b/src/responder/ifp/ifp_private.h @@ -43,6 +43,7 @@ struct ifp_ctx { int neg_timeout; struct sysbus_ctx *sysbus; + const char **user_whitelist; }; /* This is a throwaway method to ease the review of the patch. @@ -68,4 +69,6 @@ const char *ifp_path_strip_prefix(const char *path, const char *prefix); errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict, struct ldb_message_element *el); +const char **ifp_parse_attr_list(TALLOC_CTX *mem_ctx, const char *conf_str); +bool ifp_attr_allowed(const char *whitelist[], const char *attr); #endif /* _IFPSRV_PRIVATE_H_ */ diff --git a/src/responder/ifp/ifpsrv.c b/src/responder/ifp/ifpsrv.c index 978f3614a..e02b5f08a 100644 --- a/src/responder/ifp/ifpsrv.c +++ b/src/responder/ifp/ifpsrv.c @@ -205,6 +205,7 @@ int ifp_process_init(TALLOC_CTX *mem_ctx, int ret; int max_retries; char *uid_str; + char *attr_list_str; ifp_cmds = get_ifp_cmds(); ret = sss_process_init(mem_ctx, ev, cdb, @@ -271,6 +272,22 @@ int ifp_process_init(TALLOC_CTX *mem_ctx, goto fail; } + ret = confdb_get_string(ifp_ctx->rctx->cdb, ifp_ctx->rctx, + CONFDB_IFP_CONF_ENTRY, CONFDB_IFP_USER_ATTR_LIST, + NULL, &attr_list_str); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, ("Failed to get allowed UIDs.\n")); + goto fail; + } + + ifp_ctx->user_whitelist = ifp_parse_attr_list(ifp_ctx, attr_list_str); + talloc_free(attr_list_str); + if (ifp_ctx->user_whitelist == NULL) { + DEBUG(SSSDBG_FATAL_FAILURE, + ("Failed to parse the allowed attribute list\n")); + goto fail; + } + /* Enable automatic reconnection to the Data Provider */ ret = confdb_get_int(ifp_ctx->rctx->cdb, CONFDB_IFP_CONF_ENTRY, diff --git a/src/responder/ifp/ifpsrv_cmd.c b/src/responder/ifp/ifpsrv_cmd.c index 933b249df..7a200aab8 100644 --- a/src/responder/ifp/ifpsrv_cmd.c +++ b/src/responder/ifp/ifpsrv_cmd.c @@ -96,17 +96,43 @@ static errno_t ifp_user_get_attr_unpack_msg(struct ifp_attr_req *attr_req) { bool parsed; + char **attrs; + int nattrs; + int i, ai; + const char **whitelist = attr_req->ireq->ifp_ctx->user_whitelist; parsed = sbus_request_parse_or_finish(attr_req->ireq->dbus_req, DBUS_TYPE_STRING, &attr_req->name, DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, - &attr_req->attrs, - &attr_req->nattrs, + &attrs, &nattrs, DBUS_TYPE_INVALID); if (parsed == false) { + DEBUG(SSSDBG_OP_FAILURE, "Could not parse arguments\n"); return EOK; /* handled */ } + /* Copy the attributes to maintain memory hierarchy with talloc */ + attr_req->attrs = talloc_zero_array(attr_req, const char *, nattrs+1); + if (attr_req->attrs == NULL) { + return ENOMEM; + } + + ai = 0; + for (i = 0; i < nattrs; i++) { + if (ifp_attr_allowed(whitelist, attrs[i]) == false) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Attribute %s not present in the whitelist, skipping\n", + attrs[i]); + continue; + } + + attr_req->attrs[ai] = talloc_strdup(attr_req->attrs, attrs[i]); + if (attr_req->attrs[ai] == NULL) { + return ENOMEM; + } + ai++; + } + return EOK; } diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c index 2bce20186..4ef0be6af 100644 --- a/src/responder/ifp/ifpsrv_util.c +++ b/src/responder/ifp/ifpsrv_util.c @@ -24,6 +24,11 @@ #include "db/sysdb.h" #include "responder/ifp/ifp_private.h" +#define IFP_DEFAULT_ATTRS {SYSDB_NAME, SYSDB_UIDNUM, \ + SYSDB_GIDNUM, SYSDB_GECOS, \ + SYSDB_HOMEDIR, SYSDB_SHELL, \ + NULL} + errno_t ifp_req_create(struct sbus_request *dbus_req, struct ifp_ctx *ifp_ctx, struct ifp_req **_ifp_req) @@ -178,3 +183,144 @@ errno_t ifp_add_ldb_el_to_dict(DBusMessageIter *iter_dict, return EOK; } + +static inline bool +attr_in_list(const char **list, size_t nlist, const char *str) +{ + size_t i; + + for (i = 0; i < nlist; i++) { + if (strcasecmp(list[i], str) == 0) { + break; + } + } + + return (i < nlist) ? true : false; +} + +const char ** +ifp_parse_attr_list(TALLOC_CTX *mem_ctx, const char *conf_str) +{ + TALLOC_CTX *tmp_ctx; + errno_t ret; + const char **list = NULL; + const char **res = NULL; + int list_size; + char **conf_list = NULL; + int conf_list_size = 0; + const char **allow = NULL; + const char **deny = NULL; + int ai = 0, di = 0, li = 0; + int i; + const char *defaults[] = IFP_DEFAULT_ATTRS; + + tmp_ctx = talloc_new(NULL); + if (tmp_ctx == NULL) { + return NULL; + } + + if (conf_str) { + ret = split_on_separator(tmp_ctx, conf_str, ',', true, true, + &conf_list, &conf_list_size); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot parse attribute ACL list %s: %d\n", conf_str, ret); + goto done; + } + + allow = talloc_zero_array(tmp_ctx, const char *, conf_list_size); + deny = talloc_zero_array(tmp_ctx, const char *, conf_list_size); + if (allow == NULL || deny == NULL) { + goto done; + } + } + + for (i = 0; i < conf_list_size; i++) { + switch (conf_list[i][0]) { + case '+': + allow[ai] = conf_list[i] + 1; + ai++; + continue; + case '-': + deny[di] = conf_list[i] + 1; + di++; + continue; + default: + DEBUG(SSSDBG_CRIT_FAILURE, "ACL values must start with " + "either '+' (allow) or '-' (deny), got '%s'\n", + conf_list[i]); + goto done; + } + } + + /* Assume the output will have to hold defauls and all the configured, + * values, resize later + */ + list_size = 0; + while (defaults[list_size]) { + list_size++; + } + list_size += conf_list_size; + + list = talloc_zero_array(tmp_ctx, const char *, list_size + 1); + if (list == NULL) { + goto done; + } + + /* Start by copying explicitly allowed attributes */ + for (i = 0; i < ai; i++) { + /* if the attribute is explicitly denied, skip it */ + if (attr_in_list(deny, di, allow[i])) { + continue; + } + + list[li] = talloc_strdup(list, allow[i]); + if (list[li] == NULL) { + goto done; + } + li++; + + DEBUG(SSSDBG_TRACE_INTERNAL, + "Added allowed attr %s to whitelist\n", allow[i]); + } + + /* Add defaults */ + for (i = 0; defaults[i]; i++) { + /* if the attribute is explicitly denied, skip it */ + if (attr_in_list(deny, di, defaults[i])) { + continue; + } + + list[li] = talloc_strdup(list, defaults[i]); + if (list[li] == NULL) { + goto done; + } + li++; + + DEBUG(SSSDBG_TRACE_INTERNAL, + "Added default attr %s to whitelist\n", defaults[i]); + } + + res = talloc_steal(mem_ctx, list); +done: + talloc_free(tmp_ctx); + return res; +} + +bool +ifp_attr_allowed(const char *whitelist[], const char *attr) +{ + size_t i; + + if (whitelist == NULL) { + return false; + } + + for (i = 0; whitelist[i]; i++) { + if (strcasecmp(whitelist[i], attr) == 0) { + break; + } + } + + return (whitelist[i]) ? true : false; +} diff --git a/src/tests/cmocka/test_ifp.c b/src/tests/cmocka/test_ifp.c index 188508bcb..70ebf942d 100644 --- a/src/tests/cmocka/test_ifp.c +++ b/src/tests/cmocka/test_ifp.c @@ -206,6 +206,105 @@ void test_el_to_dict(void **state) assert_false(dbus_message_iter_next(&iter_dict)); } +static void assert_string_list_equal(const char **s1, + const char **s2) +{ + int i; + + for (i=0; s1[i]; i++) { + assert_non_null(s2[i]); + assert_string_equal(s1[i], s2[i]); + } + + assert_null(s2[i]); +} + +static void attr_parse_test(const char *expected[], const char *input) +{ + const char **res; + TALLOC_CTX *test_ctx; + + test_ctx = talloc_new(NULL); + assert_non_null(test_ctx); + + res = ifp_parse_attr_list(test_ctx, input); + + if (expected) { + /* Positive test */ + assert_non_null(res); + assert_string_list_equal(res, expected); + } else { + /* Negative test */ + assert_null(res); + } + + talloc_free(test_ctx); +} + +void test_attr_acl(void **state) +{ + /* Test defaults */ + const char *exp_defaults[] = { SYSDB_NAME, SYSDB_UIDNUM, + SYSDB_GIDNUM, SYSDB_GECOS, + SYSDB_HOMEDIR, SYSDB_SHELL, + NULL }; + attr_parse_test(exp_defaults, NULL); + + /* Test adding some attributes to the defaults */ + const char *exp_add[] = { "telephoneNumber", "streetAddress", + SYSDB_NAME, SYSDB_UIDNUM, + SYSDB_GIDNUM, SYSDB_GECOS, + SYSDB_HOMEDIR, SYSDB_SHELL, + NULL }; + attr_parse_test(exp_add, "+telephoneNumber, +streetAddress"); + + /* Test removing some attributes to the defaults */ + const char *exp_rm[] = { SYSDB_NAME, + SYSDB_GIDNUM, SYSDB_GECOS, + SYSDB_HOMEDIR, + NULL }; + attr_parse_test(exp_rm, "-"SYSDB_SHELL ",-"SYSDB_UIDNUM); + + /* Test both add and remove */ + const char *exp_add_rm[] = { "telephoneNumber", + SYSDB_NAME, SYSDB_UIDNUM, + SYSDB_GIDNUM, SYSDB_GECOS, + SYSDB_HOMEDIR, + NULL }; + attr_parse_test(exp_add_rm, "+telephoneNumber, -"SYSDB_SHELL); + + /* Test rm trumps add */ + const char *exp_add_rm_override[] = { SYSDB_NAME, SYSDB_UIDNUM, + SYSDB_GIDNUM, SYSDB_GECOS, + SYSDB_HOMEDIR, SYSDB_SHELL, + NULL }; + attr_parse_test(exp_add_rm_override, + "+telephoneNumber, -telephoneNumber, +telephoneNumber"); + + /* Remove all */ + const char *rm_all[] = { NULL }; + attr_parse_test(rm_all, "-"SYSDB_NAME ", -"SYSDB_UIDNUM + ", -"SYSDB_GIDNUM ", -"SYSDB_GECOS + ", -"SYSDB_HOMEDIR ", -"SYSDB_SHELL); + + /* Malformed list */ + attr_parse_test(NULL, "missing_plus_or_minus"); +} + +void test_attr_allowed(void **state) +{ + const char *whitelist[] = { "name", "gecos", NULL }; + const char *emptylist[] = { NULL }; + + assert_true(ifp_attr_allowed(whitelist, "name")); + assert_true(ifp_attr_allowed(whitelist, "gecos")); + + assert_false(ifp_attr_allowed(whitelist, "password")); + + assert_false(ifp_attr_allowed(emptylist, "name")); + assert_false(ifp_attr_allowed(NULL, "name")); +} + int main(int argc, const char *argv[]) { poptContext pc; @@ -221,6 +320,8 @@ int main(int argc, const char *argv[]) unit_test(ifp_test_req_wrong_uid), unit_test(test_path_prefix), unit_test(test_el_to_dict), + unit_test(test_attr_acl), + unit_test(test_attr_allowed), }; /* Set debug level to invalid value so we can deside if -d 0 was used. */ |