summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJakub Hrozek <jhrozek@redhat.com>2014-01-02 17:23:08 +0100
committerJakub Hrozek <jhrozek@redhat.com>2014-05-13 22:24:09 +0200
commit770dc892f867639f36f84455d65be6287935a529 (patch)
tree4b847d89ec0cb984140187bee14bf409066f0ec5
parent60cab26b12df9a2153823972cde0c38ca86e01b9 (diff)
downloadsssd-770dc892f867639f36f84455d65be6287935a529.tar.gz
sssd-770dc892f867639f36f84455d65be6287935a529.tar.xz
sssd-770dc892f867639f36f84455d65be6287935a529.zip
IFP: Per-attribute ACL for users
Introduces a new option called user_attributes that allows to specify which user attributes are allowed to be queried from the IFP responder. By default only the default POSIX set is allowed, this option allows to either add other attributes (+attrname) or remove them from the default set (-attrname). Reviewed-by: Pavel Březina <pbrezina@redhat.com>
-rw-r--r--Makefile.am2
-rw-r--r--src/confdb/confdb.h1
-rw-r--r--src/config/SSSDConfig/__init__.py.in1
-rw-r--r--src/config/etc/sssd.api.conf1
-rw-r--r--src/man/sssd-ifp.5.xml62
-rw-r--r--src/responder/ifp/ifp_private.h3
-rw-r--r--src/responder/ifp/ifpsrv.c17
-rw-r--r--src/responder/ifp/ifpsrv_cmd.c30
-rw-r--r--src/responder/ifp/ifpsrv_util.c146
-rw-r--r--src/tests/cmocka/test_ifp.c101
10 files changed, 361 insertions, 3 deletions
diff --git a/Makefile.am b/Makefile.am
index 2fbb19b6d..3bcde0599 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1719,7 +1719,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 8498adb2f..7994a6757 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -132,6 +132,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 3e5bbe9a8..e221eba27 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -97,6 +97,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 a5cb62f45..c7c1232c3 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -70,6 +70,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 2fc4308b4..cd9ab4441 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. */