125 lines
3.8 KiB
Diff
125 lines
3.8 KiB
Diff
|
From 2c10ae315375730020108cbcae0c282d0d6eff5f Mon Sep 17 00:00:00 2001
|
||
|
From: Jakub Jelen <jjelen@redhat.com>
|
||
|
Date: Mon, 26 Aug 2019 17:42:06 +0200
|
||
|
Subject: [PATCH 1/2] vcard_emul_nss: Drop the key caching to simplify error
|
||
|
handling
|
||
|
|
||
|
It could happen with PKCS#11 modules that (correctly) invalidate object
|
||
|
handles after logout (which was introduced in 0d3a683a), that the handles
|
||
|
are not valid when we try to use the objects again.
|
||
|
|
||
|
This is trying to address this use case, which I noticed was breaking
|
||
|
CI with SoftHSM PKCS#11 modules.
|
||
|
|
||
|
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
|
||
|
---
|
||
|
src/vcard_emul_nss.c | 15 +--------------
|
||
|
1 file changed, 1 insertion(+), 14 deletions(-)
|
||
|
|
||
|
diff --git a/src/vcard_emul_nss.c b/src/vcard_emul_nss.c
|
||
|
index e8f5c56..f788964 100644
|
||
|
--- a/src/vcard_emul_nss.c
|
||
|
+++ b/src/vcard_emul_nss.c
|
||
|
@@ -52,7 +52,6 @@ typedef enum {
|
||
|
struct VCardKeyStruct {
|
||
|
CERTCertificate *cert;
|
||
|
PK11SlotInfo *slot;
|
||
|
- SECKEYPrivateKey *key;
|
||
|
VCardEmulTriState failedX509;
|
||
|
};
|
||
|
|
||
|
@@ -155,10 +154,6 @@ vcard_emul_make_key(PK11SlotInfo *slot, CERTCertificate *cert)
|
||
|
key = g_new(VCardKey, 1);
|
||
|
key->slot = PK11_ReferenceSlot(slot);
|
||
|
key->cert = CERT_DupCertificate(cert);
|
||
|
- /* NOTE: if we aren't logged into the token, this could return NULL */
|
||
|
- /* NOTE: the cert is a temp cert, not necessarily the cert in the token,
|
||
|
- * use the DER version of this function */
|
||
|
- key->key = PK11_FindKeyByDERCert(slot, cert, NULL);
|
||
|
key->failedX509 = VCardEmulUnknown;
|
||
|
return key;
|
||
|
}
|
||
|
@@ -170,10 +165,6 @@ vcard_emul_delete_key(VCardKey *key)
|
||
|
if (!nss_emul_init || (key == NULL)) {
|
||
|
return;
|
||
|
}
|
||
|
- if (key->key) {
|
||
|
- SECKEY_DestroyPrivateKey(key->key);
|
||
|
- key->key = NULL;
|
||
|
- }
|
||
|
if (key->cert) {
|
||
|
CERT_DestroyCertificate(key->cert);
|
||
|
}
|
||
|
@@ -189,12 +180,8 @@ vcard_emul_delete_key(VCardKey *key)
|
||
|
static SECKEYPrivateKey *
|
||
|
vcard_emul_get_nss_key(VCardKey *key)
|
||
|
{
|
||
|
- if (key->key) {
|
||
|
- return key->key;
|
||
|
- }
|
||
|
/* NOTE: if we aren't logged into the token, this could return NULL */
|
||
|
- key->key = PK11_FindPrivateKeyFromCert(key->slot, key->cert, NULL);
|
||
|
- return key->key;
|
||
|
+ return PK11_FindPrivateKeyFromCert(key->slot, key->cert, NULL);
|
||
|
}
|
||
|
|
||
|
/*
|
||
|
--
|
||
|
2.22.0
|
||
|
|
||
|
|
||
|
From 06587ef683373690f61540935b4516b4f23238ea Mon Sep 17 00:00:00 2001
|
||
|
From: Jakub Jelen <jjelen@redhat.com>
|
||
|
Date: Tue, 27 Aug 2019 12:38:45 +0200
|
||
|
Subject: [PATCH 2/2] tests: Reproducer for pkcs11 modules invalidating object
|
||
|
handles on logout
|
||
|
|
||
|
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
|
||
|
---
|
||
|
tests/hwtests.c | 21 +++++++++++++++++++++
|
||
|
1 file changed, 21 insertions(+)
|
||
|
|
||
|
diff --git a/tests/hwtests.c b/tests/hwtests.c
|
||
|
index cd9a33b..39decfb 100644
|
||
|
--- a/tests/hwtests.c
|
||
|
+++ b/tests/hwtests.c
|
||
|
@@ -339,6 +339,26 @@ static void test_sign_bad_data_x509(void)
|
||
|
vreader_free(reader); /* get by id ref */
|
||
|
}
|
||
|
|
||
|
+/* This is a regression test for issues with PKCS#11 tokens
|
||
|
+ * invalidating object handles after logout (such as softhsm).
|
||
|
+ * See: https://bugzilla.mozilla.org/show_bug.cgi?id=1576642
|
||
|
+ */
|
||
|
+static void test_sign_logout_sign(void)
|
||
|
+{
|
||
|
+ VReader *reader = vreader_get_reader_by_id(0);
|
||
|
+
|
||
|
+ g_assert_nonnull(reader);
|
||
|
+
|
||
|
+ test_login();
|
||
|
+ test_sign();
|
||
|
+
|
||
|
+ /* This implicitly logs out the user */
|
||
|
+ test_login();
|
||
|
+ test_sign();
|
||
|
+
|
||
|
+ vreader_free(reader); /* get by id ref */
|
||
|
+}
|
||
|
+
|
||
|
static void libcacard_finalize(void)
|
||
|
{
|
||
|
VReader *reader = vreader_get_reader_by_id(0);
|
||
|
@@ -374,6 +394,7 @@ int main(int argc, char *argv[])
|
||
|
g_test_add_func("/hw-tests/sign-bad-data", test_sign_bad_data_x509);
|
||
|
g_test_add_func("/hw-tests/empty-applets", test_empty_applets);
|
||
|
g_test_add_func("/hw-tests/get-response", test_get_response);
|
||
|
+ g_test_add_func("/hw-tests/sign-logout-sign", test_sign_logout_sign);
|
||
|
|
||
|
ret = g_test_run();
|
||
|
|
||
|
--
|
||
|
2.22.0
|
||
|
|
||
|
|