Blame SOURCES/0035-KCM-Queue-requests-by-the-same-UID.patch

ecf709
From 688e8d8ffe331a1dd75a78002bf212277f2d7664 Mon Sep 17 00:00:00 2001
ecf709
From: Jakub Hrozek <jhrozek@redhat.com>
ecf709
Date: Tue, 21 Mar 2017 13:25:11 +0100
ecf709
Subject: [PATCH 35/36] KCM: Queue requests by the same UID
ecf709
MIME-Version: 1.0
ecf709
Content-Type: text/plain; charset=UTF-8
ecf709
Content-Transfer-Encoding: 8bit
ecf709
ecf709
In order to avoid race conditions, we queue requests towards the KCM
ecf709
responder coming from the same client UID.
ecf709
ecf709
Reviewed-by: Michal Židek <mzidek@redhat.com>
ecf709
Reviewed-by: Simo Sorce <simo@redhat.com>
ecf709
Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>
ecf709
---
ecf709
 Makefile.am                         |  21 ++-
ecf709
 src/responder/kcm/kcm.c             |   7 +
ecf709
 src/responder/kcm/kcmsrv_cmd.c      |  10 +-
ecf709
 src/responder/kcm/kcmsrv_op_queue.c | 264 ++++++++++++++++++++++++++
ecf709
 src/responder/kcm/kcmsrv_ops.c      |  44 ++++-
ecf709
 src/responder/kcm/kcmsrv_ops.h      |   1 +
ecf709
 src/responder/kcm/kcmsrv_pvt.h      |  20 ++
ecf709
 src/tests/cmocka/test_kcm_queue.c   | 365 ++++++++++++++++++++++++++++++++++++
ecf709
 8 files changed, 721 insertions(+), 11 deletions(-)
ecf709
 create mode 100644 src/responder/kcm/kcmsrv_op_queue.c
ecf709
 create mode 100644 src/tests/cmocka/test_kcm_queue.c
ecf709
ecf709
diff --git a/Makefile.am b/Makefile.am
ecf709
index e9eaa312c91e3aee40bcf13c90a0ad8c683045d5..91afdd669aa11a3cc316588d3b51d7e8e9c91cb8 100644
ecf709
--- a/Makefile.am
ecf709
+++ b/Makefile.am
ecf709
@@ -304,7 +304,10 @@ non_interactive_cmocka_based_tests += test_inotify
ecf709
 endif   # HAVE_INOTIFY
ecf709
 
ecf709
 if BUILD_KCM
ecf709
-non_interactive_cmocka_based_tests += test_kcm_json
ecf709
+non_interactive_cmocka_based_tests += \
ecf709
+	test_kcm_json \
ecf709
+	test_kcm_queue \
ecf709
+        $(NULL)
ecf709
 endif   # BUILD_KCM
ecf709
 
ecf709
 if BUILD_SAMBA
ecf709
@@ -1501,6 +1504,7 @@ sssd_kcm_SOURCES = \
ecf709
     src/responder/kcm/kcmsrv_ccache_json.c \
ecf709
     src/responder/kcm/kcmsrv_ccache_secrets.c \
ecf709
     src/responder/kcm/kcmsrv_ops.c \
ecf709
+    src/responder/kcm/kcmsrv_op_queue.c \
ecf709
     src/util/sss_sockets.c \
ecf709
     src/util/sss_krb5.c \
ecf709
     src/util/sss_iobuf.c \
ecf709
@@ -3402,6 +3406,21 @@ test_kcm_json_LDADD = \
ecf709
     $(SSSD_INTERNAL_LTLIBS) \
ecf709
     libsss_test_common.la \
ecf709
     $(NULL)
ecf709
+
ecf709
+test_kcm_queue_SOURCES = \
ecf709
+    src/tests/cmocka/test_kcm_queue.c \
ecf709
+    src/responder/kcm/kcmsrv_op_queue.c \
ecf709
+    $(NULL)
ecf709
+test_kcm_queue_CFLAGS = \
ecf709
+    $(AM_CFLAGS) \
ecf709
+    $(NULL)
ecf709
+test_kcm_queue_LDADD = \
ecf709
+    $(CMOCKA_LIBS) \
ecf709
+    $(SSSD_LIBS) \
ecf709
+    $(SSSD_INTERNAL_LTLIBS) \
ecf709
+    libsss_test_common.la \
ecf709
+    $(NULL)
ecf709
+
ecf709
 endif # BUILD_KCM
ecf709
 
ecf709
 endif # HAVE_CMOCKA
ecf709
diff --git a/src/responder/kcm/kcm.c b/src/responder/kcm/kcm.c
ecf709
index 063c27b915b4b92f6259496feee891aa94a498b6..3ee978066c589a5cc38b0ae358f741d389d00e7a 100644
ecf709
--- a/src/responder/kcm/kcm.c
ecf709
+++ b/src/responder/kcm/kcm.c
ecf709
@@ -133,6 +133,13 @@ static int kcm_get_config(struct kcm_ctx *kctx)
ecf709
         goto done;
ecf709
     }
ecf709
 
ecf709
+    kctx->qctx = kcm_ops_queue_create(kctx);
ecf709
+    if (ret != EOK) {
ecf709
+        DEBUG(SSSDBG_OP_FAILURE,
ecf709
+              "Cannot create KCM request queue [%d]: %s\n",
ecf709
+               ret, strerror(ret));
ecf709
+        goto done;
ecf709
+    }
ecf709
     ret = EOK;
ecf709
 done:
ecf709
     return ret;
ecf709
diff --git a/src/responder/kcm/kcmsrv_cmd.c b/src/responder/kcm/kcmsrv_cmd.c
ecf709
index 537e88953fd1a190a9a73bcdd430d8e0db8f9291..81015de4a91617de3dca444cde95b636c8d5c0d1 100644
ecf709
--- a/src/responder/kcm/kcmsrv_cmd.c
ecf709
+++ b/src/responder/kcm/kcmsrv_cmd.c
ecf709
@@ -353,14 +353,18 @@ struct kcm_req_ctx {
ecf709
 
ecf709
 static void kcm_cmd_request_done(struct tevent_req *req);
ecf709
 
ecf709
-static errno_t kcm_cmd_dispatch(struct kcm_req_ctx *req_ctx)
ecf709
+static errno_t kcm_cmd_dispatch(struct kcm_ctx *kctx,
ecf709
+                                struct kcm_req_ctx *req_ctx)
ecf709
 {
ecf709
     struct tevent_req *req;
ecf709
     struct cli_ctx *cctx;
ecf709
 
ecf709
     cctx = req_ctx->cctx;
ecf709
 
ecf709
-    req = kcm_cmd_send(req_ctx, cctx->ev, req_ctx->kctx->kcm_data,
ecf709
+    req = kcm_cmd_send(req_ctx,
ecf709
+                       cctx->ev,
ecf709
+                       kctx->qctx,
ecf709
+                       req_ctx->kctx->kcm_data,
ecf709
                        req_ctx->cctx->creds,
ecf709
                        &req_ctx->op_io.request,
ecf709
                        req_ctx->op_io.op);
ecf709
@@ -505,7 +509,7 @@ static void kcm_recv(struct cli_ctx *cctx)
ecf709
     /* do not read anymore, client is done sending */
ecf709
     TEVENT_FD_NOT_READABLE(cctx->cfde);
ecf709
 
ecf709
-    ret = kcm_cmd_dispatch(req);
ecf709
+    ret = kcm_cmd_dispatch(kctx, req);
ecf709
     if (ret != EOK) {
ecf709
         DEBUG(SSSDBG_FATAL_FAILURE,
ecf709
               "Failed to dispatch KCM operation [%d]: %s\n",
ecf709
diff --git a/src/responder/kcm/kcmsrv_op_queue.c b/src/responder/kcm/kcmsrv_op_queue.c
ecf709
new file mode 100644
ecf709
index 0000000000000000000000000000000000000000..f6c425dd5b64877c8b7401e488dd6565157fc9b5
ecf709
--- /dev/null
ecf709
+++ b/src/responder/kcm/kcmsrv_op_queue.c
ecf709
@@ -0,0 +1,264 @@
ecf709
+/*
ecf709
+   SSSD
ecf709
+
ecf709
+   KCM Server - the KCM operations wait queue
ecf709
+
ecf709
+   Copyright (C) Red Hat, 2017
ecf709
+
ecf709
+   This program is free software; you can redistribute it and/or modify
ecf709
+   it under the terms of the GNU General Public License as published by
ecf709
+   the Free Software Foundation; either version 3 of the License, or
ecf709
+   (at your option) any later version.
ecf709
+
ecf709
+   This program is distributed in the hope that it will be useful,
ecf709
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
ecf709
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
ecf709
+   GNU General Public License for more details.
ecf709
+
ecf709
+   You should have received a copy of the GNU General Public License
ecf709
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
ecf709
+*/
ecf709
+
ecf709
+#include "util/util.h"
ecf709
+#include "util/util_creds.h"
ecf709
+#include "responder/kcm/kcmsrv_pvt.h"
ecf709
+
ecf709
+#define QUEUE_HASH_SIZE      32
ecf709
+
ecf709
+struct kcm_ops_queue_entry {
ecf709
+    struct tevent_req *req;
ecf709
+    uid_t uid;
ecf709
+
ecf709
+    hash_table_t *wait_queue_hash;
ecf709
+
ecf709
+    struct kcm_ops_queue_entry *head;
ecf709
+    struct kcm_ops_queue_entry *next;
ecf709
+    struct kcm_ops_queue_entry *prev;
ecf709
+};
ecf709
+
ecf709
+struct kcm_ops_queue_ctx {
ecf709
+    /* UID: dlist of kcm_ops_queue_entry */
ecf709
+    hash_table_t *wait_queue_hash;
ecf709
+};
ecf709
+
ecf709
+/*
ecf709
+ * Per-UID wait queue
ecf709
+ *
ecf709
+ * They key in the hash table is the UID of the peer. The value of each
ecf709
+ * hash table entry is a linked list of kcm_ops_queue_entry structures
ecf709
+ * which primarily hold the tevent request being queued.
ecf709
+ */
ecf709
+struct kcm_ops_queue_ctx *kcm_ops_queue_create(TALLOC_CTX *mem_ctx)
ecf709
+{
ecf709
+    errno_t ret;
ecf709
+    struct kcm_ops_queue_ctx *queue_ctx;
ecf709
+
ecf709
+    queue_ctx = talloc_zero(mem_ctx, struct kcm_ops_queue_ctx);
ecf709
+    if (queue_ctx == NULL) {
ecf709
+        return NULL;
ecf709
+    }
ecf709
+
ecf709
+    ret = sss_hash_create_ex(mem_ctx, QUEUE_HASH_SIZE,
ecf709
+                             &queue_ctx->wait_queue_hash, 0, 0, 0, 0,
ecf709
+                             NULL, NULL);
ecf709
+    if (ret != EOK) {
ecf709
+        DEBUG(SSSDBG_CRIT_FAILURE,
ecf709
+              "sss_hash_create failed [%d]: %s\n", ret, sss_strerror(ret));
ecf709
+        talloc_free(queue_ctx);
ecf709
+        return NULL;
ecf709
+    }
ecf709
+
ecf709
+    return queue_ctx;
ecf709
+}
ecf709
+
ecf709
+static int kcm_op_queue_entry_destructor(struct kcm_ops_queue_entry *entry)
ecf709
+{
ecf709
+    int ret;
ecf709
+    struct kcm_ops_queue_entry *next_entry;
ecf709
+    hash_key_t key;
ecf709
+
ecf709
+    if (entry == NULL) {
ecf709
+        return 1;
ecf709
+    }
ecf709
+
ecf709
+    /* Take the next entry from the queue */
ecf709
+    next_entry = entry->next;
ecf709
+
ecf709
+    /* Remove the current entry from the queue */
ecf709
+    DLIST_REMOVE(entry->head, entry);
ecf709
+
ecf709
+    if (next_entry == NULL) {
ecf709
+        key.type = HASH_KEY_ULONG;
ecf709
+        key.ul = entry->uid;
ecf709
+
ecf709
+        /* If this was the last entry, remove the key (the UID) from the
ecf709
+         * hash table to signal the queue is empty
ecf709
+         */
ecf709
+        ret = hash_delete(entry->wait_queue_hash, &key);
ecf709
+        if (ret != HASH_SUCCESS) {
ecf709
+            DEBUG(SSSDBG_CRIT_FAILURE,
ecf709
+                  "Failed to remove wait queue for user %"SPRIuid"\n",
ecf709
+                  entry->uid);
ecf709
+            return 1;
ecf709
+        }
ecf709
+        return 0;
ecf709
+    }
ecf709
+
ecf709
+    /* Otherwise, mark the current head as done to run the next request */
ecf709
+    tevent_req_done(next_entry->req);
ecf709
+    return 0;
ecf709
+}
ecf709
+
ecf709
+static errno_t kcm_op_queue_add(hash_table_t *wait_queue_hash,
ecf709
+                                struct kcm_ops_queue_entry *entry,
ecf709
+                                uid_t uid)
ecf709
+{
ecf709
+    errno_t ret;
ecf709
+    hash_key_t key;
ecf709
+    hash_value_t value;
ecf709
+    struct kcm_ops_queue_entry *head = NULL;
ecf709
+
ecf709
+    key.type = HASH_KEY_ULONG;
ecf709
+    key.ul = uid;
ecf709
+
ecf709
+    ret = hash_lookup(wait_queue_hash, &key, &value);
ecf709
+    switch (ret) {
ecf709
+    case HASH_SUCCESS:
ecf709
+        /* The key with this UID already exists. Its value is request queue
ecf709
+         * for the UID, so let's just add the current request to the end
ecf709
+         * of the queue and wait for the previous requests to finish
ecf709
+         */
ecf709
+        if (value.type != HASH_VALUE_PTR) {
ecf709
+            DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected hash value type.\n");
ecf709
+            return EINVAL;
ecf709
+        }
ecf709
+
ecf709
+        head = talloc_get_type(value.ptr, struct kcm_ops_queue_entry);
ecf709
+        if (head == NULL) {
ecf709
+            DEBUG(SSSDBG_CRIT_FAILURE, "Invalid queue pointer\n");
ecf709
+            return EINVAL;
ecf709
+        }
ecf709
+
ecf709
+        entry->head = head;
ecf709
+        DLIST_ADD_END(head, entry, struct kcm_ops_queue_entry *);
ecf709
+
ecf709
+        DEBUG(SSSDBG_TRACE_LIBS, "Waiting in queue\n");
ecf709
+        ret = EAGAIN;
ecf709
+        break;
ecf709
+
ecf709
+    case HASH_ERROR_KEY_NOT_FOUND:
ecf709
+        /* No request for this UID yet. Enqueue this request in case
ecf709
+         * another one comes in and return EOK to run the current request
ecf709
+         * immediatelly
ecf709
+         */
ecf709
+        entry->head = entry;
ecf709
+
ecf709
+        value.type = HASH_VALUE_PTR;
ecf709
+        value.ptr = entry;
ecf709
+
ecf709
+        ret = hash_enter(wait_queue_hash, &key, &value);
ecf709
+        if (ret != HASH_SUCCESS) {
ecf709
+            DEBUG(SSSDBG_CRIT_FAILURE, "hash_enter failed.\n");
ecf709
+            return EIO;
ecf709
+        }
ecf709
+
ecf709
+        DEBUG(SSSDBG_TRACE_LIBS,
ecf709
+              "Added a first request to the queue, running immediately\n");
ecf709
+        ret = EOK;
ecf709
+        break;
ecf709
+
ecf709
+    default:
ecf709
+        DEBUG(SSSDBG_CRIT_FAILURE, "hash_lookup failed.\n");
ecf709
+        return EIO;
ecf709
+    }
ecf709
+
ecf709
+    talloc_steal(wait_queue_hash, entry);
ecf709
+    talloc_set_destructor(entry, kcm_op_queue_entry_destructor);
ecf709
+    return ret;
ecf709
+}
ecf709
+
ecf709
+struct kcm_op_queue_state {
ecf709
+    struct kcm_ops_queue_entry *entry;
ecf709
+};
ecf709
+
ecf709
+/*
ecf709
+ * Enqueue a request.
ecf709
+ *
ecf709
+ * If the request queue /for the given ID/ is empty, that is, if this
ecf709
+ * request is the first one in the queue, run the request immediatelly.
ecf709
+ *
ecf709
+ * Otherwise just add it to the queue and wait until the previous request
ecf709
+ * finishes and only at that point mark the current request as done, which
ecf709
+ * will trigger calling the recv function and allow the request to continue.
ecf709
+ */
ecf709
+struct tevent_req *kcm_op_queue_send(TALLOC_CTX *mem_ctx,
ecf709
+                                     struct tevent_context *ev,
ecf709
+                                     struct kcm_ops_queue_ctx *qctx,
ecf709
+                                     struct cli_creds *client)
ecf709
+{
ecf709
+    errno_t ret;
ecf709
+    struct tevent_req *req;
ecf709
+    struct kcm_op_queue_state *state;
ecf709
+    uid_t uid;
ecf709
+
ecf709
+    uid = cli_creds_get_uid(client);
ecf709
+
ecf709
+    req = tevent_req_create(mem_ctx, &state, struct kcm_op_queue_state);
ecf709
+    if (req == NULL) {
ecf709
+        return NULL;
ecf709
+    }
ecf709
+
ecf709
+    state->entry = talloc_zero(state, struct kcm_ops_queue_entry);
ecf709
+    if (state->entry == NULL) {
ecf709
+        ret = ENOMEM;
ecf709
+        goto immediate;
ecf709
+    }
ecf709
+    state->entry->req = req;
ecf709
+    state->entry->uid = uid;
ecf709
+    state->entry->wait_queue_hash = qctx->wait_queue_hash;
ecf709
+
ecf709
+    DEBUG(SSSDBG_FUNC_DATA,
ecf709
+          "Adding request by %"SPRIuid" to the wait queue\n", uid);
ecf709
+
ecf709
+    ret = kcm_op_queue_add(qctx->wait_queue_hash, state->entry, uid);
ecf709
+    if (ret == EOK) {
ecf709
+        DEBUG(SSSDBG_TRACE_LIBS,
ecf709
+              "Wait queue was empty, running immediately\n");
ecf709
+        goto immediate;
ecf709
+    } else if (ret != EAGAIN) {
ecf709
+        DEBUG(SSSDBG_OP_FAILURE,
ecf709
+              "Cannot enqueue request [%d]: %s\n", ret, sss_strerror(ret));
ecf709
+        goto immediate;
ecf709
+    }
ecf709
+
ecf709
+    DEBUG(SSSDBG_TRACE_LIBS, "Waiting our turn in the queue\n");
ecf709
+    return req;
ecf709
+
ecf709
+immediate:
ecf709
+    if (ret == EOK) {
ecf709
+        tevent_req_done(req);
ecf709
+    } else {
ecf709
+        tevent_req_error(req, ret);
ecf709
+    }
ecf709
+    tevent_req_post(req, ev);
ecf709
+    return req;
ecf709
+}
ecf709
+
ecf709
+/*
ecf709
+ * The queue recv function is called when this request is 'activated'. The queue
ecf709
+ * entry should be allocated on the same memory context as the enqueued request
ecf709
+ * to trigger freeing the kcm_ops_queue_entry structure destructor when the
ecf709
+ * parent request is done and its tevent_req freed. This would in turn unblock
ecf709
+ * the next request in the queue
ecf709
+ */
ecf709
+errno_t kcm_op_queue_recv(struct tevent_req *req,
ecf709
+                          TALLOC_CTX *mem_ctx,
ecf709
+                          struct kcm_ops_queue_entry **_entry)
ecf709
+{
ecf709
+    struct kcm_op_queue_state *state = tevent_req_data(req,
ecf709
+                                                struct kcm_op_queue_state);
ecf709
+
ecf709
+    TEVENT_REQ_RETURN_ON_ERROR(req);
ecf709
+    *_entry = talloc_steal(mem_ctx, state->entry);
ecf709
+    return EOK;
ecf709
+}
ecf709
diff --git a/src/responder/kcm/kcmsrv_ops.c b/src/responder/kcm/kcmsrv_ops.c
ecf709
index 50e8cc635424e15d53e3c8d122c5525044f59c8a..2feaf51f227ce9d90f706229ce7ac201b282dc6f 100644
ecf709
--- a/src/responder/kcm/kcmsrv_ops.c
ecf709
+++ b/src/responder/kcm/kcmsrv_ops.c
ecf709
@@ -67,17 +67,21 @@ struct kcm_op {
ecf709
 
ecf709
 struct kcm_cmd_state {
ecf709
     struct kcm_op *op;
ecf709
+    struct tevent_context *ev;
ecf709
 
ecf709
+    struct kcm_ops_queue_entry *queue_entry;
ecf709
     struct kcm_op_ctx *op_ctx;
ecf709
     struct sss_iobuf *reply;
ecf709
 
ecf709
     uint32_t op_ret;
ecf709
 };
ecf709
 
ecf709
+static void kcm_cmd_queue_done(struct tevent_req *subreq);
ecf709
 static void kcm_cmd_done(struct tevent_req *subreq);
ecf709
 
ecf709
 struct tevent_req *kcm_cmd_send(TALLOC_CTX *mem_ctx,
ecf709
                                 struct tevent_context *ev,
ecf709
+                                struct kcm_ops_queue_ctx *qctx,
ecf709
                                 struct kcm_resp_ctx *kcm_data,
ecf709
                                 struct cli_creds *client,
ecf709
                                 struct kcm_data *input,
ecf709
@@ -93,6 +97,7 @@ struct tevent_req *kcm_cmd_send(TALLOC_CTX *mem_ctx,
ecf709
         return NULL;
ecf709
     }
ecf709
     state->op = op;
ecf709
+    state->ev = ev;
ecf709
 
ecf709
     if (op == NULL) {
ecf709
         ret = EINVAL;
ecf709
@@ -154,18 +159,43 @@ struct tevent_req *kcm_cmd_send(TALLOC_CTX *mem_ctx,
ecf709
         goto immediate;
ecf709
     }
ecf709
 
ecf709
-    subreq = op->fn_send(state, ev, state->op_ctx);
ecf709
+    subreq = kcm_op_queue_send(state, ev, qctx, client);
ecf709
     if (subreq == NULL) {
ecf709
         ret = ENOMEM;
ecf709
         goto immediate;
ecf709
     }
ecf709
+    tevent_req_set_callback(subreq, kcm_cmd_queue_done, req);
ecf709
+    return req;
ecf709
+
ecf709
+immediate:
ecf709
+    tevent_req_error(req, ret);
ecf709
+    tevent_req_post(req, ev);
ecf709
+    return req;
ecf709
+}
ecf709
+
ecf709
+static void kcm_cmd_queue_done(struct tevent_req *subreq)
ecf709
+{
ecf709
+    struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req);
ecf709
+    struct kcm_cmd_state *state = tevent_req_data(req, struct kcm_cmd_state);
ecf709
+    errno_t ret;
ecf709
+
ecf709
+    /* When this request finishes, it frees the queue_entry which unblocks
ecf709
+     * other requests by the same UID
ecf709
+     */
ecf709
+    ret = kcm_op_queue_recv(subreq, state, &state->queue_entry);
ecf709
+    talloc_zfree(subreq);
ecf709
+    if (ret != EOK) {
ecf709
+        DEBUG(SSSDBG_CRIT_FAILURE, "Cannot acquire queue slot\n");
ecf709
+        tevent_req_error(req, ret);
ecf709
+        return;
ecf709
+    }
ecf709
+
ecf709
+    subreq = state->op->fn_send(state, state->ev, state->op_ctx);
ecf709
+    if (subreq == NULL) {
ecf709
+        tevent_req_error(req, ENOMEM);
ecf709
+        return;
ecf709
+    }
ecf709
     tevent_req_set_callback(subreq, kcm_cmd_done, req);
ecf709
-    return req;
ecf709
-
ecf709
-immediate:
ecf709
-    tevent_req_error(req, ret);
ecf709
-    tevent_req_post(req, ev);
ecf709
-    return req;
ecf709
 }
ecf709
 
ecf709
 static void kcm_cmd_done(struct tevent_req *subreq)
ecf709
diff --git a/src/responder/kcm/kcmsrv_ops.h b/src/responder/kcm/kcmsrv_ops.h
ecf709
index 8e6feaf56a10b73c8b6375aea9ef26c392b5b492..67d9f86026bf949548471f2280c130ebefd2f865 100644
ecf709
--- a/src/responder/kcm/kcmsrv_ops.h
ecf709
+++ b/src/responder/kcm/kcmsrv_ops.h
ecf709
@@ -34,6 +34,7 @@ const char *kcm_opt_name(struct kcm_op *op);
ecf709
 
ecf709
 struct tevent_req *kcm_cmd_send(TALLOC_CTX *mem_ctx,
ecf709
                                 struct tevent_context *ev,
ecf709
+                                struct kcm_ops_queue_ctx *qctx,
ecf709
                                 struct kcm_resp_ctx *kcm_data,
ecf709
                                 struct cli_creds *client,
ecf709
                                 struct kcm_data *input,
ecf709
diff --git a/src/responder/kcm/kcmsrv_pvt.h b/src/responder/kcm/kcmsrv_pvt.h
ecf709
index 74f30c00014105ed533744779b02c5d42523722d..f081a6bf0c6e40d2f8a83b07f9bbc2abacff359d 100644
ecf709
--- a/src/responder/kcm/kcmsrv_pvt.h
ecf709
+++ b/src/responder/kcm/kcmsrv_pvt.h
ecf709
@@ -25,6 +25,7 @@
ecf709
 #include "config.h"
ecf709
 
ecf709
 #include <sys/types.h>
ecf709
+#include <krb5/krb5.h>
ecf709
 #include "responder/common/responder.h"
ecf709
 
ecf709
 /*
ecf709
@@ -65,6 +66,7 @@ struct kcm_ctx {
ecf709
     int fd_limit;
ecf709
     char *socket_path;
ecf709
     enum kcm_ccdb_be cc_be;
ecf709
+    struct kcm_ops_queue_ctx *qctx;
ecf709
 
ecf709
     struct kcm_resp_ctx *kcm_data;
ecf709
 };
ecf709
@@ -78,4 +80,22 @@ int kcm_connection_setup(struct cli_ctx *cctx);
ecf709
  */
ecf709
 krb5_error_code sss2krb5_error(errno_t err);
ecf709
 
ecf709
+/* We enqueue all requests by the same UID to avoid concurrency issues
ecf709
+ * especially when performing multiple round-trips to sssd-secrets. In
ecf709
+ * future, we should relax the queue to allow multiple read-only operations
ecf709
+ * if no write operations are in progress.
ecf709
+ */
ecf709
+struct kcm_ops_queue_entry;
ecf709
+
ecf709
+struct kcm_ops_queue_ctx *kcm_ops_queue_create(TALLOC_CTX *mem_ctx);
ecf709
+
ecf709
+struct tevent_req *kcm_op_queue_send(TALLOC_CTX *mem_ctx,
ecf709
+                                     struct tevent_context *ev,
ecf709
+                                     struct kcm_ops_queue_ctx *qctx,
ecf709
+                                     struct cli_creds *client);
ecf709
+
ecf709
+errno_t kcm_op_queue_recv(struct tevent_req *req,
ecf709
+                          TALLOC_CTX *mem_ctx,
ecf709
+                          struct kcm_ops_queue_entry **_entry);
ecf709
+
ecf709
 #endif /* __KCMSRV_PVT_H__ */
ecf709
diff --git a/src/tests/cmocka/test_kcm_queue.c b/src/tests/cmocka/test_kcm_queue.c
ecf709
new file mode 100644
ecf709
index 0000000000000000000000000000000000000000..ba0d2405629960df5c623848f3207b7c80fa948d
ecf709
--- /dev/null
ecf709
+++ b/src/tests/cmocka/test_kcm_queue.c
ecf709
@@ -0,0 +1,365 @@
ecf709
+/*
ecf709
+    Copyright (C) 2017 Red Hat
ecf709
+
ecf709
+    SSSD tests: Test KCM wait queue
ecf709
+
ecf709
+    This program is free software; you can redistribute it and/or modify
ecf709
+    it under the terms of the GNU General Public License as published by
ecf709
+    the Free Software Foundation; either version 3 of the License, or
ecf709
+    (at your option) any later version.
ecf709
+
ecf709
+    This program is distributed in the hope that it will be useful,
ecf709
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
ecf709
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
ecf709
+    GNU General Public License for more details.
ecf709
+
ecf709
+    You should have received a copy of the GNU General Public License
ecf709
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
ecf709
+*/
ecf709
+
ecf709
+#include "config.h"
ecf709
+
ecf709
+#include <stdio.h>
ecf709
+#include <popt.h>
ecf709
+
ecf709
+#include "util/util.h"
ecf709
+#include "util/util_creds.h"
ecf709
+#include "tests/cmocka/common_mock.h"
ecf709
+#include "responder/kcm/kcmsrv_pvt.h"
ecf709
+
ecf709
+#define INVALID_ID      -1
ecf709
+#define FAST_REQ_ID     0
ecf709
+#define SLOW_REQ_ID     1
ecf709
+
ecf709
+#define FAST_REQ_DELAY  1
ecf709
+#define SLOW_REQ_DELAY  2
ecf709
+
ecf709
+struct timed_request_state {
ecf709
+    struct tevent_context *ev;
ecf709
+    struct kcm_ops_queue_ctx *qctx;
ecf709
+    struct cli_creds *client;
ecf709
+    int delay;
ecf709
+    int req_id;
ecf709
+
ecf709
+    struct kcm_ops_queue_entry *queue_entry;
ecf709
+};
ecf709
+
ecf709
+static void timed_request_start(struct tevent_req *subreq);
ecf709
+static void timed_request_done(struct tevent_context *ev,
ecf709
+                               struct tevent_timer *te,
ecf709
+                               struct timeval current_time,
ecf709
+                               void *pvt);
ecf709
+
ecf709
+static struct tevent_req *timed_request_send(TALLOC_CTX *mem_ctx,
ecf709
+                                             struct tevent_context *ev,
ecf709
+                                             struct kcm_ops_queue_ctx *qctx,
ecf709
+                                             struct cli_creds *client,
ecf709
+                                             int delay,
ecf709
+                                             int req_id)
ecf709
+{
ecf709
+    struct tevent_req *req;
ecf709
+    struct tevent_req *subreq;
ecf709
+    struct timed_request_state *state;
ecf709
+
ecf709
+    req = tevent_req_create(mem_ctx, &state, struct timed_request_state);
ecf709
+    if (req == NULL) {
ecf709
+        return NULL;
ecf709
+    }
ecf709
+    state->ev = ev;
ecf709
+    state->qctx = qctx;
ecf709
+    state->client = client;
ecf709
+    state->delay = delay;
ecf709
+    state->req_id = req_id;
ecf709
+
ecf709
+    DEBUG(SSSDBG_TRACE_ALL, "Request %p with delay %d\n", req, delay);
ecf709
+
ecf709
+    subreq = kcm_op_queue_send(state, ev, qctx, client);
ecf709
+    if (subreq == NULL) {
ecf709
+        return NULL;
ecf709
+    }
ecf709
+    tevent_req_set_callback(subreq, timed_request_start, req);
ecf709
+
ecf709
+    return req;
ecf709
+}
ecf709
+
ecf709
+static void timed_request_start(struct tevent_req *subreq)
ecf709
+{
ecf709
+    struct timeval tv;
ecf709
+    struct tevent_timer *timeout = NULL;
ecf709
+    struct tevent_req *req = tevent_req_callback_data(subreq,
ecf709
+                                                      struct tevent_req);
ecf709
+    struct timed_request_state *state = tevent_req_data(req,
ecf709
+                                                struct timed_request_state);
ecf709
+    errno_t ret;
ecf709
+
ecf709
+    ret = kcm_op_queue_recv(subreq, state, &state->queue_entry);
ecf709
+    talloc_zfree(subreq);
ecf709
+    if (ret != EOK) {
ecf709
+        tevent_req_error(req, ret);
ecf709
+        return;
ecf709
+    }
ecf709
+
ecf709
+    tv = tevent_timeval_current_ofs(state->delay, 0);
ecf709
+    timeout = tevent_add_timer(state->ev, state, tv, timed_request_done, req);
ecf709
+    if (timeout == NULL) {
ecf709
+        tevent_req_error(req, ENOMEM);
ecf709
+        return;
ecf709
+    }
ecf709
+
ecf709
+    return;
ecf709
+}
ecf709
+
ecf709
+static void timed_request_done(struct tevent_context *ev,
ecf709
+                               struct tevent_timer *te,
ecf709
+                               struct timeval current_time,
ecf709
+                               void *pvt)
ecf709
+{
ecf709
+    struct tevent_req *req = talloc_get_type(pvt, struct tevent_req);
ecf709
+    DEBUG(SSSDBG_TRACE_ALL, "Request %p done\n", req);
ecf709
+    tevent_req_done(req);
ecf709
+}
ecf709
+
ecf709
+static errno_t timed_request_recv(struct tevent_req *req,
ecf709
+                                  int *req_id)
ecf709
+{
ecf709
+    struct timed_request_state *state = tevent_req_data(req,
ecf709
+                                                struct timed_request_state);
ecf709
+
ecf709
+    TEVENT_REQ_RETURN_ON_ERROR(req);
ecf709
+    *req_id = state->req_id;
ecf709
+    return EOK;
ecf709
+}
ecf709
+
ecf709
+struct test_ctx {
ecf709
+    struct kcm_ops_queue_ctx *qctx;
ecf709
+    struct tevent_context *ev;
ecf709
+
ecf709
+    int *req_ids;
ecf709
+
ecf709
+    int num_requests;
ecf709
+    int finished_requests;
ecf709
+    bool done;
ecf709
+    errno_t error;
ecf709
+};
ecf709
+
ecf709
+static int setup_kcm_queue(void **state)
ecf709
+{
ecf709
+    struct test_ctx *tctx;
ecf709
+
ecf709
+    tctx = talloc_zero(NULL, struct test_ctx);
ecf709
+    assert_non_null(tctx);
ecf709
+
ecf709
+    tctx->ev = tevent_context_init(tctx);
ecf709
+    assert_non_null(tctx->ev);
ecf709
+
ecf709
+    tctx->qctx = kcm_ops_queue_create(tctx);
ecf709
+    assert_non_null(tctx->qctx);
ecf709
+
ecf709
+    *state = tctx;
ecf709
+    return 0;
ecf709
+}
ecf709
+
ecf709
+static int teardown_kcm_queue(void **state)
ecf709
+{
ecf709
+    struct test_ctx *tctx = talloc_get_type(*state, struct test_ctx);
ecf709
+    talloc_free(tctx);
ecf709
+    return 0;
ecf709
+}
ecf709
+
ecf709
+static void test_kcm_queue_done(struct tevent_req *req)
ecf709
+{
ecf709
+    struct test_ctx *test_ctx = tevent_req_callback_data(req,
ecf709
+                                                struct test_ctx);
ecf709
+    int req_id = INVALID_ID;
ecf709
+
ecf709
+    test_ctx->error = timed_request_recv(req, &req_id);
ecf709
+    talloc_zfree(req);
ecf709
+    if (test_ctx->error != EOK) {
ecf709
+        test_ctx->done = true;
ecf709
+        return;
ecf709
+    }
ecf709
+
ecf709
+    if (test_ctx->req_ids[test_ctx->finished_requests] != req_id) {
ecf709
+        DEBUG(SSSDBG_CRIT_FAILURE,
ecf709
+              "Request %d finished, expected %d\n",
ecf709
+              req_id, test_ctx->req_ids[test_ctx->finished_requests]);
ecf709
+        test_ctx->error = EIO;
ecf709
+        test_ctx->done = true;
ecf709
+        return;
ecf709
+    }
ecf709
+
ecf709
+    test_ctx->finished_requests++;
ecf709
+    if (test_ctx->finished_requests == test_ctx->num_requests) {
ecf709
+        test_ctx->done = true;
ecf709
+        return;
ecf709
+    }
ecf709
+}
ecf709
+
ecf709
+/*
ecf709
+ * Just make sure that a single pass through the queue works
ecf709
+ */
ecf709
+static void test_kcm_queue_single(void **state)
ecf709
+{
ecf709
+    struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
ecf709
+    struct tevent_req *req;
ecf709
+    struct cli_creds client;
ecf709
+    static int req_ids[] = { 0 };
ecf709
+
ecf709
+    client.ucred.uid = getuid();
ecf709
+    client.ucred.gid = getgid();
ecf709
+
ecf709
+    req = timed_request_send(test_ctx,
ecf709
+                             test_ctx->ev,
ecf709
+                             test_ctx->qctx,
ecf709
+                             &client, 1, 0);
ecf709
+    assert_non_null(req);
ecf709
+    tevent_req_set_callback(req, test_kcm_queue_done, test_ctx);
ecf709
+
ecf709
+    test_ctx->num_requests = 1;
ecf709
+    test_ctx->req_ids = req_ids;
ecf709
+
ecf709
+    while (test_ctx->done == false) {
ecf709
+        tevent_loop_once(test_ctx->ev);
ecf709
+    }
ecf709
+    assert_int_equal(test_ctx->error, EOK);
ecf709
+}
ecf709
+
ecf709
+/*
ecf709
+ * Test that multiple requests from the same ID wait for one another
ecf709
+ */
ecf709
+static void test_kcm_queue_multi_same_id(void **state)
ecf709
+{
ecf709
+    struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
ecf709
+    struct tevent_req *req;
ecf709
+    struct cli_creds client;
ecf709
+    /* The slow request will finish first because request from
ecf709
+     * the same ID are serialized
ecf709
+     */
ecf709
+    static int req_ids[] = { SLOW_REQ_ID, FAST_REQ_ID };
ecf709
+
ecf709
+    client.ucred.uid = getuid();
ecf709
+    client.ucred.gid = getgid();
ecf709
+
ecf709
+    req = timed_request_send(test_ctx,
ecf709
+                             test_ctx->ev,
ecf709
+                             test_ctx->qctx,
ecf709
+                             &client,
ecf709
+                             SLOW_REQ_DELAY,
ecf709
+                             SLOW_REQ_ID);
ecf709
+    assert_non_null(req);
ecf709
+    tevent_req_set_callback(req, test_kcm_queue_done, test_ctx);
ecf709
+
ecf709
+    req = timed_request_send(test_ctx,
ecf709
+                             test_ctx->ev,
ecf709
+                             test_ctx->qctx,
ecf709
+                             &client,
ecf709
+                             FAST_REQ_DELAY,
ecf709
+                             FAST_REQ_ID);
ecf709
+    assert_non_null(req);
ecf709
+    tevent_req_set_callback(req, test_kcm_queue_done, test_ctx);
ecf709
+
ecf709
+    test_ctx->num_requests = 2;
ecf709
+    test_ctx->req_ids = req_ids;
ecf709
+
ecf709
+    while (test_ctx->done == false) {
ecf709
+        tevent_loop_once(test_ctx->ev);
ecf709
+    }
ecf709
+    assert_int_equal(test_ctx->error, EOK);
ecf709
+}
ecf709
+
ecf709
+/*
ecf709
+ * Test that multiple requests from different IDs don't wait for one
ecf709
+ * another and can run concurrently
ecf709
+ */
ecf709
+static void test_kcm_queue_multi_different_id(void **state)
ecf709
+{
ecf709
+    struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
ecf709
+    struct tevent_req *req;
ecf709
+    struct cli_creds client;
ecf709
+    /* In this test, the fast request will finish sooner because
ecf709
+     * both requests are from different IDs, allowing them to run
ecf709
+     * concurrently
ecf709
+     */
ecf709
+    static int req_ids[] = { FAST_REQ_ID, SLOW_REQ_ID };
ecf709
+
ecf709
+    client.ucred.uid = getuid();
ecf709
+    client.ucred.gid = getgid();
ecf709
+
ecf709
+    req = timed_request_send(test_ctx,
ecf709
+                             test_ctx->ev,
ecf709
+                             test_ctx->qctx,
ecf709
+                             &client,
ecf709
+                             SLOW_REQ_DELAY,
ecf709
+                             SLOW_REQ_ID);
ecf709
+    assert_non_null(req);
ecf709
+    tevent_req_set_callback(req, test_kcm_queue_done, test_ctx);
ecf709
+
ecf709
+    client.ucred.uid = getuid() + 1;
ecf709
+    client.ucred.gid = getgid() + 1;
ecf709
+
ecf709
+    req = timed_request_send(test_ctx,
ecf709
+                             test_ctx->ev,
ecf709
+                             test_ctx->qctx,
ecf709
+                             &client,
ecf709
+                             FAST_REQ_DELAY,
ecf709
+                             FAST_REQ_ID);
ecf709
+    assert_non_null(req);
ecf709
+    tevent_req_set_callback(req, test_kcm_queue_done, test_ctx);
ecf709
+
ecf709
+    test_ctx->num_requests = 2;
ecf709
+    test_ctx->req_ids = req_ids;
ecf709
+
ecf709
+    while (test_ctx->done == false) {
ecf709
+        tevent_loop_once(test_ctx->ev);
ecf709
+    }
ecf709
+    assert_int_equal(test_ctx->error, EOK);
ecf709
+}
ecf709
+
ecf709
+int main(int argc, const char *argv[])
ecf709
+{
ecf709
+    poptContext pc;
ecf709
+    int opt;
ecf709
+    int rv;
ecf709
+    struct poptOption long_options[] = {
ecf709
+        POPT_AUTOHELP
ecf709
+        SSSD_DEBUG_OPTS
ecf709
+        POPT_TABLEEND
ecf709
+    };
ecf709
+
ecf709
+    const struct CMUnitTest tests[] = {
ecf709
+        cmocka_unit_test_setup_teardown(test_kcm_queue_single,
ecf709
+                                        setup_kcm_queue,
ecf709
+                                        teardown_kcm_queue),
ecf709
+        cmocka_unit_test_setup_teardown(test_kcm_queue_multi_same_id,
ecf709
+                                        setup_kcm_queue,
ecf709
+                                        teardown_kcm_queue),
ecf709
+        cmocka_unit_test_setup_teardown(test_kcm_queue_multi_different_id,
ecf709
+                                        setup_kcm_queue,
ecf709
+                                        teardown_kcm_queue),
ecf709
+    };
ecf709
+
ecf709
+    /* Set debug level to invalid value so we can deside if -d 0 was used. */
ecf709
+    debug_level = SSSDBG_INVALID;
ecf709
+
ecf709
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
ecf709
+    while((opt = poptGetNextOpt(pc)) != -1) {
ecf709
+        switch(opt) {
ecf709
+        default:
ecf709
+            fprintf(stderr, "\nInvalid option %s: %s\n\n",
ecf709
+                    poptBadOption(pc, 0), poptStrerror(opt));
ecf709
+            poptPrintUsage(pc, stderr, 0);
ecf709
+            return 1;
ecf709
+        }
ecf709
+    }
ecf709
+    poptFreeContext(pc);
ecf709
+
ecf709
+    DEBUG_CLI_INIT(debug_level);
ecf709
+
ecf709
+    /* Even though normally the tests should clean up after themselves
ecf709
+     * they might not after a failed run. Remove the old db to be sure */
ecf709
+    tests_set_cwd();
ecf709
+
ecf709
+    rv = cmocka_run_group_tests(tests, NULL, NULL);
ecf709
+
ecf709
+    return rv;
ecf709
+}
ecf709
-- 
ecf709
2.9.3
ecf709