On Tue, Jan 31, 2012 at 08:29:15PM -0500, Stephen Gallagher wrote:
> On Tue, 2012-01-31 at 15:56 +0100, Jakub Hrozek wrote:
> > On Wed, Jan 25, 2012 at 08:23:23PM +0100, Jakub Hrozek wrote:
> > > On Fri, Jan 20, 2012 at 01:22:19PM +0100, Pavel Březina wrote:
> > > > Dne 18.1.2012 10:30, Jakub Hrozek napsal(a):
> > > > >On Wed, Jan 18, 2012 at 10:16:14AM +0100, Jakub Hrozek wrote:
> > > > >>On Wed, Jan 18, 2012 at 10:05:23AM +0100, Jan Zelený wrote:
> > > > >>>>https://fedorahosted.org/sssd/ticket/1116
> > > > >>>>
> > > > >>>>Adds option 'sudo_timed' to responder.
> > > > >>>
> > > > >>>Ack. I didn't perform any real-life testing but the patch seems 
> > > > >>>simple enough.
> > > > >>>
> > > > >>>Thanks
> > > > >>>Jan
> > > > >>
> > > > >>Real life testing cannot be performed until the sudo side is ready.
> > > > >>
> > > > >>That's also why I asked Pavel to hold sending out this patch for 
> > > > >>review
> > > > >>until he can actuall test it.
> > > > >
> > > > >Also, nack. The new option needs to be added into the config API.
> > > > 
> > > > I think this should be done as part of #1144 [1] or #1144 should be
> > > > done as a part of this patch (preferably because this is going to be
> > > > the only responder related option so far).
> > > > 
> > > > [1] https://fedorahosted.org/sssd/ticket/1144
> > > 
> > > You're right, I didn't realize it was the only option. Then please
> > > include the changes into this patch and we can close both tickets.
> > > 
> > > Also, this patch needs rebasing on top of the latest iteration of
> > > "cn=defaults" patches.
> > 
> > I grabbed Pavel's WIP patch, and made some fixes (to separate the work,
> > attached as Patch #2).
> 
> This doesn't apply atop the current master. Please rebase it. (It also
> looks like maybe there's a missing patch in the series?)

Yes, sorry, I had the in-memory cache in my tree before the timed
patches. Instead of rebasing the timed patches on top of master and then
the other way around, I dediced to finish the in-memory patch.

I'm attaching the patches again, there are no changes in the code, just
rebase on top of the changed in the in-memory patch.
From 2dca0eff83bf79ea1dfb1f06a1b0a5eec7e325b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Fri, 27 Jan 2012 10:53:51 +0100
Subject: [PATCH 1/2] SUDO Integration - responder 'sudo_timed' option

https://fedorahosted.org/sssd/ticket/1116
---
 src/confdb/confdb.h                  |    2 +
 src/db/sysdb_sudo.c                  |  159 ++++++++++++++++++++++++++--------
 src/db/sysdb_sudo.h                  |   10 ++-
 src/responder/sudo/sudosrv.c         |   11 +++
 src/responder/sudo/sudosrv_cmd.c     |   21 ++++-
 src/responder/sudo/sudosrv_private.h |    1 +
 6 files changed, 166 insertions(+), 38 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 
83da4474402568ae7f5f9eab61fd3d5dcfc0e773..126cbf02952ac1e67f7290d5acba0fb6a741bc3e
 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -95,6 +95,8 @@
 #define CONFDB_SUDO_CONF_ENTRY "config/sudo"
 #define CONFDB_SUDO_CACHE_TIMEOUT "sudo_cache_timeout"
 #define CONFDB_DEFAULT_SUDO_CACHE_TIMEOUT 180
+#define CONFDB_SUDO_TIMED "sudo_timed"
+#define CONFDB_DEFAULT_SUDO_TIMED false
 
 /* Data Provider */
 #define CONFDB_DP_CONF_ENTRY "config/dp"
diff --git a/src/db/sysdb_sudo.c b/src/db/sysdb_sudo.c
index 
5adde76e75149b1b38f1618184ac89a0be463e15..cea4fcbaf2f020f48a5c3c83a468638b0309148d
 100644
--- a/src/db/sysdb_sudo.c
+++ b/src/db/sysdb_sudo.c
@@ -18,7 +18,10 @@
     along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+#define _XOPEN_SOURCE
+
 #include <talloc.h>
+#include <time.h>
 
 #include "db/sysdb.h"
 #include "db/sysdb_private.h"
@@ -32,34 +35,131 @@
 } while(0)
 
 /* ====================  Utility functions ==================== */
-static char *
-get_sudo_time_filter(TALLOC_CTX *mem_ctx)
+
+static errno_t sysdb_sudo_check_time(struct sysdb_attrs *rule,
+                                     time_t now,
+                                     bool *result)
 {
-    time_t now;
-    struct tm *tp;
-    char timebuffer[64];
-
-    /* Make sure we have a formatted timestamp for __now__. */
-    time(&now);
-    if ((tp = gmtime(&now)) == NULL) {
-        DEBUG(SSSDBG_CRIT_FAILURE, ("unable to get GMT time\n"));
-        return NULL;
+    TALLOC_CTX *tmp_ctx = NULL;
+    const char **values = NULL;
+    char *tret = NULL;
+    time_t converted;
+    struct tm tm;
+    errno_t ret;
+    int i;
+
+    tmp_ctx = talloc_new(NULL);
+    NULL_CHECK(tmp_ctx, ret, done);
+
+    /*
+     * From man sudoers.ldap:
+     *
+     * A timestamp is in the form yyyymmddHHMMZ.
+     * If multiple sudoNotBefore entries are present, the *earliest* is used.
+     * If multiple sudoNotAfter entries are present, the *last one* is used.
+     */
+
+    /* check for sudoNotBefore */
+    ret = sysdb_attrs_get_string_array(rule, SYSDB_SUDO_CACHE_AT_NOTBEFORE,
+                                       tmp_ctx, &values);
+    if (ret != EOK) {
+        goto done;
+    }
+    if (values != NULL && values[0] != NULL) {
+        tret = strptime(values[0], SYSDB_SUDO_TIME_FORMAT, &tm);
+        if (tret == NULL || *tret != '\0') {
+            DEBUG(SSSDBG_FUNC_DATA, ("Invalid time format!\n"));
+            ret = EINVAL;
+            goto done;
+        }
+        converted = mktime(&tm);
+
+        if (now < converted) {
+            *result = false;
+            goto done;
+        }
     }
 
-    /* Format the timestamp according to the RFC. */
-    if (strftime(timebuffer, sizeof(timebuffer), "%Y%m%d%H%M%SZ", tp) == 0) {
-        DEBUG(SSSDBG_CRIT_FAILURE, ("unable to format timestamp\n"));
-        return NULL;
+    /* check for sudoNotAfter */
+    ret = sysdb_attrs_get_string_array(rule, SYSDB_SUDO_CACHE_AT_NOTAFTER,
+                                       tmp_ctx, &values);
+    if (ret != EOK) {
+        goto done;
+    }
+    if (values != NULL && values[0] != NULL) {
+        /* find last value */
+        for (i = 0; values[i] != NULL; i++) {
+            // do nothing
+        }
+
+        tret = strptime(values[i - 1], SYSDB_SUDO_TIME_FORMAT, &tm);
+        if (tret == NULL || *tret != '\0') {
+            DEBUG(SSSDBG_FUNC_DATA, ("Invalid time format!\n"));
+            ret = EINVAL;
+            goto done;
+        }
+        converted = mktime(&tm);
+
+        if (now > converted) {
+            *result = false;
+            goto done;
+        }
+    }
+
+    *result = true;
+    ret = EOK;
+
+done:
+    if (ret == ENOENT) {
+        *result = true;
+        ret = EOK;
     }
 
-    return talloc_asprintf(mem_ctx, "(&(|(!(%s=*))(%s>=%s))"
-                           "(|(!(%s=*))(%s<=%s)))",
-                           SYSDB_SUDO_CACHE_AT_NOTAFTER,
-                           SYSDB_SUDO_CACHE_AT_NOTAFTER,
-                           timebuffer,
-                           SYSDB_SUDO_CACHE_AT_NOTBEFORE,
-                           SYSDB_SUDO_CACHE_AT_NOTBEFORE,
-                           timebuffer);
+    talloc_free(tmp_ctx);
+    return ret;
+}
+
+errno_t sysdb_sudo_filter_rules_by_time(TALLOC_CTX *mem_ctx,
+                                        size_t in_num_rules,
+                                        struct sysdb_attrs **in_rules,
+                                        time_t now,
+                                        size_t *_num_rules,
+                                        struct sysdb_attrs ***_rules)
+{
+    size_t num_rules = 0;
+    struct sysdb_attrs **rules = NULL;
+    TALLOC_CTX *tmp_ctx = NULL;
+    bool allowed = false;
+    errno_t ret;
+    int i;
+
+    tmp_ctx = talloc_new(NULL);
+    NULL_CHECK(tmp_ctx, ret, done);
+
+    if (now == 0) {
+        now = time(NULL);
+    }
+
+    for (i = 0; i < in_num_rules; i++) {
+        ret = sysdb_sudo_check_time(in_rules[i], now, &allowed);
+        if (ret == EOK && allowed) {
+            num_rules++;
+            rules = talloc_realloc(tmp_ctx, rules, struct sysdb_attrs *,
+                                   num_rules);
+            NULL_CHECK(rules, ret, done);
+
+            rules[num_rules - 1] = in_rules[i];
+        }
+    }
+
+    *_num_rules = num_rules;
+    *_rules = talloc_steal(mem_ctx, rules);
+
+    ret = EOK;
+
+done:
+    talloc_free(tmp_ctx);
+    return ret;
 }
 
 errno_t
@@ -70,7 +170,6 @@ sysdb_get_sudo_filter(TALLOC_CTX *mem_ctx, const char 
*username,
     TALLOC_CTX *tmp_ctx = NULL;
     char *filter = NULL;
     char *specific_filter = NULL;
-    char *time_filter = NULL;
     errno_t ret;
     int i;
 
@@ -123,24 +222,12 @@ sysdb_get_sudo_filter(TALLOC_CTX *mem_ctx, const char 
*username,
         NULL_CHECK(specific_filter, ret, done);
     }
 
-    /* build time filter */
-
-    if (flags & SYSDB_SUDO_FILTER_TIMED) {
-        time_filter = get_sudo_time_filter(tmp_ctx);
-        NULL_CHECK(time_filter, ret, done);
-    }
-
     /* build global filter */
 
     filter = talloc_asprintf(tmp_ctx, "(&(%s=%s)",
                              SYSDB_OBJECTCLASS, SYSDB_SUDO_CACHE_AT_OC);
     NULL_CHECK(filter, ret, done);
 
-    if (time_filter != NULL) {
-        filter = talloc_strdup_append(filter, time_filter);
-        NULL_CHECK(filter, ret, done);
-    }
-
     if (specific_filter[0] != '\0') {
         filter = talloc_asprintf_append(filter, "(|%s)", specific_filter);
         NULL_CHECK(filter, ret, done);
diff --git a/src/db/sysdb_sudo.h b/src/db/sysdb_sudo.h
index 
b4e3eaff8aac545d0f92e708210a2733d0d5e865..87a3b94728bdf200f19d31059f49106d6a63bac0
 100644
--- a/src/db/sysdb_sudo.h
+++ b/src/db/sysdb_sudo.h
@@ -44,13 +44,14 @@
 #define SYSDB_SUDO_CACHE_AT_NOTAFTER   "sudoNotAfter"
 #define SYSDB_SUDO_CACHE_AT_ORDER      "sudoOrder"
 
+#define SYSDB_SUDO_TIME_FORMAT "%Y%m%d%H%M%SZ"
+
 /* When constructing a sysdb filter, OR these values to include..   */
 #define SYSDB_SUDO_FILTER_NONE           0x00       /* no additional filter */
 #define SYSDB_SUDO_FILTER_USERNAME       0x01       /* username             */
 #define SYSDB_SUDO_FILTER_UID            0x02       /* uid                  */
 #define SYSDB_SUDO_FILTER_GROUPS         0x04       /* groups               */
 #define SYSDB_SUDO_FILTER_NGRS           0x08       /* netgroups            */
-#define SYSDB_SUDO_FILTER_TIMED          0x10       /* timed rules          */
 #define SYSDB_SUDO_FILTER_INCLUDE_ALL    0x20       /* ALL                  */
 #define SYSDB_SUDO_FILTER_INCLUDE_DFL    0x40       /* include cn=default   */
 #define SYSDB_SUDO_FILTER_USERINFO       SYSDB_SUDO_FILTER_USERNAME \
@@ -58,6 +59,13 @@
                                        | SYSDB_SUDO_FILTER_GROUPS \
                                        | SYSDB_SUDO_FILTER_NGRS
 
+errno_t sysdb_sudo_filter_rules_by_time(TALLOC_CTX *mem_ctx,
+                                        size_t in_num_rules,
+                                        struct sysdb_attrs **in_rules,
+                                        time_t now,
+                                        size_t *_num_rules,
+                                        struct sysdb_attrs ***_rules);
+
 errno_t
 sysdb_get_sudo_filter(TALLOC_CTX *mem_ctx, const char *username,
                       uid_t uid, char **groupnames, unsigned int flags,
diff --git a/src/responder/sudo/sudosrv.c b/src/responder/sudo/sudosrv.c
index 
6b7eae07b51a064eeccb2190f45fa9c063a17adf..c8e36adc941d67454167699e3ffad85679d17a15
 100644
--- a/src/responder/sudo/sudosrv.c
+++ b/src/responder/sudo/sudosrv.c
@@ -142,6 +142,17 @@ int sudo_process_init(TALLOC_CTX *mem_ctx,
         return ret;
     }
 
+    /* Get sudo_timed option */
+    ret = confdb_get_bool(sudo_ctx->rctx->cdb, sudo_ctx,
+                          CONFDB_SUDO_CONF_ENTRY, CONFDB_SUDO_TIMED,
+                          CONFDB_DEFAULT_SUDO_TIMED,
+                          &sudo_ctx->timed);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_FATAL_FAILURE, ("Error reading from confdb (%d) [%s]\n",
+              ret, strerror(ret)));
+        return ret;
+    }
+
     /* Initialize in-memory cache */
     ret = sudosrv_cache_init(sudo_ctx, 10, &sudo_ctx->cache);
     if (ret != EOK) {
diff --git a/src/responder/sudo/sudosrv_cmd.c b/src/responder/sudo/sudosrv_cmd.c
index 
cef245fec22072ac3ab0e415af2fb5646e29b5e9..f179b92336facb01db61300154c1a863af31d69e
 100644
--- a/src/responder/sudo/sudosrv_cmd.c
+++ b/src/responder/sudo/sudosrv_cmd.c
@@ -26,6 +26,7 @@
 #include "responder/common/responder.h"
 #include "responder/common/responder_packet.h"
 #include "responder/sudo/sudosrv_private.h"
+#include "db/sysdb_sudo.h"
 
 static errno_t sudosrv_cmd_send_reply(struct sudo_cmd_ctx *cmd_ctx,
                                       uint8_t *response_body,
@@ -90,12 +91,30 @@ errno_t sudosrv_cmd_done(struct sudo_dom_ctx *dctx, int ret)
 {
     uint8_t *response_body = NULL;
     size_t response_len = 0;
+    size_t num_rules = dctx->res_count;
+    struct sysdb_attrs **rules = dctx->res;
 
     switch (ret) {
     case EOK:
+        /*
+         * Parent of dctx->res is in-memory cache, we must not talloc_free it!
+         */
+        if (!dctx->cmd_ctx->sudo_ctx->timed) {
+            num_rules = dctx->res_count;
+            rules = dctx->res;
+        } else {
+            /* filter rules by time */
+            ret = sysdb_sudo_filter_rules_by_time(dctx, dctx->res_count,
+                                                  dctx->res, 0,
+                                                  &num_rules, &rules);
+            if (ret != EOK) {
+                return EFAULT;
+            }
+        }
+
         /* send result */
         ret = sudosrv_get_sudorules_build_response(dctx->cmd_ctx, 
SSS_SUDO_ERROR_OK,
-                                                   dctx->res_count, dctx->res,
+                                                   num_rules, rules,
                                                    &response_body, 
&response_len);
         if (ret != EOK) {
             return EFAULT;
diff --git a/src/responder/sudo/sudosrv_private.h 
b/src/responder/sudo/sudosrv_private.h
index 
c3feb19bf797a706176f4b41b51c2edcade1cfed..7a7acc0c59eff82d8cb2c58deb4e9fd615706f45
 100644
--- a/src/responder/sudo/sudosrv_private.h
+++ b/src/responder/sudo/sudosrv_private.h
@@ -43,6 +43,7 @@ struct sudo_ctx {
      * options
      */
     int cache_timeout;
+    bool timed;
 
     /*
      * Key: domain          for SSS_DP_SUDO_DEFAULTS
-- 
1.7.7.6

From fe3b97db62b4ffa9f264a88c42bd05ba0dd27251 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 31 Jan 2012 15:51:35 +0100
Subject: [PATCH 2/2] Fixes for sudo_timed

https://fedorahosted.org/sssd/ticket/1116
---
 src/db/sysdb_sudo.c     |   65 +++++++++++++++++++++++++++++-----------------
 src/man/sssd.conf.5.xml |   13 +++++++++
 2 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/src/db/sysdb_sudo.c b/src/db/sysdb_sudo.c
index 
cea4fcbaf2f020f48a5c3c83a468638b0309148d..5f87a80cd85f44ae9b0feff04d77a7096d599f50
 100644
--- a/src/db/sysdb_sudo.c
+++ b/src/db/sysdb_sudo.c
@@ -43,20 +43,29 @@ static errno_t sysdb_sudo_check_time(struct sysdb_attrs 
*rule,
     TALLOC_CTX *tmp_ctx = NULL;
     const char **values = NULL;
     char *tret = NULL;
+    time_t notBefore = 0;
+    time_t notAfter = 0;
     time_t converted;
     struct tm tm;
     errno_t ret;
     int i;
 
+    if (!result) return EINVAL;
+    *result = false;
+
     tmp_ctx = talloc_new(NULL);
     NULL_CHECK(tmp_ctx, ret, done);
 
     /*
      * From man sudoers.ldap:
      *
-     * A timestamp is in the form yyyymmddHHMMZ.
+     * A timestamp is in the form yyyymmddHHMMSSZ.
      * If multiple sudoNotBefore entries are present, the *earliest* is used.
      * If multiple sudoNotAfter entries are present, the *last one* is used.
+     *
+     * From sudo sources, ldap.c:
+     * If either the sudoNotAfter or sudoNotBefore attributes are missing,
+     * no time restriction shall be imposed.
      */
 
     /* check for sudoNotBefore */
@@ -64,19 +73,27 @@ static errno_t sysdb_sudo_check_time(struct sysdb_attrs 
*rule,
                                        tmp_ctx, &values);
     if (ret != EOK) {
         goto done;
+    } else if (ret == ENOENT) {
+        DEBUG(SSSDBG_TRACE_LIBS,
+              ("notBefore attribute is missing, the rule is valid\n"));
+        *result = true;
+        ret = EOK;
     }
-    if (values != NULL && values[0] != NULL) {
-        tret = strptime(values[0], SYSDB_SUDO_TIME_FORMAT, &tm);
+
+    for (i=0; values[i] ; i++) {
+        tret = strptime(values[i], SYSDB_SUDO_TIME_FORMAT, &tm);
         if (tret == NULL || *tret != '\0') {
-            DEBUG(SSSDBG_FUNC_DATA, ("Invalid time format!\n"));
+            DEBUG(SSSDBG_MINOR_FAILURE, ("Invalid time format!\n"));
             ret = EINVAL;
             goto done;
         }
         converted = mktime(&tm);
 
-        if (now < converted) {
-            *result = false;
-            goto done;
+        /* Grab the earliest */
+        if (!notBefore) {
+            notBefore = converted;
+        } else if (notBefore > converted) {
+            notBefore = converted;
         }
     }
 
@@ -85,36 +102,36 @@ static errno_t sysdb_sudo_check_time(struct sysdb_attrs 
*rule,
                                        tmp_ctx, &values);
     if (ret != EOK) {
         goto done;
+    } else if (ret == ENOENT) {
+        DEBUG(SSSDBG_TRACE_LIBS,
+              ("notAfter attribute is missing, the rule is valid\n"));
+        *result = true;
+        ret = EOK;
     }
-    if (values != NULL && values[0] != NULL) {
-        /* find last value */
-        for (i = 0; values[i] != NULL; i++) {
-            // do nothing
-        }
 
-        tret = strptime(values[i - 1], SYSDB_SUDO_TIME_FORMAT, &tm);
+    for (i=0; values[i] ; i++) {
+        tret = strptime(values[i], SYSDB_SUDO_TIME_FORMAT, &tm);
         if (tret == NULL || *tret != '\0') {
-            DEBUG(SSSDBG_FUNC_DATA, ("Invalid time format!\n"));
+            DEBUG(SSSDBG_MINOR_FAILURE, ("Invalid time format!\n"));
             ret = EINVAL;
             goto done;
         }
         converted = mktime(&tm);
 
-        if (now > converted) {
-            *result = false;
-            goto done;
+        /* Grab the latest */
+        if (!notAfter) {
+            notAfter = converted;
+        } else if (notAfter < converted) {
+            notAfter = converted;
         }
     }
 
-    *result = true;
+    if (now >= notBefore && now <= notAfter) {
+        *result = true;
+    }
+
     ret = EOK;
-
 done:
-    if (ret == ENOENT) {
-        *result = true;
-        ret = EOK;
-    }
-
     talloc_free(tmp_ctx);
     return ret;
 }
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 
b12534b3c12020bc96f4175237ee7e9e486828ae..e06232fcb65ca97eb5550c74f7aef722548be311
 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -629,6 +629,19 @@
                         </para>
                     </listitem>
                 </varlistentry>
+                <varlistentry>
+                    <term>sudo_timed (bool)</term>
+                    <listitem>
+                        <para>
+                            Whether or not to evaluate the sudoNotBefore
+                            and sudoNotAfter attributes that implement
+                            time-dependent sudoers entries.
+                        </para>
+                        <para>
+                            Default: false
+                        </para>
+                    </listitem>
+                </varlistentry>
             </variablelist>
         </refsect2>
 
-- 
1.7.7.6

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

Reply via email to