On (23/04/14 22:43), Jakub Hrozek wrote:
Hi,
the attached patches upstream functionality that mod_lookup_identity has
been using for a while, but which we couldn't push to master due to the
pending sbus changes. Now that these are accepted, it's time to merge
the DBus methods themselves. Previously, mod_lookup_identity relied on a
private branch with quite a bit of ad-hoc code.
Please note that when the work in progress to expose users, groups and
domains is merged as well, this interface will become obsolete in favor
of the objects. But we shouldn't let a released version of
mod_lookup_identity rely on unreleased sssd code any longer. Moreover,
all patches except the last one will be used even later on and there's
no fixed timeline on converting mod_lookup_identity.
[PATCH 1/6] SBUS: two trivial style fixes
SSIA
[PATCH 2/6] SBUS: Add a convenience function
Adds a convenience function that constructs a DBusError internally and
as such can be used to mark an sbus request as failed without having
to create a DBusError instance by the caller. The error message must be
either allocated on top of dbus_req or a constant, DBus itself doesn't
copy the error message! This is reflected in the doxygen docstring.
[PATCH 3/6] IFP: Add utility functions
Adds a number of utility functions, most importanly ifp_req_create().
The ifp_req is a structure that will be passed along with the ifp
request and contains the sbus request and some ifp-specific data, like
caller ID or the responder context.
[PATCH 4/6] IFP: use a list of allowed_uids for authentication
Similar to the PAC responder, the InfoPipe uses a list of UIDs that are
allowed to communicate with the IFP responder.
[PATCH 5/6] IFP: Initialize negative cache timeout
In order to avoid hitting the back end with repetitive requests, the
InfoPipe responder needs a negative cache, too. This patch follows the
convention set by other responders, where the negative cache timeouts are
read from the [nss] section. This is not ideal, however, and ticket #2318
tracks moving the configuration to the [ifp] section primarily.
The timeout is also a separate parameter in the NSS context. We should
consider moving it to the negcache context instead (#2317).
[PATCH 6/6] IFP: Add GetUserAttrs call
Adds a DBus method that allows the caller to retrieve attributes of a
user. The synopsis of the call is as follows:
<method name="GetUserAttr">
<arg type="s" name="user" direction="in"/>
<arg type="as" name="attr" direction="in"/>
<arg type="a{sv}" name="values"
direction="out"/>
</method>
The return value is an array (one attribute per array member) of
dictionaries. The key of the dictionary is the attribute name, the value
is a variant containing the attribute values as strings.
If an attribute does not exist or is not permitted to be read, no error
is returned. If the users does not exist, the method returns an error.
In future patches this function will be marked as obsolete in favor of
object-oriented approach.
ifp_user_get_attr_unpack_msg is a separate function to allow extending
it in a later patch.
The function to check the cache validity duplicates quite a bit of code
with the NSS responder. The refactoring would be nice to get done along
with #843. I have a patch in my private branch, but it's not complete,
sorry.
From 53a9af5e3659293e2845b9a7aafea4e8bfbe818e Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Mon, 6 Jan 2014 15:27:44 +0100
Subject: [PATCH 3/6] IFP: Add utility functions
Adds a number of utility functions, most importanly ifp_req_create().
The ifp_req is a structure that will be passed along with the ifp
request and contains the sbus request and some ifp-specific data, like
caller ID.
---
Makefile.am | 29 ++++-
src/responder/ifp/ifp_private.h | 16 +++
src/responder/ifp/ifpsrv_util.c | 197 ++++++++++++++++++++++++++++++++++
src/tests/cmocka/test_ifp.c | 231 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 472 insertions(+), 1 deletion(-)
create mode 100644 src/responder/ifp/ifpsrv_util.c
create mode 100644 src/tests/cmocka/test_ifp.c
diff --git a/Makefile.am b/Makefile.am
index 56d8e1df648822a1fe8b3ca5fc8995f3c78a8566..e05461a3ab400e34bf685de89f2ec038822648fe
100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -176,7 +176,12 @@ if HAVE_CMOCKA
dp_opt_tests \
responder-get-domains-tests \
test_search_bases
-endif
+
+if BUILD_IFP
+non_interactive_cmocka_based_tests += ifp_tests
+endif # BUILD_IFP
+
+endif # HAVE_CMOCKA
check_PROGRAMS = \
stress-tests \
@@ -818,6 +823,7 @@ sssd_ifp_SOURCES = \
src/responder/ifp/ifpsrv_cmd.c \
src/responder/ifp/ifp_iface_generated.c \
src/responder/ifp/ifp_iface_generated.h \
+ src/responder/ifp/ifpsrv_util.c \
$(SSSD_UTIL_OBJ) \
$(SSSD_RESPONDER_OBJ)
sssd_ifp_CFLAGS = \
@@ -1676,8 +1682,29 @@ dp_opt_tests_LDADD = \
$(SSSD_INTERNAL_LTLIBS) \
libsss_test_common.la
+if BUILD_IFP
+ifp_tests_DEPENDENCIES = \
^^^^
Use prefix EXTRA_ if you need memberof pluging for this test.
Otherwise, it can break parallel build.
We have this prefix also on other places.
+ $(ldblib_LTLIBRARIES)
+ifp_tests_SOURCES = \
+ $(TEST_MOCK_RESP_OBJ) \
+ src/tests/cmocka/test_ifp.c \
+ src/responder/ifp/ifpsrv_cmd.c \
+ src/responder/ifp/ifpsrv_util.c
+ifp_tests_CFLAGS = \
+ $(AM_CFLAGS)
+ifp_tests_LDFLAGS = \
+ -Wl,-wrap,sbus_conn_send_reply \
+ -Wl,-wrap,dbus_message_get_sender \
+ -Wl,-wrap,dbus_bus_get_unix_user
+ifp_tests_LDADD = \
+ $(CMOCKA_LIBS) \
+ $(SSSD_LIBS) \
+ $(SSSD_INTERNAL_LTLIBS) \
+ libsss_test_common.la
endif
+endif # HAVE_CMOCKA
+
noinst_PROGRAMS = pam_test_client
if BUILD_SUDO
noinst_PROGRAMS += sss_sudo_cli
diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h
index b97cb8a7d9e55f550a3eda0e9acc034329d4ba17..d33009a5dbc95f8c0ca8905d9c8b9e42eaa24988
100644
--- a/src/responder/ifp/ifp_private.h
+++ b/src/responder/ifp/ifp_private.h
@@ -46,4 +46,20 @@ struct ifp_ctx {
* It will be removed later */
int ifp_ping(struct sbus_request *dbus_req, void *data);
+/* == Utility functions == */
+struct ifp_req {
+ struct sbus_request *dbus_req;
+ struct ifp_ctx *ifp_ctx;
+ uid_t caller;
+};
+
+struct ifp_req *ifp_req_create(struct sbus_request *dbus_req,
+ struct ifp_ctx *ifp_ctx);
+
+int ifp_enomem(struct ifp_req *ireq);
+int ifp_return_failure(struct ifp_req *ireq, const char *err_msg);
+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);
+
#endif /* _IFPSRV_PRIVATE_H_ */
//snip
From 533aa8adef72fb5fa5cece55059ecfe1d1f030b9 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Tue, 15 Apr 2014 15:42:15 +0200
Subject: [PATCH 6/6] IFP: Add GetUserAttrs call
Adds a DBus method that allows the caller to retrieve attributes of a
user. The synopsis of the call is as follows:
<method name="GetUserAttr">
<arg type="s" name="user" direction="in"/>
<arg type="as" name="attr" direction="in"/>
<arg type="a{sv}" name="values"
direction="out"/>
</method>
The return value is an array (one attribute per array member) of
dictionaries. The key of the dictionary is the attribute name, the value
is a variant containing the attribute values as strings.
If an attribute does not exist or is not permitted to be read, no error
is returned. If the users does not exist, the method returns an error.
In future patches this function will be marked as obsolete in favor of
object-oriented approach.
ifp_user_get_attr_unpack_msg is a separate function to allow extending
it in a later patch.
The function to check the cache validity duplicates quite a bit of code
with the NSS responder. The refactoring would be nice to get done along
with #843.
---
There are warnings introduced in this last patch.
src/responder/ifp/ifpsrv_cmd.c: In function ‘ifp_user_get_attr_process’:
src/responder/ifp/ifpsrv_cmd.c:174:38: warning: ‘res’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
el = ldb_msg_find_element(res->msgs[0], attrs[ai]);
^
src/responder/ifp/ifpsrv_cmd.c:119:24: note: ‘res’ was declared here
struct ldb_result *res;
^
And some in tests.
src/tests/cmocka/test_ifp.c: In function ‘test_el_to_dict’:
src/tests/cmocka/test_ifp.c:186:26: warning: cast discards ‘__attribute__((const))’
qualifier from pointer target type [-Wcast-qual]
el->values[0].data = (uint8_t *) "one";
^
src/tests/cmocka/test_ifp.c:188:26: warning: cast discards ‘__attribute__((const))’
qualifier from pointer target type [-Wcast-qual]
el->values[1].data = (uint8_t *) "two";
LS