URL: https://github.com/SSSD/sssd/pull/474 Author: sumit-bose Title: #474: p11_child: properly check results of CERT_VerifyCertificateNow Action: opened
PR body: """ I'm sorry but my recent commit 'p11_child: make sure OCSP checks are done' was not the right fix for the observed issue and might lead to do the OCSP check twice. Hence this PR reverts this commit and adds a better fix (after checking the related NSS code carefully and discussing it with NSS developers). With certificateUsageCheckAllUsages not only the return code of CERT_VerifyCertificateNow() should be checked but also the usages for which the certificate was verified. The usages checked here will all involve CA signature checks and OCSP checks if OCSP is enabled. Related to https://pagure.io/SSSD/sssd/issue/3560 """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/474/head:pr474 git checkout pr474
From d3d516c84ce128bbfe54d4cd1a0b7cd00e7579cf Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Tue, 12 Dec 2017 15:28:27 +0100 Subject: [PATCH 1/2] Revert "p11_child: make sure OCSP checks are done" This reverts commit 2297cc7d6cd5c38a7d64027165e4e82ca497f418. --- src/p11_child/p11_child_nss.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c index bf533f3ef..21c508eb1 100644 --- a/src/p11_child/p11_child_nss.c +++ b/src/p11_child/p11_child_nss.c @@ -338,23 +338,6 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, PR_GetError(), PORT_ErrorToString(PR_GetError())); continue; } - - /* with 'certificateUsageCheckAllUsages' set - * CERT_VerifyCertificateNow() does not do OCSP so it must be done - * explicitly */ - if (cert_verify_opts->do_ocsp) { - rv = CERT_CheckOCSPStatus(handle, cert_list_node->cert, - PR_Now(), NULL); - if (rv != SECSuccess) { - DEBUG(SSSDBG_OP_FAILURE, - "Certificate [%s][%s] failed OCSP check [%d][%s], " - "skipping.\n", - cert_list_node->cert->nickname, - cert_list_node->cert->subjectName, - PR_GetError(), PORT_ErrorToString(PR_GetError())); - continue; - } - } } if (key_id_in != NULL) { From c1b2853e4c3cc0b21c1c3366776d3859e3a8090e Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> Date: Tue, 12 Dec 2017 15:24:57 +0100 Subject: [PATCH 2/2] p11_child: properly check results of CERT_VerifyCertificateNow With certificateUsageCheckAllUsages not only the return code of CERT_VerifyCertificateNow() should be checked but also the usages for which the certificate was verified. The usages checked here will all involve CA signature checks and OCSP checks if OCSP is enabled. Related to https://pagure.io/SSSD/sssd/issue/3560 --- src/p11_child/p11_child_nss.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c index 21c508eb1..cb894280c 100644 --- a/src/p11_child/p11_child_nss.c +++ b/src/p11_child/p11_child_nss.c @@ -45,6 +45,15 @@ #include "util/crypto/sss_crypto.h" #include "util/cert.h" +#define EXP_USAGES ( certificateUsageSSLClient \ + | certificateUsageSSLServer \ + | certificateUsageSSLServerWithStepUp \ + | certificateUsageEmailSigner \ + | certificateUsageEmailRecipient \ + | certificateUsageObjectSigner \ + | certificateUsageStatusResponder \ + | certificateUsageSSLCA ) + enum op_mode { OP_NONE, OP_AUTH, @@ -136,6 +145,7 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, char *cert_b64 = NULL; char *multi = NULL; PRCList *node; + SECCertificateUsage returned_usage = 0; nss_ctx = NSS_InitContext(nss_db, "", "", SECMOD_DB, ¶meters, flags); if (nss_ctx == NULL) { @@ -329,8 +339,8 @@ int do_work(TALLOC_CTX *mem_ctx, const char *nss_db, rv = CERT_VerifyCertificateNow(handle, cert_list_node->cert, PR_TRUE, certificateUsageCheckAllUsages, - NULL, NULL); - if (rv != SECSuccess) { + NULL, &returned_usage); + if (rv != SECSuccess || ((returned_usage & EXP_USAGES) == 0)) { DEBUG(SSSDBG_OP_FAILURE, "Certificate [%s][%s] not valid [%d][%s], skipping.\n", cert_list_node->cert->nickname,
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
