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, ¶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