[Pkg-sssd-devel] SUDOERs Bug in SSD < 1.13; Fix Backport Requested

Aaron Peschel apeschel at zendesk.com
Tue Feb 9 00:06:12 UTC 2016


diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index df45433..9aa2648 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -124,6 +124,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 49de53e..6294d59 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -92,6 +92,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 cf6ce63..2e5b02e 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -63,6 +63,7 @@ pam_account_expired_message = str, 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 2499586..ff5d92e 100644
--- a/src/responder/sudo/sudosrv.c
+++ b/src/responder/sudo/sudosrv.c
@@ -149,6 +149,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 34d63bd..a0b09e6 100644
--- a/src/responder/sudo/sudosrv_get_sudorules.c
+++ b/src/responder/sudo/sudosrv_get_sudorules.c
@@ -325,6 +325,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);

@@ -386,6 +387,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 "
@@ -597,6 +599,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,
@@ -622,7 +625,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
sss_domain_info *domain,
@@ -631,6 +634,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)
 {
@@ -680,7 +684,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");
@@ -697,7 +701,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;
@@ -730,19 +734,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 3c53755..186ed2c 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 {

On Mon, Feb 8, 2016 at 4:05 PM, Aaron Peschel <apeschel at zendesk.com> wrote:
> Hello Timo,
>
> Thanks for the very quick response!
>
> I tracked down the commit that fixed the bug, here's a link to it:
>
> https://git.fedorahosted.org/cgit/sssd.git/commit/?id=52e3ee5c5ff2c5a4341041826a803ad42d2b2de7
>
> I took a look at the source for 1.12.5 to see if the commit had
> already been backported, but unfortunately, the bug is present in your
> latest 1.12.5 release.
>
> I backported the commit to work with your 1.11.7 release, I'll attach
> it in the following email
>
> -Aaron
>
> On Mon, Feb 8, 2016 at 2:40 PM, Timo Aaltonen <tjaalton at debian.org> wrote:
>> 09.02.2016, 00:06, Aaron Peschel kirjoitti:
>>> There's a bug in SSSD versions prior to 1.13 that causes the order of
>>> LDAP SUDOERs rules using the sudoOrder attribute to be incorrect.
>>>
>>> https://www.sudo.ws/pipermail/sudo-users/2016-January/005723.html
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1138576
>>>
>>> This bug exists in the SSD packages for all Ubuntu releases except
>>> Xenial. Unfortunately, since Xenial is a systemd based release, it's
>>> package cannot directly be backported to the previous Ubuntu releases.
>>>
>>> It would be greatly appreciated if you could backport the fix for this
>>> bug to the other releases, or provide a backported version of the 1.13
>>> package for Upstart releases. 12.04 (precise) is the release I am
>>> personally interested in a fix for.
>>
>> Have you tried 1.12.5?
>>
>> 12.04 has 1.11.x, full backport of 1.13 would need quite a bit of work I
>> think, and is not something I'm willing to spend time on. Bisect the
>> patch that's needed and I can add it to 1.11.
>>
>> --
>> t




More information about the Pkg-sssd-devel mailing list