PAM: only allow missing user name for certificate authentication

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

Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>

2 files changed, 47 insertions, 3 deletions

diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index 27dddcf43..2823f8133 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -957,11 +957,13 @@ static errno_t pam_forwarder_parse_data(struct cli_ctx *cctx, struct pam_data *p
     } 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) {
+        if (pd->cmd == SSS_PAM_PREAUTH
+                && may_do_cert_auth(talloc_get_type(cctx->rctx->pvt_ctx,
+                                                    struct pam_ctx), pd)) {
             ret = EOK;
         } else {
             DEBUG(SSSDBG_CRIT_FAILURE, "Missing logon name in PAM request.\n");
-            ret = EINVAL;
+            ret = ERR_NO_CREDS;
             goto done;
         }
     }
@@ -1104,7 +1106,6 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
         }
         goto done;
     } else if (ret != EOK) {
-        ret = EINVAL;
         goto done;
     }
 
@@ -1610,6 +1611,11 @@ static int pam_check_user_done(struct pam_auth_req *preq, int ret)
         pam_reply(preq);
         break;
 
+    case ERR_NO_CREDS:
+        preq->pd->pam_status = PAM_CRED_INSUFFICIENT;
+        pam_reply(preq);
+        break;
+
     default:
         preq->pd->pam_status = PAM_SYSTEM_ERR;
         pam_reply(preq);
diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c
index ab33433fd..dbdc4ae08 100644
--- a/src/tests/cmocka/test_pam_srv.c
+++ b/src/tests/cmocka/test_pam_srv.c
@@ -623,6 +623,23 @@ static int test_pam_wrong_pw_offline_auth_check(uint32_t status,
     return test_pam_simple_check(status, body, blen);
 }
 
+static int test_pam_creds_insufficient_check(uint32_t status,
+                                             uint8_t *body, size_t blen)
+{
+    size_t rp = 0;
+    uint32_t val;
+
+    assert_int_equal(status, 0);
+
+    SAFEALIGN_COPY_UINT32(&val, body + rp, &rp);
+    assert_int_equal(val, PAM_CRED_INSUFFICIENT);
+
+    SAFEALIGN_COPY_UINT32(&val, body + rp, &rp);
+    assert_int_equal(val, 0);
+
+    return EOK;
+}
+
 static int test_pam_user_unknown_check(uint32_t status,
                                        uint8_t *body, size_t blen)
 {
@@ -1127,6 +1144,25 @@ void test_pam_offline_chauthtok(void **state)
     assert_int_equal(ret, EOK);
 }
 
+void test_pam_preauth_no_logon_name(void **state)
+{
+    int ret;
+
+    mock_input_pam_cert(pam_test_ctx, NULL, NULL);
+
+    will_return(__wrap_sss_packet_get_cmd, SSS_PAM_PREAUTH);
+    will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+
+    set_cmd_cb(test_pam_creds_insufficient_check);
+    ret = sss_cmd_execute(pam_test_ctx->cctx, SSS_PAM_PREAUTH,
+                          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);
+}
+
 static void set_cert_auth_param(struct pam_ctx *pctx, const char *dbpath)
 {
     pam_test_ctx->pctx->cert_auth = true;
@@ -1432,6 +1468,8 @@ int main(int argc, const char *argv[])
                                         pam_test_setup, pam_test_teardown),
         cmocka_unit_test_setup_teardown(test_pam_offline_chauthtok,
                                         pam_test_setup, pam_test_teardown),
+        cmocka_unit_test_setup_teardown(test_pam_preauth_no_logon_name,
+                                        pam_test_setup, pam_test_teardown),
 /* p11_child is not built without NSS */
 #ifdef HAVE_NSS
         cmocka_unit_test_setup_teardown(test_pam_preauth_cert_nocert,