From 4d427176895d698c6f25f046bebd3eb210a80d1f Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 1 Aug 2024 10:59:53 -0400 Subject: [PATCH 1/3] Make helper write titles to provider debug file Makes it easier to check what operations each test sends to the token by writing the title of "PARA" sections to the provider's debug file. Signed-off-by: Simo Sorce --- tests/helpers.sh | 3 +++ tests/setup-kryoptic.sh | 1 + tests/setup-softhsm.sh | 1 + tests/setup-softokn.sh | 1 + 4 files changed, 6 insertions(+) diff --git a/tests/helpers.sh b/tests/helpers.sh index a9c0d0d6..6fc1f2cb 100755 --- a/tests/helpers.sh +++ b/tests/helpers.sh @@ -23,6 +23,9 @@ title() shift 1 echo "" echo "## $*" + if [ -f "${PPDBGFILE}" ]; then + echo "[TEST]: $*" >> "${PPDBGFILE}" + fi ;; "LINE") shift 1 diff --git a/tests/setup-kryoptic.sh b/tests/setup-kryoptic.sh index b62ca822..176c550d 100755 --- a/tests/setup-kryoptic.sh +++ b/tests/setup-kryoptic.sh @@ -403,6 +403,7 @@ title LINE "Export test variables to ${TMPPDIR}/testvars" cat >> "${TMPPDIR}/testvars" <> "${TMPPDIR}/testvars" < "${TMPPDIR}/testvars" < Date: Thu, 1 Aug 2024 10:58:34 -0400 Subject: [PATCH 2/3] Add test to force ECDH on the token Among other things this tests that a public key sourced by a PEM file gets correctly imported in the token for the on-token ECDH operation. Signed-off-by: Simo Sorce --- .reuse/dep5 | 1 + tests/tecdh | 27 +++++++++++++++++++++++++++ tests/testp256.pri.pem | 5 +++++ tests/testp256.pub.pem | 4 ++++ 4 files changed, 37 insertions(+) create mode 100644 tests/testp256.pri.pem create mode 100644 tests/testp256.pub.pem diff --git a/.reuse/dep5 b/.reuse/dep5 index 16aa7698..c0941da9 100644 --- a/.reuse/dep5 +++ b/.reuse/dep5 @@ -25,6 +25,7 @@ Files: .github/* docs/* tests/lsan.supp tools/openssl*.cnf + tests/*.pem Copyright: (C) 2022 Simo Sorce License: Apache-2.0 diff --git a/tests/tecdh b/tests/tecdh index 7969567e..b9a68ba4 100755 --- a/tests/tecdh +++ b/tests/tecdh @@ -10,4 +10,31 @@ pkeyutl -derive -inkey ${ECBASEURI} -peerkey ${ECPEERPUBURI} -out ${TMPPDIR}/secret.ecdh.bin' + +# Now test by forcing all operations on the token +title PARA "ECDH Exchange forcing PKCS11 Provider" +ORIG_OPENSSL_CONF=${OPENSSL_CONF} +sed -e "s/#MORECONF/alg_section = algorithm_sec\n\n[algorithm_sec]\ndefault_properties = ?provider=pkcs11/" \ + "${OPENSSL_CONF}" > "${OPENSSL_CONF}.forcetoken" +OPENSSL_CONF=${OPENSSL_CONF}.forcetoken +title PARA "ECDH Exchange forced: public key in file" +ossl ' +pkeyutl -derive -inkey ${ECBASEURI} + -peerkey ${TESTSSRCDIR}/testp256.pub.pem + -out ${TMPPDIR}/forced.pub.ecdh.bin' + +### Private EC Key import not supported yet +#title PARA "ECDH Exchange forced: private key in file" +#ossl ' +#pkeyutl -derive -inkey ${TESTSSRCDIR}/testp256.pri.pem +# -peerkey ${ECPEERPUBURI} +# -out ${TMPPDIR}/forced.pri.ecdh.bin' + +#title PARA "ECDH Exchange forced: both key in file" +#ossl ' +#pkeyutl -derive -inkey ${TESTSSRCDIR}/testp256.pri.pem +# -peerkey ${TESTSSRCDIR}/testp256.pub.pem +# -out ${TMPPDIR}/forced.both.ecdh.bin' +OPENSSL_CONF=${ORIG_OPENSSL_CONF} + exit 0 diff --git a/tests/testp256.pri.pem b/tests/testp256.pri.pem new file mode 100644 index 00000000..35c65dd3 --- /dev/null +++ b/tests/testp256.pri.pem @@ -0,0 +1,5 @@ +-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgQj80pwUNIHHjzQaJ +yP+vAPE8KPBmrVwafor5xar9sq+hRANCAATXOFIB00W2LsAwzDxBpg/uFzFu4uIK +5otxalZiroOusrSBYA/vS2MC/6vaR+zrdnxRlYoHIbhe7H+PlEHPuq/a +-----END PRIVATE KEY----- diff --git a/tests/testp256.pub.pem b/tests/testp256.pub.pem new file mode 100644 index 00000000..2a8c1f5d --- /dev/null +++ b/tests/testp256.pub.pem @@ -0,0 +1,4 @@ +-----BEGIN PUBLIC KEY----- +MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE1zhSAdNFti7AMMw8QaYP7hcxbuLi +CuaLcWpWYq6DrrK0gWAP70tjAv+r2kfs63Z8UZWKByG4Xux/j5RBz7qv2g== +-----END PUBLIC KEY----- From 02cb59f74a04f217acb613d54991492a46b27a91 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 1 Aug 2024 15:10:39 -0400 Subject: [PATCH 3/3] Fix issue found by addrss sanitizer If an attribute had an empty value (which is apparently possible with some tokens), the copy function would fail to initialize the pointer causing an invalid free when the attributes array is freed. Ensure pValue is zeroed on copy if the attri bute length is 0, and also ensure the attribute length is correctly set to 0. Then belt&suspender approach ensure the buffer is allocated with OPENSSL_zalloc instead of OPENSSL_malloc, which will eansure by default all addresses and legth are a safe default NULL/0 value. Signed-off-by: Simo Sorce --- src/objects.c | 2 +- src/util.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/objects.c b/src/objects.c index d0f97f04..e4519fa1 100644 --- a/src/objects.c +++ b/src/objects.c @@ -2762,7 +2762,7 @@ static CK_RV return_dup_key(P11PROV_OBJ *dst, P11PROV_OBJ *src) dst->cka_token = src->cka_token; dst->data.key = src->data.key; - dst->attrs = OPENSSL_malloc(sizeof(CK_ATTRIBUTE) * src->numattrs); + dst->attrs = OPENSSL_zalloc(sizeof(CK_ATTRIBUTE) * src->numattrs); if (!dst->attrs) { rv = CKR_HOST_MEMORY; P11PROV_raise(dst->ctx, rv, "Failed allocation"); diff --git a/src/util.c b/src/util.c index 59f52796..a6140f8f 100644 --- a/src/util.c +++ b/src/util.c @@ -1016,8 +1016,10 @@ CK_RV p11prov_copy_attr(CK_ATTRIBUTE *dst, CK_ATTRIBUTE *src) return CKR_HOST_MEMORY; } memcpy(dst->pValue, src->pValue, src->ulValueLen); - dst->ulValueLen = src->ulValueLen; + } else { + dst->pValue = NULL; } + dst->ulValueLen = src->ulValueLen; dst->type = src->type; return CKR_OK;