From f6e57537cbeaf6e3f313e700f08e0022a32a7d6c Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

Date: Mon, 30 Oct 2017 17:11:56 +0100

Subject: [PATCH 41/46] PAM: allow missing logon_name during certificate

 authentication

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

If only one certificate is available and the logon_name is the user is

not given the PAM responder already tried to find the name during the

pre-auth step. With multiple certificates this might cause useless extra

effort and the name should be determined after the certificate is

selected in the authentication step. This might currently only happen

with GDM because all other PAM clients will prompt for the user name

unconditionally.

New unit tests are added to cover this new case.

Related to https://pagure.io/SSSD/sssd/issue/3560

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

Tested-by: Scott Poore <spoore@redhat.com>

(cherry picked from commit fd6f4047b58686bd4057c9859c3c804a77b136d8)

---

 src/responder/pam/pamsrv_cmd.c  | 63 ++++++++++++++++++++++++++-----

 src/tests/cmocka/test_pam_srv.c | 82 +++++++++++++++++++++++++++++++++++++++++

 2 files changed, 135 insertions(+), 10 deletions(-)

diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c

index 8b2c086e206796ad4c977495be957c56b3255e7f..caf6c99489b8378d2e850473191223709920cd79 100644

--- a/src/responder/pam/pamsrv_cmd.c

+++ b/src/responder/pam/pamsrv_cmd.c

@@ -1151,6 +1151,7 @@ static errno_t pam_forwarder_parse_data(struct cli_ctx *cctx, struct pam_data *p

     size_t blen;

     errno_t ret;

     uint32_t terminator;

+    const char *key_id;

     prctx = talloc_get_type(cctx->protocol_ctx, struct cli_protocol);

@@ -1191,9 +1192,33 @@ static errno_t pam_forwarder_parse_data(struct cli_ctx *cctx, struct pam_data *p

                                          pd->logon_name,

                                          &pd->domain, &pd->user);

     } else {

-        /* Only SSS_PAM_PREAUTH request may have a missing name, e.g. if the

-         * name is determined with the help of a certificate */

-        if (pd->cmd == SSS_PAM_PREAUTH

+        /* SSS_PAM_PREAUTH request may have a missing name, e.g. if the

+         * name is determined with the help of a certificate. During

+         * SSS_PAM_AUTHENTICATE at least a key ID is needed to identify the

+         * selected certificate. */

+        if (pd->cmd == SSS_PAM_AUTHENTICATE

+                && may_do_cert_auth(talloc_get_type(cctx->rctx->pvt_ctx,

+                                                    struct pam_ctx), pd)

+                && (sss_authtok_get_type(pd->authtok) == SSS_AUTHTOK_TYPE_SC_PIN

+                    || sss_authtok_get_type(pd->authtok)

+                                               == SSS_AUTHTOK_TYPE_SC_KEYPAD)) {

+            ret = sss_authtok_get_sc(pd->authtok, NULL, NULL, NULL, NULL, NULL,

+                                     NULL, &key_id, NULL);

+            if (ret != EOK) {

+                DEBUG(SSSDBG_OP_FAILURE, "sss_authtok_get_sc failed.\n");

+                goto done;

+            }

+

+            if (key_id == NULL || *key_id == '\0') {

+                DEBUG(SSSDBG_CRIT_FAILURE,

+                      "Missing logon and Smartcard key ID during "

+                      "authentication.\n");

+                ret = ERR_NO_CREDS;

+                goto done;

+            }

+

+            ret = EOK;

+        } else if (pd->cmd == SSS_PAM_PREAUTH

                 && may_do_cert_auth(talloc_get_type(cctx->rctx->pvt_ctx,

                                                     struct pam_ctx), pd)) {

             ret = EOK;

@@ -1375,9 +1400,12 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)

     /* Determine what domain type to contact */

     preq->req_dom_type = get_domain_request_type(preq, pctx);

-    /* try backend first for authentication before doing local Smartcard

-     * authentication */

-    if (pd->cmd != SSS_PAM_AUTHENTICATE && may_do_cert_auth(pctx, pd)) {

+    /* Try backend first for authentication before doing local Smartcard

+     * authentication if a logon name is available. Otherwise try to derive

+     * the logon name from the certificate first. */

+    if ((pd->cmd != SSS_PAM_AUTHENTICATE

+                || (pd->cmd == SSS_PAM_AUTHENTICATE && pd->logon_name == NULL))

+            && may_do_cert_auth(pctx, pd)) {

         ret = check_cert(cctx, cctx->ev, pctx, preq, pd);

         /* Finish here */

         goto done;

@@ -1577,9 +1605,10 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req)

     } else {

         if (preq->pd->logon_name == NULL) {

-            if (preq->pd->cmd != SSS_PAM_PREAUTH) {

+            if (preq->pd->cmd != SSS_PAM_PREAUTH

+                    && preq->pd->cmd != SSS_PAM_AUTHENTICATE) {

                 DEBUG(SSSDBG_CRIT_FAILURE,

-                      "Missing logon name only allowed during pre-auth.\n");

+                      "Missing logon name only allowed during (pre-)auth.\n");

                 ret = ENOENT;

                 goto done;

             }

@@ -1641,7 +1670,8 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req)

                 }

             }

-            if (preq->cctx->rctx->domains->user_name_hint) {

+            if (preq->cctx->rctx->domains->user_name_hint

+                    && preq->pd->cmd == SSS_PAM_PREAUTH) {

                 ret = add_pam_cert_response(preq->pd, cert_user,

                                             preq->cert_list,

                                             SSS_PAM_CERT_INFO_WITH_HINT);

@@ -1664,6 +1694,20 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req)

                 goto done;

             }

+            /* If logon_name was not given during authentication add a

+             * SSS_PAM_CERT_INFO message to send the name to the caller. */

+            if (preq->pd->cmd == SSS_PAM_AUTHENTICATE

+                    && preq->pd->logon_name == NULL) {

+                ret = add_pam_cert_response(preq->pd, cert_user,

+                                            preq->cert_list,

+                                            SSS_PAM_CERT_INFO);

+                if (ret != EOK) {

+                    DEBUG(SSSDBG_OP_FAILURE, "add_pam_cert_response failed.\n");

+                    preq->pd->pam_status = PAM_AUTHINFO_UNAVAIL;

+                    goto done;

+                }

+            }

+

             /* cert_user will be returned to the PAM client as user name, so

              * we can use it here already e.g. to set in initgroups timeout */

             preq->pd->logon_name = talloc_strdup(preq->pd, cert_user);

@@ -2039,7 +2083,6 @@ static void pam_dom_forwarder(struct pam_auth_req *preq)

                                   "the backend.\n");

                         }

-                        /* FIXME: use the right cert info */

                         ret = add_pam_cert_response(preq->pd, cert_user,

                                                     preq->current_cert,

                                                     SSS_PAM_CERT_INFO);

diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c

index 50d3ed005468375ff02c60bebd1c61047ca1c6d4..b6845320ca41d6933280aa2836a3d984dacfcc5e 100644

--- a/src/tests/cmocka/test_pam_srv.c

+++ b/src/tests/cmocka/test_pam_srv.c

@@ -967,6 +967,16 @@ static int test_pam_cert_check(uint32_t status, uint8_t *body, size_t blen)

                                   NULL);

 }

+static int test_pam_cert_check_auth_success(uint32_t status, uint8_t *body,

+                                            size_t blen)

+{

+    assert_int_equal(pam_test_ctx->exp_pam_status, PAM_BAD_ITEM);

+    pam_test_ctx->exp_pam_status = PAM_SUCCESS;

+    return test_pam_cert_check_ex(status, body, blen,

+                                  SSS_PAM_CERT_INFO, "pamuser@"TEST_DOM_NAME,

+                                  NULL);

+}

+

 static int test_pam_cert_check_with_hint(uint32_t status, uint8_t *body,

                                          size_t blen)

 {

@@ -2265,6 +2275,74 @@ void test_pam_cert_auth(void **state)

     assert_int_equal(ret, EOK);

 }

+void test_pam_cert_auth_no_logon_name(void **state)

+{

+    int ret;

+

+    set_cert_auth_param(pam_test_ctx->pctx, NSS_DB);

+

+    /* Here the last option must be set to true because the backend is only

+     * connected once. During authentication the backend is connected first to

+     * see if it can handle Smartcard authentication, but before that the user

+     * is looked up. Since the first mocked reply already adds the certificate

+     * to the user entry the lookup by certificate will already find the user

+     * in the cache and no second request to the backend is needed. */

+    mock_input_pam_cert(pam_test_ctx, NULL, "123456", "SSSD Test Token",

+                        "NSS-Internal",

+                        "A5EF7DEE625CA5996C8D1BA7D036708161FD49E7", NULL,

+                        test_lookup_by_cert_cb, TEST_TOKEN_CERT, true);

+

+    mock_account_recv_simple();

+    mock_parse_inp("pamuser", NULL, EOK);

+

+    will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE);

+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);

+

+    /* Assume backend cannot handle Smartcard credentials */

+    pam_test_ctx->exp_pam_status = PAM_BAD_ITEM;

+

+    set_cmd_cb(test_pam_cert_check_auth_success);

+    ret = sss_cmd_execute(pam_test_ctx->cctx, SSS_PAM_AUTHENTICATE,

+                          pam_test_ctx->pam_cmds);

+    assert_int_equal(ret, EOK);

+

+    /* Wait until the test finishes with EOK */

+    ret = test_ev_loop(pam_test_ctx->tctx);

+    assert_int_equal(ret, EOK);

+}

+

+void test_pam_cert_auth_no_logon_name_no_key_id(void **state)

+{

+    int ret;

+

+    set_cert_auth_param(pam_test_ctx->pctx, NSS_DB);

+

+    /* Here the last option must be set to true because the backend is only

+     * connected once. During authentication the backend is connected first to

+     * see if it can handle Smartcard authentication, but before that the user

+     * is looked up. Since the first mocked reply already adds the certificate

+     * to the user entry the lookup by certificate will already find the user

+     * in the cache and no second request to the backend is needed. */

+    mock_input_pam_cert(pam_test_ctx, NULL, "123456", "SSSD Test Token",

+                        "NSS-Internal", NULL, NULL,

+                        NULL, NULL, false);

+

+    will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE);

+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);

+

+    /* Assume backend cannot handle Smartcard credentials */

+    pam_test_ctx->exp_pam_status = PAM_BAD_ITEM;

+

+    set_cmd_cb(test_pam_creds_insufficient_check);

+    ret = sss_cmd_execute(pam_test_ctx->cctx, SSS_PAM_AUTHENTICATE,

+                          pam_test_ctx->pam_cmds);

+    assert_int_equal(ret, EOK);

+

+    /* Wait until the test finishes with EOK */

+    ret = test_ev_loop(pam_test_ctx->tctx);

+    assert_int_equal(ret, EOK);

+}

+

 void test_pam_cert_auth_double_cert(void **state)

 {

     int ret;

@@ -2770,6 +2848,10 @@ int main(int argc, const char *argv[])

                                         pam_test_setup, pam_test_teardown),

         cmocka_unit_test_setup_teardown(test_pam_cert_preauth_2certs_two_mappings,

                                         pam_test_setup, pam_test_teardown),

+        cmocka_unit_test_setup_teardown(test_pam_cert_auth_no_logon_name,

+                                        pam_test_setup, pam_test_teardown),

+        cmocka_unit_test_setup_teardown(test_pam_cert_auth_no_logon_name_no_key_id,

+                                        pam_test_setup, pam_test_teardown),

 #endif /* HAVE_NSS */

         cmocka_unit_test_setup_teardown(test_filter_response,

-- 

2.13.6