From 8458e1b1b35e69ecdc57c5c92c5780c38695f3f0 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@redhat.com>
Date: Thu, 16 Jun 2022 22:22:17 +0200
Subject: [PATCH 1/3] m4: Update code coverage
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
---
m4/ax_code_coverage.m4 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/m4/ax_code_coverage.m4 b/m4/ax_code_coverage.m4
index 3d36924..352165b 100644
--- a/m4/ax_code_coverage.m4
+++ b/m4/ax_code_coverage.m4
@@ -74,7 +74,7 @@
# You should have received a copy of the GNU Lesser General Public License
# along with this program. If not, see <https://www.gnu.org/licenses/>.
-#serial 32
+#serial 34
m4_define(_AX_CODE_COVERAGE_RULES,[
AX_ADD_AM_MACRO_STATIC([
@@ -138,7 +138,7 @@ CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT ?=\
CODE_COVERAGE_GENHTML_OPTIONS ?= \$(CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT)
CODE_COVERAGE_IGNORE_PATTERN ?=
-GITIGNOREFILES = \$(GITIGNOREFILES) \$(CODE_COVERAGE_OUTPUT_FILE) \$(CODE_COVERAGE_OUTPUT_DIRECTORY)
+GITIGNOREFILES := \$(GITIGNOREFILES) \$(CODE_COVERAGE_OUTPUT_FILE) \$(CODE_COVERAGE_OUTPUT_DIRECTORY)
code_coverage_v_lcov_cap = \$(code_coverage_v_lcov_cap_\$(V))
code_coverage_v_lcov_cap_ = \$(code_coverage_v_lcov_cap_\$(AM_DEFAULT_VERBOSITY))
code_coverage_v_lcov_cap_0 = @echo \" LCOV --capture\" \$(CODE_COVERAGE_OUTPUT_FILE);
@@ -175,7 +175,7 @@ code-coverage-clean:
code-coverage-dist-clean:
-A][M_DISTCHECK_CONFIGURE_FLAGS = \$(A][M_DISTCHECK_CONFIGURE_FLAGS) --disable-code-coverage
+A][M_DISTCHECK_CONFIGURE_FLAGS := \$(A][M_DISTCHECK_CONFIGURE_FLAGS) --disable-code-coverage
else # ifneq (\$(abs_builddir), \$(abs_top_builddir))
check-code-coverage:
--
2.35.3
From 0f4cd1279ea826a306bb0f10c691af9f0c40ad2e Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@redhat.com>
Date: Thu, 16 Jun 2022 22:23:03 +0200
Subject: [PATCH 2/3] Sort certificates by ID
This is needed to avoid non-deterministic order of the certificates in
case the underlying pkcs11 module does not guarantee that (such as
softhsm). Without this change, the signing and encryption certificate
might get mixed up and application might try to use wrong one for
verification or decryption.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
---
src/vcard_emul_nss.c | 43 +++++++++++++++++++++++++++++++++++++------
1 file changed, 37 insertions(+), 6 deletions(-)
diff --git a/src/vcard_emul_nss.c b/src/vcard_emul_nss.c
index b63105d..2d2062d 100644
--- a/src/vcard_emul_nss.c
+++ b/src/vcard_emul_nss.c
@@ -706,8 +706,9 @@ vcard_emul_mirror_card(VReader *vreader)
* us the real certs until we log in.
*/
PK11GenericObject *firstObj, *thisObj;
- int cert_count;
+ int cert_count, i;
unsigned char **certs;
+ SECItem **ids;
int *cert_len;
VCardKey **keys;
PK11SlotInfo *slot;
@@ -734,12 +735,13 @@ vcard_emul_mirror_card(VReader *vreader)
/* allocate the arrays */
vcard_emul_alloc_arrays(&certs, &cert_len, &keys, cert_count);
+ ids = g_new(SECItem *, cert_count);
/* fill in the arrays */
- cert_count = 0;
+ cert_count = i = 0;
for (thisObj = firstObj; thisObj;
thisObj = PK11_GetNextGenericObject(thisObj)) {
- SECItem derCert;
+ SECItem derCert, *id;
CERTCertificate *cert;
SECStatus rv;
@@ -749,22 +751,51 @@ vcard_emul_mirror_card(VReader *vreader)
if (rv != SECSuccess) {
continue;
}
+ /* Read ID and try to sort by this to get reproducible results
+ * in case of underlying pkcs11 module does not provide it */
+ id = SECITEM_AllocItem(NULL, NULL, 0);
+ rv = PK11_ReadRawAttribute(PK11_TypeGeneric, thisObj, CKA_ID, id);
+ if (rv != SECSuccess) {
+ SECITEM_FreeItem(&derCert, PR_FALSE);
+ SECITEM_FreeItem(id, PR_TRUE);
+ continue;
+ }
/* create floating temp cert. This gives us a cert structure even if
* the token isn't logged in */
cert = CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &derCert,
NULL, PR_FALSE, PR_TRUE);
SECITEM_FreeItem(&derCert, PR_FALSE);
if (cert == NULL) {
+ SECITEM_FreeItem(id, PR_TRUE);
continue;
}
- certs[cert_count] = cert->derCert.data;
- cert_len[cert_count] = cert->derCert.len;
- keys[cert_count] = vcard_emul_make_key(slot, cert);
+ for (i = 0; i < cert_count; i++) {
+ if (SECITEM_CompareItem(id, ids[i]) < SECEqual) {
+ int j;
+ /* Make space for the item here, move the rest of the items */
+ for (j = cert_count; j > i; j--) {
+ certs[j] = certs[j - 1];
+ cert_len[j] = cert_len[j - 1];
+ keys[j] = keys[j - 1];
+ ids[j] = ids[j - 1];
+ }
+ break;
+ }
+ }
+ certs[i] = cert->derCert.data;
+ cert_len[i] = cert->derCert.len;
+ keys[i] = vcard_emul_make_key(slot, cert);
+ ids[i] = id;
cert_count++;
CERT_DestroyCertificate(cert); /* key obj still has a reference */
}
PK11_DestroyGenericObjects(firstObj);
+ /* No longer needed */
+ for (i = 0; i < cert_count; i++) {
+ SECITEM_FreeItem(ids[i], PR_TRUE);
+ }
+ g_free(ids);
/* now create the card */
card = vcard_emul_make_card(vreader, certs, cert_len, keys, cert_count);
--
2.35.3
From 1a415a16f9d3d914e3d1f5b45d3e6b30160280c9 Mon Sep 17 00:00:00 2001
From: Jakub Jelen <jjelen@redhat.com>
Date: Fri, 17 Jun 2022 12:36:18 +0200
Subject: [PATCH 3/3] Implement tests with second PKI object
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
---
tests/common.c | 42 ++++++++++++++++++++++++++++++++++------
tests/common.h | 15 +++++++-------
tests/db2.crypt | Bin 0 -> 256 bytes
tests/hwtests.c | 27 ++++++++++++++++++++++++--
tests/libcacard.c | 36 +++++++++++++++++++++++++++++++++-
tests/setup-softhsm2.sh | 2 +-
6 files changed, 105 insertions(+), 17 deletions(-)
create mode 100644 tests/db2.crypt
diff --git a/tests/common.c b/tests/common.c
index e5bc3e2..d1681f2 100644
--- a/tests/common.c
+++ b/tests/common.c
@@ -192,7 +192,7 @@ void get_properties_coid(VReader *reader, const unsigned char coid[2],
case 0x43: /* PKI properties */
g_assert_cmphex(p2[0], ==, 0x06);
- if (hw_tests) {
+ if (hw_tests && object_type == TEST_PKI) {
/* Assuming CAC card with 1024 b RSA keys */
key_bits = 1024;
} else {
@@ -248,7 +248,7 @@ void get_properties_coid(VReader *reader, const unsigned char coid[2],
g_assert_cmpint(num_objects_expected, ==, 0);
}
- if (object_type == TEST_PKI) {
+ if (object_type == TEST_PKI || object_type == TEST_PKI_2) {
g_assert_cmpint(verified_pki_properties, ==, 1);
}
@@ -307,12 +307,17 @@ void get_properties(VReader *reader, int object_type)
unsigned char coid[2];
switch (object_type) {
case TEST_PKI:
- // XXX only the first PKI for now
coid[0] = 0x01;
coid[1] = 0x00;
get_properties_coid(reader, coid, object_type);
break;
+ case TEST_PKI_2:
+ coid[0] = 0x01;
+ coid[1] = 0x01;
+ get_properties_coid(reader, coid, object_type);
+ break;
+
case TEST_CCC:
coid[0] = 0xDB;
coid[1] = 0x00;
@@ -426,6 +431,10 @@ void select_applet(VReader *reader, int type)
/* Select first PKI Applet */
0xa0, 0x00, 0x00, 0x00, 0x79, 0x01, 0x00
};
+ uint8_t selfile_pki_2[] = {
+ /* Select second PKI Applet */
+ 0xa0, 0x00, 0x00, 0x00, 0x79, 0x01, 0x01
+ };
uint8_t selfile_passthrough[] = {
/* Select Person Instance (passthrough) */
0xa0, 0x00, 0x00, 0x00, 0x79, 0x02, 0x00
@@ -442,6 +451,11 @@ void select_applet(VReader *reader, int type)
aid_len = sizeof(selfile_pki);
break;
+ case TEST_PKI_2:
+ aid = selfile_pki_2;
+ aid_len = sizeof(selfile_pki_2);
+ break;
+
case TEST_CCC:
aid = selfile_ccc;
aid_len = sizeof(selfile_ccc);
@@ -562,7 +576,7 @@ void do_sign(VReader *reader, int parts)
}
-void do_decipher(VReader *reader)
+void do_decipher(VReader *reader, int type)
{
VReaderStatus status;
int dwRecvLength = APDUBufSize;
@@ -589,14 +603,30 @@ void do_decipher(VReader *reader)
/* Read the encrypted file */
if (hw_tests) {
- filename = g_test_build_filename(G_TEST_BUILT, "01.crypt", NULL);
+ const char *name;
+ if (type == TEST_PKI) {
+ name = "01.crypt";
+ } else if (type == TEST_PKI_2) {
+ name = "02.crypt";
+ } else {
+ g_assert_not_reached();
+ }
+ filename = g_test_build_filename(G_TEST_BUILT, name, NULL);
} else {
/* Generated from existing db using:
* echo "1234567890" > data
* certutil -L -d sql:$PWD/tests/db/ -n cert1 -r > tests/db.cert
* openssl rsautl -encrypt -inkey "tests/db.cert" -keyform DER -certin -in data -out "tests/db.crypt"
*/
- filename = g_test_build_filename(G_TEST_DIST, "db.crypt", NULL);
+ const char *name;
+ if (type == TEST_PKI) {
+ name = "db.crypt";
+ } else if (type == TEST_PKI_2) {
+ name = "db2.crypt";
+ } else {
+ g_assert_not_reached();
+ }
+ filename = g_test_build_filename(G_TEST_DIST, name, NULL);
}
if (!g_file_get_contents(filename, &ciphertext, &ciphertext_len, NULL)) {
g_test_skip("The encrypted file not found");
diff --git a/tests/common.h b/tests/common.h
index db217b4..459d980 100644
--- a/tests/common.h
+++ b/tests/common.h
@@ -17,12 +17,13 @@
enum {
TEST_PKI = 1,
- TEST_CCC = 2,
- TEST_ACA = 3,
- TEST_GENERIC = 4,
- TEST_EMPTY_BUFFER = 5,
- TEST_EMPTY = 6,
- TEST_PASSTHROUGH = 7,
+ TEST_PKI_2,
+ TEST_CCC,
+ TEST_ACA,
+ TEST_GENERIC,
+ TEST_EMPTY_BUFFER,
+ TEST_EMPTY,
+ TEST_PASSTHROUGH,
};
void select_coid_good(VReader *reader, unsigned char *coid);
@@ -40,7 +41,7 @@ void read_buffer(VReader *reader, uint8_t type, int object_type);
void do_sign(VReader *reader, int parts);
-void do_decipher(VReader *reader);
+void do_decipher(VReader *reader, int type);
void test_empty_applets(void);
diff --git a/tests/hwtests.c b/tests/hwtests.c
index 3684642..2474578 100644
--- a/tests/hwtests.c
+++ b/tests/hwtests.c
@@ -256,6 +256,17 @@ static void test_sign(void)
/* test also multipart signatures */
do_sign(reader, 1);
+ /* select the second PKI */
+ select_applet(reader, TEST_PKI_2);
+
+ /* get properties to figure out the key length */
+ get_properties(reader, TEST_PKI_2);
+
+ do_sign(reader, 0);
+
+ /* test also multipart signatures */
+ do_sign(reader, 1);
+
vreader_free(reader); /* get by id ref */
}
@@ -281,7 +292,15 @@ static void test_decipher(void)
/* get properties to figure out the key length */
get_properties(reader, TEST_PKI);
- do_decipher(reader);
+ do_decipher(reader, TEST_PKI);
+
+ /* select the second PKI */
+ select_applet(reader, TEST_PKI_2);
+
+ /* get properties to figure out the key length */
+ get_properties(reader, TEST_PKI_2);
+
+ do_decipher(reader, TEST_PKI_2);
vreader_free(reader); /* get by id ref */
}
@@ -318,7 +337,7 @@ static void test_sign_bad_data_x509(void)
0x00 /* <-- [Le] */
};
int sign_len = sizeof(sign);
- int key_bits = getBits();
+ int key_bits;
g_assert_nonnull(reader);
@@ -329,6 +348,10 @@ static void test_sign_bad_data_x509(void)
return;
}
+ /* get properties to figure out the key length */
+ select_applet(reader, TEST_PKI);
+ get_properties(reader, TEST_PKI);
+
/* run the actual test */
key_bits = getBits();
diff --git a/tests/libcacard.c b/tests/libcacard.c
index 5328ace..37dedbb 100644
--- a/tests/libcacard.c
+++ b/tests/libcacard.c
@@ -515,6 +515,25 @@ static void test_cac_pki(void)
vreader_free(reader); /* get by id ref */
}
+static void test_cac_pki_2(void)
+{
+ VReader *reader = vreader_get_reader_by_id(0);
+
+ /* select the first PKI applet */
+ select_applet(reader, TEST_PKI_2);
+
+ /* get properties */
+ get_properties(reader, TEST_PKI_2);
+
+ /* get the TAG buffer length */
+ read_buffer(reader, CAC_FILE_TAG, TEST_PKI_2);
+
+ /* get the VALUE buffer length */
+ read_buffer(reader, CAC_FILE_VALUE, TEST_PKI_2);
+
+ vreader_free(reader); /* get by id ref */
+}
+
static void test_cac_ccc(void)
{
VReader *reader = vreader_get_reader_by_id(0);
@@ -579,6 +598,14 @@ static void test_sign(void)
/* test also multipart signatures */
do_sign(reader, 1);
+ /* select the second PKI */
+ select_applet(reader, TEST_PKI_2);
+
+ do_sign(reader, 0);
+
+ /* test also multipart signatures */
+ do_sign(reader, 1);
+
vreader_free(reader); /* get by id ref */
}
@@ -594,7 +621,12 @@ static void test_decipher(void)
/* select the PKI */
select_applet(reader, TEST_PKI);
- do_decipher(reader);
+ do_decipher(reader, TEST_PKI);
+
+ /* select the PKI */
+ select_applet(reader, TEST_PKI_2);
+
+ do_decipher(reader, TEST_PKI_2);
vreader_free(reader); /* get by id ref */
}
@@ -925,6 +957,7 @@ static void test_invalid_read_buffer(void)
test_invalid_read_buffer_applet(reader, TEST_CCC);
test_invalid_read_buffer_applet(reader, TEST_PKI);
+ test_invalid_read_buffer_applet(reader, TEST_PKI_2);
test_invalid_read_buffer_applet(reader, TEST_PASSTHROUGH);
test_invalid_read_buffer_applet(reader, TEST_EMPTY);
@@ -1122,6 +1155,7 @@ int main(int argc, char *argv[])
g_test_add_func("/libcacard/xfer", test_xfer);
g_test_add_func("/libcacard/select-coid", test_select_coid);
g_test_add_func("/libcacard/cac-pki", test_cac_pki);
+ g_test_add_func("/libcacard/cac-pki-2", test_cac_pki_2);
g_test_add_func("/libcacard/cac-ccc", test_cac_ccc);
g_test_add_func("/libcacard/cac-aca", test_cac_aca);
g_test_add_func("/libcacard/get-response", test_get_response);
diff --git a/tests/setup-softhsm2.sh b/tests/setup-softhsm2.sh
index c3874e5..f187191 100755
--- a/tests/setup-softhsm2.sh
+++ b/tests/setup-softhsm2.sh
@@ -111,7 +111,7 @@ if [ ! -d "tokens" ]; then
# Generate 1024b RSA Key pair
generate_cert "RSA:1024" "01" "RSA_auth"
- #generate_cert "RSA:1024" "02" "RSA_sign"
+ generate_cert "RSA:2048" "02" "RSA_sign"
fi
# NSS DB
if [ ! -d "$NSSDB" ]; then
--
2.35.3