From 69c820abacd963a3699fc9ea84a17bb99f9eaf3a Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Mon, 6 Nov 2017 15:26:38 +0100 Subject: [PATCH 43/46] pam: filter certificates in the responder not in the child MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the new selection option and the handling of multiple certificates in the PAM responder it is not needed anymore to filter the certificates in p11_child but the matching rules can be applied by the PAM responder directly. Related to https://pagure.io/SSSD/sssd/issue/3560 Reviewed-by: Fabiano Fidêncio Tested-by: Scott Poore (cherry picked from commit 177ab84f0e336b75289a3ac0b2df25bd5ab5198b) --- src/p11_child/p11_child_nss.c | 18 +----- src/responder/pam/pamsrv.h | 6 ++ src/responder/pam/pamsrv_cmd.c | 10 ++- src/responder/pam/pamsrv_p11.c | 135 +++++++++++++++++++++++++++++++++++++++- src/tests/cmocka/test_pam_srv.c | 3 + 5 files changed, 152 insertions(+), 20 deletions(-) diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c index 5f289688e41f4ea610292b907036e05cf95eb29d..e59aba0d1561f58206252f7251ecd88315836b1b 100644 --- a/src/p11_child/p11_child_nss.c +++ b/src/p11_child/p11_child_nss.c @@ -264,22 +264,6 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, } } - rv = CERT_FilterCertListByUsage(cert_list, certUsageSSLClient, PR_FALSE); - if (rv != SECSuccess) { - DEBUG(SSSDBG_OP_FAILURE, "CERT_FilterCertListByUsage failed: [%d][%s].\n", - PR_GetError(), PORT_ErrorToString(PR_GetError())); - return EIO; - } - - rv = CERT_FilterCertListForUserCerts(cert_list); - if (rv != SECSuccess) { - DEBUG(SSSDBG_OP_FAILURE, - "CERT_FilterCertListForUserCerts failed: [%d][%s].\n", - PR_GetError(), PORT_ErrorToString(PR_GetError())); - return EIO; - } - - handle = CERT_GetDefaultCertDB(); if (handle == NULL) { DEBUG(SSSDBG_OP_FAILURE, "CERT_GetDefaultCertDB failed: [%d][%s].\n", @@ -344,7 +328,7 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, if (cert_verify_opts->do_verification) { rv = CERT_VerifyCertificateNow(handle, cert_list_node->cert, PR_TRUE, - certificateUsageSSLClient, + certificateUsageCheckAllUsages, NULL, NULL); if (rv != SECSuccess) { DEBUG(SSSDBG_OP_FAILURE, diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h index f15f7f19f1f38626288416c9f2038371c6f58b47..0bc229212844602ed461d1c7db48bf51ac2e2194 100644 --- a/src/responder/pam/pamsrv.h +++ b/src/responder/pam/pamsrv.h @@ -27,6 +27,7 @@ #include "sbus/sssd_dbus.h" #include "responder/common/responder.h" #include "responder/common/cache_req/cache_req.h" +#include "lib/certmap/sss_certmap.h" struct pam_auth_req; @@ -49,6 +50,7 @@ struct pam_ctx { bool cert_auth; int p11_child_debug_fd; char *nss_db; + struct sss_certmap_ctx *sss_certmap_ctx; }; struct pam_auth_dp_req { @@ -104,6 +106,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, const char *nss_db, time_t timeout, const char *verify_opts, + struct sss_certmap_ctx *sss_certmap_ctx, struct pam_data *pd); errno_t pam_check_cert_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, struct cert_auth_info **cert_list); @@ -114,6 +117,9 @@ errno_t add_pam_cert_response(struct pam_data *pd, const char *sysdb_username, bool may_do_cert_auth(struct pam_ctx *pctx, struct pam_data *pd); +errno_t p11_refresh_certmap_ctx(struct pam_ctx *pctx, + struct certmap_info **certmap_list); + errno_t pam_set_last_online_auth_with_curr_token(struct sss_domain_info *domain, const char *username, diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index caf6c99489b8378d2e850473191223709920cd79..0e76c9e772f1775635677f35b870e9613b2faf64 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1336,7 +1336,8 @@ static errno_t check_cert(TALLOC_CTX *mctx, req = pam_check_cert_send(mctx, ev, pctx->p11_child_debug_fd, pctx->nss_db, p11_child_timeout, - cert_verification_opts, pd); + cert_verification_opts, pctx->sss_certmap_ctx, + pd); if (req == NULL) { DEBUG(SSSDBG_OP_FAILURE, "pam_check_cert_send failed.\n"); return ENOMEM; @@ -1749,6 +1750,13 @@ static void pam_forwarder_cb(struct tevent_req *req) goto done; } + ret = p11_refresh_certmap_ctx(pctx, pctx->rctx->domains->certmaps); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "p11_refresh_certmap_ctx failed, " + "certificate matching might not work as expected"); + } + pd = preq->pd; ret = pam_forwarder_parse_data(cctx, pd); diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c index 5a3eeff0ec977829a9ad8c80b4fc6b2e06857097..ec52c5ae7163d41144fe082643a201b766a1e201 100644 --- a/src/responder/pam/pamsrv_p11.c +++ b/src/responder/pam/pamsrv_p11.c @@ -36,6 +36,7 @@ #define P11_CHILD_LOG_FILE "p11_child" #define P11_CHILD_PATH SSSD_LIBEXEC_PATH"/p11_child" +#define CERT_AUTH_DEFAULT_MATCHING_RULE "KRB5:clientAuth" struct cert_auth_info { char *cert; @@ -116,8 +117,110 @@ void sss_cai_check_users(struct cert_auth_info **list, size_t *_cert_count, return; } +struct priv_sss_debug { + int level; +}; + +static void ext_debug(void *private, const char *file, long line, + const char *function, const char *format, ...) +{ + va_list ap; + struct priv_sss_debug *data = private; + int level = SSSDBG_OP_FAILURE; + + if (data != NULL) { + level = data->level; + } + + if (DEBUG_IS_SET(level)) { + va_start(ap, format); + sss_vdebug_fn(file, line, function, level, APPEND_LINE_FEED, + format, ap); + va_end(ap); + } +} + +errno_t p11_refresh_certmap_ctx(struct pam_ctx *pctx, + struct certmap_info **certmap_list) +{ + int ret; + struct sss_certmap_ctx *sss_certmap_ctx = NULL; + size_t c; + + ret = sss_certmap_init(pctx, ext_debug, NULL, &sss_certmap_ctx); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sss_certmap_init failed.\n"); + goto done; + } + + if (certmap_list == NULL || *certmap_list == NULL) { + /* Try to add default matching rule */ + ret = sss_certmap_add_rule(sss_certmap_ctx, SSS_CERTMAP_MIN_PRIO, + CERT_AUTH_DEFAULT_MATCHING_RULE, NULL, NULL); + if (ret != 0) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to add default matching rule.\n"); + } + + goto done; + } + + for (c = 0; certmap_list[c] != NULL; c++) { + DEBUG(SSSDBG_TRACE_ALL, + "Trying to add rule [%s][%d][%s][%s].\n", + certmap_list[c]->name, certmap_list[c]->priority, + certmap_list[c]->match_rule, certmap_list[c]->map_rule); + + ret = sss_certmap_add_rule(sss_certmap_ctx, certmap_list[c]->priority, + certmap_list[c]->match_rule, + certmap_list[c]->map_rule, + certmap_list[c]->domains); + if (ret != 0) { + DEBUG(SSSDBG_CRIT_FAILURE, + "sss_certmap_add_rule failed for rule [%s] " + "with error [%d][%s], skipping. " + "Please check for typos and if rule syntax is supported.\n", + certmap_list[c]->name, ret, sss_strerror(ret)); + continue; + } + } + + ret = EOK; + +done: + if (ret == EOK) { + sss_certmap_free_ctx(pctx->sss_certmap_ctx); + pctx->sss_certmap_ctx = sss_certmap_ctx; + } else { + sss_certmap_free_ctx(sss_certmap_ctx); + } + + return ret; +} + errno_t p11_child_init(struct pam_ctx *pctx) { + int ret; + struct certmap_info **certmaps; + bool user_name_hint; + struct sss_domain_info *dom = pctx->rctx->domains; + + ret = sysdb_get_certmap(dom, dom->sysdb, &certmaps, &user_name_hint); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sysdb_get_certmap failed.\n"); + return ret; + } + + dom->user_name_hint = user_name_hint; + talloc_free(dom->certmaps); + dom->certmaps = certmaps; + + ret = p11_refresh_certmap_ctx(pctx, dom->certmaps); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "p11_refresh_certmap_ctx failed.\n"); + return ret; + } + return child_debug_init(P11_CHILD_LOG_FILE, &pctx->p11_child_debug_fd); } @@ -214,6 +317,7 @@ 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, + struct sss_certmap_ctx *sss_certmap_ctx, struct cert_auth_info **_cert_list) { int ret; @@ -222,6 +326,8 @@ static errno_t parse_p11_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, uint8_t *pn; struct cert_auth_info *cert_list = NULL; struct cert_auth_info *cert_auth_info; + unsigned char *der = NULL; + size_t der_size; if (buf_len < 0) { DEBUG(SSSDBG_CRIT_FAILURE, @@ -347,7 +453,22 @@ static errno_t parse_p11_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, } DEBUG(SSSDBG_TRACE_ALL, "Found cert [%s].\n", cert_auth_info->cert); - DLIST_ADD(cert_list, cert_auth_info); + der = sss_base64_decode(tmp_ctx, cert_auth_info->cert, &der_size); + if (der == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "sss_base64_decode failed.\n"); + ret = EIO; + goto done; + } + + ret = sss_certmap_match_cert(sss_certmap_ctx, der, der_size); + if (ret == 0) { + DLIST_ADD(cert_list, cert_auth_info); + } else { + DEBUG(SSSDBG_TRACE_LIBS, + "Cert [%s] does not match matching rules and is ignored.\n", + cert_auth_info->cert); + talloc_free(cert_auth_info); + } p = ++pn; } while ((pn - buf) < buf_len); @@ -373,6 +494,7 @@ struct pam_check_cert_state { struct sss_child_ctx_old *child_ctx; struct tevent_timer *timeout_handler; struct tevent_context *ev; + struct sss_certmap_ctx *sss_certmap_ctx; struct child_io_fds *io; @@ -391,6 +513,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, const char *nss_db, time_t timeout, const char *verify_opts, + struct sss_certmap_ctx *sss_certmap_ctx, struct pam_data *pd) { errno_t ret; @@ -420,6 +543,12 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, goto done; } + if (sss_certmap_ctx == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Missing certificate matching context.\n"); + ret = EINVAL; + goto done; + } + /* extra_args are added in revers order */ arg_c = 0; extra_args[arg_c++] = nss_db; @@ -476,6 +605,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, } state->ev = ev; + state->sss_certmap_ctx = sss_certmap_ctx; state->child_status = EFAULT; state->io = talloc(state, struct child_io_fds); if (state->io == NULL) { @@ -639,7 +769,8 @@ 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_list); + ret = parse_p11_child_response(state, buf, buf_len, state->sss_certmap_ctx, + &state->cert_list); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "parse_p11_child_response failed.\n"); tevent_req_error(req, ret); diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c index b6845320ca41d6933280aa2836a3d984dacfcc5e..bccf9972dacbb414076904a783772198620fd73c 100644 --- a/src/tests/cmocka/test_pam_srv.c +++ b/src/tests/cmocka/test_pam_srv.c @@ -287,6 +287,9 @@ struct pam_ctx *mock_pctx(TALLOC_CTX *mem_ctx) return NULL; } + ret = p11_refresh_certmap_ctx(pctx, NULL); + assert_int_equal(ret, 0); + return pctx; } -- 2.13.6