URL: https://github.com/SSSD/sssd/pull/5872
Author: dpward
Title: #5872: p11_child: Fixes for authentication
Action: opened
PR body:
"""
This addresses several issues when searching for a token for
(pre-)authentication, in particular when `--wait_for_card` is used. It includes
a partial fix for #5025 (RHBZ 1989996), so that tokens for authentication may
be inserted into any slot on a single PKCS #11 module.
This also includes fixes for possible memory leaks and unhandled error
conditions in this part of the code.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5872/head:pr5872
git checkout pr5872
From d8954f01437ebd777eed9d36256703fd9d722637 Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 01/11] p11_child: Fix printing of non-null-terminated strings
in wait_for_card()
---
src/p11_child/p11_child_openssl.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index be9bc42421..89649d71a8 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1609,9 +1609,14 @@ static errno_t wait_for_card(CK_FUNCTION_LIST *module, CK_SLOT_ID *slot_id,
return EIO;
}
DEBUG(SSSDBG_TRACE_ALL,
- "Description [%s] Manufacturer [%s] flags [%lu] "
+ "Description [%.*s] Manufacturer [%.*s] flags [%lu] "
"removable [%s] token present [%s].\n",
- info->slotDescription, info->manufacturerID, info->flags,
+ (int) p11_kit_space_strlen(info->slotDescription,
+ sizeof(info->slotDescription)),
+ info->slotDescription,
+ (int) p11_kit_space_strlen(info->manufacturerID,
+ sizeof(info->manufacturerID)),
+ info->manufacturerID, info->flags,
(info->flags & CKF_REMOVABLE_DEVICE) ? "true": "false",
(info->flags & CKF_TOKEN_PRESENT) ? "true": "false");
From 59e9ee3607d9ad31d6fbc8fad920e6d1c5d9ed7a Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 02/11] p11_child: Include return value of PKCS #11 API calls
in debug messages
---
src/p11_child/p11_child_openssl.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 89649d71a8..36f3e5516d 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1605,7 +1605,8 @@ static errno_t wait_for_card(CK_FUNCTION_LIST *module, CK_SLOT_ID *slot_id,
rv = module->C_GetSlotInfo(*slot_id, info);
if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetSlotInfo failed\n");
+ DEBUG(SSSDBG_OP_FAILURE, "C_GetSlotInfo failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
return EIO;
}
DEBUG(SSSDBG_TRACE_ALL,
@@ -1706,7 +1707,8 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
memset(&module_info, 0, sizeof(CK_INFO));
rv = modules[c]->C_GetInfo(&module_info);
if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetInfo failed.\n");
+ DEBUG(SSSDBG_OP_FAILURE, "C_GetInfo failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
ret = EIO;
goto done;
}
@@ -1722,7 +1724,8 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
num_slots = MAX_SLOTS;
rv = modules[c]->C_GetSlotList(CK_FALSE, slots, &num_slots);
if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetSlotList failed.\n");
+ DEBUG(SSSDBG_OP_FAILURE, "C_GetSlotList failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
ret = EIO;
goto done;
}
@@ -1730,7 +1733,9 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
for (s = 0; s < num_slots; s++) {
rv = modules[c]->C_GetSlotInfo(slots[s], &info);
if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetSlotInfo failed\n");
+ DEBUG(SSSDBG_OP_FAILURE,
+ "C_GetSlotInfo failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
ret = EIO;
goto done;
}
@@ -1761,7 +1766,9 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
if ((info.flags & CKF_TOKEN_PRESENT) && uri != NULL) {
rv = modules[c]->C_GetTokenInfo(slots[s], &token_info);
if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetTokenInfo failed.\n");
+ DEBUG(SSSDBG_OP_FAILURE,
+ "C_GetTokenInfo failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
ret = EIO;
goto done;
}
@@ -1833,7 +1840,8 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
rv = module->C_GetTokenInfo(slot_id, &token_info);
if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetTokenInfo failed.\n");
+ DEBUG(SSSDBG_OP_FAILURE, "C_GetTokenInfo failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
ret = EIO;
goto done;
}
From b02d05c12d18ae2f954ae2b492c44403bc806d1b Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 03/11] p11_child: Make debug messages about URI matching more
specific
Indicate whether the URI does not match the module info, slot info, slot ID
or token info. Only print the URI once in the debug messages.
---
src/p11_child/p11_child_openssl.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 36f3e5516d..65e232190d 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1649,7 +1649,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
CK_ULONG num_slots;
CK_SLOT_ID slots[MAX_SLOTS];
CK_SLOT_ID slot_id;
- CK_SLOT_ID uri_slot_id;
+ CK_SLOT_ID uri_slot_id = -1;
CK_SLOT_INFO info;
CK_TOKEN_INFO token_info;
CK_INFO module_info;
@@ -1682,6 +1682,10 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
ret = EINVAL;
goto done;
}
+
+ uri_slot_id = p11_kit_uri_get_slot_id(uri);
+
+ DEBUG(SSSDBG_TRACE_ALL, "URI: %s\n", uri_str);
}
@@ -1716,7 +1720,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
/* Skip modules which do not match the PKCS#11 URI */
if (p11_kit_uri_match_module_info(uri, &module_info) != 1) {
DEBUG(SSSDBG_TRACE_ALL,
- "Not matching URI [%s], skipping.\n", uri_str);
+ "Module info does not match URI; skipping.\n");
continue;
}
}
@@ -1753,12 +1757,16 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
/* Skip slots which do not match the PKCS#11 URI */
if (uri != NULL) {
- uri_slot_id = p11_kit_uri_get_slot_id(uri);
- if ((uri_slot_id != (CK_SLOT_ID)-1
- && uri_slot_id != slots[s])
- || p11_kit_uri_match_slot_info(uri, &info) != 1) {
+ if (uri_slot_id != (CK_SLOT_ID)-1
+ && uri_slot_id != slots[s]) {
DEBUG(SSSDBG_TRACE_ALL,
- "Not matching URI [%s], skipping.\n", uri_str);
+ "Slot ID does not match URI; skipping.\n");
+ continue;
+ }
+
+ if (p11_kit_uri_match_slot_info(uri, &info) != 1) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ "Slot info does not match URI; skipping.\n");
continue;
}
}
@@ -1778,8 +1786,8 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
token_info.label);
if (p11_kit_uri_match_token_info(uri, &token_info) != 1) {
- DEBUG(SSSDBG_CONF_SETTINGS,
- "No matching uri [%s], skipping.\n", uri_str);
+ DEBUG(SSSDBG_TRACE_ALL,
+ "Token info does not match URI; skipping.\n");
continue;
}
From c46a4249f13cad12f7ec51d6c43a7eb2fa1dbe90 Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 04/11] p11_child: Perform URI matching inside wait_for_card()
If the slot or token does not match the URI, continue waiting for another
token instead of failing.
---
src/p11_child/p11_child_openssl.c | 59 ++++++++++++++++++++++++-------
1 file changed, 47 insertions(+), 12 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 65e232190d..662e22f0c1 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1584,11 +1584,17 @@ static int sign_data(CK_FUNCTION_LIST *module, CK_SESSION_HANDLE session,
}
static errno_t wait_for_card(CK_FUNCTION_LIST *module, CK_SLOT_ID *slot_id,
- CK_SLOT_INFO *info)
+ CK_SLOT_INFO *info, CK_TOKEN_INFO *token_info,
+ P11KitUri *uri)
{
+ CK_SLOT_ID uri_slot_id = -1;
CK_FLAGS wait_flags = 0;
CK_RV rv;
+ if (uri != NULL) {
+ uri_slot_id = p11_kit_uri_get_slot_id(uri);
+ }
+
do {
rv = module->C_WaitForSlotEvent(wait_flags, slot_id, NULL);
if (rv != CKR_OK && rv != CKR_FUNCTION_NOT_SUPPORTED) {
@@ -1622,10 +1628,46 @@ static errno_t wait_for_card(CK_FUNCTION_LIST *module, CK_SLOT_ID *slot_id,
(info->flags & CKF_TOKEN_PRESENT) ? "true": "false");
/* Check if really a token is present */
- if ((info->flags & CKF_REMOVABLE_DEVICE)
- && (info->flags & CKF_TOKEN_PRESENT)) {
- break;
+ if (!(info->flags & CKF_REMOVABLE_DEVICE)
+ || !(info->flags & CKF_TOKEN_PRESENT)) {
+ continue;
+ }
+
+ if (uri != NULL) {
+ if (uri_slot_id != (CK_SLOT_ID)-1
+ && uri_slot_id != *slot_id) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ "Slot ID does not match URI; skipping.\n");
+ continue;
+ }
+
+ if (p11_kit_uri_match_slot_info(uri, info) != 1) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ "Slot info does not match URI; skipping.\n");
+ continue;
+ }
}
+
+ rv = module->C_GetTokenInfo(*slot_id, token_info);
+ if (rv != CKR_OK) {
+ DEBUG(SSSDBG_OP_FAILURE, "C_GetTokenInfo failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
+ return EIO;
+ }
+ DEBUG(SSSDBG_TRACE_ALL, "Token label [%.*s].\n",
+ (int) p11_kit_space_strlen(token_info->label,
+ sizeof(token_info->label)),
+ token_info->label);
+
+ if (uri != NULL) {
+ if (p11_kit_uri_match_token_info(uri, token_info) != 1) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ "Token info does not match URI; skipping.\n");
+ continue;
+ }
+ }
+
+ break;
} while (true);
return EOK;
@@ -1835,7 +1877,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
if (!(info.flags & CKF_TOKEN_PRESENT)) {
DEBUG(SSSDBG_TRACE_ALL, "Token not present.\n");
if (p11_ctx->wait_for_card) {
- ret = wait_for_card(module, &slot_id, &info);
+ ret = wait_for_card(module, &slot_id, &info, &token_info, uri);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "wait_for_card failed.\n");
goto done;
@@ -1854,13 +1896,6 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
goto done;
}
- if (uri != NULL && p11_kit_uri_match_token_info(uri, &token_info) != 1) {
- DEBUG(SSSDBG_CONF_SETTINGS, "No token matching uri [%s] found.",
- uri_str);
- ret = ENOENT;
- goto done;
- }
-
module_id = c;
slot_name = p11_kit_space_strdup(info.slotDescription,
sizeof(info.slotDescription));
From 61da1c619cc8a64b211c49a5e4e3e1f2c055809c Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 05/11] p11_child: Check if module supports
C_WaitForSlotEvent()
If the module does not support blocking calls to C_WaitForSlotEvent(), use
non-blocking calls separated by a one-second delay. If these calls are not
supported either, then return with failure.
Before this change, if blocking calls were not supported, wait_for_card()
passed an uninitialized slot ID to C_GetSlotInfo() after a 10-second wait.
---
src/p11_child/p11_child.h | 2 ++
src/p11_child/p11_child_openssl.c | 15 +++++++++------
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/p11_child/p11_child.h b/src/p11_child/p11_child.h
index cbc5a9df95..8f99f265d5 100644
--- a/src/p11_child/p11_child.h
+++ b/src/p11_child/p11_child.h
@@ -31,6 +31,8 @@
/* Time to wait during a C_Finalize C_Initialize cycle to discover
* new slots. */
#define PKCS11_FINIALIZE_INITIALIZE_WAIT_TIME 3
+/* Time to wait for new slot events. */
+#define PKCS11_SLOT_EVENT_WAIT_TIME 1
struct p11_ctx;
struct cert_verify_opts {
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 662e22f0c1..24f6f7379c 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1597,18 +1597,21 @@ static errno_t wait_for_card(CK_FUNCTION_LIST *module, CK_SLOT_ID *slot_id,
do {
rv = module->C_WaitForSlotEvent(wait_flags, slot_id, NULL);
- if (rv != CKR_OK && rv != CKR_FUNCTION_NOT_SUPPORTED) {
+ if (rv == CKR_FUNCTION_NOT_SUPPORTED
+ && !(wait_flags & CKF_DONT_BLOCK)) {
+ wait_flags |= CKF_DONT_BLOCK;
+ continue;
+ } else if (rv == CKR_NO_EVENT) {
+ /* Poor man's wait */
+ sleep(PKCS11_SLOT_EVENT_WAIT_TIME);
+ continue;
+ } else if (rv != CKR_OK) {
DEBUG(SSSDBG_OP_FAILURE,
"C_WaitForSlotEvent failed [%lu][%s].\n",
rv, p11_kit_strerror(rv));
return EIO;
}
- if (rv == CKR_FUNCTION_NOT_SUPPORTED) {
- /* Poor man's wait */
- sleep(10);
- }
-
rv = module->C_GetSlotInfo(*slot_id, info);
if (rv != CKR_OK) {
DEBUG(SSSDBG_OP_FAILURE, "C_GetSlotInfo failed [%lu][%s].\n",
From 26f7ebef941e278b45ac389778f3543bdb3ca703 Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 06/11] p11_child: Allow slot changes to take effect before
resuming search
After the slot list has been obtained with C_GetSlotList(), a module cannot
expose any new or removed slots, until C_GetSlotList() is called again with
NULL as the second argument.
Do this instead of reloading all of the modules before resuming the search
for a slot/token.
---
src/p11_child/p11_child.h | 3 ---
src/p11_child/p11_child_openssl.c | 33 ++++++++++++++++++++-----------
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/src/p11_child/p11_child.h b/src/p11_child/p11_child.h
index 8f99f265d5..a1b30bc896 100644
--- a/src/p11_child/p11_child.h
+++ b/src/p11_child/p11_child.h
@@ -28,9 +28,6 @@
/* for CK_MECHANISM_TYPE */
#include <p11-kit/pkcs11.h>
-/* Time to wait during a C_Finalize C_Initialize cycle to discover
- * new slots. */
-#define PKCS11_FINIALIZE_INITIALIZE_WAIT_TIME 3
/* Time to wait for new slot events. */
#define PKCS11_SLOT_EVENT_WAIT_TIME 1
struct p11_ctx;
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 24f6f7379c..1c88835488 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1770,6 +1770,16 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
}
}
+ /* After obtaining the module's slot list (previously in this loop),
+ * this call is needed to let any changes in slots take effect. */
+ rv = modules[c]->C_GetSlotList(CK_FALSE, NULL, &num_slots);
+ if (rv != CKR_OK) {
+ DEBUG(SSSDBG_OP_FAILURE, "C_GetSlotList failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
+ ret = EIO;
+ goto done;
+ }
+
num_slots = MAX_SLOTS;
rv = modules[c]->C_GetSlotList(CK_FALSE, slots, &num_slots);
if (rv != CKR_OK) {
@@ -1854,18 +1864,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
/* When e.g. using Yubikeys the slot isn't present until the device is
* inserted, so we should wait for a slot as well. */
if (p11_ctx->wait_for_card && module == NULL) {
- p11_kit_modules_finalize_and_release(modules);
-
- sleep(PKCS11_FINIALIZE_INITIALIZE_WAIT_TIME);
-
- modules = p11_kit_modules_load_and_initialize(0);
- if (modules == NULL) {
- DEBUG(SSSDBG_OP_FAILURE,
- "p11_kit_modules_load_and_initialize failed.\n");
- ret = EIO;
- goto done;
- }
-
+ sleep(PKCS11_SLOT_EVENT_WAIT_TIME);
} else {
break;
}
@@ -1880,6 +1879,16 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
if (!(info.flags & CKF_TOKEN_PRESENT)) {
DEBUG(SSSDBG_TRACE_ALL, "Token not present.\n");
if (p11_ctx->wait_for_card) {
+ /* After obtaining the module's slot list (in the loop above), this
+ * call is needed to let any changes in slots take effect. */
+ rv = module->C_GetSlotList(CK_FALSE, NULL, &num_slots);
+ if (rv != CKR_OK) {
+ DEBUG(SSSDBG_OP_FAILURE, "C_GetSlotList failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
+ ret = EIO;
+ goto done;
+ }
+
ret = wait_for_card(module, &slot_id, &info, &token_info, uri);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "wait_for_card failed.\n");
From 0294457e4d50986d99d9e68fdb72e24765a1e473 Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 07/11] p11_child: Adjust exit conditions when looping over
modules/slots
When a slot is found that supports removable tokens, set "module". If the
slot contains a usable token, set "slot_id", and use this condition to exit
the loop immediately.
With this change, the flags in the slot info can be checked earlier.
---
src/p11_child/p11_child_openssl.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 1c88835488..d0ffcf59af 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1693,7 +1693,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
char *mod_file_name;
CK_ULONG num_slots;
CK_SLOT_ID slots[MAX_SLOTS];
- CK_SLOT_ID slot_id;
+ CK_SLOT_ID slot_id = -1;
CK_SLOT_ID uri_slot_id = -1;
CK_SLOT_INFO info;
CK_TOKEN_INFO token_info;
@@ -1810,6 +1810,16 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
(info.flags & CKF_REMOVABLE_DEVICE) ? "true": "false",
(info.flags & CKF_TOKEN_PRESENT) ? "true": "false");
+ if (!(info.flags & CKF_REMOVABLE_DEVICE)) {
+ continue;
+ }
+
+ module = modules[c];
+
+ if (!(info.flags & CKF_TOKEN_PRESENT)) {
+ continue;
+ }
+
/* Skip slots which do not match the PKCS#11 URI */
if (uri != NULL) {
if (uri_slot_id != (CK_SLOT_ID)-1
@@ -1826,7 +1836,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
}
}
- if ((info.flags & CKF_TOKEN_PRESENT) && uri != NULL) {
+ if (uri != NULL) {
rv = modules[c]->C_GetTokenInfo(slots[s], &token_info);
if (rv != CKR_OK) {
DEBUG(SSSDBG_OP_FAILURE,
@@ -1848,15 +1858,10 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
}
- if ((info.flags & CKF_REMOVABLE_DEVICE)) {
- module = modules[c];
- slot_id = slots[s];
- if ((info.flags & CKF_TOKEN_PRESENT)) {
- break;
- }
- }
+ slot_id = slots[s];
+ break;
}
- if (s != num_slots) {
+ if (slot_id != (CK_SLOT_ID)-1) {
break;
}
}
@@ -1876,7 +1881,7 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
goto done;
}
- if (!(info.flags & CKF_TOKEN_PRESENT)) {
+ if (slot_id == (CK_SLOT_ID)-1) {
DEBUG(SSSDBG_TRACE_ALL, "Token not present.\n");
if (p11_ctx->wait_for_card) {
/* After obtaining the module's slot list (in the loop above), this
From 8e2896bbc17dd6dc8be4cccf5be65e857a62fba7 Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 08/11] p11_child: Skip uninitialized tokens
These cannot be used for authentication, and attempting to open a session
results in failure.
With this change, obtain token_info unconditionally when looping over
slots/tokens.
---
src/p11_child/p11_child_openssl.c | 47 +++++++++++++++++--------------
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index d0ffcf59af..52ce930c4f 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1657,6 +1657,12 @@ static errno_t wait_for_card(CK_FUNCTION_LIST *module, CK_SLOT_ID *slot_id,
rv, p11_kit_strerror(rv));
return EIO;
}
+
+ if (!(token_info->flags & CKF_TOKEN_INITIALIZED)) {
+ DEBUG(SSSDBG_TRACE_ALL, "Token is not initialized; skipping.\n");
+ continue;
+ }
+
DEBUG(SSSDBG_TRACE_ALL, "Token label [%.*s].\n",
(int) p11_kit_space_strlen(token_info->label,
sizeof(token_info->label)),
@@ -1836,20 +1842,27 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
}
}
- if (uri != NULL) {
- rv = modules[c]->C_GetTokenInfo(slots[s], &token_info);
- if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE,
- "C_GetTokenInfo failed [%lu][%s].\n",
- rv, p11_kit_strerror(rv));
- ret = EIO;
- goto done;
- }
- DEBUG(SSSDBG_TRACE_ALL, "Token label [%.*s].\n",
- (int) p11_kit_space_strlen(token_info.label,
- sizeof(token_info.label)),
- token_info.label);
+ rv = modules[c]->C_GetTokenInfo(slots[s], &token_info);
+ if (rv != CKR_OK) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "C_GetTokenInfo failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
+ ret = EIO;
+ goto done;
+ }
+ if (!(token_info.flags & CKF_TOKEN_INITIALIZED)) {
+ DEBUG(SSSDBG_TRACE_ALL,
+ "Token is not initialized; skipping.\n");
+ continue;
+ }
+
+ DEBUG(SSSDBG_TRACE_ALL, "Token label [%.*s].\n",
+ (int) p11_kit_space_strlen(token_info.label,
+ sizeof(token_info.label)),
+ token_info.label);
+
+ if (uri != NULL) {
if (p11_kit_uri_match_token_info(uri, &token_info) != 1) {
DEBUG(SSSDBG_TRACE_ALL,
"Token info does not match URI; skipping.\n");
@@ -1905,14 +1918,6 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
}
}
- rv = module->C_GetTokenInfo(slot_id, &token_info);
- if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetTokenInfo failed [%lu][%s].\n",
- rv, p11_kit_strerror(rv));
- ret = EIO;
- goto done;
- }
-
module_id = c;
slot_name = p11_kit_space_strdup(info.slotDescription,
sizeof(info.slotDescription));
From 0c047e48074f417148d882fb54f453695d2c8fc1 Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 09/11] p11_child: Combine subsequent loops over certificate
list
With this change, obtain module_info unconditionally when looping over
slots/tokens.
---
src/p11_child/p11_child_openssl.c | 41 +++++++++++--------------------
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 52ce930c4f..91ff7321e5 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1758,17 +1758,16 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
free(mod_name);
free(mod_file_name);
- if (uri != NULL) {
- memset(&module_info, 0, sizeof(CK_INFO));
- rv = modules[c]->C_GetInfo(&module_info);
- if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetInfo failed [%lu][%s].\n",
- rv, p11_kit_strerror(rv));
- ret = EIO;
- goto done;
- }
+ rv = modules[c]->C_GetInfo(&module_info);
+ if (rv != CKR_OK) {
+ DEBUG(SSSDBG_OP_FAILURE, "C_GetInfo failed [%lu][%s].\n",
+ rv, p11_kit_strerror(rv));
+ ret = EIO;
+ goto done;
+ }
- /* Skip modules which do not match the PKCS#11 URI */
+ /* Skip modules which do not match the PKCS#11 URI */
+ if (uri != NULL) {
if (p11_kit_uri_match_module_info(uri, &module_info) != 1) {
DEBUG(SSSDBG_TRACE_ALL,
"Module info does not match URI; skipping.\n");
@@ -2011,6 +2010,12 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
|| (key_id_in != NULL && item->id != NULL
&& strcmp(key_id_in, item->id) == 0)))) {
+ item->uri = get_pkcs11_uri(mem_ctx, &module_info, &info, slot_id,
+ &token_info,
+ &item->attributes[1] /* label */,
+ &item->attributes[0] /* id */);
+ DEBUG(SSSDBG_TRACE_ALL, "uri: %s.\n", item->uri);
+
tmp_cert = talloc_memdup(mem_ctx, item, sizeof(struct cert_list));
if (tmp_cert == NULL) {
DEBUG(SSSDBG_OP_FAILURE, "talloc_memdup failed.\n");
@@ -2025,22 +2030,6 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
}
}
- memset(&module_info, 0, sizeof(CK_INFO));
- rv = module->C_GetInfo(&module_info);
- if (rv != CKR_OK) {
- DEBUG(SSSDBG_OP_FAILURE, "C_GetInfo failed.\n");
- ret = EIO;
- goto done;
- }
-
- DLIST_FOR_EACH(item, cert_list) {
- item->uri = get_pkcs11_uri(mem_ctx, &module_info, &info, slot_id,
- &token_info,
- &item->attributes[1] /* label */,
- &item->attributes[0] /* id */);
- DEBUG(SSSDBG_TRACE_ALL, "uri: %s.\n", item->uri);
- }
-
/* TODO: check module_name_in, token_name_in, key_id_in */
if (cert_list == NULL) {
From bd56cbcfef14af3da66c8940a0e30a97b69e9e1b Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 10/11] p11_child: Filter certificate list in place
A subset of the items in all_cert_list are copied in memory and added to
cert_list. all_cert_list does not get used again, and its items are never
freed directly. Instead, just populate cert_list and remove the unwanted
items from it (freeing their memory after doing so).
---
src/p11_child/p11_child_openssl.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 91ff7321e5..3e5d95a61b 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1710,10 +1710,9 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
char *slot_name = NULL;
char *token_name = NULL;
CK_SESSION_HANDLE session = 0;
- struct cert_list *all_cert_list = NULL;
struct cert_list *cert_list = NULL;
struct cert_list *item = NULL;
- struct cert_list *tmp_cert = NULL;
+ struct cert_list *next_item = NULL;
char *multi = NULL;
bool pkcs11_session = false;
bool pkcs11_login = false;
@@ -1971,13 +1970,13 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
DEBUG(SSSDBG_TRACE_ALL, "Login NOT required.\n");
}
- ret = read_certs(mem_ctx, module, session, p11_ctx, &all_cert_list);
+ ret = read_certs(mem_ctx, module, session, p11_ctx, &cert_list);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "read_certs failed.\n");
goto done;
}
- DLIST_FOR_EACH(item, all_cert_list) {
+ DLIST_FOR_EACH_SAFE(item, next_item, cert_list) {
/* Check if we found the certificates we needed for authentication or
* the requested ones for pre-auth. For authentication all attributes
* except the label must be given and match. The label is optional for
@@ -2016,17 +2015,9 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
&item->attributes[0] /* id */);
DEBUG(SSSDBG_TRACE_ALL, "uri: %s.\n", item->uri);
- tmp_cert = talloc_memdup(mem_ctx, item, sizeof(struct cert_list));
- if (tmp_cert == NULL) {
- DEBUG(SSSDBG_OP_FAILURE, "talloc_memdup failed.\n");
- ret = ENOMEM;
- goto done;
- }
- tmp_cert->prev = NULL;
- tmp_cert->next = NULL;
-
- DLIST_ADD(cert_list, tmp_cert);
-
+ } else {
+ DLIST_REMOVE(cert_list, item);
+ talloc_free(item);
}
}
From ec89cfba691cbb2692987be588b1c7e2c0ac91ac Mon Sep 17 00:00:00 2001
From: David Ward <david.w...@ll.mit.edu>
Date: Thu, 11 Nov 2021 11:00:00 -0500
Subject: [PATCH 11/11] p11_child: Handle failure when obtaining module list or
names
---
src/p11_child/p11_child_openssl.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c
index 3e5d95a61b..a06b6d2ff0 100644
--- a/src/p11_child/p11_child_openssl.c
+++ b/src/p11_child/p11_child_openssl.c
@@ -1744,16 +1744,32 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
if (modules == NULL) {
DEBUG(SSSDBG_OP_FAILURE,
"p11_kit_modules_load_and_initialize failed.\n");
- return EIO;
+ ret = EIO;
+ goto done;
}
for (;;) {
DEBUG(SSSDBG_TRACE_ALL, "Module List:\n");
for (c = 0; modules[c] != NULL; c++) {
mod_name = p11_kit_module_get_name(modules[c]);
+ if (mod_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "p11_kit_module_get_name failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
mod_file_name = p11_kit_module_get_filename(modules[c]);
+ if (mod_file_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE,
+ "p11_kit_module_get_filename failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
+
DEBUG(SSSDBG_TRACE_ALL, "common name: [%s].\n", mod_name);
DEBUG(SSSDBG_TRACE_ALL, "dll name: [%s].\n", mod_file_name);
+
free(mod_name);
free(mod_file_name);
@@ -1934,6 +1950,11 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx,
}
module_file_name = p11_kit_module_get_filename(module);
+ if (module_file_name == NULL) {
+ DEBUG(SSSDBG_OP_FAILURE, "p11_kit_module_get_filename failed.\n");
+ ret = ENOMEM;
+ goto done;
+ }
DEBUG(SSSDBG_TRACE_ALL, "Found [%s] in slot [%s][%d] of module [%d][%s].\n",
token_name, slot_name, (int) slot_id, (int) module_id,
_______________________________________________
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
