From 426dd731fe7c804c4b9d96c62d1a60a9022dcb09 Mon Sep 17 00:00:00 2001

From: Sumit Bose <sbose@redhat.com>

Date: Tue, 17 Jan 2017 15:55:18 +0100

Subject: [PATCH 36/46] p11_child: return multiple certs

MIME-Version: 1.0

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

Content-Transfer-Encoding: 8bit

This patch refactors the handling of certificates in p11_child. Not only

the first but all certificates suitable for authentication are returned.

The PAM responder component calling p11_child is refactored to handle

multiple certificate returned by p11_child but so far only returns the

first one to its callers.

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 39fd336e4390ece3a8465714735ef4203f329e54)

---

 src/p11_child/p11_child_nss.c  | 131 +++++++++++++---------

 src/responder/pam/pamsrv.h     |   2 +

 src/responder/pam/pamsrv_p11.c | 242 +++++++++++++++++++++++------------------

 3 files changed, 219 insertions(+), 156 deletions(-)

diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c

index b0ec69be321c4b4186ce851c07bfcc3e1afe9694..50bde2f4f91f6c00260b0db383d0962112686ebc 100644

--- a/src/p11_child/p11_child_nss.c

+++ b/src/p11_child/p11_child_nss.c

@@ -72,8 +72,7 @@ static char *password_passthrough(PK11SlotInfo *slot, PRBool retry, void *arg)

 int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in,

             enum op_mode mode, const char *pin,

             struct cert_verify_opts *cert_verify_opts,

-            char **cert, char **token_name_out, char **module_name_out,

-            char **key_id_out)

+            char **_multi)

 {

     int ret;

     SECStatus rv;

@@ -110,7 +109,10 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in,

     PK11SlotListElement *le;

     SECItem *key_id = NULL;

     char *key_id_str = NULL;

-

+    CERTCertList *valid_certs = NULL;

+    char *cert_b64 = NULL;

+    char *multi = NULL;

+    PRCList *node;

     nss_ctx = NSS_InitContext(nss_db, "", "", SECMOD_DB, &parameters, flags);

     if (nss_ctx == NULL) {

@@ -303,6 +305,14 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in,

     }

     found_cert = NULL;

+    valid_certs = CERT_NewCertList();

+    if (valid_certs == NULL) {

+        DEBUG(SSSDBG_OP_FAILURE, "CERT_NewCertList failed [%d].\n",

+                                 PR_GetError());

+        ret = ENOMEM;

+        goto done;

+    }

+

     DEBUG(SSSDBG_TRACE_ALL, "Filtered certificates:\n");

     for (cert_list_node = CERT_LIST_HEAD(cert_list);

                 !CERT_LIST_END(cert_list_node, cert_list);

@@ -326,6 +336,13 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in,

                 }

             }

+            rv = CERT_AddCertToListTail(valid_certs, cert_list_node->cert);

+            if (rv != SECSuccess) {

+                DEBUG(SSSDBG_OP_FAILURE,

+                      "CERT_AddCertToListTail failed [%d].\n", PR_GetError());

+                ret = EIO;

+                goto done;

+            }

             if (found_cert == NULL) {

                 found_cert = cert_list_node->cert;

@@ -352,9 +369,7 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in,

     if (found_cert == NULL) {

         DEBUG(SSSDBG_TRACE_ALL, "No certificate found.\n");

-        *cert = NULL;

-        *token_name_out = NULL;

-        *module_name_out = NULL;

+        *_multi = NULL;

         ret = EOK;

         goto done;

     }

@@ -421,51 +436,55 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in,

               "Certificate verified and validated.\n");

     }

-    key_id = PK11_GetLowLevelKeyIDForCert(slot, found_cert, NULL);

-    if (key_id == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "PK11_GetLowLevelKeyIDForCert failed [%d].\n",

-                                 PR_GetError());

-        ret = EINVAL;

-        goto done;

-    }

-

-    key_id_str = CERT_Hexify(key_id, PR_FALSE);

-    if (key_id_str == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "CERT_Hexify failed [%d].\n", PR_GetError());

+    multi = talloc_strdup(mem_ctx, "");

+    if (multi == NULL) {

+        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to create output string.\n");

         ret = ENOMEM;

         goto done;

     }

-    DEBUG(SSSDBG_TRACE_ALL, "Found certificate has key id [%s].\n", key_id_str);

+    for (cert_list_node = CERT_LIST_HEAD(valid_certs);

+                !CERT_LIST_END(cert_list_node, valid_certs);

+                cert_list_node = CERT_LIST_NEXT(cert_list_node)) {

-    *key_id_out = talloc_strdup(mem_ctx, key_id_str);

-    if (*key_id_out == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to copy key id.\n");

-        ret = ENOMEM;

-        goto done;

-    }

+        found_cert = cert_list_node->cert;

-    *cert = sss_base64_encode(mem_ctx, found_cert->derCert.data,

-                                       found_cert->derCert.len);

-    if (*cert == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "sss_base64_encode failed.\n");

-        ret = ENOMEM;

-        goto done;

-    }

+        SECITEM_FreeItem(key_id, PR_TRUE);

+        PORT_Free(key_id_str);

+        key_id = PK11_GetLowLevelKeyIDForCert(slot, found_cert, NULL);

+        if (key_id == NULL) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "PK11_GetLowLevelKeyIDForCert failed [%d].\n",

+                  PR_GetError());

+            ret = EINVAL;

+            goto done;

+        }

-    *token_name_out = talloc_strdup(mem_ctx, token_name);

-    if (*token_name_out == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to copy slot name.\n");

-        ret = ENOMEM;

-        goto done;

-    }

+        key_id_str = CERT_Hexify(key_id, PR_FALSE);

+        if (key_id_str == NULL) {

+            DEBUG(SSSDBG_OP_FAILURE, "CERT_Hexify failed [%d].\n",

+                  PR_GetError());

+            ret = ENOMEM;

+            goto done;

+        }

-    *module_name_out = talloc_strdup(mem_ctx, module_name);

-    if (*module_name_out == NULL) {

-        DEBUG(SSSDBG_CRIT_FAILURE, "Failed to copy module_name_out name.\n");

-        ret = ENOMEM;

-        goto done;

+        talloc_free(cert_b64);

+        cert_b64 = sss_base64_encode(mem_ctx, found_cert->derCert.data,

+                                     found_cert->derCert.len);

+        if (cert_b64 == NULL) {

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

+            ret = ENOMEM;

+            goto done;

+        }

+

+        DEBUG(SSSDBG_TRACE_ALL, "Found certificate has key id [%s].\n",

+              key_id_str);

+

+        multi = talloc_asprintf_append(multi, "%s\n%s\n%s\n%s\n",

+                                       token_name, module_name, key_id_str,

+                                       cert_b64);

     }

+    *_multi = multi;

     ret = EOK;

@@ -474,6 +493,18 @@ done:

         PK11_FreeSlot(slot);

     }

+    if (valid_certs != NULL) {

+        /* The certificates can be found in valid_certs and cert_list and

+         * CERT_DestroyCertList() will free the certificates as well. To avoid

+         * a double free the nodes from valid_certs are removed first because

+         * valid_certs will only have a sub-set of the certificates. */

+        while (!PR_CLIST_IS_EMPTY(&valid_certs->list)) {

+            node = PR_LIST_HEAD(&valid_certs->list);

+            PR_REMOVE_LINK(node);

+        }

+        CERT_DestroyCertList(valid_certs);

+    }

+

     if (cert_list != NULL) {

         CERT_DestroyCertList(cert_list);

     }

@@ -483,6 +514,8 @@ done:

     PORT_Free(signed_random_value.data);

+    talloc_free(cert_b64);

+

     rv = NSS_ShutdownContext(nss_ctx);

     if (rv != SECSuccess) {

         DEBUG(SSSDBG_OP_FAILURE, "NSS_ShutdownContext failed [%d].\n",

@@ -540,17 +573,14 @@ int main(int argc, const char *argv[])

     const char *opt_logger = NULL;

     errno_t ret;

     TALLOC_CTX *main_ctx = NULL;

-    char *cert;

     enum op_mode mode = OP_NONE;

     enum pin_mode pin_mode = PIN_NONE;

     char *pin = NULL;

     char *slot_name_in = NULL;

-    char *token_name_out = NULL;

-    char *module_name_out = NULL;

-    char *key_id_out = NULL;

     char *nss_db = NULL;

     struct cert_verify_opts *cert_verify_opts;

     char *verify_opts = NULL;

+    char *multi = NULL;

     struct poptOption long_options[] = {

         POPT_AUTOHELP

@@ -715,17 +745,14 @@ int main(int argc, const char *argv[])

     }

     ret = do_work(main_ctx, nss_db, slot_name_in, mode, pin, cert_verify_opts,

-                  &cert, &token_name_out, &module_name_out, &key_id_out);

+                  &multi);

     if (ret != EOK) {

         DEBUG(SSSDBG_OP_FAILURE, "do_work failed.\n");

         goto fail;

     }

-    if (cert != NULL) {

-        fprintf(stdout, "%s\n", token_name_out);

-        fprintf(stdout, "%s\n", module_name_out);

-        fprintf(stdout, "%s\n", key_id_out);

-        fprintf(stdout, "%s\n", cert);

+    if (multi != NULL) {

+        fprintf(stdout, "%s", multi);

     }

     talloc_free(main_ctx);

diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h

index 57a37b72594f030995f5e22255eb7a8fcd63d10e..896f71befbc9947a53b5eb20cba0bb3d104c4cf2 100644

--- a/src/responder/pam/pamsrv.h

+++ b/src/responder/pam/pamsrv.h

@@ -88,6 +88,8 @@ int LOCAL_pam_handler(struct pam_auth_req *preq);

 errno_t p11_child_init(struct pam_ctx *pctx);

+struct cert_auth_info;

+

 struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx,

                                        struct tevent_context *ev,

                                        int child_debug_fd,

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

index 4dce43800c3c6b026c545df35c846269cbb49610..ff32d1e726808caa36ca7cca557220866ef1a9ab 100644

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

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

@@ -35,6 +35,15 @@

 #define P11_CHILD_LOG_FILE "p11_child"

 #define P11_CHILD_PATH SSSD_LIBEXEC_PATH"/p11_child"

+struct cert_auth_info {

+    char *cert;

+    char *token_name;

+    char *module_name;

+    char *key_id;

+    struct cert_auth_info *prev;

+    struct cert_auth_info *next;

+};

+

 errno_t p11_child_init(struct pam_ctx *pctx)

 {

     return child_debug_init(P11_CHILD_LOG_FILE, &pctx->p11_child_debug_fd);

@@ -132,18 +141,15 @@ static errno_t get_p11_child_write_buffer(TALLOC_CTX *mem_ctx,

 }

 static errno_t parse_p11_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf,

-                                        ssize_t buf_len, char **_cert,

-                                        char **_token_name, char **_module_name,

-                                        char **_key_id)

+                                        ssize_t buf_len,

+                                        struct cert_auth_info **_cert_list)

 {

     int ret;

     TALLOC_CTX *tmp_ctx = NULL;

     uint8_t *p;

     uint8_t *pn;

-    char *cert = NULL;

-    char *token_name = NULL;

-    char *module_name = NULL;

-    char *key_id = NULL;

+    struct cert_auth_info *cert_list = NULL;

+    struct cert_auth_info *cert_auth_info;

     if (buf_len < 0) {

         DEBUG(SSSDBG_CRIT_FAILURE,

@@ -157,108 +163,132 @@ static errno_t parse_p11_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf,

         goto done;

     }

-    p = memchr(buf, '\n', buf_len);

-    if (p == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "Missing new-line in p11_child response.\n");

-        return EINVAL;

-    }

-    if (p == buf) {

-        DEBUG(SSSDBG_OP_FAILURE, "Missing counter in p11_child response.\n");

-        return EINVAL;

-    }

-

     tmp_ctx = talloc_new(NULL);

     if (tmp_ctx == NULL) {

         DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");

         return ENOMEM;

     }

-    token_name = talloc_strndup(tmp_ctx, (char*) buf, (p - buf));

-    if (token_name == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strndup failed.\n");

-        ret = ENOMEM;

-        goto done;

-    }

+    p = buf;

-    p++;

-    pn = memchr(p, '\n', buf_len - (p - buf));

-    if (pn == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE,

-              "Missing new-line in p11_child response.\n");

-        ret = EINVAL;

-        goto done;

-    }

+    do {

+        cert_auth_info = talloc_zero(tmp_ctx, struct cert_auth_info);

+        if (cert_auth_info == NULL) {

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

+            return ENOMEM;

+        }

-    if (pn == p) {

-        DEBUG(SSSDBG_OP_FAILURE,

-              "Missing module name in p11_child response.\n");

-        ret = EINVAL;

-        goto done;

-    }

+        pn = memchr(p, '\n', buf_len - (p - buf));

+        if (pn == NULL) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "Missing new-line in p11_child response.\n");

+            return EINVAL;

+        }

+        if (pn == p) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "Missing counter in p11_child response.\n");

+            return EINVAL;

+        }

-    module_name = talloc_strndup(tmp_ctx, (char *) p, (pn - p));

-    if (module_name == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strndup failed.\n");

-        ret = ENOMEM;

-        goto done;

-    }

-    DEBUG(SSSDBG_TRACE_ALL, "Found module name [%s].\n", module_name);

+        cert_auth_info->token_name = talloc_strndup(cert_auth_info, (char *)p,

+                                                    (pn - p));

+        if (cert_auth_info->token_name == NULL) {

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

+            ret = ENOMEM;

+            goto done;

+        }

+        DEBUG(SSSDBG_TRACE_ALL, "Found token name [%s].\n",

+              cert_auth_info->token_name);

-    p = ++pn;

-    pn = memchr(p, '\n', buf_len - (p - buf));

-    if (pn == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE,

-              "Missing new-line in p11_child response.\n");

-        ret = EINVAL;

-        goto done;

-    }

+        p = ++pn;

+        pn = memchr(p, '\n', buf_len - (p - buf));

+        if (pn == NULL) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "Missing new-line in p11_child response.\n");

+            ret = EINVAL;

+            goto done;

+        }

-    if (pn == p) {

-        DEBUG(SSSDBG_OP_FAILURE,

-              "Missing key id in p11_child response.\n");

-        ret = EINVAL;

-        goto done;

-    }

+        if (pn == p) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "Missing module name in p11_child response.\n");

+            ret = EINVAL;

+            goto done;

+        }

-    key_id = talloc_strndup(tmp_ctx, (char *) p, (pn - p));

-    if (key_id == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strndup failed.\n");

-        ret = ENOMEM;

-        goto done;

-    }

-    DEBUG(SSSDBG_TRACE_ALL, "Found key id [%s].\n", key_id);

+        cert_auth_info->module_name = talloc_strndup(cert_auth_info, (char *)p,

+                                                     (pn - p));

+        if (cert_auth_info->module_name == NULL) {

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

+            ret = ENOMEM;

+            goto done;

+        }

+        DEBUG(SSSDBG_TRACE_ALL, "Found module name [%s].\n",

+              cert_auth_info->module_name);

-    p = pn + 1;

-    pn = memchr(p, '\n', buf_len - (p - buf));

-    if (pn == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE,

-              "Missing new-line in p11_child response.\n");

-        ret = EINVAL;

-        goto done;

-    }

+        p = ++pn;

+        pn = memchr(p, '\n', buf_len - (p - buf));

+        if (pn == NULL) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "Missing new-line in p11_child response.\n");

+            ret = EINVAL;

+            goto done;

+        }

-    if (pn == p) {

-        DEBUG(SSSDBG_OP_FAILURE, "Missing cert in p11_child response.\n");

-        ret = EINVAL;

-        goto done;

-    }

+        if (pn == p) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "Missing key id in p11_child response.\n");

+            ret = EINVAL;

+            goto done;

+        }

-    cert = talloc_strndup(tmp_ctx, (char *) p, (pn - p));

-    if(cert == NULL) {

-        DEBUG(SSSDBG_OP_FAILURE, "talloc_strndup failed.\n");

-        ret = ENOMEM;

-        goto done;

-    }

-    DEBUG(SSSDBG_TRACE_ALL, "Found cert [%s].\n", cert);

+        cert_auth_info->key_id = talloc_strndup(cert_auth_info, (char *)p,

+                                                (pn - p));

+        if (cert_auth_info->key_id == NULL) {

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

+            ret = ENOMEM;

+            goto done;

+        }

+        DEBUG(SSSDBG_TRACE_ALL, "Found key id [%s].\n", cert_auth_info->key_id);

+

+        p = ++pn;

+        pn = memchr(p, '\n', buf_len - (p - buf));

+        if (pn == NULL) {

+            DEBUG(SSSDBG_OP_FAILURE,

+                  "Missing new-line in p11_child response.\n");

+            ret = EINVAL;

+            goto done;

+        }

+

+        if (pn == p) {

+            DEBUG(SSSDBG_OP_FAILURE, "Missing cert in p11_child response.\n");

+            ret = EINVAL;

+            goto done;

+        }

+

+        cert_auth_info->cert = talloc_strndup(cert_auth_info, (char *)p,

+                                              (pn - p));

+        if (cert_auth_info->cert == NULL) {

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

+            ret = ENOMEM;

+            goto done;

+        }

+        DEBUG(SSSDBG_TRACE_ALL, "Found cert [%s].\n", cert_auth_info->cert);

+

+        DLIST_ADD(cert_list, cert_auth_info);

+

+        p = ++pn;

+    } while ((pn - buf) < buf_len);

     ret = EOK;

 done:

     if (ret == EOK) {

-        *_token_name = talloc_steal(mem_ctx, token_name);

-        *_cert = talloc_steal(mem_ctx, cert);

-        *_module_name = talloc_steal(mem_ctx, module_name);

-        *_key_id = talloc_steal(mem_ctx, key_id);

+        DLIST_FOR_EACH(cert_auth_info, cert_list) {

+            talloc_steal(mem_ctx, cert_auth_info);

+        }

+

+        *_cert_list = cert_list;

     }

     talloc_free(tmp_ctx);

@@ -273,10 +303,8 @@ struct pam_check_cert_state {

     struct tevent_context *ev;

     struct child_io_fds *io;

-    char *cert;

-    char *token_name;

-    char *module_name;

-    char *key_id;

+

+    struct cert_auth_info *cert_list;

 };

 static void p11_child_write_done(struct tevent_req *subreq);

@@ -349,9 +377,6 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx,

     state->ev = ev;

     state->child_status = EFAULT;

-    state->cert = NULL;

-    state->token_name = NULL;

-    state->module_name = NULL;

     state->io = talloc(state, struct child_io_fds);

     if (state->io == NULL) {

         DEBUG(SSSDBG_CRIT_FAILURE, "talloc failed.\n");

@@ -514,11 +539,9 @@ static void p11_child_done(struct tevent_req *subreq)

     PIPE_FD_CLOSE(state->io->read_from_child_fd);

-    ret = parse_p11_child_response(state, buf, buf_len, &state->cert,

-                                   &state->token_name, &state->module_name,

-                                   &state->key_id);

+    ret = parse_p11_child_response(state, buf, buf_len, &state->cert_list);

     if (ret != EOK) {

-        DEBUG(SSSDBG_OP_FAILURE, "parse_p11_child_respose failed.\n");

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

         tevent_req_error(req, ret);

         return;

     }

@@ -551,20 +574,31 @@ errno_t pam_check_cert_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,

     TEVENT_REQ_RETURN_ON_ERROR(req);

+    if (state->cert_list == NULL) {

+        *token_name = NULL;

+        *cert = NULL;

+        *module_name = NULL;

+        *key_id = NULL;

+    }

+

     if (cert != NULL) {

-        *cert = talloc_steal(mem_ctx, state->cert);

+        *cert = (state->cert_list == NULL) ? NULL

+                                : talloc_steal(mem_ctx, state->cert_list->cert);

     }

     if (token_name != NULL) {

-        *token_name = talloc_steal(mem_ctx, state->token_name);

+        *token_name = (state->cert_list == NULL) ? NULL

+                          : talloc_steal(mem_ctx, state->cert_list->token_name);

     }

     if (module_name != NULL) {

-        *module_name = talloc_steal(mem_ctx, state->module_name);

+        *module_name = (state->cert_list == NULL) ? NULL

+                         : talloc_steal(mem_ctx, state->cert_list->module_name);

     }

     if (key_id != NULL) {

-        *key_id = talloc_steal(mem_ctx, state->key_id);

+        *key_id = (state->cert_list == NULL) ? NULL

+                              : talloc_steal(mem_ctx, state->cert_list->key_id);

     }

     return EOK;

-- 

2.13.6