URL: https://github.com/SSSD/sssd/pull/138
Author: justin-stephenson
 Title: #138: IPA: Skip conflict entries associated with sudo rules
Action: opened

PR body:
"""
SSSD retrieves sudo rule information from the IPA LDAP tree, conflict entries 
will cause problems for SSSD and disallow sudo access when SSSD code is parsing 
entries associated with sudo rules. This PR sets a skip_entry boolean when it 
is appropriate and skips over these conflict entries.

Ticket: https://fedorahosted.org/sssd/ticket/3288

Reproducer steps: Create host conflict entry and associate it with a sudo rule 
that is assigned to certain hosts, attempt to sudo as IDM user. I had some 
difficulty attempting to force replication issues causing the creation of a 
conflict entry, the below manual ldapmodify steps will work also:

- Retrieve the DN of the sudoRule

`# ipa sudorule-find --all --raw | grep 'dn: '
  dn: 
ipaUniqueID=e9025c46-ddab-11e6-9096-525400af7498,cn=sudorules,cn=sudo,dc=jstephen,dc=local`

- Run ldapmodify similar to below

dn: 
ipaUniqueID=e9025c46-ddab-11e6-9096-525400af7498,cn=sudorules,cn=sudo,dc=jstephen,dc=local
changetype: modify
add: memberHost
memberHost: 
fqdn=testhost.jstephen.local+nsuniqueid=cb3d7383-ddb511e6-8c9996c1-71a1e36a,cn=computers,cn=accounts,dc=jstephen,dc=local
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/138/head:pr138
git checkout pr138
From 78b69d7549aaed19e8f955944cfed97948be9c3b Mon Sep 17 00:00:00 2001
From: Justin Stephenson <[email protected]>
Date: Fri, 20 Jan 2017 15:43:34 -0500
Subject: [PATCH 1/3] SUDO: Add skip_entry boolean to convert_ functions

Pass boolean as argument to sudo conversion functions to add logic for
skipping unexpected entries like replication conflicts.

Resolves:
https://fedorahosted.org/sssd/ticket/3288
---
 src/providers/ipa/ipa_sudo_conversion.c | 42 +++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/src/providers/ipa/ipa_sudo_conversion.c b/src/providers/ipa/ipa_sudo_conversion.c
index 9dbc860..d19817a 100644
--- a/src/providers/ipa/ipa_sudo_conversion.c
+++ b/src/providers/ipa/ipa_sudo_conversion.c
@@ -746,12 +746,15 @@ struct ipa_sudo_conv_result_ctx {
 static const char *
 convert_host(TALLOC_CTX *mem_ctx,
              struct ipa_sudo_conv *conv,
-             const char *value)
+             const char *value,
+             bool *skip_entry)
 {
     char *rdn;
     const char *group;
     errno_t ret;
 
+    *skip_entry = false;
+
     ret = ipa_get_rdn(mem_ctx, conv->dom->sysdb, value, &rdn,
                       MATCHRDN_HOST(conv->map_host));
     if (ret == EOK) {
@@ -765,7 +768,8 @@ convert_host(TALLOC_CTX *mem_ctx,
     ret = ipa_get_rdn(mem_ctx, conv->dom->sysdb, value, &rdn,
                       MATCHRDN_HOSTGROUP(conv->map_hostgroup));
     if (ret == ENOENT) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected DN %s\n", value);
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected DN %s: Skipping\n", value);
+        *skip_entry = true;
         return NULL;
     } else if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "ipa_get_rdn() failed on value %s [%d]: %s\n",
@@ -782,12 +786,15 @@ convert_host(TALLOC_CTX *mem_ctx,
 static const char *
 convert_user(TALLOC_CTX *mem_ctx,
              struct ipa_sudo_conv *conv,
-             const char *value)
+             const char *value,
+             bool *skip_entry)
 {
     char *rdn;
     const char *group;
     errno_t ret;
 
+    *skip_entry = false;
+
     ret = ipa_get_rdn(mem_ctx, conv->dom->sysdb, value, &rdn,
                       MATCHRDN_USER(conv->map_user));
     if (ret == EOK) {
@@ -801,7 +808,8 @@ convert_user(TALLOC_CTX *mem_ctx,
     ret = ipa_get_rdn(mem_ctx, conv->dom->sysdb, value, &rdn,
                       MATCHRDN_GROUP(conv->map_group));
     if (ret == ENOENT) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected DN %s\n", value);
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected DN %s: Skipping\n", value);
+        *skip_entry = true;
         return NULL;
     } else if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "ipa_get_rdn() failed on value %s [%d]: %s\n",
@@ -818,12 +826,15 @@ convert_user(TALLOC_CTX *mem_ctx,
 static const char *
 convert_user_fqdn(TALLOC_CTX *mem_ctx,
                   struct ipa_sudo_conv *conv,
-                  const char *value)
+                  const char *value,
+                  bool *skip_entry)
 {
     const char *shortname = NULL;
     char *fqdn = NULL;
 
-    shortname = convert_user(mem_ctx, conv, value);
+    *skip_entry = false;
+
+    shortname = convert_user(mem_ctx, conv, value, skip_entry);
     if (shortname == NULL) {
         return NULL;
     }
@@ -836,15 +847,19 @@ convert_user_fqdn(TALLOC_CTX *mem_ctx,
 static const char *
 convert_group(TALLOC_CTX *mem_ctx,
               struct ipa_sudo_conv *conv,
-              const char *value)
+              const char *value,
+              bool *skip_entry)
 {
     char *rdn;
     errno_t ret;
 
+    *skip_entry = false;
+
     ret = ipa_get_rdn(mem_ctx, conv->dom->sysdb, value, &rdn,
                       MATCHRDN_GROUP(conv->map_group));
     if (ret == ENOENT) {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected DN %s\n", value);
+        DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected DN %s: Skipping\n", value);
+        *skip_entry = true;
         return NULL;
     } else if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "ipa_get_rdn() failed on value %s [%d]: %s\n",
@@ -858,16 +873,23 @@ convert_group(TALLOC_CTX *mem_ctx,
 static const char *
 convert_runasextusergroup(TALLOC_CTX *mem_ctx,
                           struct ipa_sudo_conv *conv,
-                          const char *value)
+                          const char *value,
+                          bool *skip_entry)
 {
+    skip_entry = false;
+
     return talloc_asprintf(mem_ctx, "%%%s", value);
 }
 
 static const char *
 convert_cat(TALLOC_CTX *mem_ctx,
             struct ipa_sudo_conv *conv,
-            const char *value)
+            const char *value,
+            bool *skip_entry)
 {
+
+    *skip_entry = false;
+
     if (strcmp(value, "all") == 0) {
         return talloc_strdup(mem_ctx, "ALL");
     }

From fd8ef10540a2651e2f3307fd578a1190f616e23e Mon Sep 17 00:00:00 2001
From: Justin Stephenson <[email protected]>
Date: Fri, 20 Jan 2017 15:48:43 -0500
Subject: [PATCH 2/3] SUDO: Add boolean to primary conversion function

Add boolean to convert_attributes function allowing certain entries to
be skipped instead of failing the complete conversion operation entirely

Resolves:
https://fedorahosted.org/sssd/ticket/3288
---
 src/providers/ipa/ipa_sudo_conversion.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/providers/ipa/ipa_sudo_conversion.c b/src/providers/ipa/ipa_sudo_conversion.c
index d19817a..3ef077a 100644
--- a/src/providers/ipa/ipa_sudo_conversion.c
+++ b/src/providers/ipa/ipa_sudo_conversion.c
@@ -907,12 +907,14 @@ convert_attributes(struct ipa_sudo_conv *conv,
     const char *value;
     errno_t ret;
     int i, j;
+    bool skip_entry;
     static struct {
         const char *ipa;
         const char *sudo;
         const char *(*conv_fn)(TALLOC_CTX *mem_ctx,
                                struct ipa_sudo_conv *conv,
-                               const char *value);
+                               const char *value,
+                               bool *skip_entry);
     } table[] = {{SYSDB_NAME,                            SYSDB_SUDO_CACHE_AT_CN         , NULL},
                  {SYSDB_IPA_SUDORULE_HOST,               SYSDB_SUDO_CACHE_AT_HOST       , convert_host},
                  {SYSDB_IPA_SUDORULE_USER,               SYSDB_SUDO_CACHE_AT_USER       , convert_user_fqdn},
@@ -953,10 +955,15 @@ convert_attributes(struct ipa_sudo_conv *conv,
 
         for (j = 0; values[j] != NULL; j++) {
             if (table[i].conv_fn != NULL) {
-                value = table[i].conv_fn(tmp_ctx, conv, values[j]);
+                value = table[i].conv_fn(tmp_ctx, conv, values[j], &skip_entry);
                 if (value == NULL) {
-                    ret = ENOMEM;
-                    goto done;
+                    if (skip_entry) {
+                        ret = ENOENT;
+                        continue;
+                    } else {
+                        ret = ENOMEM;
+                        goto done;
+                    }
                 }
             } else {
                 value = values[j];

From 7ff4f3dc5d3d84d02ecf6869b53ebcb645482049 Mon Sep 17 00:00:00 2001
From: Justin Stephenson <[email protected]>
Date: Wed, 25 Jan 2017 17:05:01 -0500
Subject: [PATCH 3/3] TESTS: Add to IPA DN test

Add test to ensure conflict entries return ENOENT

Resolves:
https://fedorahosted.org/sssd/ticket/3288
---
 src/tests/cmocka/test_ipa_dn.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/tests/cmocka/test_ipa_dn.c b/src/tests/cmocka/test_ipa_dn.c
index a6e26ec..1cd5013 100644
--- a/src/tests/cmocka/test_ipa_dn.c
+++ b/src/tests/cmocka/test_ipa_dn.c
@@ -169,6 +169,11 @@ static void ipa_get_rdn_test(void **state)
     ret = ipa_get_rdn(test_ctx, test_ctx->sysdb, "cn=rdn,attr1=value1", &rdn, "cn", "attr1", "value1");
     assert_int_equal(ret, ENOENT);
     assert_null(rdn);
+
+    ret = ipa_get_rdn(test_ctx, test_ctx->sysdb, "cn=rdn+nsuniqueid=9b1e3301-c32611e6-bdcae37a-ef905e7c,attr1=value1,attr2=value2,dc=example,dc=com",
+                      &rdn, "cn", "attr1", "value1", "attr2", "value2");
+    assert_int_equal(ret, ENOENT);
+    assert_null(rdn);
 }
 
 int main(int argc, const char *argv[])
_______________________________________________
sssd-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to