[SSSD] [PATCH] Chain authentication requests in all Kerberos-based providers

Jakub Hrozek jhrozek at redhat.com
Fri Jul 3 09:54:46 UTC 2015


Hi,

the attached patches fix https://fedorahosted.org/sssd/ticket/2701

The first patch just adds a common function instead of copying the same
pattern again to the new test.

The second adds a new request krb5_auth_queue_send() that wraps
krb5_auth_send() and also uses the Kerberos authentication queue. I hope
the unit tests cover a lot of use-cases, if not, please suggest more!

btw I was thinking that the chaining might not always be necessary if
the ccache is of type MEMORY and I hope that the serializaton wouldn't
be perceived as performance regression for users. Shall we say that
Pavel's cached auth patches are a more systematic solution that doesn't
rely on properties of the ccache type in that case?
-------------- next part --------------
>From ad138ded237a1f3433fa4f379c70f96d66d6797a Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 3 Jul 2015 11:15:32 +0200
Subject: [PATCH 1/2] tests: Reduce duplication with new function test_ev_done

---
 src/tests/cmocka/test_ipa_subdomains_server.c | 11 ++---------
 src/tests/cmocka/test_resolv_fake.c           |  3 +--
 src/tests/cmocka/test_responder_common.c      |  8 ++++----
 src/tests/common.h                            |  3 +++
 src/tests/common_tev.c                        |  6 ++++++
 5 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/tests/cmocka/test_ipa_subdomains_server.c b/src/tests/cmocka/test_ipa_subdomains_server.c
index 28e998b57489eccb5d3b0357df650f0faf3976fc..a4cb8c2b7538dc84b74e0227205b73780844b652 100644
--- a/src/tests/cmocka/test_ipa_subdomains_server.c
+++ b/src/tests/cmocka/test_ipa_subdomains_server.c
@@ -266,7 +266,6 @@ static int test_ipa_server_create_trusts_teardown(void **state)
 
 static void test_ipa_server_create_trusts_none(struct tevent_req *req);
 static void test_ipa_server_create_trusts_twoway(struct tevent_req *req);
-static void test_ipa_server_create_trusts_finish(struct trust_test_ctx *test_ctx);
 
 static void test_ipa_server_create_trusts(void **state)
 {
@@ -427,13 +426,7 @@ static void test_ipa_server_create_trusts_twoway(struct tevent_req *req)
                         DOM_REALM);
     assert_null(test_ctx->ipa_ctx->server_mode->trusts->next);
 
-    test_ipa_server_create_trusts_finish(test_ctx);
-}
-
-static void test_ipa_server_create_trusts_finish(struct trust_test_ctx *test_ctx)
-{
-    test_ctx->tctx->error = EOK;
-    test_ctx->tctx->done = true;
+    test_ev_done(test_ctx->tctx, EOK);
 }
 
 static void
@@ -669,7 +662,7 @@ static void test_ipa_server_create_trusts_oneway(struct tevent_req *req)
                         SUBDOM_REALM);
 
     assert_null(test_ctx->ipa_ctx->server_mode->trusts->next->next);
-    test_ipa_server_create_trusts_finish(test_ctx);
+    test_ev_done(test_ctx->tctx, EOK);
 }
 
 static void test_ipa_server_create_oneway_kt_exists(void **state)
diff --git a/src/tests/cmocka/test_resolv_fake.c b/src/tests/cmocka/test_resolv_fake.c
index 2c846ce4cc63cbf80b30bd15091136002ec84cb3..a1e9ee4cba9b17d506eeae7e6088de28c301364d 100644
--- a/src/tests/cmocka/test_resolv_fake.c
+++ b/src/tests/cmocka/test_resolv_fake.c
@@ -293,8 +293,7 @@ void test_resolv_fake_srv_done(struct tevent_req *req)
     assert_int_equal(ttl, 500);
 
     talloc_free(tmp_ctx);
-    test_ctx->ctx->error = EOK;
-    test_ctx->ctx->done = true;
+    test_ev_done(test_ctx->ctx, EOK);
 }
 
 void test_resolv_fake_srv(void **state)
diff --git a/src/tests/cmocka/test_responder_common.c b/src/tests/cmocka/test_responder_common.c
index 0a4d4bb49c5058b5a006912b6b3b61ab0332cc29..cb57c97fe6b0995c9ec69f969015ef500e065fad 100644
--- a/src/tests/cmocka/test_responder_common.c
+++ b/src/tests/cmocka/test_responder_common.c
@@ -161,8 +161,9 @@ void parse_inp_simple_done(struct tevent_req *req)
     char *domname = NULL;
 
     ret = sss_parse_inp_recv(req, parse_inp_ctx, &name, &domname);
-    parse_inp_ctx->tctx->done = true;
     assert_int_equal(ret, EOK);
+
+    test_ev_done(parse_inp_ctx->tctx, EOK);
     talloc_free(req);
 
     assert_string_equal(name, NAME);
@@ -239,8 +240,8 @@ void parse_inp_neg_done(struct tevent_req *req)
     char *domname = NULL;
 
     ret = sss_parse_inp_recv(req, parse_inp_ctx, &name, &domname);
-    parse_inp_ctx->tctx->done = true;
     assert_int_equal(ret, ERR_INPUT_PARSE);
+    test_ev_done(parse_inp_ctx->tctx, EOK);
     talloc_free(req);
 
     assert_null(name);
@@ -273,8 +274,7 @@ struct sss_nc_ctx {
 errno_t sss_ncache_reset_repopulate_permanent(struct resp_ctx *rctx,
                                               struct sss_nc_ctx *dummy_ncache_ptr)
 {
-    dummy_ncache_ptr->pctx->tctx->error = EOK;
-    dummy_ncache_ptr->pctx->tctx->done = true;
+    test_ev_done(dummy_ncache_ptr->pctx->tctx, EOK);
     return EOK;
 }
 
diff --git a/src/tests/common.h b/src/tests/common.h
index 714be6988aa032d935c09bc7db524545a4c9154e..0b351f5d647a8f72bdbc399c6fe02579d4b4e1be 100644
--- a/src/tests/common.h
+++ b/src/tests/common.h
@@ -112,6 +112,9 @@ errno_t test_request_recv(struct tevent_req *req);
 
 int test_ev_loop(struct sss_test_ctx *tctx);
 
+/* Mark the test as done with an error code */
+void test_ev_done(struct sss_test_ctx *tctx, errno_t ret);
+
 bool ldb_modules_path_is_set(void);
 
 DBusConnection *
diff --git a/src/tests/common_tev.c b/src/tests/common_tev.c
index 770f97d83684696498ff691dc14b45c233008fd3..808efd459ca66a52874eda2897ab9cdd084523e1 100644
--- a/src/tests/common_tev.c
+++ b/src/tests/common_tev.c
@@ -83,3 +83,9 @@ int test_ev_loop(struct sss_test_ctx *tctx)
 
     return tctx->error;
 }
+
+void test_ev_done(struct sss_test_ctx *tctx, errno_t ret)
+{
+    tctx->error = ret;
+    tctx->done = true;
+}
-- 
2.4.3

-------------- next part --------------
>From 167d7c8c56e33bd2369ddc62b502e39b50633edd Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 30 Jun 2015 19:40:46 +0200
Subject: [PATCH 2/2] KRB5: Add and use krb5_auth_queue_send to queue requests
 by default

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

Previously, only the krb5 provides used to queue requests, which
resulted in concurrent authentication requests stepping on one another.

This patch queues requests by default.
---
 Makefile.am                                        |  15 +
 src/providers/ipa/ipa_auth.c                       |  12 +-
 src/providers/krb5/krb5_auth.c                     |  28 +-
 src/providers/krb5/krb5_auth.h                     |  18 +-
 .../krb5/krb5_delayed_online_authentication.c      |  11 +-
 src/providers/krb5/krb5_renew_tgt.c                |   6 +-
 src/providers/krb5/krb5_wait_queue.c               | 183 ++++++++++-
 src/tests/cmocka/test_krb5_wait_queue.c            | 365 +++++++++++++++++++++
 8 files changed, 584 insertions(+), 54 deletions(-)
 create mode 100644 src/tests/cmocka/test_krb5_wait_queue.c

diff --git a/Makefile.am b/Makefile.am
index 8fa532d30e2af52ce65046e0e6a70739ecce3d17..a37e8d1417bc5b936b4e9248e89b47ccfe5f1d4d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -230,6 +230,7 @@ if HAVE_CMOCKA
         pam-srv-tests \
         test_ipa_subdom_util \
         test_ipa_subdom_server \
+        test_krb5_wait_queue \
         test_cert_utils \
         $(NULL)
 
@@ -2515,6 +2516,20 @@ test_ipa_subdom_server_LDADD = \
     libsss_test_common.la \
     $(NULL)
 
+test_krb5_wait_queue_SOURCES = \
+    src/tests/cmocka/common_mock_be.c \
+    src/tests/cmocka/test_krb5_wait_queue.c \
+    src/providers/krb5/krb5_wait_queue.c \
+    $(NULL)
+test_krb5_wait_queue_CFLAGS = \
+    $(AM_CFLAGS) \
+    $(NULL)
+test_krb5_wait_queue_LDADD = \
+    $(CMOCKA_LIBS) \
+    $(SSSD_INTERNAL_LTLIBS) \
+    libsss_test_common.la \
+    $(NULL)
+
 test_cert_utils_SOURCES = \
     src/tests/cmocka/test_cert_utils.c \
     $(NULL)
diff --git a/src/providers/ipa/ipa_auth.c b/src/providers/ipa/ipa_auth.c
index 79e891b77205f75671c146af0192b764f12e13b0..b1bfa3ffe6aaae6b19ba3dd5334fd4444933ef51 100644
--- a/src/providers/ipa/ipa_auth.c
+++ b/src/providers/ipa/ipa_auth.c
@@ -224,8 +224,8 @@ void ipa_auth(struct be_req *be_req)
             goto fail;
     }
 
-    req = krb5_auth_send(state, state->ev, be_ctx, state->pd,
-                         state->ipa_auth_ctx->krb5_auth_ctx);
+    req = krb5_auth_queue_send(state, state->ev, be_ctx, state->pd,
+                               state->ipa_auth_ctx->krb5_auth_ctx);
     if (req == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "krb5_auth_send failed.\n");
         goto fail;
@@ -248,7 +248,7 @@ static void ipa_auth_handler_done(struct tevent_req *req)
     int pam_status = PAM_SYSTEM_ERR;
     int dp_err;
 
-    ret = krb5_auth_recv(req, &pam_status, &dp_err);
+    ret = krb5_auth_queue_recv(req, &pam_status, &dp_err);
     talloc_zfree(req);
     state->pd->pam_status = pam_status;
     if (ret != EOK && pam_status != PAM_CRED_ERR) {
@@ -423,8 +423,8 @@ static void ipa_auth_ldap_done(struct tevent_req *req)
     DEBUG(SSSDBG_TRACE_FUNC, "LDAP authentication succeded, "
                               "trying Kerberos authentication again.\n");
 
-    req = krb5_auth_send(state, state->ev, be_ctx, state->pd,
-                         state->ipa_auth_ctx->krb5_auth_ctx);
+    req = krb5_auth_queue_send(state, state->ev, be_ctx, state->pd,
+                               state->ipa_auth_ctx->krb5_auth_ctx);
     if (req == NULL) {
         DEBUG(SSSDBG_OP_FAILURE, "krb5_auth_send failed.\n");
         goto done;
@@ -445,7 +445,7 @@ static void ipa_auth_handler_retry_done(struct tevent_req *req)
     int pam_status;
     int dp_err;
 
-    ret = krb5_auth_recv(req, &pam_status, &dp_err);
+    ret = krb5_auth_queue_recv(req, &pam_status, &dp_err);
     talloc_zfree(req);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, "krb5_auth_recv request failed.\n");
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 812e215240ec28e2cd627da19da0059286909793..c94f2b26f8d4b8d5d3bd5d7836052c478e13aba7 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -1139,7 +1139,6 @@ void krb5_pam_handler(struct be_req *be_req)
     struct pam_data *pd;
     struct krb5_ctx *krb5_ctx;
     int dp_err = DP_ERR_FATAL;
-    int ret;
 
     pd = talloc_get_type(be_req_get_data(be_req), struct pam_data);
     pd->pam_status = PAM_SYSTEM_ERR;
@@ -1155,22 +1154,7 @@ void krb5_pam_handler(struct be_req *be_req)
         case SSS_CMD_RENEW:
         case SSS_PAM_CHAUTHTOK_PRELIM:
         case SSS_PAM_CHAUTHTOK:
-            ret = add_to_wait_queue(be_req, pd, krb5_ctx);
-            if (ret == EOK) {
-                DEBUG(SSSDBG_TRACE_LIBS,
-                      "Request successfully added to wait queue "
-                          "of user [%s].\n", pd->user);
-                return;
-            } else if (ret == ENOENT) {
-                DEBUG(SSSDBG_TRACE_LIBS, "Wait queue of user [%s] is empty, "
-                          "running request immediately.\n", pd->user);
-            } else {
-                DEBUG(SSSDBG_TRACE_LIBS,
-                      "Failed to add request to wait queue of user [%s], "
-                          "running request immediately.\n", pd->user);
-            }
-
-            req = krb5_auth_send(be_req, be_ctx->ev, be_ctx, pd, krb5_ctx);
+            req = krb5_auth_queue_send(be_req, be_ctx->ev, be_ctx, pd, krb5_ctx);
             if (req == NULL) {
                 DEBUG(SSSDBG_CRIT_FAILURE, "krb5_auth_send failed.\n");
                 goto done;
@@ -1215,11 +1199,10 @@ void krb5_pam_handler_auth_done(struct tevent_req *req)
     int pam_status;
     int dp_err;
     struct pam_data *pd;
-    struct krb5_ctx *krb5_ctx;
 
     pd = talloc_get_type(be_req_get_data(be_req), struct pam_data);
 
-    ret = krb5_auth_recv(req, &pam_status, &dp_err);
+    ret = krb5_auth_queue_recv(req, &pam_status, &dp_err);
     talloc_zfree(req);
     if (ret) {
         pd->pam_status = PAM_SYSTEM_ERR;
@@ -1228,13 +1211,6 @@ void krb5_pam_handler_auth_done(struct tevent_req *req)
         pd->pam_status = pam_status;
     }
 
-    krb5_ctx = get_krb5_ctx(be_req);
-    if (krb5_ctx != NULL) {
-        check_wait_queue(krb5_ctx, pd->user);
-    } else {
-        DEBUG(SSSDBG_CRIT_FAILURE, "Kerberos context not available.\n");
-    }
-
     be_req_terminate(be_req, dp_err, pd->pam_status, NULL);
 }
 
diff --git a/src/providers/krb5/krb5_auth.h b/src/providers/krb5/krb5_auth.h
index a00f1d9d9be38453da11c9e64ab73fe23f67d475..205e70665531803d6374f6fef084cd24114a6a97 100644
--- a/src/providers/krb5/krb5_auth.h
+++ b/src/providers/krb5/krb5_auth.h
@@ -66,6 +66,11 @@ errno_t krb5_setup(TALLOC_CTX *mem_ctx, struct pam_data *pd,
 void krb5_pam_handler(struct be_req *be_req);
 void krb5_pam_handler_auth_done(struct tevent_req *req);
 
+/* Please use krb5_auth_send/recv *only* if you're certain there can't
+ * be concurrent logins happening. With some ccache back ends, the ccache
+ * files might clobber one another. Please use krb5_auth_queue_send()
+ * instead that queues the requests
+ */
 struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
                                   struct tevent_context *ev,
                                   struct be_ctx *be_ctx,
@@ -114,7 +119,14 @@ struct tevent_req *krb5_access_send(TALLOC_CTX *mem_ctx,
 int krb5_access_recv(struct tevent_req *req, bool *access_allowed);
 
 /* krb5_wait_queue.c */
-errno_t add_to_wait_queue(struct be_req *be_req, struct pam_data *pd,
-                          struct krb5_ctx *krb5_ctx);
-void check_wait_queue(struct krb5_ctx *krb5_ctx, char *username);
+struct tevent_req *krb5_auth_queue_send(TALLOC_CTX *mem_ctx,
+                                        struct tevent_context *ev,
+                                        struct be_ctx *be_ctx,
+                                        struct pam_data *pd,
+                                        struct krb5_ctx *krb5_ctx);
+
+int krb5_auth_queue_recv(struct tevent_req *req,
+                         int *_pam_status,
+                         int *_dp_err);
+
 #endif /* __KRB5_AUTH_H__ */
diff --git a/src/providers/krb5/krb5_delayed_online_authentication.c b/src/providers/krb5/krb5_delayed_online_authentication.c
index 5f13eac8e1dbe7494423b553458f75aacab8c258..bf2ef775573ba6bad79a99ad43b5d9748516e794 100644
--- a/src/providers/krb5/krb5_delayed_online_authentication.c
+++ b/src/providers/krb5/krb5_delayed_online_authentication.c
@@ -103,8 +103,8 @@ static void authenticate_user(struct tevent_context *ev,
     }
 #endif
 
-    req = krb5_auth_send(auth_data, ev, auth_data->be_ctx, auth_data->pd,
-                         auth_data->krb5_ctx);
+    req = krb5_auth_queue_send(auth_data, ev, auth_data->be_ctx,
+                               auth_data->pd, auth_data->krb5_ctx);
     if (req == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, "krb5_auth_send failed.\n");
         talloc_free(auth_data);
@@ -114,14 +114,15 @@ static void authenticate_user(struct tevent_context *ev,
     tevent_req_set_callback(req, authenticate_user_done, auth_data);
 }
 
-static void authenticate_user_done(struct tevent_req *req) {
+static void authenticate_user_done(struct tevent_req *req)
+{
     struct auth_data *auth_data = tevent_req_callback_data(req,
                                                            struct auth_data);
     int ret;
     int pam_status = PAM_SYSTEM_ERR;
-    int dp_err;
+    int dp_err = DP_ERR_OK;
 
-    ret = krb5_auth_recv(req, &pam_status, &dp_err);
+    ret = krb5_auth_queue_recv(req, &pam_status, &dp_err);
     talloc_free(req);
     if (ret) {
         DEBUG(SSSDBG_CRIT_FAILURE, "krb5_auth request failed.\n");
diff --git a/src/providers/krb5/krb5_renew_tgt.c b/src/providers/krb5/krb5_renew_tgt.c
index 5277c0f7691dd272a69b3024cb2c0bd7545a8fbd..498097792b65e68dc849c09501ad5b1882e4a498 100644
--- a/src/providers/krb5/krb5_renew_tgt.c
+++ b/src/providers/krb5/krb5_renew_tgt.c
@@ -66,8 +66,8 @@ static void renew_tgt(struct tevent_context *ev, struct tevent_timer *te,
                                                   struct auth_data);
     struct tevent_req *req;
 
-    req = krb5_auth_send(auth_data, ev, auth_data->be_ctx, auth_data->pd,
-                         auth_data->krb5_ctx);
+    req = krb5_auth_queue_send(auth_data, ev, auth_data->be_ctx, auth_data->pd,
+                               auth_data->krb5_ctx);
     if (req == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, "krb5_auth_send failed.\n");
 /* Give back the pam data to the renewal item to be able to retry at the next
@@ -90,7 +90,7 @@ static void renew_tgt_done(struct tevent_req *req)
     int dp_err;
     hash_value_t value;
 
-    ret = krb5_auth_recv(req, &pam_status, &dp_err);
+    ret = krb5_auth_queue_recv(req, &pam_status, &dp_err);
     talloc_free(req);
     if (ret) {
         DEBUG(SSSDBG_CRIT_FAILURE, "krb5_auth request failed.\n");
diff --git a/src/providers/krb5/krb5_wait_queue.c b/src/providers/krb5/krb5_wait_queue.c
index 10c50d81b8c8b81aab0b5f73fa86b57f708a2e54..d0cc0c14d6c39a1248cb155e70d1bb6917768848 100644
--- a/src/providers/krb5/krb5_wait_queue.c
+++ b/src/providers/krb5/krb5_wait_queue.c
@@ -25,6 +25,8 @@
 #include <tevent.h>
 #include <dhash.h>
 
+#include <security/pam_modules.h>
+
 #include "src/providers/krb5/krb5_auth.h"
 
 #define INIT_HASH_SIZE 5
@@ -33,28 +35,53 @@ struct queue_entry {
     struct queue_entry *prev;
     struct queue_entry *next;
 
+    struct be_ctx *be_ctx;
     struct be_req *be_req;
+    struct tevent_req *parent_req;
     struct pam_data *pd;
     struct krb5_ctx *krb5_ctx;
 };
 
+static void wait_queue_auth_done(struct tevent_req *req);
+
+static void krb5_auth_queue_finish(struct tevent_req *req, errno_t ret,
+                                   int pam_status, int dp_err);
+
 static void wait_queue_auth(struct tevent_context *ev, struct tevent_timer *te,
-                             struct timeval current_time, void *private_data)
+                            struct timeval current_time, void *private_data)
 {
     struct queue_entry *qe = talloc_get_type(private_data, struct queue_entry);
-    struct be_ctx *be_ctx = be_req_get_be_ctx(qe->be_req);
     struct tevent_req *req;
 
-    req = krb5_auth_send(qe->be_req, be_ctx->ev, be_ctx, qe->pd, qe->krb5_ctx);
+    req = krb5_auth_send(qe->parent_req, qe->be_ctx->ev,
+                         qe->be_ctx, qe->pd, qe->krb5_ctx);
     if (req == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, "krb5_auth_send failed.\n");
     } else {
-        tevent_req_set_callback(req, krb5_pam_handler_auth_done, qe->be_req);
+        tevent_req_set_callback(req, wait_queue_auth_done,
+                                qe->parent_req);
     }
 
     talloc_zfree(qe);
 }
 
+static void wait_queue_auth_done(struct tevent_req *req)
+{
+    struct tevent_req *parent_req = \
+                tevent_req_callback_data(req, struct tevent_req);
+    int pam_status;
+    int dp_err;
+    errno_t ret;
+
+    ret = krb5_auth_recv(req, &pam_status, &dp_err);
+    talloc_zfree(req);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "krb5_auth_recv failed: %d\n", ret);
+    }
+
+    krb5_auth_queue_finish(parent_req, ret, pam_status, dp_err);
+}
+
 static void wait_queue_del_cb(hash_entry_t *entry, hash_destroy_enum type,
                               void *pvt)
 {
@@ -70,8 +97,10 @@ static void wait_queue_del_cb(hash_entry_t *entry, hash_destroy_enum type,
           "Unexpected value type [%d].\n", entry->value.type);
 }
 
-errno_t add_to_wait_queue(struct be_req *be_req, struct pam_data *pd,
-                          struct krb5_ctx *krb5_ctx)
+static errno_t add_to_wait_queue(struct be_ctx *be_ctx,
+                                 struct tevent_req *parent_req,
+                                 struct pam_data *pd,
+                                 struct krb5_ctx *krb5_ctx)
 {
     int ret;
     hash_key_t key;
@@ -108,7 +137,8 @@ errno_t add_to_wait_queue(struct be_req *be_req, struct pam_data *pd,
                 return ENOMEM;
             }
 
-            queue_entry->be_req = be_req;
+            queue_entry->be_ctx = be_ctx;
+            queue_entry->parent_req = parent_req;
             queue_entry->pd = pd;
             queue_entry->krb5_ctx = krb5_ctx;
 
@@ -144,7 +174,7 @@ errno_t add_to_wait_queue(struct be_req *be_req, struct pam_data *pd,
     }
 }
 
-void check_wait_queue(struct krb5_ctx *krb5_ctx, char *username)
+static void check_wait_queue(struct krb5_ctx *krb5_ctx, char *username)
 {
     int ret;
     hash_key_t key;
@@ -152,7 +182,6 @@ void check_wait_queue(struct krb5_ctx *krb5_ctx, char *username)
     struct queue_entry *head;
     struct queue_entry *queue_entry;
     struct tevent_timer *te;
-    struct be_ctx *be_ctx;
 
     if (krb5_ctx->wait_queue_hash == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, "No wait queue available.\n");
@@ -181,8 +210,7 @@ void check_wait_queue(struct krb5_ctx *krb5_ctx, char *username)
 
                 DLIST_REMOVE(head, queue_entry);
 
-                be_ctx = be_req_get_be_ctx(queue_entry->be_req);
-                te = tevent_add_timer(be_ctx->ev, krb5_ctx,
+                te = tevent_add_timer(queue_entry->be_ctx->ev, krb5_ctx,
                                       tevent_timeval_current(), wait_queue_auth,
                                       queue_entry);
                 if (te == NULL) {
@@ -211,3 +239,136 @@ void check_wait_queue(struct krb5_ctx *krb5_ctx, char *username)
     return;
 }
 
+struct krb5_auth_queue_state {
+    struct krb5_ctx *krb5_ctx;
+    struct pam_data *pd;
+
+    int pam_status;
+    int dp_err;
+};
+
+static void krb5_auth_queue_done(struct tevent_req *subreq);
+
+struct tevent_req *krb5_auth_queue_send(TALLOC_CTX *mem_ctx,
+                                        struct tevent_context *ev,
+                                        struct be_ctx *be_ctx,
+                                        struct pam_data *pd,
+                                        struct krb5_ctx *krb5_ctx)
+{
+    errno_t ret;
+    struct tevent_req *req;
+    struct tevent_req *subreq;
+    struct krb5_auth_queue_state *state;
+
+    req = tevent_req_create(mem_ctx, &state, struct krb5_auth_queue_state);
+    if (req == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create failed.\n");
+        return NULL;
+    }
+    state->krb5_ctx = krb5_ctx;
+    state->pd = pd;
+
+    ret = add_to_wait_queue(be_ctx, req, pd, krb5_ctx);
+    if (ret == EOK) {
+        DEBUG(SSSDBG_TRACE_LIBS,
+              "Request successfully added to wait queue "
+              "of user [%s].\n", pd->user);
+        ret = EOK;
+        goto immediate;
+    } else if (ret == ENOENT) {
+        DEBUG(SSSDBG_TRACE_LIBS, "Wait queue of user [%s] is empty, "
+              "running request immediately.\n", pd->user);
+    } else {
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              "Failed to add request to wait queue of user [%s], "
+              "running request immediately.\n", pd->user);
+    }
+
+    subreq = krb5_auth_send(req, ev, be_ctx, pd, krb5_ctx);
+    if (req == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "krb5_auth_send failed.\n");
+        ret = ENOMEM;
+        goto immediate;
+    }
+
+    tevent_req_set_callback(subreq, krb5_auth_queue_done, req);
+
+    ret = EOK;
+
+immediate:
+    if (ret != EOK) {
+        tevent_req_error(req, ret);
+        tevent_req_post(req, ev);
+    }
+    return req;
+}
+
+static void krb5_auth_queue_done(struct tevent_req *subreq)
+{
+    struct tevent_req *req = \
+                tevent_req_callback_data(subreq, struct tevent_req);
+    struct krb5_auth_queue_state *state = \
+                tevent_req_data(req, struct krb5_auth_queue_state);
+    errno_t ret;
+
+    ret = krb5_auth_recv(subreq, &state->pam_status, &state->dp_err);
+    talloc_zfree(subreq);
+
+    check_wait_queue(state->krb5_ctx, state->pd->user);
+
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, "krb5_auth_recv failed with: %d\n", ret);
+        tevent_req_error(req, ret);
+        return;
+    }
+
+    DEBUG(SSSDBG_TRACE_INTERNAL, "krb5_auth_queue done\n");
+    tevent_req_done(req);
+}
+
+/* This is a violation of the tevent_req style. Ideally, the wait queue would
+ * be rewritten to the tevent_req style in the future, expose per-request recv
+ * and not hide the request underneath. But this function allows us to expose
+ * a tevent_req API for users of this module
+ */
+static void krb5_auth_queue_finish(struct tevent_req *req,
+                                   errno_t ret,
+                                   int pam_status,
+                                   int dp_err)
+{
+    struct krb5_auth_queue_state *state = \
+                tevent_req_data(req, struct krb5_auth_queue_state);
+
+    check_wait_queue(state->krb5_ctx, state->pd->user);
+
+    state->pam_status = pam_status;
+    state->dp_err = dp_err;
+    if (ret != EOK) {
+        tevent_req_error(req, ret);
+    } else {
+        tevent_req_done(req);
+    }
+}
+
+int krb5_auth_queue_recv(struct tevent_req *req,
+                         int *_pam_status,
+                         int *_dp_err)
+{
+    struct krb5_auth_queue_state *state = \
+                tevent_req_data(req, struct krb5_auth_queue_state);
+
+    /* Returning values even on failure is not typical, but IPA password migration
+     * relies on receiving PAM_CRED_ERR even if the request fails..
+     */
+    if (_pam_status) {
+        *_pam_status = state->pam_status;
+    }
+
+    if (_dp_err) {
+        *_dp_err = state->pam_status;
+    }
+
+    TEVENT_REQ_RETURN_ON_ERROR(req);
+
+    return EOK;
+}
diff --git a/src/tests/cmocka/test_krb5_wait_queue.c b/src/tests/cmocka/test_krb5_wait_queue.c
new file mode 100644
index 0000000000000000000000000000000000000000..9e72d05d16d071815936855bd56b804d266717e5
--- /dev/null
+++ b/src/tests/cmocka/test_krb5_wait_queue.c
@@ -0,0 +1,365 @@
+/*
+    Copyright (C) 2015 Red Hat
+
+    SSSD tests: Kerberos wait queue tests
+
+    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 <talloc.h>
+#include <tevent.h>
+#include <errno.h>
+#include <popt.h>
+#include <stdlib.h>
+#include <security/pam_modules.h>
+
+#include "util/util.h"
+#include "providers/krb5/krb5_common.h"
+#include "providers/krb5/krb5_auth.h"
+#include "tests/cmocka/common_mock.h"
+#include "tests/cmocka/common_mock_be.h"
+
+struct krb5_mocked_auth_state {
+    const char *user;
+    time_t us_delay;
+    int ret;
+    int pam_status;
+    int dp_err;
+};
+
+static void krb5_mocked_auth_done(struct tevent_context *ev,
+                                  struct tevent_timer *tt,
+                                  struct timeval tv,
+                                  void *pvt);
+
+struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
+                                  struct tevent_context *ev,
+                                  struct be_ctx *be_ctx,
+                                  struct pam_data *pd,
+                                  struct krb5_ctx *krb5_ctx)
+{
+    struct tevent_req *req;
+    struct krb5_mocked_auth_state *state;
+    struct tevent_timer *tt;
+    struct timeval tv;
+
+    req = tevent_req_create(mem_ctx, &state, struct krb5_mocked_auth_state);
+    if (req == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create failed.\n");
+        return NULL;
+    }
+
+    state->user = sss_mock_ptr_type(const char *);
+    state->us_delay = sss_mock_type(time_t);
+    state->ret = sss_mock_type(int);
+    state->pam_status = sss_mock_type(int);
+    state->dp_err = sss_mock_type(int);
+
+    tv = tevent_timeval_current_ofs(0, state->us_delay);
+
+    tt = tevent_add_timer(ev, req, tv, krb5_mocked_auth_done, req);
+    if (tt == NULL) {
+        return NULL;
+    }
+
+    return req;
+}
+
+static void krb5_mocked_auth_done(struct tevent_context *ev,
+                                  struct tevent_timer *tt,
+                                  struct timeval tv,
+                                  void *pvt)
+{
+    struct tevent_req *req;
+    struct krb5_mocked_auth_state *state;
+
+    req = talloc_get_type(pvt, struct tevent_req);
+    state = tevent_req_data(req, struct krb5_mocked_auth_state);
+
+    DEBUG(SSSDBG_TRACE_LIBS, "Finished auth request of %s\n", state->user);
+
+    if (state->ret == 0) {
+        tevent_req_done(req);
+    } else {
+        tevent_req_error(req, state->ret);
+    }
+}
+
+int krb5_auth_recv(struct tevent_req *req,
+                   int *_pam_status,
+                   int *_dp_err)
+{
+    struct krb5_mocked_auth_state *state;
+
+    state = tevent_req_data(req, struct krb5_mocked_auth_state);
+
+    if (_pam_status != NULL) {
+        *_pam_status = state->pam_status;
+    }
+
+    if (_dp_err != NULL) {
+        *_dp_err = state->dp_err;
+    }
+
+    TEVENT_REQ_RETURN_ON_ERROR(req);
+    return EOK;
+}
+
+struct test_krb5_wait_queue {
+    struct sss_test_ctx *tctx;
+    int num_auths;
+    int num_finished_auths;
+
+    struct be_ctx *be_ctx;
+    struct pam_data *pd;
+    struct krb5_ctx *krb5_ctx;
+};
+
+static int test_krb5_wait_queue_setup(void **state)
+{
+    struct test_krb5_wait_queue *test_ctx;
+
+    test_ctx = talloc_zero(global_talloc_context,
+                           struct test_krb5_wait_queue);
+    assert_non_null(test_ctx);
+
+    test_ctx->tctx = create_ev_test_ctx(test_ctx);
+    assert_non_null(test_ctx);
+
+    test_ctx->be_ctx = mock_be_ctx(test_ctx, test_ctx->tctx);
+    assert_non_null(test_ctx->be_ctx);
+
+    test_ctx->pd = talloc_zero(test_ctx, struct pam_data);
+    assert_non_null(test_ctx->pd);
+
+    test_ctx->krb5_ctx = talloc_zero(test_ctx, struct krb5_ctx);
+    assert_non_null(test_ctx->krb5_ctx);
+
+    *state = test_ctx;
+    return 0;
+}
+
+static int test_krb5_wait_queue_teardown(void **state)
+{
+    struct test_krb5_wait_queue *test_ctx =
+        talloc_get_type(*state, struct test_krb5_wait_queue);
+
+    talloc_free(test_ctx);
+    return 0;
+}
+
+static void test_krb5_wait_mock(struct test_krb5_wait_queue *test_ctx, 
+                                const char *username,
+                                time_t us_delay,
+                                int ret,
+                                int pam_status,
+                                int dp_err)
+{
+    test_ctx->pd->user = discard_const(username);
+
+    will_return(krb5_auth_send, username);
+    will_return(krb5_auth_send, us_delay);
+    will_return(krb5_auth_send, ret);
+    will_return(krb5_auth_send, pam_status);
+    will_return(krb5_auth_send, dp_err);
+}
+
+static void test_krb5_wait_mock_success(struct test_krb5_wait_queue *test_ctx,
+                                        const char *username)
+{
+    return test_krb5_wait_mock(test_ctx, username, 200, 0, 0, 0);
+}
+
+static void test_krb5_wait_queue_single_done(struct tevent_req *req);
+
+static void test_krb5_wait_queue_single(void **state)
+{
+    errno_t ret;
+    struct tevent_req *req;
+    struct test_krb5_wait_queue *test_ctx =
+        talloc_get_type(*state, struct test_krb5_wait_queue);
+
+    test_krb5_wait_mock_success(test_ctx, "krb5_user");
+
+    req = krb5_auth_queue_send(test_ctx,
+                               test_ctx->tctx->ev,
+                               test_ctx->be_ctx,
+                               test_ctx->pd,
+                               test_ctx->krb5_ctx);
+    assert_non_null(req);
+    tevent_req_set_callback(req, test_krb5_wait_queue_single_done, test_ctx);
+
+    ret = test_ev_loop(test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+}
+
+static void test_krb5_wait_queue_single_done(struct tevent_req *req)
+{
+    struct test_krb5_wait_queue *test_ctx = \
+        tevent_req_callback_data(req, struct test_krb5_wait_queue);
+    errno_t ret;
+    int pam_status;
+    int dp_err;
+
+    ret = krb5_auth_queue_recv(req, &pam_status, &dp_err);
+    talloc_free(req);
+    assert_int_equal(ret, EOK);
+
+    test_ev_done(test_ctx->tctx, EOK);
+}
+
+static void test_krb5_wait_queue_multi_done(struct tevent_req *req);
+
+static void test_krb5_wait_queue_multi(void **state)
+{
+    int i;
+    errno_t ret;
+    struct tevent_req *req;
+    struct test_krb5_wait_queue *test_ctx =
+        talloc_get_type(*state, struct test_krb5_wait_queue);
+
+    test_ctx->num_auths = 1000;
+
+    for (i=0; i < test_ctx->num_auths; i++) {
+        test_krb5_wait_mock_success(test_ctx, "krb5_user");
+
+        req = krb5_auth_queue_send(test_ctx,
+                                   test_ctx->tctx->ev,
+                                   test_ctx->be_ctx,
+                                   test_ctx->pd,
+                                   test_ctx->krb5_ctx);
+        assert_non_null(req);
+        tevent_req_set_callback(req, test_krb5_wait_queue_multi_done, test_ctx);
+    }
+
+    ret = test_ev_loop(test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+}
+
+static void test_krb5_wait_queue_multi_done(struct tevent_req *req)
+{
+    struct test_krb5_wait_queue *test_ctx = \
+        tevent_req_callback_data(req, struct test_krb5_wait_queue);
+    errno_t ret;
+    int pam_status;
+    int dp_err;
+
+    ret = krb5_auth_queue_recv(req, &pam_status, &dp_err);
+    talloc_free(req);
+    assert_int_equal(ret, EOK);
+
+    test_ctx->num_finished_auths++;
+
+    if (test_ctx->num_finished_auths == test_ctx->num_auths) {
+        test_ev_done(test_ctx->tctx, EOK);
+    }
+}
+
+static void test_krb5_wait_queue_fail_odd_done(struct tevent_req *req);
+
+static void test_krb5_wait_queue_fail_odd(void **state)
+{
+    int i;
+    errno_t ret;
+    struct tevent_req *req;
+    struct test_krb5_wait_queue *test_ctx =
+        talloc_get_type(*state, struct test_krb5_wait_queue);
+
+    test_ctx->num_auths = 10;
+
+    for (i=0; i < test_ctx->num_auths; i++) {
+        test_krb5_wait_mock(test_ctx, "krb5_user", 0, i+1 % 2, PAM_SUCCESS, 0);
+
+        req = krb5_auth_queue_send(test_ctx,
+                                   test_ctx->tctx->ev,
+                                   test_ctx->be_ctx,
+                                   test_ctx->pd,
+                                   test_ctx->krb5_ctx);
+        assert_non_null(req);
+        tevent_req_set_callback(req, test_krb5_wait_queue_fail_odd_done, test_ctx);
+    }
+
+    ret = test_ev_loop(test_ctx->tctx);
+    assert_int_equal(ret, EOK);
+}
+
+static void test_krb5_wait_queue_fail_odd_done(struct tevent_req *req)
+{
+    struct test_krb5_wait_queue *test_ctx = \
+        tevent_req_callback_data(req, struct test_krb5_wait_queue);
+    errno_t ret;
+    int pam_status;
+    int dp_err;
+
+    ret = krb5_auth_queue_recv(req, &pam_status, &dp_err);
+    talloc_free(req);
+    assert_int_equal(ret, test_ctx->num_finished_auths+1 % 2);
+
+    test_ctx->num_finished_auths++;
+
+    if (test_ctx->num_finished_auths == test_ctx->num_auths) {
+        test_ev_done(test_ctx->tctx, EOK);
+    }
+}
+
+int main(int argc, const char *argv[])
+{
+    poptContext pc;
+    int opt;
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        SSSD_DEBUG_OPTS
+        POPT_TABLEEND
+    };
+
+    const struct CMUnitTest tests[] = {
+        /* Run a single auth request */
+        cmocka_unit_test_setup_teardown(test_krb5_wait_queue_single,
+                                        test_krb5_wait_queue_setup,
+                                        test_krb5_wait_queue_teardown),
+
+        /* Run multiple auth requests */
+        cmocka_unit_test_setup_teardown(test_krb5_wait_queue_multi,
+                                        test_krb5_wait_queue_setup,
+                                        test_krb5_wait_queue_teardown),
+
+        /* Make sure that all requests in queue run even if some fail */
+        cmocka_unit_test_setup_teardown(test_krb5_wait_queue_fail_odd,
+                                        test_krb5_wait_queue_setup,
+                                        test_krb5_wait_queue_teardown),
+    };
+
+    /* Set debug level to invalid value so we can deside if -d 0 was used. */
+    debug_level = SSSDBG_INVALID;
+
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+    while((opt = poptGetNextOpt(pc)) != -1) {
+        switch(opt) {
+        default:
+            fprintf(stderr, "\nInvalid option %s: %s\n\n",
+                    poptBadOption(pc, 0), poptStrerror(opt));
+            poptPrintUsage(pc, stderr, 0);
+            return 1;
+        }
+    }
+    poptFreeContext(pc);
+
+    DEBUG_CLI_INIT(debug_level);
+
+    /* Even though normally the tests should clean up after themselves
+     * they might not after a failed run. Remove the old db to be sure */
+    tests_set_cwd();
+
+    return cmocka_run_group_tests(tests, NULL, NULL);
+}
-- 
2.4.3



More information about the sssd-devel mailing list