On Fri, Aug 14, 2015 at 11:00:25PM +0200, Jakub Hrozek wrote:
> On Thu, Aug 13, 2015 at 05:17:32PM +0200, Jakub Hrozek wrote:
> > ACK
> > 
> > I'll just wait for CI results before pushing.
> 
> * master: 52e3ee5c5ff2c5a4341041826a803ad42d2b2de7

Attached is a backport to the sssd-1-11 branch. Please sanity-check
before I push..
>From 7ce09a4d04b7e18b729b0c064ba51f06e4df981d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Wed, 29 Jul 2015 14:51:30 +0200
Subject: [PATCH] sudo: use "higher value wins" when ordering rules

This commit changes the default ordering logic (lower value wins) to
a correct one that is used by native ldap support. It also adds a new
option sudo_inverse_order to switch to the original SSSD (incorrect)
behaviour if needed.

Resolves:
https://fedorahosted.org/sssd/ticket/2682

Reviewed-by: Jakub Hrozek <jhro...@redhat.com>
(cherry picked from commit 52e3ee5c5ff2c5a4341041826a803ad42d2b2de7)
---
 src/confdb/confdb.h                        |  2 ++
 src/config/SSSDConfig/__init__.py.in       |  1 +
 src/config/etc/sssd.api.conf               |  1 +
 src/responder/sudo/sudosrv.c               | 11 ++++++
 src/responder/sudo/sudosrv_get_sudorules.c | 54 ++++++++++++++++++++++++------
 src/responder/sudo/sudosrv_private.h       |  1 +
 6 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 
10ec13bf716f53bf9a10e709fcf3761d00594119..95711a5577a39bc2bd2c5993114ae535143c882e
 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -117,6 +117,8 @@
 #define CONFDB_DEFAULT_SUDO_CACHE_TIMEOUT 180
 #define CONFDB_SUDO_TIMED "sudo_timed"
 #define CONFDB_DEFAULT_SUDO_TIMED false
+#define CONFDB_SUDO_INVERSE_ORDER "sudo_inverse_order"
+#define CONFDB_DEFAULT_SUDO_INVERSE_ORDER false
 
 /* autofs */
 #define CONFDB_AUTOFS_CONF_ENTRY "config/autofs"
diff --git a/src/config/SSSDConfig/__init__.py.in 
b/src/config/SSSDConfig/__init__.py.in
index 
4242df5078fe53c209db95c0854ff1f95f6735dd..3814bb9a3ab42ea34d1c5fc474099e86f852b3e4
 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -84,6 +84,7 @@ option_strings = {
 
     # [sudo]
     'sudo_timed' : _('Whether to evaluate the time-based attributes in sudo 
rules'),
+    'sudo_inverse_order' : _('If true, SSSD will switch back to lower-wins 
ordering logic'),
 
     # [autofs]
     'autofs_negative_timeout' : _('Negative cache timeout length (seconds)'),
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 
0514d86091703f262af45766accc8ee0e2908519..ebdd21563e28ad21b9fb38b572112839182a5cf1
 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -59,6 +59,7 @@ get_domains_timeout = int, None, false
 [sudo]
 # sudo service
 sudo_timed = bool, None, false
+sudo_inverse_order = bool, None, false
 
 [autofs]
 # autofs service
diff --git a/src/responder/sudo/sudosrv.c b/src/responder/sudo/sudosrv.c
index 
25c9d585b8780f3a1d0dd7d246481a9d2455f8f0..df8a1e3210791167cf735ce6926d9368bfbb4baa
 100644
--- a/src/responder/sudo/sudosrv.c
+++ b/src/responder/sudo/sudosrv.c
@@ -167,6 +167,17 @@ int sudo_process_init(TALLOC_CTX *mem_ctx,
         goto fail;
     }
 
+    /* Get sudo_inverse_order option */
+    ret = confdb_get_bool(sudo_ctx->rctx->cdb,
+                          CONFDB_SUDO_CONF_ENTRY, CONFDB_SUDO_INVERSE_ORDER,
+                          CONFDB_DEFAULT_SUDO_INVERSE_ORDER,
+                          &sudo_ctx->inverse_order);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_FATAL_FAILURE, "Error reading from confdb (%d) [%s]\n",
+              ret, strerror(ret));
+        goto fail;
+    }
+
     ret = schedule_get_domains_task(rctx, rctx->ev, rctx);
     if (ret != EOK) {
         DEBUG(SSSDBG_FATAL_FAILURE, "schedule_get_domains_tasks failed.\n");
diff --git a/src/responder/sudo/sudosrv_get_sudorules.c 
b/src/responder/sudo/sudosrv_get_sudorules.c
index 
579874d13304926c196bcdc75e895ee0059bd713..b9ac808469f60ee11d05b8e05ac1c5d550216d10
 100644
--- a/src/responder/sudo/sudosrv_get_sudorules.c
+++ b/src/responder/sudo/sudosrv_get_sudorules.c
@@ -319,6 +319,7 @@ static errno_t sudosrv_get_sudorules_query_cache(TALLOC_CTX 
*mem_ctx,
                                                  const char *username,
                                                  uid_t uid,
                                                  char **groupnames,
+                                                 bool inverse_order,
                                                  struct sysdb_attrs ***_rules,
                                                  uint32_t *_count);
 
@@ -380,6 +381,7 @@ errno_t sudosrv_get_rules(struct sudo_cmd_ctx *cmd_ctx)
                                             cmd_ctx->domain, attrs, flags,
                                             cmd_ctx->orig_username,
                                             cmd_ctx->uid, groupnames,
+                                            cmd_ctx->sudo_ctx->inverse_order,
                                             &expired_rules, 
&expired_rules_num);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Unable to retrieve expired sudo rules "
@@ -591,6 +593,7 @@ static errno_t sudosrv_get_sudorules_from_cache(TALLOC_CTX 
*mem_ctx,
                                             cmd_ctx->domain, attrs, flags,
                                             cmd_ctx->orig_username,
                                             cmd_ctx->uid, groupnames,
+                                            cmd_ctx->sudo_ctx->inverse_order,
                                             &rules, &num_rules);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE,
@@ -616,7 +619,7 @@ done:
 }
 
 static errno_t
-sort_sudo_rules(struct sysdb_attrs **rules, size_t count);
+sort_sudo_rules(struct sysdb_attrs **rules, size_t count, bool higher_wins);
 
 static errno_t sudosrv_get_sudorules_query_cache(TALLOC_CTX *mem_ctx,
                                                  struct sysdb_ctx *sysdb,
@@ -626,6 +629,7 @@ static errno_t sudosrv_get_sudorules_query_cache(TALLOC_CTX 
*mem_ctx,
                                                  const char *username,
                                                  uid_t uid,
                                                  char **groupnames,
+                                                 bool inverse_order,
                                                  struct sysdb_attrs ***_rules,
                                                  uint32_t *_count)
 {
@@ -675,7 +679,7 @@ static errno_t sudosrv_get_sudorules_query_cache(TALLOC_CTX 
*mem_ctx,
         goto done;
     }
 
-    ret = sort_sudo_rules(rules, count);
+    ret = sort_sudo_rules(rules, count, inverse_order);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
               "Could not sort rules by sudoOrder\n");
@@ -692,7 +696,7 @@ done:
 }
 
 static int
-sudo_order_cmp_fn(const void *a, const void *b)
+sudo_order_cmp(const void *a, const void *b, bool lower_wins)
 {
     struct sysdb_attrs *r1, *r2;
     uint32_t o1, o2;
@@ -725,19 +729,49 @@ sudo_order_cmp_fn(const void *a, const void *b)
         return 0;
     }
 
-    if (o1 > o2) {
-        return 1;
-    } else if (o1 < o2) {
-        return -1;
+    if (lower_wins) {
+        /* The lowest value takes priority. Original wrong SSSD behaviour. */
+        if (o1 > o2) {
+            return 1;
+        } else if (o1 < o2) {
+            return -1;
+        }
+    } else {
+        /* The higher value takes priority. Standard LDAP behaviour. */
+        if (o1 < o2) {
+            return 1;
+        } else if (o1 > o2) {
+            return -1;
+        }
     }
 
     return 0;
 }
 
+static int
+sudo_order_low_cmp_fn(const void *a, const void *b)
+{
+    return sudo_order_cmp(a, b, true);
+}
+
+static int
+sudo_order_high_cmp_fn(const void *a, const void *b)
+{
+    return sudo_order_cmp(a, b, false);
+}
+
 static errno_t
-sort_sudo_rules(struct sysdb_attrs **rules, size_t count)
+sort_sudo_rules(struct sysdb_attrs **rules, size_t count, bool lower_wins)
 {
-    qsort(rules, count, sizeof(struct sysdb_attrs *),
-          sudo_order_cmp_fn);
+    if (lower_wins) {
+        DEBUG(SSSDBG_TRACE_FUNC, "Sorting rules with lower-wins logic\n");
+        qsort(rules, count, sizeof(struct sysdb_attrs *),
+              sudo_order_low_cmp_fn);
+    } else {
+        DEBUG(SSSDBG_TRACE_FUNC, "Sorting rules with higher-wins logic\n");
+        qsort(rules, count, sizeof(struct sysdb_attrs *),
+              sudo_order_high_cmp_fn);
+    }
+
     return EOK;
 }
diff --git a/src/responder/sudo/sudosrv_private.h 
b/src/responder/sudo/sudosrv_private.h
index 
3c53755f9e8ec56f3dea52021d14b50f715a54e7..186ed2cb5114d00524b41b801b5f32bac50f7153
 100644
--- a/src/responder/sudo/sudosrv_private.h
+++ b/src/responder/sudo/sudosrv_private.h
@@ -50,6 +50,7 @@ struct sudo_ctx {
      * options
      */
     bool timed;
+    bool inverse_order;
 };
 
 struct sudo_cmd_ctx {
-- 
2.4.3

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to