PAM: make initgroups timeout work across multiple clients

Instead of timing out the initgroups lookup on a per-cctx basis,
we will maintain a hash table of recently-seen users and use this
instead. This will allow SSSD to handle user's logging into
multiple services simultaneously more graciously, as well as
playing nicer with SSH (which makes calls to PAM both before and
after a fork).

https://fedorahosted.org/sssd/ticket/1063

7 files changed, 243 insertions, 6 deletions

diff --git a/Makefile.am b/Makefile.am
index f6b340a57..4c330a4ed 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -309,6 +309,7 @@ dist_noinst_HEADERS = \
     src/responder/common/responder.h \
     src/responder/common/responder_packet.h \
     src/responder/pam/pamsrv.h \
+    src/responder/pam/pam_helpers.h \
     src/responder/nss/nsssrv.h \
     src/responder/nss/nsssrv_private.h \
     src/responder/nss/nsssrv_netgroup.h \
@@ -445,6 +446,7 @@ sssd_pam_SOURCES = \
     src/responder/pam/pamsrv.c \
     src/responder/pam/pamsrv_cmd.c \
     src/responder/pam/pamsrv_dp.c \
+    src/responder/pam/pam_helpers.c \
     $(SSSD_RESPONDER_OBJ)
 sssd_pam_LDADD = \
     $(TDB_LIBS) \
diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index f6784e916..cbac67b56 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -120,8 +120,6 @@ struct cli_ctx {
 
     char *netgr_name;
     int netgrent_cur;
-
-    time_t pam_timeout;
 };
 
 struct sss_cmd_table {
diff --git a/src/responder/pam/pam_helpers.c b/src/responder/pam/pam_helpers.c
new file mode 100644
index 000000000..d2068e57c
--- /dev/null
+++ b/src/responder/pam/pam_helpers.c
@@ -0,0 +1,153 @@
+/*
+    SSSD
+
+    Authors:
+        Stephen Gallagher <sgallagh@redhat.com>
+
+    Copyright (C) 2011 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+
+#include "util/util.h"
+#include "src/responder/pam/pam_helpers.h"
+
+struct pam_initgr_table_ctx {
+    hash_table_t *id_table;
+    char *name;
+};
+
+static void pam_initgr_cache_remove(struct tevent_context *ev,
+                                    struct tevent_timer *te,
+                                    struct timeval tv,
+                                    void *pvt);
+
+errno_t pam_initgr_cache_set(struct tevent_context *ev,
+                             hash_table_t *id_table,
+                             char *name,
+                             long timeout)
+{
+    errno_t ret;
+    hash_key_t key;
+    hash_value_t val;
+    int hret;
+    struct tevent_timer *te;
+    struct timeval tv;
+    struct pam_initgr_table_ctx *table_ctx;
+
+    table_ctx = talloc_zero(id_table, struct pam_initgr_table_ctx);
+    if (!table_ctx) return ENOMEM;
+
+    table_ctx->id_table = id_table;
+    table_ctx->name = talloc_strdup(table_ctx, name);
+    if (!table_ctx->name) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    key.type = HASH_KEY_STRING;
+    key.str = name;
+
+    /* The value isn't relevant, since we're using
+     * a timer to remove the entry.
+     */
+    val.type = HASH_VALUE_UNDEF;
+
+    hret = hash_enter(id_table, &key, &val);
+    if (hret != HASH_SUCCESS) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              ("Could not update initgr cache for [%s]: [%s]\n",
+               name, hash_error_string(hret)));
+        ret = EIO;
+        goto done;
+    } else {
+        DEBUG(SSSDBG_TRACE_INTERNAL,
+              ("[%s] added to PAM initgroup cache\n",
+               name));
+    }
+
+    /* Create a timer event to remove the entry from the cache */
+    tv = tevent_timeval_current_ofs(timeout, 0);
+    te = tevent_add_timer(ev, table_ctx, tv,
+                          pam_initgr_cache_remove,
+                          table_ctx);
+    if (!te) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    ret = EOK;
+
+done:
+    if (ret != EOK) {
+        talloc_free(table_ctx);
+    }
+    return ret;
+}
+
+static void pam_initgr_cache_remove(struct tevent_context *ev,
+                                    struct tevent_timer *te,
+                                    struct timeval tv,
+                                    void *pvt)
+{
+    int hret;
+    hash_key_t key;
+
+    struct pam_initgr_table_ctx *table_ctx =
+            talloc_get_type(pvt, struct pam_initgr_table_ctx);
+
+    key.type = HASH_KEY_STRING;
+    key.str = table_ctx->name;
+
+    hret = hash_delete(table_ctx->id_table, &key);
+    if (hret != HASH_SUCCESS
+            && hret != HASH_ERROR_KEY_NOT_FOUND) {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              ("Could not clear [%s] from initgr cache: [%s]\n",
+               table_ctx->name,
+               hash_error_string(hret)));
+    } else {
+        DEBUG(SSSDBG_TRACE_INTERNAL,
+              ("[%s] removed from PAM initgroup cache\n",
+               table_ctx->name));
+    }
+
+    talloc_free(table_ctx);
+}
+
+errno_t pam_initgr_check_timeout(hash_table_t *id_table,
+                                 char *name)
+{
+    hash_key_t key;
+    hash_value_t val;
+    int hret;
+
+    key.type = HASH_KEY_STRING;
+    key.str = name;
+
+    hret = hash_lookup(id_table, &key, &val);
+    if (hret != HASH_SUCCESS
+            && hret != HASH_ERROR_KEY_NOT_FOUND) {
+        return EIO;
+    } else if (hret == HASH_ERROR_KEY_NOT_FOUND) {
+        return ENOENT;
+    }
+
+    /* If there's a value here, then the cache
+     * entry is still valid.
+     */
+    return EOK;
+}
+
diff --git a/src/responder/pam/pam_helpers.h b/src/responder/pam/pam_helpers.h
new file mode 100644
index 000000000..1e1d914df
--- /dev/null
+++ b/src/responder/pam/pam_helpers.h
@@ -0,0 +1,38 @@
+/*
+    SSSD
+
+    Authors:
+        Stephen Gallagher <sgallagh@redhat.com>
+
+    Copyright (C) 2011 Red Hat
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef PAM_HELPERS_H_
+#define PAM_HELPERS_H_
+
+errno_t pam_initgr_cache_set(struct tevent_context *ev,
+                             hash_table_t *id_table,
+                             char *name,
+                             long timeout);
+
+/* Returns EOK if the cache is still valid
+ * Returns ENOENT if the user is not found or is expired
+ * May report other errors if the hash lookup fails.
+ */
+errno_t pam_initgr_check_timeout(hash_table_t *id_table,
+                              char *name);
+
+#endif /* PAM_HELPERS_H_ */
diff --git a/src/responder/pam/pamsrv.c b/src/responder/pam/pamsrv.c
index 2933c79f0..be459b116 100644
--- a/src/responder/pam/pamsrv.c
+++ b/src/responder/pam/pamsrv.c
@@ -174,6 +174,15 @@ static int pam_process_init(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
+    /* Create table for initgroup lookups */
+    ret = sss_hash_create(pctx, 10, &pctx->id_table);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_FATAL_FAILURE,
+              ("Could not create initgroups hash table: [%s]",
+               strerror(ret)));
+        goto done;
+    }
+
     ret = EOK;
 
 done:
diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h
index 3ffc17087..3617231d0 100644
--- a/src/responder/pam/pamsrv.h
+++ b/src/responder/pam/pamsrv.h
@@ -36,6 +36,7 @@ struct pam_ctx {
     struct sss_nc_ctx *ncache;
     int neg_timeout;
     time_t id_timeout;
+    hash_table_t *id_table;
 };
 
 struct pam_auth_req {
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index 7c13ab11d..ced9df063 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -29,6 +29,7 @@
 #include "responder/common/negcache.h"
 #include "providers/data_provider.h"
 #include "responder/pam/pamsrv.h"
+#include "responder/pam/pam_helpers.h"
 #include "db/sysdb.h"
 
 enum pam_verbosity {
@@ -836,6 +837,8 @@ static int pam_check_user_search(struct pam_auth_req *preq)
     int ret;
     struct tevent_req *dpreq;
     struct dp_callback_ctx *cb_ctx;
+    struct pam_ctx *pctx =
+            talloc_get_type(preq->cctx->rctx->pvt_ctx, struct pam_ctx);
 
     while (dom) {
        /* if it is a domainless search, skip domains that require fully
@@ -858,14 +861,26 @@ static int pam_check_user_search(struct pam_auth_req *preq)
         talloc_free(name);
         name = dom->case_sensitive ? talloc_strdup(preq, preq->pd->user) :
                                 sss_tc_utf8_str_tolower(preq, preq->pd->user);
+        if (!name) {
+            return ENOMEM;
+        }
 
         /* Refresh the user's cache entry on any PAM query
          * We put a timeout in the client context so that we limit
          * the number of updates within a reasonable timeout
          */
-        if (preq->check_provider && cctx->pam_timeout < time(NULL)) {
-            /* Call provider first */
-            break;
+        if (preq->check_provider) {
+            ret = pam_initgr_check_timeout(pctx->id_table, name);
+            if (ret != EOK
+                    && ret != ENOENT) {
+                DEBUG(SSSDBG_OP_FAILURE,
+                      ("Could not look up initgroup timout\n"));
+                return EIO;
+            } else if (ret == ENOENT) {
+                /* Call provider first */
+                break;
+            }
+            /* Entry is still valid, get it from the sysdb */
         }
 
         DEBUG(4, ("Requesting info for [%s@%s]\n", name, dom->name));
@@ -1020,6 +1035,7 @@ static void pam_check_user_dp_callback(uint16_t err_maj, uint32_t err_min,
     int ret;
     struct pam_ctx *pctx =
             talloc_get_type(preq->cctx->rctx->pvt_ctx, struct pam_ctx);
+    char *name;
 
     if (err_maj) {
         DEBUG(2, ("Unable to get information from Data Provider\n"
@@ -1030,7 +1046,25 @@ static void pam_check_user_dp_callback(uint16_t err_maj, uint32_t err_min,
     ret = pam_check_user_search(preq);
     if (ret == EOK || ret == ENOENT) {
         /* Make sure we don't go to the ID provider too often */
-        preq->cctx->pam_timeout = time(NULL) + pctx->id_timeout;
+        name = preq->domain->case_sensitive ?
+                talloc_strdup(preq, preq->pd->user) :
+                sss_tc_utf8_str_tolower(preq, preq->pd->user);
+        if (!name) {
+            ret = ENOMEM;
+            goto done;
+        }
+
+        ret = pam_initgr_cache_set(pctx->rctx->ev, pctx->id_table,
+                                   name, pctx->id_timeout);
+        talloc_free(name);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE,
+                  ("Could not save initgr timestamp. "
+                   "Proceeding with PAM actions\n"));
+            /* This is non-fatal, we'll just end up going to the
+             * data provider again next time.
+             */
+        }
     }
 
     if (ret == EOK) {
@@ -1038,6 +1072,8 @@ static void pam_check_user_dp_callback(uint16_t err_maj, uint32_t err_min,
     }
 
     ret = pam_check_user_done(preq, ret);
+
+done:
     if (ret) {
         preq->pd->pam_status = PAM_SYSTEM_ERR;
         pam_reply(preq);