From b1b858bad79dc3921412d6a301471b1016eac79a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= Date: Thu, 12 Jan 2012 09:41:52 +0100 Subject: [PATCH 1/4] SUDO Integration review issues --- src/config/SSSDConfig.py | 14 +++++++++++++ src/config/SSSDConfigTest.py | 2 +- src/config/etc/sssd.api.conf | 1 + src/config/etc/sssd.api.d/sssd-ldap.conf | 14 +++++++++++++ src/db/sysdb.c | 2 +- src/db/sysdb_sudo.c | 30 ++++++++++----------------- src/providers/data_provider_be.c | 8 +++--- src/providers/ipa/ipa_common.c | 4 +- src/providers/ldap/ldap_common.c | 2 +- src/providers/ldap/sdap_sudo.c | 3 +- src/responder/sudo/sudosrv_dp.c | 2 + src/responder/sudo/sudosrv_get_sudorules.c | 3 +- src/sss_client/sudo/sss_sudo.c | 6 ++-- src/sss_client/sudo/sss_sudo.h | 6 ++-- src/sss_client/sudo/sss_sudo_response.c | 1 + src/sss_client/sudo_testcli/sudo_testcli.c | 2 +- 16 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/config/SSSDConfig.py b/src/config/SSSDConfig.py index 44bfb69..f103ed3 100644 --- a/src/config/SSSDConfig.py +++ b/src/config/SSSDConfig.py @@ -237,6 +237,20 @@ option_strings = { # [provider/ldap/chpass] 'ldap_chpass_uri' : _('URI of an LDAP server where password changes are allowed'), 'ldap_chpass_dns_service_name' : _('DNS service name for LDAP password change server'), + + # [provider/ldap/sudo] + 'ldap_sudo_search_base' : _('Base DN for sudo rules lookups'), + 'ldap_sudorule_object_class' : _('Object class for sudo rules'), + 'ldap_sudorule_name' : _('Sudo rule name'), + 'ldap_sudorule_command' : _('Sudo rule command attribute'), + 'ldap_sudorule_host' : _('Sudo rule host attribute'), + 'ldap_sudorule_user' : _('Sudo rule user attribute'), + 'ldap_sudorule_option' : _('Sudo rule option attribute'), + 'ldap_sudorule_runasuser' : _('Sudo rule runasuser attribute'), + 'ldap_sudorule_runasgroup' : _('Sudo rule runasgroup attribute'), + 'ldap_sudorule_notbefore' : _('Sudo rule notbefore attribute'), + 'ldap_sudorule_notafter' : _('Sudo rule notafter attribute'), + 'ldap_sudorule_order' : _('Sudo rule order attribute'), # [provider/simple/access] 'simple_allow_users' : _('Comma separated list of allowed users'), diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index 8421a09..07e772c 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -681,7 +681,7 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase): control_provider_dict = { 'ipa': ['id', 'auth', 'access', 'chpass'], 'local': ['id', 'auth', 'chpass'], - 'ldap': ['id', 'auth', 'access', 'chpass'], + 'ldap': ['id', 'auth', 'access', 'chpass', 'sudo'], 'krb5': ['auth', 'access', 'chpass'], 'proxy': ['id', 'auth'], 'simple': ['access'], diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index c2c425c..34b67de 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -49,6 +49,7 @@ id_provider = str, None, true auth_provider = str, None, false access_provider = str, None, false chpass_provider = str, None, false +sudo_provider = str, None, false [domain] # Options available to all domains diff --git a/src/config/etc/sssd.api.d/sssd-ldap.conf b/src/config/etc/sssd.api.d/sssd-ldap.conf index 9a89bfe..b155c2b 100644 --- a/src/config/etc/sssd.api.d/sssd-ldap.conf +++ b/src/config/etc/sssd.api.d/sssd-ldap.conf @@ -106,3 +106,17 @@ ldap_access_order = str, None, false [provider/ldap/chpass] ldap_chpass_uri = str, None, false ldap_chpass_dns_service_name = str, None, false + +[provider/ldap/sudo] +ldap_sudo_search_base = str, None, false +ldap_sudorule_object_class = str, None, false +ldap_sudorule_name = str, None, false +ldap_sudorule_command = str, None, false +ldap_sudorule_host = str, None, false +ldap_sudorule_user = str, None, false +ldap_sudorule_option = str, None, false +ldap_sudorule_runasuser = str, None, false +ldap_sudorule_runasgroup = str, None, false +ldap_sudorule_notbefore = str, None, false +ldap_sudorule_notafter = str, None, false +ldap_sudorule_order = str, None, false diff --git a/src/db/sysdb.c b/src/db/sysdb.c index 8ca4c17..9fcb7ae 100644 --- a/src/db/sysdb.c +++ b/src/db/sysdb.c @@ -1782,7 +1782,7 @@ errno_t sysdb_msg2attrs(TALLOC_CTX *mem_ctx, size_t count, for (i = 0; i < count; i++) { a[i] = talloc(a, struct sysdb_attrs); if (a[i] == NULL) { - DEBUG(1, ("talloc_array failed.\n")); + DEBUG(1, ("talloc failed.\n")); talloc_free(a); return ENOMEM; } diff --git a/src/db/sysdb_sudo.c b/src/db/sysdb_sudo.c index 1703e78..81b330c 100644 --- a/src/db/sysdb_sudo.c +++ b/src/db/sysdb_sudo.c @@ -73,7 +73,7 @@ sysdb_get_sudo_filter(TALLOC_CTX *mem_ctx, const char *username, int i; tmp_ctx = talloc_new(NULL); - if (tmp_ctx == NULL) return ENOMEM; + NULL_CHECK(tmp_ctx, ret, done); /* AND with objectclass */ filter = talloc_asprintf(tmp_ctx, "(&(%s=%s)", @@ -156,7 +156,7 @@ sysdb_get_sudo_user_info(TALLOC_CTX *mem_ctx, const char *username, int i; tmp_ctx = talloc_new(NULL); - if (tmp_ctx == NULL) return ENOMEM; + NULL_CHECK(tmp_ctx, ret, done); attrs[0] = SYSDB_MEMBEROF; attrs[1] = SYSDB_UIDNUM; @@ -181,13 +181,10 @@ sysdb_get_sudo_user_info(TALLOC_CTX *mem_ctx, const char *username, sysdb_groupnames = NULL; } else { sysdb_groupnames = talloc_array(tmp_ctx, char *, groups->num_values+1); - if (!sysdb_groupnames) { - ret = ENOMEM; - goto done; - } + NULL_CHECK(sysdb_groupnames, ret, done); /* Get a list of the groups by groupname only */ - for (i=0; i < groups->num_values; i++) { + for (i = 0; i < groups->num_values; i++) { ret = sysdb_group_dn_name(sysdb, sysdb_groupnames, (const char *)groups->values[i].data, @@ -218,17 +215,10 @@ sysdb_sudo_purge_subdir(struct sysdb_ctx *sysdb, errno_t ret; tmp_ctx = talloc_new(NULL); - if (tmp_ctx == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_new() failed\n")); - ret = ENOMEM; - goto done; - } + NULL_CHECK(tmp_ctx, ret, done); base_dn = sysdb_custom_subtree_dn(sysdb, tmp_ctx, domain->name, subdir); - if (base_dn == NULL) { - ret = ENOMEM; - goto done; - } + NULL_CHECK(base_dn, ret, done); ret = sysdb_delete_recursive(sysdb, base_dn, true); if (ret != EOK) { @@ -289,7 +279,8 @@ sysdb_purge_sudorule_subtree(struct sysdb_ctx *sysdb, errno_t ret; const char *attrs[] = { SYSDB_OBJECTCLASS SYSDB_SUDO_CACHE_AT_OC, - SYSDB_SUDO_CACHE_AT_CN }; + SYSDB_SUDO_CACHE_AT_CN, + NULL }; /* just purge all if there's no filter */ if (!filter) { @@ -297,7 +288,7 @@ sysdb_purge_sudorule_subtree(struct sysdb_ctx *sysdb, } tmp_ctx = talloc_new(NULL); - if (tmp_ctx == NULL) return ENOMEM; + NULL_CHECK(tmp_ctx, ret, done); /* match entries based on the filter and remove them one by one */ ret = sysdb_search_custom(tmp_ctx, sysdb, filter, @@ -316,7 +307,8 @@ sysdb_purge_sudorule_subtree(struct sysdb_ctx *sysdb, name = ldb_msg_find_attr_as_string(msgs[i], SYSDB_NAME, NULL); if (name == NULL) { DEBUG(SSSDBG_OP_FAILURE, ("A rule without a name?\n")); - goto done; + /* skip this one but still delete other entries */ + continue; } ret = sysdb_delete_custom(sysdb, name, SUDORULE_SUBDIR); diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index bf77c5f..e30395d 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -696,11 +696,11 @@ static int be_sudo_handler(DBusMessage *message, struct sbus_connection *conn) DBUS_TYPE_STRING, &(be_sudo_req->username), DBUS_TYPE_INVALID); - if (dbus_error_is_set(&dbus_error)) { - dbus_error_free(&dbus_error); - } - if (!dbus_ret) { + if (dbus_error_is_set(&dbus_error)) { + dbus_error_free(&dbus_error); + } + DEBUG(SSSDBG_CRIT_FAILURE, ("dbus_message_get_args failed.\n")); ret = EINVAL; goto fail; diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c index 4f90b18..c3ea8c3 100644 --- a/src/providers/ipa/ipa_common.c +++ b/src/providers/ipa/ipa_common.c @@ -468,14 +468,14 @@ int ipa_get_id_options(struct ipa_options *ipa_opts, if (NULL == dp_opt_get_string(ipa_opts->id->basic, SDAP_SUDO_SEARCH_BASE)) { #if 0 - ret = dp_opt_set_string(ipa_opts->id->basic, SDAP_NETGROUP_SEARCH_BASE, + ret = dp_opt_set_string(ipa_opts->id->basic, SDAP_SUDO_SEARCH_BASE, dp_opt_get_string(ipa_opts->id->basic, SDAP_SEARCH_BASE)); if (ret != EOK) { goto done; } #else - /* We don't yet have support for the native representation + /* We don't yet have support for the representation * of sudo in IPA. For now, we need to point at the * compat tree */ diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index 7192196..6ca6f34 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -271,7 +271,7 @@ int ldap_get_options(TALLOC_CTX *memctx, /* Handle search bases */ search_base = dp_opt_get_string(opts->basic, SDAP_SEARCH_BASE); if (search_base != NULL) { - /* set user/group/netgroup/sudo search bases if they are not */ + /* set user/group/netgroup search bases if they are not */ for (o = 0; search_base_options[o] != -1; o++) { if (NULL == dp_opt_get_string(opts->basic, search_base_options[o])) { ret = dp_opt_set_string(opts->basic, search_base_options[o], diff --git a/src/providers/ldap/sdap_sudo.c b/src/providers/ldap/sdap_sudo.c index 68cb47c..387cf0c 100644 --- a/src/providers/ldap/sdap_sudo.c +++ b/src/providers/ldap/sdap_sudo.c @@ -408,8 +408,6 @@ void sdap_sudo_load_sudoers_done(struct tevent_req *req) DEBUG(SSSDBG_TRACE_FUNC, ("Received %d rules\n", rules_count)); /* purge cache */ - /* TODO purge with filter */ - DEBUG(SSSDBG_TRACE_FUNC, ("Purging sudo cache with filter %s\n", "")); ret = sdap_sudo_purge_sudoers(sudo_ctx); if (ret != EOK) { goto done; @@ -448,6 +446,7 @@ int sdap_sudo_purge_sudoers(struct sdap_sudo_ctx *sudo_ctx) } /* Purge rules */ + DEBUG(SSSDBG_TRACE_FUNC, ("Purging sudo cache with filter [%s]\n", filter)); ret = sysdb_purge_sudorule_subtree(sysdb_ctx, sudo_ctx->be_ctx->domain, filter); if (ret != EOK) { diff --git a/src/responder/sudo/sudosrv_dp.c b/src/responder/sudo/sudosrv_dp.c index 0c621f5..27f01f9 100644 --- a/src/responder/sudo/sudosrv_dp.c +++ b/src/responder/sudo/sudosrv_dp.c @@ -107,6 +107,8 @@ struct tevent_req * sudosrv_dp_refresh_send(struct resp_ctx *rctx, error: tevent_req_error(req, ret); tevent_req_post(req, rctx->ev); + dbus_message_unref(msg); + return req; } diff --git a/src/responder/sudo/sudosrv_get_sudorules.c b/src/responder/sudo/sudosrv_get_sudorules.c index 5d54f95..fca6257 100644 --- a/src/responder/sudo/sudosrv_get_sudorules.c +++ b/src/responder/sudo/sudosrv_get_sudorules.c @@ -481,7 +481,8 @@ char * sudosrv_get_sudorules_parse_query(TALLOC_CTX *mem_ctx, const char *query_body, int query_len) { - if (query_len < 2 || ((query_len - 1) != strlen(query_body))) { + /* empty string or not NULL terminated */ + if (query_len < 2 || strnlen(query_body, query_len) == query_len) { DEBUG(SSSDBG_CRIT_FAILURE, ("Invalid query.\n")); return NULL; } diff --git a/src/sss_client/sudo/sss_sudo.c b/src/sss_client/sudo/sss_sudo.c index 9a74945..01fdee0 100644 --- a/src/sss_client/sudo/sss_sudo.c +++ b/src/sss_client/sudo/sss_sudo.c @@ -36,9 +36,9 @@ static void sss_sudo_free_rules(unsigned int num_rules, static void sss_sudo_free_attrs(unsigned int num_attrs, struct sss_attr *attrs); -int sss_sudo_get_result(const char *username, - uint32_t *_error, - struct sss_result **_result) +int sss_sudo_send_recv(const char *username, + uint32_t *_error, + struct sss_result **_result) { struct sss_result *result = NULL; struct sss_cli_req_data request; diff --git a/src/sss_client/sudo/sss_sudo.h b/src/sss_client/sudo/sss_sudo.h index 04e19db..1b55467 100644 --- a/src/sss_client/sudo/sss_sudo.h +++ b/src/sss_client/sudo/sss_sudo.h @@ -45,9 +45,9 @@ struct sss_result { struct sss_rule *rules; }; -int sss_sudo_get_result(const char *username, - uint32_t *_error, - struct sss_result **_result); +int sss_sudo_send_recv(const char *username, + uint32_t *_error, + struct sss_result **_result); void sss_sudo_free_result(struct sss_result *result); diff --git a/src/sss_client/sudo/sss_sudo_response.c b/src/sss_client/sudo/sss_sudo_response.c index d33215a..2b158b7 100644 --- a/src/sss_client/sudo/sss_sudo_response.c +++ b/src/sss_client/sudo/sss_sudo_response.c @@ -188,6 +188,7 @@ int sss_sudo_parse_uint32(const char *message, return EINVAL; } + /* expanded SAFEALIGN_COPY_UINT32 macro from util.h */ memcpy(_number, message + start_pos, sizeof(uint32_t)); *_cursor = start_pos + sizeof(uint32_t); diff --git a/src/sss_client/sudo_testcli/sudo_testcli.c b/src/sss_client/sudo_testcli/sudo_testcli.c index e7da035..be31037 100644 --- a/src/sss_client/sudo_testcli/sudo_testcli.c +++ b/src/sss_client/sudo_testcli/sudo_testcli.c @@ -96,7 +96,7 @@ int main(int argc, char **argv) /* get sss_result - it will send new query to responder */ - ret = sss_sudo_get_result(username, &error, &result); + ret = sss_sudo_send_recv(username, &error, &result); if (ret != EOK) { fprintf(stderr, "Usss_sudo_get_result() failed: %s\n", strerror(ret)); goto fail; -- 1.7.6.4