From 2c3fa3a3264c957957db48c6c488049b6cf8b7a1 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 6 Nov 2013 14:12:11 +0100 Subject: IFP: use a list of allowed_uids for authentication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar to the PAC responder, the InfoPipe uses a list of UIDs that are allowed to communicate with the IFP responder. Reviewed-by: Pavel Březina Reviewed-by: Stef Walter (cherry picked from commit 3660f49f81e4db07be66fe0887af9d62065f1f2c) --- src/config/SSSDConfig/__init__.py.in | 3 +++ src/config/SSSDConfigTest.py | 6 +++-- src/config/etc/sssd.api.conf | 4 +++ src/man/sssd-ifp.5.xml | 32 ++++++++++++++++++++++++ src/responder/ifp/ifp_private.h | 3 +++ src/responder/ifp/ifpsrv.c | 20 +++++++++++++++ src/responder/ifp/ifpsrv_cmd.c | 6 +++++ src/responder/ifp/ifpsrv_util.c | 47 +++++++++++++++++++++++++++++++++++- src/tests/cmocka/test_ifp.c | 44 ++++++++++++++++++++++++++++++--- 9 files changed, 159 insertions(+), 6 deletions(-) diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index de0348b1d..7070b88a1 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -94,6 +94,9 @@ option_strings = { # [pac] 'allowed_uids': _('List of UIDs or user names allowed to access the PAC responder'), + # [ifp] + 'allowed_uids': _('List of UIDs or user names allowed to access the InfoPipe responder'), + # [provider] 'id_provider' : _('Identity provider'), 'auth_provider' : _('Authentication provider'), diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index a539b805f..6e8af1dab 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -1234,7 +1234,8 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase): 'sudo', 'autofs', 'ssh', - 'pac'] + 'pac', + 'ifp'] for section in control_list: self.assertTrue(sssdconfig.has_section(section), "Section [%s] missing" % @@ -1327,7 +1328,8 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase): 'sudo', 'autofs', 'ssh', - 'pac'] + 'pac', + 'ifp'] service_list = sssdconfig.list_services() for service in control_list: self.assertTrue(service in service_list, diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index 9805dddfe..d6f2d6b45 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -72,6 +72,10 @@ ssh_known_hosts_timeout = int, None, false # PAC responder allowed_uids = str, None, false +[ifp] +# InfoPipe responder +allowed_uids = str, None, false + [provider] #Available provider types id_provider = str, None, true diff --git a/src/man/sssd-ifp.5.xml b/src/man/sssd-ifp.5.xml index dfac25298..7e2ea7cfc 100644 --- a/src/man/sssd-ifp.5.xml +++ b/src/man/sssd-ifp.5.xml @@ -40,6 +40,38 @@ + + CONFIGURATION OPTIONS + + These options can be used to configure the InfoPipe responder. + + + + allowed_uids (string) + + + Specifies the comma-separated list of UID values or + user names that are allowed to access the InfoPipe + responder. User names are resolved to UIDs at + startup. + + + Default: 0 (only the root user is allowed to access + the InfoPipe responder) + + + Please note that although the UID 0 is used as the + default it will be overwritten with this option. If + you still want to allow the root user to access the + InfoPipe responder, which would be the typical + case, you have to add 0 to the list of allowed UIDs + as well. + + + + + + diff --git a/src/responder/ifp/ifp_private.h b/src/responder/ifp/ifp_private.h index 472d10441..bc476291a 100644 --- a/src/responder/ifp/ifp_private.h +++ b/src/responder/ifp/ifp_private.h @@ -56,6 +56,9 @@ errno_t ifp_req_create(struct sbus_request *dbus_req, struct ifp_ctx *ifp_ctx, struct ifp_req **_ifp_req); +/* Returns an appropriate DBus error for specific ifp_req_create failures */ +int ifp_req_create_handle_failure(struct sbus_request *dbus_req, errno_t err); + 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); diff --git a/src/responder/ifp/ifpsrv.c b/src/responder/ifp/ifpsrv.c index 2f98cf838..65613b941 100644 --- a/src/responder/ifp/ifpsrv.c +++ b/src/responder/ifp/ifpsrv.c @@ -40,6 +40,8 @@ #include "responder/ifp/ifp_private.h" #include "responder/common/responder_sbus.h" +#define DEFAULT_ALLOWED_UIDS "0" + struct mon_cli_iface monitor_ifp_methods = { { &mon_cli_iface_meta, 0 }, .ping = monitor_common_pong, @@ -201,6 +203,7 @@ int ifp_process_init(TALLOC_CTX *mem_ctx, struct be_conn *iter; int ret; int max_retries; + char *uid_str; ifp_cmds = get_ifp_cmds(); ret = sss_process_init(mem_ctx, ev, cdb, @@ -236,6 +239,23 @@ 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_SERVICE_ALLOWED_UIDS, + DEFAULT_ALLOWED_UIDS, &uid_str); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, ("Failed to get allowed UIDs.\n")); + goto fail; + } + + ret = csv_string_to_uid_array(ifp_ctx->rctx, uid_str, true, + &ifp_ctx->rctx->allowed_uids_count, + &ifp_ctx->rctx->allowed_uids); + talloc_free(uid_str); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, ("Failed to set allowed UIDs.\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 4f73342dd..e26bcfa58 100644 --- a/src/responder/ifp/ifpsrv_cmd.c +++ b/src/responder/ifp/ifpsrv_cmd.c @@ -39,12 +39,18 @@ int ifp_ping(struct sbus_request *dbus_req, void *data) static const char *pong = "PONG"; const char *request; DBusError dberr; + errno_t ret; + struct ifp_req *ifp_req; if (ifp_ctx == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "Invalid pointer!\n"); return sbus_request_return_and_finish(dbus_req, DBUS_TYPE_INVALID); } + ret = ifp_req_create(dbus_req, ifp_ctx, &ifp_req); + if (ret != EOK) { + return ifp_req_create_handle_failure(dbus_req, ret); + } if (!sbus_request_parse_or_finish(dbus_req, DBUS_TYPE_STRING, &request, diff --git a/src/responder/ifp/ifpsrv_util.c b/src/responder/ifp/ifpsrv_util.c index e16d36279..2bce20186 100644 --- a/src/responder/ifp/ifpsrv_util.c +++ b/src/responder/ifp/ifpsrv_util.c @@ -29,6 +29,7 @@ errno_t ifp_req_create(struct sbus_request *dbus_req, struct ifp_req **_ifp_req) { struct ifp_req *ireq = NULL; + errno_t ret; if (ifp_ctx->sysbus == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "Responder not connected to sysbus!\n"); @@ -43,8 +44,52 @@ errno_t ifp_req_create(struct sbus_request *dbus_req, ireq->ifp_ctx = ifp_ctx; ireq->dbus_req = dbus_req; + if (dbus_req->client == -1) { + /* We got a sysbus message but couldn't identify the + * caller? Bail out! */ + DEBUG(SSSDBG_CRIT_FAILURE, + "BUG: Received a message without a known caller!\n"); + ret = EACCES; + goto done; + } + + ret = check_allowed_uids(dbus_req->client, + ifp_ctx->rctx->allowed_uids_count, + ifp_ctx->rctx->allowed_uids); + if (ret == EACCES) { + DEBUG(SSSDBG_MINOR_FAILURE, + "User %"PRIi64" not in ACL\n", dbus_req->client); + goto done; + } else if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot check if user %"PRIi64" is present in ACL\n", + dbus_req->client); + goto done; + } + *_ifp_req = ireq; - return EOK; + ret = EOK; +done: + if (ret != EOK) { + talloc_free(ireq); + } + return ret; +} + +int ifp_req_create_handle_failure(struct sbus_request *dbus_req, errno_t err) +{ + if (err == EACCES) { + return sbus_request_fail_and_finish(dbus_req, + sbus_error_new(dbus_req, + DBUS_ERROR_ACCESS_DENIED, + "User %"PRIi64" not in ACL\n", + dbus_req->client)); + } + + return sbus_request_fail_and_finish(dbus_req, + sbus_error_new(dbus_req, + DBUS_ERROR_FAILED, + "Cannot create IFP request\n")); } const char *ifp_path_strip_prefix(const char *path, const char *prefix) diff --git a/src/tests/cmocka/test_ifp.c b/src/tests/cmocka/test_ifp.c index 161f8ffe4..188508bcb 100644 --- a/src/tests/cmocka/test_ifp.c +++ b/src/tests/cmocka/test_ifp.c @@ -24,6 +24,7 @@ #include "db/sysdb.h" #include "tests/cmocka/common_mock.h" +#include "tests/cmocka/common_mock_resp.h" #include "responder/ifp/ifp_private.h" #include "sbus/sssd_dbus_private.h" @@ -35,6 +36,14 @@ mock_ifp_ctx(TALLOC_CTX *mem_ctx) ifp_ctx = talloc_zero(mem_ctx, struct ifp_ctx); assert_non_null(ifp_ctx); + ifp_ctx->rctx = mock_rctx(ifp_ctx, NULL, NULL, NULL); + assert_non_null(ifp_ctx->rctx); + + ifp_ctx->rctx->allowed_uids = talloc_array(ifp_ctx->rctx, uint32_t, 1); + assert_non_null(ifp_ctx->rctx->allowed_uids); + ifp_ctx->rctx->allowed_uids[0] = geteuid(); + ifp_ctx->rctx->allowed_uids_count = 1; + ifp_ctx->sysbus = talloc_zero(ifp_ctx, struct sysbus_ctx); assert_non_null(ifp_ctx->sysbus); @@ -45,7 +54,7 @@ mock_ifp_ctx(TALLOC_CTX *mem_ctx) } static struct sbus_request * -mock_sbus_request(TALLOC_CTX *mem_ctx) +mock_sbus_request(TALLOC_CTX *mem_ctx, uid_t client) { struct sbus_request *sr; @@ -59,6 +68,8 @@ mock_sbus_request(TALLOC_CTX *mem_ctx) assert_non_null(sr->message); dbus_message_set_serial(sr->message, 1); + sr->client = client; + return sr; } @@ -75,7 +86,7 @@ void ifp_test_req_create(void **state) assert_non_null(ifp_ctx); check_leaks_push(ifp_ctx); - sr = mock_sbus_request(ifp_ctx); + sr = mock_sbus_request(ifp_ctx, geteuid()); assert_non_null(sr); check_leaks_push(sr); @@ -92,6 +103,32 @@ void ifp_test_req_create(void **state) assert_true(leak_check_teardown()); } +void ifp_test_req_wrong_uid(void **state) +{ + struct ifp_req *ireq; + struct sbus_request *sr; + struct ifp_ctx *ifp_ctx; + errno_t ret; + + assert_true(leak_check_setup()); + + ifp_ctx = mock_ifp_ctx(global_talloc_context); + assert_non_null(ifp_ctx); + check_leaks_push(ifp_ctx); + + sr = mock_sbus_request(ifp_ctx, geteuid()+1); + assert_non_null(sr); + + ret = ifp_req_create(sr, ifp_ctx, &ireq); + assert_int_equal(ret, EACCES); + talloc_free(sr); + + assert_true(check_leaks_pop(ifp_ctx) == true); + talloc_free(ifp_ctx); + + assert_true(leak_check_teardown()); +} + void test_path_prefix(void **state) { const char *prefix = "foo"; @@ -111,7 +148,7 @@ void test_el_to_dict(void **state) char *attr_name; char *attr_val; - sr = mock_sbus_request(global_talloc_context); + sr = mock_sbus_request(global_talloc_context, geteuid()); assert_non_null(sr); el = talloc(sr, struct ldb_message_element); @@ -181,6 +218,7 @@ int main(int argc, const char *argv[]) const UnitTest tests[] = { unit_test(ifp_test_req_create), + unit_test(ifp_test_req_wrong_uid), unit_test(test_path_prefix), unit_test(test_el_to_dict), }; -- cgit