On Tue, May 10, 2016 at 06:10:15PM +0200, Sumit Bose wrote: > On Tue, May 10, 2016 at 04:42:17PM +0200, Jakub Hrozek wrote: > > On Thu, Apr 14, 2016 at 01:48:50PM +0200, Sumit Bose wrote: > > > Hi, > > > > > > the following 3 patches are related to the Smartcard authentication > > > feature but imo can be tested even without having one. > > > > > > The first patch just adds some missing pieces. The second adds a new > > > 'no_verification' switch to the 'certificate_verification' option, which > > > is already tested by the unit tests. > > > > > > The third adds two new OCSP related switches. With OCSP a certificate > > > can be validates online by talking to a server which is listed in the > > > certificate. Of course it might not always be possible to directly talk > > > to this server. We already have the 'no_ocsp' switch to disable OCSP > > > completely. The two new switches allow SSSD to talk to a different > > > server or a proxy. To see how it is working you can do to following: > > > > > > - call 'make check' to build and rung all the tests > > > - call './pam-srv-tests' to run the PAM responder tests but do not let > > > it complete but stop it with CTRL-C. This is needed to create the test > > > nss database in /dev/shm/tp_pam_srv_tests-test_pam_srv/, it can be > > > created differently but this way it is most easy :-) > > > - add a OCSP signing cert with > > > > > > echo > > > "MIIDaTCCAlGgAwIBAgIBAjANBgkqhkiG9w0BAQsFADA0MRIwEAYDVQQKDAlJUEEuREVWRUwxHjAcBgNVBAMMFUNlcnRpZmljYXRlIEF1dGhvcml0eTAeFw0xNTA5MjMxMTI2MjBaFw0xNzA5MTIxMTI2MjBaMC0xEjAQBgNVBAoMCUlQQS5ERVZFTDEXMBUGA1UEAwwOT0NTUCBTdWJzeXN0ZW0wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDDEXVzkTN9R03+hvAbkiCYqdKKe7Nr0YDlchny+IsJl5cYiFBtWqiVJK5N0L6uCS9bVBtuG7y9Vv1TJrv2U8hqjXYZFM6HzYwwmbE2tv1Yyx0dQyYiYTT0nVyXPrXDJe0PFdYTsAlw7C54wZuBsdDADUTA1ThAP22Z6Zy+UyQFtlqEfCErvHK+VolCfkTJ4dYlQb5x7Gs6fEmLbnaGpJXW1JnRzMZ4hM+/mV3xSsjFuYwwoFCHG6GQu/LJgosBX9M+OMavMOO/AzeUlHfUoyMUNn0iNeiDqeYPBCNf4czK0CpeJdE4qBBgu0vSfC0mzjKRuFQEAUuBGxGE+2BP09uXAgMBAAGjgYwwgYkwHwYDVR0jBBgwFoAU0aLh2me2OKzMbu3zckJNNu3Hd1swDgYDVR0PAQH/BAQDAgHGMEEGCCsGAQUFBwEBBDUwMzAxBggrBgEFBQcwAYYlaHR0cDovL2lwYS1kZXZlbC5pcGEuZGV2ZWw6ODAvY2Evb2NzcDATBgNVHSUEDDAKBggrBgEFBQcDCTANBgkqhkiG9w0BAQsFAAOCAQEAAJzUjm39nsMlxc0ivEm77PN9ZFFWIfY6fOteNpJnOADlOatkXKq6PTFS0lRo/53HjYvmvrjnUTYHK3hmRlDvwr+49UqhSkKai8v6PSS4jYJplETME032OwGL17qCjoX2yU55Ovm3CIamUNNOSIvMBbS7HB0EBe/KHMhme5+Uhtfgjql9B9ihuwT1U3vXQP+cavxe37PJOCUuuATLyd0GX+Islkq2v1pEKX0dsXhMpDwLOYLqckBZHOoAKEo2VYZN0P1KZZkJd4kQHGsADYfuIrLACwzLxlEWmCIq4AOwpYtkMSx85DgT6lW3ECgfbFYVVUzqHUuTiogEsgJ0kouCJw==" > > > | base64 -d | certutil -A -d sql:/dev/shm/tp_pam_srv_tests-test_pam_srv > > > -t TC,TC,TC -n ocsp_cert > > > > > > the NSS library call check this certificate first before trying to > > > connect to > > > the OCSP responder, so a valid one with the right key usage must be > > > added to > > > make NSS try to reach the new OCSP responder > > > > > > - call > > > > > > strace -s 128 -f -esend .libs/lt-p11_child --debug-microseconds=1 > > > --debug-timestamps=1 --debug-to-stderr --debug-level=10 --pre --nssdb > > > sql:/dev/shm/tp_pam_srv_tests-test_pam_srv > > > > > > where you should see lines like > > > > > > send(7, "\313D\1\0\0\1\0\0\0\0\0\0\6ipa-ca\3ipa\5devel\0\0\1\0\1", > > > 34, MSG_NOSIGNAL) = 34 > > > > > > from the DNS lookups for ipa-ca.ipa.devel which is the OCSP server from > > > the > > > ticket > > > > > > - call > > > > > > strace -s 128 -f -esend ./p11_child --debug-microseconds=1 > > > --debug-timestamps=1 --debug-to-stderr --debug-level=10 --pre --nssdb > > > sql:/dev/shm/tp_pam_srv_tests-test_pam_srv --verify > > > 'ocsp_default_responder=http://oooo.cccc.ssss.pppp:80,ocsp_default_responder_signing_cert=ocsp_cert' > > > > > > where you should now see lines like > > > > > > send(7, "yO\1\0\0\1\0\0\0\0\0\0\4oooo\4cccc\4ssss\4pppp\0\0\1\0\1", > > > 37, MSG_NOSIGNAL) = 37 > > > > > > from the DNS lookups for the OCSP responder from the command line. > > > > > > Of course all the validations will fail with "Certificate [SSSD Test > > > Token:Server-Cert][CN=ipa-devel.ipa.devel,O=IPA.DEVEL] not valid [-8071], > > > skipping" because none of the OCSP responders are available but I think > > > this > > > test is sufficient to see that the patch is working as expected. > > > > Thank you for the patches and the tests. I only have one question about > > the first patch.. > > > > > From c2eccab2c12b58a74cdc6fd10efe775dbcd8c1e1 Mon Sep 17 00:00:00 2001 > > > From: Sumit Bose <sb...@redhat.com> > > > Date: Fri, 18 Mar 2016 16:24:18 +0100 > > > Subject: [PATCH 1/3] p11: add missing man page entry and config API > > > > > > The pam_cert_auth and pam_cert_db_path option where missing in the > > > config API and had no man page entries. > > > > Did you also want to document the pam_cert_auth option? > > oops, yes I guess this would be a good idea. I'll send a new patch. >
new version attached. bye, Sumit
From 94c193becdf736129e5d2f9eb38e64ace6a6b759 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Fri, 18 Mar 2016 16:24:18 +0100 Subject: [PATCH 1/3] p11: add missing man page entry and config API The pam_cert_auth and pam_cert_db_path option where missing in the config API and had no man page entries. --- src/config/SSSDConfig/__init__.py.in | 2 ++ src/config/etc/sssd.api.conf | 2 ++ src/man/sssd.conf.5.xml | 26 ++++++++++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 1a0893cbc180394d994b9d97fc0fa863da656549..e7bf43dfd309ec6e2e47e70cb353c27435dcacfb 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -92,6 +92,8 @@ option_strings = { 'pam_public_domains' : _('List of domains accessible even for untrusted users.'), 'pam_account_expired_message' : _('Message printed when user account is expired.'), 'pam_account_locked_message' : _('Message printed when user account is locked.'), + 'pam_cert_auth' : _('Allow certificate based/Smartcard authentication.'), + 'pam_cert_db_path' : _('Path to certificate databse with PKCS#11 modules.'), 'p11_child_timeout' : _('How many seconds will pam_sss wait for p11_child to finish'), # [sudo] diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index a15f2bd05c3046f8d76b13b3d8f28f9001d8fded..a0a82543f2c91b95eb00149e413043f0e6f6f4ea 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -62,6 +62,8 @@ pam_trusted_users = str, None, false pam_public_domains = str, None, false pam_account_expired_message = str, None, false pam_account_locked_message = str, None, false +pam_cert_auth = bool, None, false +pam_cert_db_path = str, None, false p11_child_timeout = int, None, false [sudo] diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 09db9cd32673c911991b335e986692e3d8d856d0..9633dacb77ff26358e83021d5c14b0091eeafe72 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -1027,6 +1027,32 @@ pam_account_locked_message = Account locked, please contact help desk. </listitem> </varlistentry> <varlistentry> + <term>pam_cert_auth (bool)</term> + <listitem> + <para> + Enable certificate based Smartcard authentication. + Since this requires additional communication with + the Smartcard which will delay the authentication + process this option is disabled by default. + </para> + <para> + Default: False + </para> + </listitem> + </varlistentry> + <varlistentry> + <term>pam_cert_db_path (string)</term> + <listitem> + <para> + The path to the certificate database which contain + the PKCS#11 modules to access the Smartcard. + </para> + <para> + Default: /etc/pki/nssdb (NSS version) + </para> + </listitem> + </varlistentry> + <varlistentry> <term>p11_child_timeout (integer)</term> <listitem> <para> -- 2.1.0
From c9afd60b95757a02e675a2eadec2790fb42c7956 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Thu, 24 Mar 2016 20:42:12 +0100 Subject: [PATCH 2/3] p11: add no_verification option --- src/man/sssd.conf.5.xml | 8 ++++++ src/p11_child/p11_child_nss.c | 44 ++++++++++++++++-------------- src/responder/ssh/sshsrv_cmd.c | 7 +++-- src/tests/cmocka/test_cert_utils.c | 4 ++- src/tests/cmocka/test_pam_srv.c | 27 ++++++++++++++++++ src/util/cert.h | 2 +- src/util/cert/libcrypto/cert.c | 2 +- src/util/cert/nss/cert.c | 20 ++++++++------ src/util/util.c | 56 ++++++++++++++++++++++++++++++-------- src/util/util.h | 8 +++++- 10 files changed, 130 insertions(+), 48 deletions(-) diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 9633dacb77ff26358e83021d5c14b0091eeafe72..5396a490a0b8793bb55e510896c57758d1f3fdb0 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -383,6 +383,14 @@ the client.</para> </listitem> </varlistentry> + <varlistentry> + <term>no_verification</term> + <listitem> + <para>Disables verification completely. + This option should only be used for + testing.</para> + </listitem> + </varlistentry> </variablelist> </para> <para> diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c index 8a8e68aeec078340b77f1245162f2183fc5aa63f..be3f33981e4540185d189f3d05e22fd598562f19 100644 --- a/src/p11_child/p11_child_nss.c +++ b/src/p11_child/p11_child_nss.c @@ -70,8 +70,9 @@ 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, bool do_ocsp, char **cert, - char **token_name_out) + enum op_mode mode, const char *pin, + struct cert_verify_opts *cert_verify_opts, + char **cert, char **token_name_out) { int ret; SECStatus rv; @@ -263,7 +264,7 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in, return EIO; } - if (do_ocsp) { + if (cert_verify_opts->do_ocsp) { rv = CERT_EnableOCSPChecking(handle); if (rv != SECSuccess) { DEBUG(SSSDBG_OP_FAILURE, "CERT_EnableOCSPChecking failed: [%d].\n", @@ -282,15 +283,18 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in, cert_list_node->cert->nickname, cert_list_node->cert->subjectName); - rv = CERT_VerifyCertificateNow(handle, cert_list_node->cert, - PR_TRUE, certificateUsageSSLClient, - NULL, NULL); - if (rv != SECSuccess) { - DEBUG(SSSDBG_OP_FAILURE, - "Certificate [%s][%s] not valid [%d], skipping.\n", - cert_list_node->cert->nickname, - cert_list_node->cert->subjectName, PR_GetError()); - continue; + if (cert_verify_opts->do_verification) { + rv = CERT_VerifyCertificateNow(handle, cert_list_node->cert, + PR_TRUE, + certificateUsageSSLClient, + NULL, NULL); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, + "Certificate [%s][%s] not valid [%d], skipping.\n", + cert_list_node->cert->nickname, + cert_list_node->cert->subjectName, PR_GetError()); + continue; + } } @@ -466,7 +470,7 @@ 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; + struct cert_verify_opts *cert_verify_opts; char *verify_opts = NULL; struct poptOption long_options[] = { @@ -613,12 +617,10 @@ 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; - } + ret = parse_cert_verify_opts(main_ctx, verify_opts, &cert_verify_opts); + if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Failed to parse verifiy option.\n"); + goto fail; } if (mode == OP_AUTH && pin_mode == PIN_STDIN) { @@ -629,8 +631,8 @@ int main(int argc, const char *argv[]) } } - ret = do_work(main_ctx, nss_db, slot_name_in, mode, pin, do_ocsp, &cert, - &token_name_out); + ret = do_work(main_ctx, nss_db, slot_name_in, mode, pin, cert_verify_opts, + &cert, &token_name_out); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "do_work failed.\n"); goto fail; diff --git a/src/responder/ssh/sshsrv_cmd.c b/src/responder/ssh/sshsrv_cmd.c index af385fde8b382da513dbac29f496462a90c4c269..5954cec1ba3a688ae2d14f2304d722d91c26d592 100644 --- a/src/responder/ssh/sshsrv_cmd.c +++ b/src/responder/ssh/sshsrv_cmd.c @@ -798,7 +798,7 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx, size_t d; TALLOC_CTX *tmp_ctx; char *cert_verification_opts; - bool do_ocsp = true; + struct cert_verify_opts *cert_verify_opts; if (el == NULL) { DEBUG(SSSDBG_TRACE_ALL, "Mssing element, nothing to do.\n"); @@ -826,7 +826,8 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx, } if (cert_verification_opts != NULL) { - ret = parse_cert_verify_opts(cert_verification_opts, &do_ocsp); + ret = parse_cert_verify_opts(tmp_ctx, cert_verification_opts, + &cert_verify_opts); if (ret != EOK) { DEBUG(SSSDBG_FATAL_FAILURE, "Failed to parse verifiy option.\n"); @@ -836,7 +837,7 @@ static errno_t decode_and_add_base64_data(struct ssh_cmd_ctx *cmd_ctx, ret = cert_to_ssh_key(tmp_ctx, ssh_ctx->ca_db, el->values[d].data, el->values[d].length, - do_ocsp, &key, &key_len); + cert_verify_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 658391d147154d7381d4f1ab74f199453fcab352..35e8cb7513968079861048a7e8b0631229f202c0 100644 --- a/src/tests/cmocka/test_cert_utils.c +++ b/src/tests/cmocka/test_cert_utils.c @@ -345,6 +345,8 @@ void test_cert_to_ssh_key(void **state) size_t exp_key_size; uint8_t *der; size_t der_size; + struct cert_verify_opts cert_verify_opts = { .do_ocsp = false, + .do_verification = true }; struct test_state *ts = talloc_get_type_abort(*state, struct test_state); assert_non_null(ts); @@ -356,7 +358,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, false, &key, &key_size); + der, der_size, &cert_verify_opts, &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 1e3ac542cf4610cd411f7b335930d8e1a1753e89..6da1f416ce2c78dfb4e5f7ec8ae98c7029927287 100644 --- a/src/tests/cmocka/test_pam_srv.c +++ b/src/tests/cmocka/test_pam_srv.c @@ -320,6 +320,30 @@ static int pam_test_setup(void **state) return 0; } +static int pam_test_setup_no_verification(void **state) +{ + struct sss_test_conf_param dom_params[] = { + { "enumerate", "false" }, + { "cache_credentials", "true" }, + { NULL, NULL }, /* Sentinel */ + }; + + struct sss_test_conf_param pam_params[] = { + { "p11_child_timeout", "30" }, + { NULL, NULL }, /* Sentinel */ + }; + + struct sss_test_conf_param monitor_params[] = { + { "certificate_verification", "no_verification"}, + { NULL, NULL }, /* Sentinel */ + }; + + test_pam_setup(dom_params, pam_params, monitor_params, state); + + pam_test_setup_common(); + return 0; +} + static int pam_cached_test_setup(void **state) { struct sss_test_conf_param dom_params[] = { @@ -1701,6 +1725,9 @@ int main(int argc, const char *argv[]) pam_test_setup, pam_test_teardown), cmocka_unit_test_setup_teardown(test_pam_cert_auth, pam_test_setup, pam_test_teardown), + cmocka_unit_test_setup_teardown(test_pam_cert_auth, + pam_test_setup_no_verification, + pam_test_teardown), #endif /* HAVE_NSS */ }; diff --git a/src/util/cert.h b/src/util/cert.h index c8c42548745baaed5a39addde911d4d6e9305462..bb64d0d7a0a48207df60f6e6e554da5e16a16b03 100644 --- a/src/util/cert.h +++ b/src/util/cert.h @@ -47,6 +47,6 @@ 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, - bool do_ocsp, + struct cert_verify_opts *cert_verify_opts, uint8_t **key, size_t *key_size); #endif /* __CERT_H__ */ diff --git a/src/util/cert/libcrypto/cert.c b/src/util/cert/libcrypto/cert.c index 4e2dbe70cafcb31e9ca4cc232dacb0cc176b0dc0..a7752d7c12ffede5b1a12d7c9ea3ca827128ca37 100644 --- a/src/util/cert/libcrypto/cert.c +++ b/src/util/cert/libcrypto/cert.c @@ -172,7 +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, - bool do_ocsp, + struct cert_verify_opts *cert_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 fbd063cf5f0c0100dab32f8ff14339b80979039a..9c1c965dd29d118092a264d315b392f74d905142 100644 --- a/src/util/cert/nss/cert.c +++ b/src/util/cert/nss/cert.c @@ -223,7 +223,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, - bool do_ocsp, + struct cert_verify_opts *cert_verify_opts, uint8_t **key, size_t *key_size) { CERTCertDBHandle *handle; @@ -259,7 +259,7 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, handle = CERT_GetDefaultCertDB(); - if (do_ocsp) { + if (cert_verify_opts->do_ocsp) { rv = CERT_EnableOCSPChecking(handle); if (rv != SECSuccess) { DEBUG(SSSDBG_OP_FAILURE, "CERT_EnableOCSPChecking failed: [%d].\n", @@ -278,13 +278,15 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, goto done; } - rv = CERT_VerifyCertificateNow(handle, cert, PR_TRUE, - certificateUsageSSLClient, NULL, NULL); - if (rv != SECSuccess) { - DEBUG(SSSDBG_CRIT_FAILURE, "CERT_VerifyCertificateNow failed [%d].\n", - PR_GetError()); - ret = EACCES; - goto done; + if (cert_verify_opts->do_verification) { + rv = CERT_VerifyCertificateNow(handle, cert, PR_TRUE, + certificateUsageSSLClient, NULL, NULL); + if (rv != SECSuccess) { + DEBUG(SSSDBG_CRIT_FAILURE, "CERT_VerifyCertificateNow failed [%d].\n", + PR_GetError()); + ret = EACCES; + goto done; + } } cert_pub_key = CERT_ExtractPublicKey(cert); diff --git a/src/util/util.c b/src/util/util.c index 60da8852811d99808af21f8c252bd4ff0d2fdd8c..2449a0ff3a961e33a4397ce27bd441139bffb566 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1107,23 +1107,47 @@ errno_t sss_unique_filename(TALLOC_CTX *owner, char *path_tmpl) return ret; } -errno_t parse_cert_verify_opts(const char *verify_opts, bool *do_ocsp) +static struct cert_verify_opts *init_cert_verify_opts(TALLOC_CTX *mem_ctx) +{ + struct cert_verify_opts *cert_verify_opts; + + cert_verify_opts = talloc(mem_ctx, struct cert_verify_opts); + if (cert_verify_opts == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); + return NULL; + } + + cert_verify_opts->do_ocsp = true; + cert_verify_opts->do_verification = true; + + return cert_verify_opts; +} + +errno_t parse_cert_verify_opts(TALLOC_CTX *mem_ctx, const char *verify_opts, + struct cert_verify_opts **_cert_verify_opts) { int ret; TALLOC_CTX *tmp_ctx; char **opts; size_t c; + struct cert_verify_opts *cert_verify_opts; + + tmp_ctx = talloc_new(NULL); + if (tmp_ctx == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); + return ENOMEM; + } + + cert_verify_opts = init_cert_verify_opts(tmp_ctx); + if (cert_verify_opts == NULL) { + DEBUG(SSSDBG_OP_FAILURE, "init_cert_verify_opts failed.\n"); + ret = ENOMEM; + goto done; + } 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 = EOK; + goto done; } ret = split_on_separator(tmp_ctx, verify_opts, ',', true, true, &opts, @@ -1137,7 +1161,13 @@ errno_t parse_cert_verify_opts(const char *verify_opts, bool *do_ocsp) if (strcasecmp(opts[c], "no_ocsp") == 0) { DEBUG(SSSDBG_TRACE_ALL, "Found 'no_ocsp' option, disabling OCSP.\n"); - *do_ocsp = false; + cert_verify_opts->do_ocsp = false; + } else if (strcasecmp(opts[c], "no_verification") == 0) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Found 'no_verification' option, " + "disabling verification completely. " + "This should not be used in production.\n"); + cert_verify_opts->do_verification = false; } else { DEBUG(SSSDBG_CRIT_FAILURE, "Unsupported certificate verification option [%s], " \ @@ -1148,6 +1178,10 @@ errno_t parse_cert_verify_opts(const char *verify_opts, bool *do_ocsp) ret = EOK; done: + if (ret == EOK) { + *_cert_verify_opts = talloc_steal(mem_ctx, cert_verify_opts); + } + talloc_free(tmp_ctx); return ret; diff --git a/src/util/util.h b/src/util/util.h index 11054c1064df443c0bb73eeb8cfc218815efc506..83738c973a5ac8ae7ea490d993df4ccff5f2d5a9 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -313,7 +313,13 @@ int split_on_separator(TALLOC_CTX *mem_ctx, const char *str, char **parse_args(const char *str); -errno_t parse_cert_verify_opts(const char *verify_opts, bool *do_ocsp); +struct cert_verify_opts { + bool do_ocsp; + bool do_verification; +}; + +errno_t parse_cert_verify_opts(TALLOC_CTX *mem_ctx, const char *verify_opts, + struct cert_verify_opts **cert_verify_opts); errno_t sss_hash_create(TALLOC_CTX *mem_ctx, -- 2.1.0
From c7bbf934a55016238614209216a6b9fc294718d4 Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Tue, 12 Apr 2016 18:14:08 +0200 Subject: [PATCH 3/3] p11: add OCSP default responder options --- src/man/sssd.conf.5.xml | 28 +++++++++++++++ src/p11_child/p11_child_nss.c | 33 ++++++++++++++++++ src/tests/cmocka/test_utils.c | 80 +++++++++++++++++++++++++++++++++++++++++++ src/util/cert/nss/cert.c | 43 +++++++++++++++++++++-- src/util/util.c | 60 +++++++++++++++++++++++++++++++- src/util/util.h | 2 ++ 6 files changed, 242 insertions(+), 4 deletions(-) diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml index 5396a490a0b8793bb55e510896c57758d1f3fdb0..6cff0dc87c8eff21a0ea6405d741f33b8f7ecd65 100644 --- a/src/man/sssd.conf.5.xml +++ b/src/man/sssd.conf.5.xml @@ -391,6 +391,34 @@ testing.</para> </listitem> </varlistentry> + <varlistentry> + <term>ocsp_default_responder=URL</term> + <listitem> + <para>Sets the OCSP default responder + which should be used instead of the one + mentioned in the certificate. URL must + be replaced with the URL of the OCSP + default responder e.g. + http://example.com:80/ocsp.</para> + <para>This option must be used together + with + ocsp_default_responder_signing_cert. + </para> + </listitem> + </varlistentry> + <varlistentry> + <term> + ocsp_default_responder_signing_cert=NAME</term> + <listitem> + <para>The nickname of the cert to trust + (expected) to sign the OCSP responses. + The certificate with the given nickname + must be availble in the systems NSS + database.</para> + <para>This option must be used together + with ocsp_default_responder.</para> + </listitem> + </varlistentry> </variablelist> </para> <para> diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c index be3f33981e4540185d189f3d05e22fd598562f19..feae884360025b33c8090460e9e0a1f916365ab7 100644 --- a/src/p11_child/p11_child_nss.c +++ b/src/p11_child/p11_child_nss.c @@ -271,6 +271,27 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in, PR_GetError()); return EIO; } + + if (cert_verify_opts->ocsp_default_responder != NULL + && cert_verify_opts->ocsp_default_responder_signing_cert != NULL) { + rv = CERT_SetOCSPDefaultResponder(handle, + cert_verify_opts->ocsp_default_responder, + cert_verify_opts->ocsp_default_responder_signing_cert); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, + "CERT_SetOCSPDefaultResponder failed: [%d].\n", + PR_GetError()); + return EIO; + } + + rv = CERT_EnableOCSPDefaultResponder(handle); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, + "CERT_EnableOCSPDefaultResponder failed: [%d].\n", + PR_GetError()); + return EIO; + } + } } found_cert = NULL; @@ -309,6 +330,18 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, const char *slot_name_in, } } + /* Disable OCSP default responder so that NSS can shutdown properly */ + if (cert_verify_opts->do_ocsp + && cert_verify_opts->ocsp_default_responder != NULL + && cert_verify_opts->ocsp_default_responder_signing_cert != NULL) { + rv = CERT_DisableOCSPDefaultResponder(handle); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, + "CERT_DisableOCSPDefaultResponder failed: [%d].\n", + PR_GetError()); + } + } + if (found_cert == NULL) { DEBUG(SSSDBG_TRACE_ALL, "No certificate found.\n"); *cert = NULL; diff --git a/src/tests/cmocka/test_utils.c b/src/tests/cmocka/test_utils.c index 3aea17f3610d436ea0ec8f58ec8b01000bc07d5c..aaba2df6d06d396bc88d7fa23edc8840eebeddb6 100644 --- a/src/tests/cmocka/test_utils.c +++ b/src/tests/cmocka/test_utils.c @@ -1487,6 +1487,83 @@ static void test_sss_unique_filename_destruct(void **state) sss_unique_filename_test(test_ctx, true); } +static void test_parse_cert_verify_opts(void **state) +{ + int ret; + struct cert_verify_opts *cv_opts; + + ret = parse_cert_verify_opts(global_talloc_context, NULL, &cv_opts); + assert_int_equal(ret, EOK); + assert_true(cv_opts->do_verification); + assert_true(cv_opts->do_ocsp); + assert_null(cv_opts->ocsp_default_responder); + assert_null(cv_opts->ocsp_default_responder_signing_cert); + talloc_free(cv_opts); + + ret = parse_cert_verify_opts(global_talloc_context, "wedfkwefjk", &cv_opts); + assert_int_equal(ret, EOK); + assert_true(cv_opts->do_verification); + assert_true(cv_opts->do_ocsp); + assert_null(cv_opts->ocsp_default_responder); + assert_null(cv_opts->ocsp_default_responder_signing_cert); + talloc_free(cv_opts); + + ret = parse_cert_verify_opts(global_talloc_context, "no_ocsp", &cv_opts); + assert_int_equal(ret, EOK); + assert_true(cv_opts->do_verification); + assert_false(cv_opts->do_ocsp); + assert_null(cv_opts->ocsp_default_responder); + assert_null(cv_opts->ocsp_default_responder_signing_cert); + talloc_free(cv_opts); + + ret = parse_cert_verify_opts(global_talloc_context, "no_verification", + &cv_opts); + assert_int_equal(ret, EOK); + assert_false(cv_opts->do_verification); + assert_true(cv_opts->do_ocsp); + assert_null(cv_opts->ocsp_default_responder); + assert_null(cv_opts->ocsp_default_responder_signing_cert); + talloc_free(cv_opts); + + ret = parse_cert_verify_opts(global_talloc_context, + "no_ocsp,no_verification", &cv_opts); + assert_int_equal(ret, EOK); + assert_false(cv_opts->do_verification); + assert_false(cv_opts->do_ocsp); + assert_null(cv_opts->ocsp_default_responder); + assert_null(cv_opts->ocsp_default_responder_signing_cert); + talloc_free(cv_opts); + + ret = parse_cert_verify_opts(global_talloc_context, + "ocsp_default_responder=", &cv_opts); + assert_int_equal(ret, EINVAL); + + ret = parse_cert_verify_opts(global_talloc_context, + "ocsp_default_responder_signing_cert=", + &cv_opts); + assert_int_equal(ret, EINVAL); + + ret = parse_cert_verify_opts(global_talloc_context, + "ocsp_default_responder=abc", &cv_opts); + assert_int_equal(ret, EINVAL); + + ret = parse_cert_verify_opts(global_talloc_context, + "ocsp_default_responder_signing_cert=def", + &cv_opts); + assert_int_equal(ret, EINVAL); + + ret = parse_cert_verify_opts(global_talloc_context, + "ocsp_default_responder=abc," + "ocsp_default_responder_signing_cert=def", + &cv_opts); + assert_int_equal(ret, EOK); + assert_true(cv_opts->do_verification); + assert_true(cv_opts->do_ocsp); + assert_string_equal(cv_opts->ocsp_default_responder, "abc"); + assert_string_equal(cv_opts->ocsp_default_responder_signing_cert, "def"); + talloc_free(cv_opts); +} + int main(int argc, const char *argv[]) { poptContext pc; @@ -1566,6 +1643,9 @@ int main(int argc, const char *argv[]) cmocka_unit_test_setup_teardown(test_sss_unique_filename_destruct, unique_file_test_setup, unique_file_test_teardown), + cmocka_unit_test_setup_teardown(test_parse_cert_verify_opts, + setup_add_strings_lists, + teardown_add_strings_lists), }; /* Set debug level to invalid value so we can deside if -d 0 was used. */ diff --git a/src/util/cert/nss/cert.c b/src/util/cert/nss/cert.c index 9c1c965dd29d118092a264d315b392f74d905142..7bf9a8bfcb5d8da36698c50b8255ed50ca8b67cb 100644 --- a/src/util/cert/nss/cert.c +++ b/src/util/cert/nss/cert.c @@ -238,6 +238,7 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, NSSInitParameters parameters = { 0 }; parameters.length = sizeof (parameters); SECStatus rv; + SECStatus rv_verify; if (der_blob == NULL || der_size == 0) { return EINVAL; @@ -266,6 +267,27 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, PR_GetError()); return EIO; } + + if (cert_verify_opts->ocsp_default_responder != NULL + && cert_verify_opts->ocsp_default_responder_signing_cert != NULL) { + rv = CERT_SetOCSPDefaultResponder(handle, + cert_verify_opts->ocsp_default_responder, + cert_verify_opts->ocsp_default_responder_signing_cert); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, + "CERT_SetOCSPDefaultResponder failed: [%d].\n", + PR_GetError()); + return EIO; + } + + rv = CERT_EnableOCSPDefaultResponder(handle); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, + "CERT_EnableOCSPDefaultResponder failed: [%d].\n", + PR_GetError()); + return EIO; + } + } } der_item.len = der_size; @@ -279,9 +301,24 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db, } if (cert_verify_opts->do_verification) { - rv = CERT_VerifyCertificateNow(handle, cert, PR_TRUE, - certificateUsageSSLClient, NULL, NULL); - if (rv != SECSuccess) { + rv_verify = CERT_VerifyCertificateNow(handle, cert, PR_TRUE, + certificateUsageSSLClient, + NULL, NULL); + + /* Disable OCSP default responder so that NSS can shutdown properly */ + if (cert_verify_opts->do_ocsp + && cert_verify_opts->ocsp_default_responder != NULL + && cert_verify_opts->ocsp_default_responder_signing_cert + != NULL) { + rv = CERT_DisableOCSPDefaultResponder(handle); + if (rv != SECSuccess) { + DEBUG(SSSDBG_OP_FAILURE, + "CERT_DisableOCSPDefaultResponder failed: [%d].\n", + PR_GetError()); + } + } + + if (rv_verify != SECSuccess) { DEBUG(SSSDBG_CRIT_FAILURE, "CERT_VerifyCertificateNow failed [%d].\n", PR_GetError()); ret = EACCES; diff --git a/src/util/util.c b/src/util/util.c index 2449a0ff3a961e33a4397ce27bd441139bffb566..c1024c9739f1ed99d7224411592d0fd82f7ef5e9 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1111,7 +1111,7 @@ static struct cert_verify_opts *init_cert_verify_opts(TALLOC_CTX *mem_ctx) { struct cert_verify_opts *cert_verify_opts; - cert_verify_opts = talloc(mem_ctx, struct cert_verify_opts); + cert_verify_opts = talloc_zero(mem_ctx, struct cert_verify_opts); if (cert_verify_opts == NULL) { DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); return NULL; @@ -1119,10 +1119,20 @@ static struct cert_verify_opts *init_cert_verify_opts(TALLOC_CTX *mem_ctx) cert_verify_opts->do_ocsp = true; cert_verify_opts->do_verification = true; + cert_verify_opts->ocsp_default_responder = NULL; + cert_verify_opts->ocsp_default_responder_signing_cert = NULL; return cert_verify_opts; } +#define OCSP_DEFAUL_RESPONDER "ocsp_default_responder=" +#define OCSP_DEFAUL_RESPONDER_LEN (sizeof(OCSP_DEFAUL_RESPONDER) - 1) + +#define OCSP_DEFAUL_RESPONDER_SIGNING_CERT \ + "ocsp_default_responder_signing_cert=" +#define OCSP_DEFAUL_RESPONDER_SIGNING_CERT_LEN \ + (sizeof(OCSP_DEFAUL_RESPONDER_SIGNING_CERT) - 1) + errno_t parse_cert_verify_opts(TALLOC_CTX *mem_ctx, const char *verify_opts, struct cert_verify_opts **_cert_verify_opts) { @@ -1168,6 +1178,41 @@ errno_t parse_cert_verify_opts(TALLOC_CTX *mem_ctx, const char *verify_opts, "disabling verification completely. " "This should not be used in production.\n"); cert_verify_opts->do_verification = false; + } else if (strncasecmp(opts[c], OCSP_DEFAUL_RESPONDER, + OCSP_DEFAUL_RESPONDER_LEN) == 0) { + cert_verify_opts->ocsp_default_responder = + talloc_strdup(cert_verify_opts, + &opts[c][OCSP_DEFAUL_RESPONDER_LEN]); + if (cert_verify_opts->ocsp_default_responder == NULL + || *cert_verify_opts->ocsp_default_responder == '\0') { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to parse ocsp_default_responder option [%s].\n", + opts[c]); + ret = EINVAL; + goto done; + } + + DEBUG(SSSDBG_TRACE_ALL, "Using OCSP default responder [%s]\n", + cert_verify_opts->ocsp_default_responder); + } else if (strncasecmp(opts[c], + OCSP_DEFAUL_RESPONDER_SIGNING_CERT, + OCSP_DEFAUL_RESPONDER_SIGNING_CERT_LEN) == 0) { + cert_verify_opts->ocsp_default_responder_signing_cert = + talloc_strdup(cert_verify_opts, + &opts[c][OCSP_DEFAUL_RESPONDER_SIGNING_CERT_LEN]); + if (cert_verify_opts->ocsp_default_responder_signing_cert == NULL + || *cert_verify_opts->ocsp_default_responder_signing_cert + == '\0') { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to parse ocsp_default_responder_signing_cert " + "option [%s].\n", opts[c]); + ret = EINVAL; + goto done; + } + + DEBUG(SSSDBG_TRACE_ALL, "Using OCSP default responder " + "signing cert nickname [%s]\n", + cert_verify_opts->ocsp_default_responder_signing_cert); } else { DEBUG(SSSDBG_CRIT_FAILURE, "Unsupported certificate verification option [%s], " \ @@ -1175,6 +1220,19 @@ errno_t parse_cert_verify_opts(TALLOC_CTX *mem_ctx, const char *verify_opts, } } + if ((cert_verify_opts->ocsp_default_responder == NULL + && cert_verify_opts->ocsp_default_responder_signing_cert != NULL) + || (cert_verify_opts->ocsp_default_responder != NULL + && cert_verify_opts->ocsp_default_responder_signing_cert == NULL)) { + + DEBUG(SSSDBG_CRIT_FAILURE, + "ocsp_default_responder and ocsp_default_responder_signing_cert " + "must be used together.\n"); + + ret = EINVAL; + goto done; + } + ret = EOK; done: diff --git a/src/util/util.h b/src/util/util.h index 83738c973a5ac8ae7ea490d993df4ccff5f2d5a9..ed7274a6a3278431ad8469773c443cd6c4427ffe 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -316,6 +316,8 @@ char **parse_args(const char *str); struct cert_verify_opts { bool do_ocsp; bool do_verification; + char *ocsp_default_responder; + char *ocsp_default_responder_signing_cert; }; errno_t parse_cert_verify_opts(TALLOC_CTX *mem_ctx, const char *verify_opts, -- 2.1.0
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org