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