From 64701f85a3c76ce420b974b6fd94186007e53257 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Thu, 5 Nov 2015 18:20:27 +0100 Subject: [PATCH] p11: enable ocsp checks This patch enables the Online Certificate Status Protocol in NSS and adds an option to disable it if needed. To make further tuning of certificate verification more easy it is not an option on its own but an option to the new certificate_verification configuration option. Resolves https://fedorahosted.org/sssd/ticket/2812 --- Makefile.am | 5 ++-- src/confdb/confdb.h | 1 + src/config/SSSDConfig/__init__.py.in | 1 + src/config/SSSDConfigTest.py | 3 ++- src/config/etc/sssd.api.conf | 1 + src/man/sssd.conf.5.xml | 28 ++++++++++++++++++++++ src/p11_child/p11_child_nss.c | 25 ++++++++++++++++++-- src/responder/pam/pamsrv.h | 1 + src/responder/pam/pamsrv_cmd.c | 14 ++++++++++- src/responder/pam/pamsrv_p11.c | 21 ++++++++++------ src/responder/ssh/sshsrv_cmd.c | 15 +++++++++++- src/tests/cmocka/test_cert_utils.c | 2 +- src/tests/cmocka/test_pam_srv.c | 34 ++++++++++++++++++++------ src/util/cert.h | 3 +++ src/util/cert/cert_common.c | 46 ++++++++++++++++++++++++++++++++++++ src/util/cert/libcrypto/cert.c | 1 + src/util/cert/nss/cert.c | 20 ++++++++++++++++ 17 files changed, 199 insertions(+), 22 deletions(-) diff --git a/Makefile.am b/Makefile.am index a0abb8fb3a9cc19cfabad8066c6c65d78e78d206..ee97d095e07f6d485232a1afd87f9e4057688f22 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2579,8 +2579,7 @@ test_cert_utils_LDADD = \ $(CRYPTO_LIBS) \ libsss_debug.la \ libsss_test_common.la \ - libsss_cert.la \ - libsss_crypt.la \ + libsss_util.la \ $(NULL) test_data_provider_be_SOURCES = \ @@ -3156,6 +3155,7 @@ proxy_child_LDADD = \ p11_child_SOURCES = \ src/p11_child/p11_child_nss.c \ src/util/atomic_io.c \ + src/util/util.c \ $(NULL) p11_child_CFLAGS = \ $(AM_CFLAGS) \ @@ -3168,6 +3168,7 @@ p11_child_LDADD = \ $(POPT_LIBS) \ $(NSS_LIBS) \ libsss_crypt.la \ + libsss_cert.la \ $(NULL) memberof_la_SOURCES = \ diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 37b5fd7c7629e2618a1699e3ffd58110171db605..c6a5e3f61d8bfd045eb2699d0f5e279cb7d89f86 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -71,6 +71,7 @@ #define CONFDB_MONITOR_DEFAULT_DOMAIN "default_domain_suffix" #define CONFDB_MONITOR_OVERRIDE_SPACE "override_space" #define CONFDB_MONITOR_USER_RUNAS "user" +#define CONFDB_MONITOR_CERT_VERIFICATION "certificate_verification" /* Both monitor and domains */ #define CONFDB_NAME_REGEX "re_expression" diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index bf61c402796122050fa43cf41128faec4771c5d2..2cb857013fe4bddfd2e79e589d3ba9721dc3ca4f 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -60,6 +60,7 @@ option_strings = { 'krb5_rcache_dir' : _('Directory on the filesystem where SSSD should store Kerberos replay cache files.'), 'default_domain_suffix' : _('Domain to add to names without a domain component.'), 'user' : _('The user to drop privileges to'), + 'certificate_verification' : _('Tune certificate verification'), # [nss] 'enum_cache_timeout' : _('Enumeration cache timeout length (seconds)'), diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index 45562214da5d227b45914abbcb298e043048adf5..8277d1ee4198f342b5cff50f8d44c973fce4a29b 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -307,7 +307,8 @@ class SSSDConfigTestSSSDService(unittest.TestCase): 'reconnection_retries', 'fd_limit', 'client_idle_timeout', - 'description'] + 'description', + 'certificate_verification'] self.assertTrue(type(options) == dict, "Options should be a dictionary") diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index 72abb8b3f427e9bcf3cf3dcaa67aaf4e3e50226c..b6a396a75e564355d0828fa24858337eb06ff4bf 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -25,6 +25,7 @@ full_name_format = str, None, false krb5_rcache_dir = str, None, false user = str, None, false default_domain_suffix = str, None, false +certificate_verification = str, None, false [nss] # Name service diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 573f421a7d885d28d5dbc03423e6c6dd84d7b8fd..05dcb4555e02da9e94c76baee65f1d7168f8026b 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -365,6 +365,34 @@ + + certificate_verification (string) + + + With this parameter the certificate verification + can be tuned with a comma separated list of + options. Supported options are: + + + no_ocsp + + Disables Online Certificate Status + Protocol (OCSP) checks. This might be + needed if the OCSP servers defined in + the certificate are not reachable from + the client. + + + + + + Unknown options are reported but ignored. + + + Default: not set + + + diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c index 22e449d547c64c94a3ee9e29939377c98443b47d..de5494d03fbec2f66a882521638e94942e964f90 100644 --- a/src/p11_child/p11_child_nss.c +++ b/src/p11_child/p11_child_nss.c @@ -38,10 +38,12 @@ #include #include #include +#include #include "util/child_common.h" #include "providers/dp_backend.h" #include "util/crypto/sss_crypto.h" +#include "util/cert.h" enum op_mode { OP_NONE, @@ -68,7 +70,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, char **cert, + enum op_mode mode, const char *pin, bool do_ocsp, char **cert, char **token_name_out) { int ret; @@ -261,6 +263,14 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in, return EIO; } + if (do_ocsp) { + rv = CERT_EnableOCSPChecking(handle); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, "CERT_EnableOCSPChecking failed: [%d].\n", + PR_GetError()); + return EIO; + } + } found_cert = NULL; DEBUG(SSSDBG_TRACE_ALL, "Filtered certificates:\n"); @@ -456,6 +466,8 @@ int main(int argc, const char *argv[]) char *slot_name_in = NULL; char *token_name_out = NULL; char *nss_db = NULL; + bool do_ocsp = true; + char *verify_opts = NULL; struct poptOption long_options[] = { POPT_AUTOHELP @@ -475,6 +487,8 @@ int main(int argc, const char *argv[]) {"pin", 0, POPT_ARG_NONE, NULL, 'i', _("Expect PIN on stdin"), NULL}, {"keypad", 0, POPT_ARG_NONE, NULL, 'k', _("Expect PIN on keypad"), NULL}, + {"verify", 0, POPT_ARG_STRING, &verify_opts, 0 , _("Tune validation"), + NULL}, {"nssdb", 0, POPT_ARG_STRING, &nss_db, 0, _("NSS DB to use"), NULL}, POPT_TABLEEND @@ -614,6 +628,13 @@ int main(int argc, const char *argv[]) } talloc_steal(main_ctx, debug_prg_name); + if (verify_opts != NULL) { + ret = parse_cert_verify_opts(verify_opts, &do_ocsp); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Failed to parse verifiy option.\n"); + goto fail; + } + } if (mode == OP_AUTH && pin_mode == PIN_STDIN) { ret = p11c_recv_data(main_ctx, STDIN_FILENO, &pin); @@ -623,7 +644,7 @@ int main(int argc, const char *argv[]) } } - ret = do_work(main_ctx, nss_db, slot_name_in, mode, pin, &cert, + ret = do_work(main_ctx, nss_db, slot_name_in, mode, pin, do_ocsp, &cert, &token_name_out); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "do_work failed.\n"); diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h index 59831f2e73f923053e53cad838fac715330546dd..9d907771b79ba63362dfee2484b1f2a5ac825b1c 100644 --- a/src/responder/pam/pamsrv.h +++ b/src/responder/pam/pamsrv.h @@ -87,6 +87,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, int child_debug_fd, const char *nss_db, time_t timeout, + const char *verify_opts, struct pam_data *pd); errno_t pam_check_cert_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, char **cert, char **token_name); diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 4bb3e27b102584656e32546f9ad0035750eaeb06..0d3cfa94ec24f1c93faae2fcdcb3f3fffee730e5 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1032,6 +1032,7 @@ static errno_t check_cert(TALLOC_CTX *mctx, { int p11_child_timeout; const int P11_CHILD_TIMEOUT_DEFAULT = 10; + char *cert_verification_opts; errno_t ret; struct tevent_req *req; @@ -1046,8 +1047,19 @@ static errno_t check_cert(TALLOC_CTX *mctx, return ret; } + ret = confdb_get_string(pctx->rctx->cdb, mctx, CONFDB_MONITOR_CONF_ENTRY, + CONFDB_MONITOR_CERT_VERIFICATION, NULL, + &cert_verification_opts); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to read certificate_verification from confdb: [%d]: %s\n", + ret, sss_strerror(ret)); + return ret; + } + req = pam_check_cert_send(mctx, ev, pctx->p11_child_debug_fd, - pctx->nss_db, p11_child_timeout, pd); + pctx->nss_db, p11_child_timeout, + cert_verification_opts, pd); if (req == NULL) { DEBUG(SSSDBG_OP_FAILURE, "pam_check_cert_send failed.\n"); return ENOMEM; diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c index afb28fd529245975efbf34a04624adb3f09704f2..58310a2530287fc6d08a7195c8e879f96dcc5403 100644 --- a/src/responder/pam/pamsrv_p11.c +++ b/src/responder/pam/pamsrv_p11.c @@ -236,6 +236,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, int child_debug_fd, const char *nss_db, time_t timeout, + const char *verify_opts, struct pam_data *pd) { errno_t ret; @@ -246,9 +247,10 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, struct timeval tv; int pipefd_to_child[2]; int pipefd_from_child[2]; - const char *extra_args[5] = {NULL, NULL, NULL, NULL, NULL}; + const char *extra_args[7] = { NULL }; uint8_t *write_buf = NULL; size_t write_buf_len = 0; + size_t arg_c; req = tevent_req_create(mem_ctx, &state, struct pam_check_cert_state); if (req == NULL) { @@ -262,16 +264,21 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, } /* extra_args are added in revers order */ - extra_args[1] = "--nssdb"; - extra_args[0] = nss_db; + arg_c = 0; + extra_args[arg_c++] = nss_db; + extra_args[arg_c++] = "--nssdb"; + if (verify_opts != NULL) { + extra_args[arg_c++] = verify_opts; + extra_args[arg_c++] = "--verify"; + } if (pd->cmd == SSS_PAM_AUTHENTICATE) { - extra_args[2] = "--auth"; + extra_args[arg_c++] = "--auth"; switch (sss_authtok_get_type(pd->authtok)) { case SSS_AUTHTOK_TYPE_SC_PIN: - extra_args[3] = "--pin"; + extra_args[arg_c++] = "--pin"; break; case SSS_AUTHTOK_TYPE_SC_KEYPAD: - extra_args[3] = "--keypad"; + extra_args[arg_c++] = "--keypad"; break; default: DEBUG(SSSDBG_OP_FAILURE, "Unsupported authtok type.\n"); @@ -279,7 +286,7 @@ struct tevent_req *pam_check_cert_send(TALLOC_CTX *mem_ctx, goto done; } } else if (pd->cmd == SSS_PAM_PREAUTH) { - extra_args[2] = "--pre"; + extra_args[arg_c++] = "--pre"; } else { DEBUG(SSSDBG_CRIT_FAILURE, "Unexpected PAM command [%d}.\n", pd->cmd); ret = EINVAL; diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c index 5f5487035b298dcc540de246017731b94139a413..27099a66e85dc88b83a4e2d63db09a26be4c66c8 100644 --- a/src/responder/ssh/sshsrv_cmd.c +++ b/src/responder/ssh/sshsrv_cmd.c @@ -797,6 +797,7 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx, int ret; size_t d; TALLOC_CTX *tmp_ctx; + char *cert_verification_opts; if (el == NULL) { DEBUG(SSSDBG_TRACE_ALL, "Mssing element, nothing to do.\n"); @@ -811,9 +812,21 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx, for (d = 0; d < el->num_values; d++) { if (cert_data) { + + ret = confdb_get_string(cctx->rctx->cdb, tmp_ctx, + CONFDB_MONITOR_CONF_ENTRY, + CONFDB_MONITOR_CERT_VERIFICATION, NULL, + &cert_verification_opts); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to read p11_child_timeout from confdb: [%d] %s\n", + ret, sss_strerror(ret)); + return ret; + } + ret = cert_to_ssh_key(tmp_ctx, ssh_ctx->ca_db, el->values[d].data, el->values[d].length, - &key, &key_len); + cert_verification_opts, &key, &key_len); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "cert_to_ssh_key failed.\n"); return ret; diff --git a/src/tests/cmocka/test_cert_utils.c b/src/tests/cmocka/test_cert_utils.c index 7bd8cf2344003421e9ec84dc5e1b2305a861ab38..70abfe9a73dae0de3e62caa1912e8c648a4ceb40 100644 --- a/src/tests/cmocka/test_cert_utils.c +++ b/src/tests/cmocka/test_cert_utils.c @@ -356,7 +356,7 @@ void test_cert_to_ssh_key(void **state) assert_non_null(exp_key); ret = cert_to_ssh_key(ts, "sql:" ABS_SRC_DIR "/src/tests/cmocka/p11_nssdb", - der, der_size, &key, &key_size); + der, der_size, "no_ocsp", &key, &key_size); assert_int_equal(ret, EOK); assert_int_equal(key_size, exp_key_size); assert_memory_equal(key, exp_key, exp_key_size); diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c index 9f0cdcdb053b76b28f6cee08d7e5cffc90733161..1d843766544bcc83e9eb0cc0cd23d091802436ec 100644 --- a/src/tests/cmocka/test_pam_srv.c +++ b/src/tests/cmocka/test_pam_srv.c @@ -184,26 +184,38 @@ struct pam_ctx *mock_pctx(TALLOC_CTX *mem_ctx) return pctx; } -static int add_pam_params(struct sss_test_conf_param pam_params[], - struct confdb_ctx *cdb) +static int add_confdb_params(struct sss_test_conf_param params[], + struct confdb_ctx *cdb, const char *section) { const char *val[2]; int ret; val[1] = NULL; - for (int i = 0; pam_params[i].key; i++) { - val[0] = pam_params[i].value; - ret = confdb_add_param(cdb, true, CONFDB_PAM_CONF_ENTRY, - pam_params[i].key, val); + for (int i = 0; params[i].key; i++) { + val[0] = params[i].value; + ret = confdb_add_param(cdb, true, section, params[i].key, val); assert_int_equal(ret, EOK); } return EOK; } +static int add_pam_params(struct sss_test_conf_param pam_params[], + struct confdb_ctx *cdb) +{ + return add_confdb_params(pam_params, cdb, CONFDB_PAM_CONF_ENTRY); +} + +static int add_monitor_params(struct sss_test_conf_param monitor_params[], + struct confdb_ctx *cdb) +{ + return add_confdb_params(monitor_params, cdb, CONFDB_MONITOR_CONF_ENTRY); +} + void test_pam_setup(struct sss_test_conf_param dom_params[], struct sss_test_conf_param pam_params[], + struct sss_test_conf_param monitor_params[], void **state) { errno_t ret; @@ -239,6 +251,9 @@ void test_pam_setup(struct sss_test_conf_param dom_params[], ret = add_pam_params(pam_params, pam_test_ctx->rctx->cdb); assert_int_equal(ret, EOK); + ret = add_monitor_params(monitor_params, pam_test_ctx->rctx->cdb); + assert_int_equal(ret, EOK); + /* Create client context */ pam_test_ctx->cctx = mock_cctx(pam_test_ctx, pam_test_ctx->rctx); assert_non_null(pam_test_ctx->cctx); @@ -262,7 +277,12 @@ static int pam_test_setup(void **state) { NULL, NULL }, /* Sentinel */ }; - test_pam_setup(dom_params, pam_params, state); + struct sss_test_conf_param monitor_params[] = { + { "certificate_verification", "no_ocsp"}, + { NULL, NULL }, /* Sentinel */ + }; + + test_pam_setup(dom_params, pam_params, monitor_params, state); /* Prime the cache with a valid user */ ret = sysdb_add_user(pam_test_ctx->tctx->dom, diff --git a/src/util/cert.h b/src/util/cert.h index edbafc492a1ed42ad616d0bf2fae882046711746..527e04544734cf85a4104b4f4eff852258133e28 100644 --- a/src/util/cert.h +++ b/src/util/cert.h @@ -47,5 +47,8 @@ errno_t bin_to_ldap_filter_value(TALLOC_CTX *mem_ctx, errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, const uint8_t *der_blob, size_t der_size, + const char *verify_opts, uint8_t **key, size_t *key_size); + +errno_t parse_cert_verify_opts(const char *verify_opts, bool *do_ocsp); #endif /* __CERT_H__ */ diff --git a/src/util/cert/cert_common.c b/src/util/cert/cert_common.c index a29696ed3cd9f2168f47323fac97d44e9b49f921..35aac26950029f4d5b480834af35ef8c7ed08519 100644 --- a/src/util/cert/cert_common.c +++ b/src/util/cert/cert_common.c @@ -152,3 +152,49 @@ done: return ret; } + +errno_t parse_cert_verify_opts(const char *verify_opts, bool *do_ocsp) +{ + int ret; + TALLOC_CTX *tmp_ctx; + char **opts; + size_t c; + + if (verify_opts == NULL) { + *do_ocsp = true; + + return EOK; + } + + tmp_ctx = talloc_new(NULL); + if (tmp_ctx == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); + return ENOMEM; + } + + ret = split_on_separator(tmp_ctx, verify_opts, ',', true, true, &opts, + NULL); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "split_on_separator failed.\n"); + goto done; + } + + for (c = 0; opts[c] != NULL; c++) { + if (strcasecmp(opts[c], "no_ocsp") == 0) { + DEBUG(SSSDBG_TRACE_ALL, + "Found 'no_ocsp' option, disabling OCSP.\n"); + *do_ocsp = false; + } else { + DEBUG(SSSDBG_CRIT_FAILURE, + "Unsupported certificate verification option [%s], " \ + "skipping.\n", opts[c]); + } + } + + ret = EOK; + +done: + talloc_free(tmp_ctx); + + return ret; +} diff --git a/src/util/cert/libcrypto/cert.c b/src/util/cert/libcrypto/cert.c index 01f9554b990d6a139bb9a1d8d558c1c3f6bb745c..bbae03b049360716c262cdfc0014bb7504ed45eb 100644 --- a/src/util/cert/libcrypto/cert.c +++ b/src/util/cert/libcrypto/cert.c @@ -172,6 +172,7 @@ done: errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, const uint8_t *der_blob, size_t der_size, + const char *verify_opts, uint8_t **key, size_t *key_size) { int ret; diff --git a/src/util/cert/nss/cert.c b/src/util/cert/nss/cert.c index 1ada35b6321b9e0f3daf87b3412cf7a124cbf2c7..ab3113d9269971243ef7d83d54bf00582c1d3f9b 100644 --- a/src/util/cert/nss/cert.c +++ b/src/util/cert/nss/cert.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "util/crypto/sss_crypto.h" #include "util/crypto/nss/nss_util.h" @@ -220,6 +221,7 @@ done: errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, const uint8_t *der_blob, size_t der_size, + const char *verify_opts, uint8_t **key, size_t *key_size) { CERTCertDBHandle *handle; @@ -234,11 +236,20 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, NSSInitParameters parameters = { 0 }; parameters.length = sizeof (parameters); SECStatus rv; + bool do_ocsp = true; if (der_blob == NULL || der_size == 0) { return EINVAL; } + if (verify_opts != NULL) { + ret = parse_cert_verify_opts(verify_opts, &do_ocsp); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Failed to parse verifiy option.\n"); + return ret; + } + } + /* initialize NSS with context, we might have already called * NSS_NoDB_Init() but for validation we need to have access to a DB with * the trusted issuer cert. Only NSS_InitContext will really open the DB @@ -255,6 +266,15 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, handle = CERT_GetDefaultCertDB(); + if (do_ocsp) { + rv = CERT_EnableOCSPChecking(handle); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, "CERT_EnableOCSPChecking failed: [%d].\n", + PR_GetError()); + return EIO; + } + } + der_item.len = der_size; der_item.data = discard_const(der_blob); -- 2.1.0