SUDO Integration review issues

16 files changed, 68 insertions, 41 deletions

diff --git a/src/config/SSSDConfig.py b/src/config/SSSDConfig.py
index 44bfb69f..0a73893b 100644
--- a/src/config/SSSDConfig.py
+++ b/src/config/SSSDConfig.py
@@ -238,6 +238,20 @@ option_strings = {
     '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'),
     'simple_deny_users' : _('Comma separated list of prohibited users'),
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 8421a091..afc207c0 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -489,7 +489,8 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
             'id_provider',
             'auth_provider',
             'access_provider',
-            'chpass_provider']
+            'chpass_provider',
+            'sudo_provider']
 
         self.assertTrue(type(options) == dict,
                         "Options should be a dictionary")
@@ -681,7 +682,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'],
@@ -807,7 +808,8 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase):
             'id_provider',
             'auth_provider',
             'access_provider',
-            'chpass_provider']
+            'chpass_provider',
+            'sudo_provider']
 
         self.assertTrue(type(options) == dict,
                         "Options should be a dictionary")
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index c2c425ce..34b67dec 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 9a89bfe2..b155c2bc 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 8ca4c17f..9fcb7ae5 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 1703e78e..784b642e 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) {
@@ -287,9 +277,11 @@ sysdb_purge_sudorule_subtree(struct sysdb_ctx *sysdb,
     const char *name;
     int i;
     errno_t ret;
-    const char *attrs[] = { SYSDB_OBJECTCLASS
+    const char *attrs[] = { SYSDB_OBJECTCLASS,
+                            SYSDB_NAME,
                             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 +289,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 +308,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 bf77c5f6..e30395de 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 4f90b185..c3ea8c37 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 71921963..6ca6f346 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 68cb47cd..387cf0c1 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 0c621f5d..27f01f92 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 5d54f95a..fca6257d 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 9a749455..01fdee05 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 04e19db2..1b554678 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 d33215a0..2b158b7d 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 e7da035e..be31037e 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;