From 426dd731fe7c804c4b9d96c62d1a60a9022dcb09 Mon Sep 17 00:00:00 2001 From: Sumit Bose 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 Tested-by: Scott Poore (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, ¶meters, 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