URL: https://github.com/SSSD/sssd/pull/5760
Author: dpward
Title: #5760: p11_child: Fixes for init_p11_ctx() and do_card()
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5760/head:pr5760
git checkout pr5760
From 47bf9eed05f34a9b47fd0466c7c7693a668c42a8 Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Tue, 24 Aug 2021 23:18:36 -0400
Subject: [PATCH 1/3] Revert "p11_child: do_card partially fix loop exit
condition when searching for token"
Commit b9f8c2f99d04da6d75bdde5111f2a389e1faff8b broke existing assumptions
in the code about the control flow. If --wait_for_card is given, and slots
are present but none of them contain a token, then all PKCS#11 modules are
now unloaded and reloaded repeatedly; that is not intended. The code after
the for-loop which handles --wait_for_card is no longer called either.
Instead, this code most likely requires refactoring in order to fix #5025,
either partially or fully.
---
src/p11_child/p11_child_openssl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index b5a10de570..78c05f5cb3 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1745,7 +1745,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
}
- if ((info.flags & CKF_REMOVABLE_DEVICE) && (info.flags & CKF_TOKEN_PRESENT)) {
+ if ((info.flags & CKF_REMOVABLE_DEVICE)) {
break;
}
}
From a59d2c0c4a2f5290259f01bb42596852275148dd Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Tue, 24 Aug 2021 23:18:36 -0400
Subject: [PATCH 2/3] p11_child: Ensure OpenSSL cleanup is performed
OpenSSL is initialized during init_p11_ctx(), which also sets a destructor
that will perform OpenSSL cleanup when p11_ctx is freed.
During init_verification(), the destructor for p11_ctx is replaced, and as
a result OpenSSL cleanup will no longer occur. Merge these destructors into
one which works correctly whether or not init_verification() was called.
Additionally, OpenSSL cleanup does not occur if the memory allocation for
p11_ctx fails. Re-order the steps in init_p11_ctx() so this is not needed.
---
src/p11_child/p11_child_openssl.c | 34 ++++++++++++++++---------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 78c05f5cb3..db4fc08353 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -570,8 +570,10 @@ static char *get_pkcs11_uri(TALLOC_CTX *mem_ctx, CK_INFO *module_info,
return uri_str;
}
-static int talloc_cleanup_openssl(struct p11_ctx *p11_ctx)
+static int p11_ctx_destructor(struct p11_ctx *p11_ctx)
{
+ X509_STORE_free(p11_ctx->x509_store);
+
CRYPTO_cleanup_all_ex_data();
return 0;
@@ -583,6 +585,12 @@ errno_t init_p11_ctx(TALLOC_CTX *mem_ctx, const char *ca_db,
int ret;
struct p11_ctx *ctx;
+ ctx = talloc_zero(mem_ctx, struct p11_ctx);
+ if (ctx == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
+ return ENOMEM;
+ }
+
/* See https://wiki.openssl.org/index.php/Library_Initialization for
* details. */
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
@@ -592,29 +600,24 @@ errno_t init_p11_ctx(TALLOC_CTX *mem_ctx, const char *ca_db,
#endif
if (ret != 1) {
DEBUG(SSSDBG_FATAL_FAILURE, "Failed to initialize OpenSSL.\n");
- return EIO;
- }
-
- ctx = talloc_zero(mem_ctx, struct p11_ctx);
- if (ctx == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "talloc_zero failed.\n");
- return ENOMEM;
+ ret = EIO;
+ goto done;
}
ctx->ca_db = ca_db;
ctx->wait_for_card = wait_for_card;
- talloc_set_destructor(ctx, talloc_cleanup_openssl);
+ talloc_set_destructor(ctx, p11_ctx_destructor);
*p11_ctx = ctx;
- return EOK;
-}
+ ret = EOK;
-static int talloc_free_x509_store(struct p11_ctx *p11_ctx)
-{
- X509_STORE_free(p11_ctx->x509_store);
+done:
+ if (ret != EOK) {
+ talloc_free(ctx);
+ }
- return 0;
+ return ret;
}
static int ensure_verify_param(X509_VERIFY_PARAM **verify_param_out)
@@ -702,7 +705,6 @@ errno_t init_verification(struct p11_ctx *p11_ctx,
p11_ctx->x509_store = store;
p11_ctx->cert_verify_opts = cert_verify_opts;
- talloc_set_destructor(p11_ctx, talloc_free_x509_store);
ret = EOK;
From f71440c90fd505baeba11ea22ca5855bb86e3dbe Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Tue, 24 Aug 2021 23:18:36 -0400
Subject: [PATCH 3/3] p11_child: Handle failure from p11_kit_uri_new()
---
src/p11_child/p11_child_openssl.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index db4fc08353..e7feeabd65 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1650,6 +1650,11 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
if (uri_str != NULL) {
uri = p11_kit_uri_new();
+ if (uri == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "p11_kit_uri_new failed.\n");
+ return ENOMEM;
+ }
+
ret = p11_kit_uri_parse(uri_str, P11_KIT_URI_FOR_ANY, uri);
if (ret != P11_KIT_URI_OK) {
DEBUG(SSSDBG_OP_FAILURE, "p11_kit_uri_parse failed [%d][%s].\n",
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
