From 13a849ee04f9d082d6b37e028ae1148c594eb149 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Thu, 17 Sep 2026 09:06:29 -0700 Subject: [PATCH 1/2] F-13239 - Copy CKO_DATA payload attributes in C_CopyObject --- src/crypto.c | 8 +- src/internal.c | 101 +++++++++++-- tests/copy_data_object_test.c | 145 ++++++++++++++++++ tests/copy_data_object_token_test.c | 218 ++++++++++++++++++++++++++++ tests/include.am | 15 ++ 5 files changed, 474 insertions(+), 13 deletions(-) create mode 100644 tests/copy_data_object_test.c create mode 100644 tests/copy_data_object_token_test.c diff --git a/src/crypto.c b/src/crypto.c index 1282b4b6..4046b10c 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -1797,10 +1797,12 @@ CK_RV C_CopyObject(CK_SESSION_HANDLE hSession, CK_OBJECT_HANDLE hObject, } /* copy all the attributes from the original object to the new object */ - rv = WP11_Object_Copy(obj, newObj); - if (rv != CKR_OK) { + ret = WP11_Object_Copy(obj, newObj); + if (ret != 0) { WP11_Object_Free(newObj); - return rv; + if (ret == MEMORY_E) + return CKR_DEVICE_MEMORY; + return CKR_FUNCTION_FAILED; } if (pTemplate != NULL) { diff --git a/src/internal.c b/src/internal.c index 501f0bed..18d3c1e2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2757,19 +2757,56 @@ static long GetRsaExponentValue(unsigned char* eData, word32 eSz) #define OBJ_COPY_DATA(src, dest, field) \ do { \ - if (src->field != NULL) { \ - dest->field = (unsigned char*)XMALLOC(src->field##Len, NULL, \ - DYNAMIC_TYPE_TMP_BUFFER); \ - if (dest->field == NULL) \ - return MEMORY_E; \ - XMEMCPY(dest->field, src->field, src->field##Len); \ - dest->field##Len = src->field##Len; \ - } else { \ - dest->field = NULL; \ - dest->field##Len = 0; \ + if (ret == 0) { \ + if (src->field != NULL) { \ + dest->field = (unsigned char*)XMALLOC(src->field##Len, NULL, \ + DYNAMIC_TYPE_TMP_BUFFER); \ + if (dest->field == NULL) \ + ret = MEMORY_E; \ + else { \ + XMEMCPY(dest->field, src->field, src->field##Len); \ + dest->field##Len = src->field##Len; \ + } \ + } else { \ + dest->field = NULL; \ + dest->field##Len = 0; \ + } \ } \ } while (0) +/** + * Duplicate a length-prefixed buffer for object copying. On success the + * destination owns a freshly allocated copy; on allocation failure the + * destination is left NULL and the caller frees any earlier copies through + * WP11_Object_Free. + */ +static int wp11_Object_CopyBuffer(byte* src, word32 srcLen, byte** dst, + word32* dstLen) +{ + int ret = 0; + + /* A NULL buffer with a non-zero length is an inconsistent source; reject + * it rather than silently producing an empty copy. */ + if (src == NULL && srcLen != 0) + return BAD_FUNC_ARG; + + if (src != NULL && srcLen > 0) { + *dst = (byte*)XMALLOC(srcLen, NULL, DYNAMIC_TYPE_CERT); + if (*dst == NULL) + ret = MEMORY_E; + else { + XMEMCPY(*dst, src, srcLen); + *dstLen = srcLen; + } + } + else { + *dst = NULL; + *dstLen = 0; + } + + return ret; +} + /** * Copy an object. Not all fields are supported. * @param src [in] Source object. @@ -2786,6 +2823,12 @@ int WP11_Object_Copy(WP11_Object *src, WP11_Object *dest) /* We save data copying for the last step */ + /* Copy the common mutable fields, and a data object's payload, under the + * source lock so a concurrent C_SetAttributeValue cannot free any of them + * mid-copy. */ + if (src->onToken) + WP11_Lock_LockRO(src->lock); + dest->size = src->size; #ifndef WOLFPKCS11_NO_STORE OBJ_COPY_DATA(src, dest, keyData); @@ -2806,6 +2849,30 @@ int WP11_Object_Copy(WP11_Object *src, WP11_Object *dest) dest->category = src->category; dest->devId = src->devId; + if (ret == 0 && src->objClass == CKO_DATA) { + ret = wp11_Object_CopyBuffer(src->data.genericData.data, + src->data.genericData.dataLen, + &dest->data.genericData.data, &dest->data.genericData.dataLen); + if (ret == 0) { + ret = wp11_Object_CopyBuffer(src->data.genericData.application, + src->data.genericData.applicationLen, + &dest->data.genericData.application, + &dest->data.genericData.applicationLen); + } + if (ret == 0) { + ret = wp11_Object_CopyBuffer(src->data.genericData.objectId, + src->data.genericData.objectIdLen, + &dest->data.genericData.objectId, + &dest->data.genericData.objectIdLen); + } + } + + if (src->onToken) + WP11_Lock_UnlockRO(src->lock); + + if (ret != 0) + return ret; + if (src->objClass == CKO_CERTIFICATE) { return BAD_FUNC_ARG; } @@ -2814,6 +2881,9 @@ int WP11_Object_Copy(WP11_Object *src, WP11_Object *dest) return BAD_FUNC_ARG; } #endif + else if (src->objClass == CKO_DATA) { + /* Payload copied above under the source lock. */ + } else { #ifdef WOLFPKCS11_TPM /* Handle TPM keys - copy tpmKey structure directly */ @@ -10479,8 +10549,19 @@ void WP11_Object_Free(WP11_Object* object) certFreed = 1; } else if (object->objClass == CKO_DATA) { + /* A data object's value may hold keying material, so clear each + * payload buffer before releasing it. */ + if (object->data.genericData.data != NULL) + wc_ForceZero(object->data.genericData.data, + object->data.genericData.dataLen); XFREE(object->data.genericData.data, NULL, DYNAMIC_TYPE_CERT); + if (object->data.genericData.application != NULL) + wc_ForceZero(object->data.genericData.application, + object->data.genericData.applicationLen); XFREE(object->data.genericData.application, NULL, DYNAMIC_TYPE_CERT); + if (object->data.genericData.objectId != NULL) + wc_ForceZero(object->data.genericData.objectId, + object->data.genericData.objectIdLen); XFREE(object->data.genericData.objectId, NULL, DYNAMIC_TYPE_CERT); } else { diff --git a/tests/copy_data_object_test.c b/tests/copy_data_object_test.c new file mode 100644 index 00000000..0216dd4f --- /dev/null +++ b/tests/copy_data_object_test.c @@ -0,0 +1,145 @@ +/* copy_data_object_test.c + * + * Copyright (C) 2006-2025 wolfSSL Inc. + * + * This file is part of wolfPKCS11. + * + * wolfPKCS11 is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfPKCS11 is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + * + * Copying a data object must reproduce its payload attributes (value, + * application, and object id) in the copy. + */ + +#ifdef HAVE_CONFIG_H + #include +#endif + +#include +#include + +#ifndef WOLFSSL_USER_SETTINGS + #include +#endif +#include +#include + +#ifndef WOLFPKCS11_USER_SETTINGS + #include +#endif +#include + +#ifndef HAVE_PKCS11_STATIC +#include +#endif + +#include "testdata.h" +#include "pkcs11_test_util.h" + +#define TEST_DIR "./store/copy_data_object_test" + +static const byte valueData[] = "the-quick-brown-fox-payload"; +static const byte appData[] = "wolfPKCS11-test-application"; +static const byte objectIdData[] = { 0x2A, 0x03, 0x04, 0x05, 0x06, 0x07 }; + +static int check_attr(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE obj, + CK_ATTRIBUTE_TYPE type, const byte* expect, + CK_ULONG expectLen, const char* name) +{ + CK_RV rv; + byte buf[64]; + CK_ATTRIBUTE attr; + + attr.type = type; + attr.pValue = buf; + attr.ulValueLen = sizeof(buf); + rv = funcList->C_GetAttributeValue(session, obj, &attr, 1); + CHECK_RV(rv, name, CKR_OK); + if (rv != CKR_OK) + return -1; + CHECK_TRUE(attr.ulValueLen == expectLen && + XMEMCMP(buf, expect, expectLen) == 0, name); + return 0; +} + +static int run_test(void) +{ + CK_RV rv; + CK_SESSION_HANDLE session = 0; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE copy = CK_INVALID_HANDLE; + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_BBOOL ckFalse = CK_FALSE; + CK_ATTRIBUTE createTmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + { CKA_TOKEN, &ckFalse, sizeof(ckFalse) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, + { CKA_VALUE, (void*)valueData, sizeof(valueData) - 1 }, + { CKA_APPLICATION, (void*)appData, sizeof(appData) - 1 }, + { CKA_OBJECT_ID, (void*)objectIdData, sizeof(objectIdData) }, + }; + CK_ULONG createTmplCnt = sizeof(createTmpl) / sizeof(*createTmpl); + + rv = pkcs11_load(); + CHECK_RV(rv, "load library", CKR_OK); + if (rv != CKR_OK) + return -1; + + rv = pkcs11_open_session(&session); + CHECK_RV(rv, "open session", CKR_OK); + if (rv != CKR_OK) + goto out; + + rv = funcList->C_CreateObject(session, createTmpl, createTmplCnt, &obj); + CHECK_RV(rv, "C_CreateObject(data object)", CKR_OK); + if (rv != CKR_OK) + goto out; + + rv = funcList->C_CopyObject(session, obj, NULL, 0, ©); + CHECK_RV(rv, "C_CopyObject(data object)", CKR_OK); + if (rv != CKR_OK) + goto out; + + check_attr(session, copy, CKA_VALUE, valueData, sizeof(valueData) - 1, + "copy preserves CKA_VALUE"); + check_attr(session, copy, CKA_APPLICATION, appData, sizeof(appData) - 1, + "copy preserves CKA_APPLICATION"); + check_attr(session, copy, CKA_OBJECT_ID, objectIdData, + sizeof(objectIdData), "copy preserves CKA_OBJECT_ID"); + +out: + if (copy != CK_INVALID_HANDLE) + funcList->C_DestroyObject(session, copy); + if (obj != CK_INVALID_HANDLE) + funcList->C_DestroyObject(session, obj); + if (session != 0) + funcList->C_CloseSession(session); + funcList->C_Finalize(NULL); + pkcs11_unload(); + return 0; +} + +int main(int argc, char* argv[]) +{ + (void)argc; + (void)argv; + +#ifndef WOLFPKCS11_NO_ENV + XSETENV("WOLFPKCS11_TOKEN_PATH", TEST_DIR, 1); +#endif + + printf("=== wolfPKCS11 C_CopyObject data-object payload test ===\n"); + run_test(); + return pkcs11_test_summary(); +} diff --git a/tests/copy_data_object_token_test.c b/tests/copy_data_object_token_test.c new file mode 100644 index 00000000..d288ed3a --- /dev/null +++ b/tests/copy_data_object_token_test.c @@ -0,0 +1,218 @@ +/* copy_data_object_token_test.c + * + * Copyright (C) 2006-2025 wolfSSL Inc. + * + * This file is part of wolfPKCS11. + * + * wolfPKCS11 is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfPKCS11 is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + * + * Copying a token data object reproduces its common attributes and payload. + * This exercises the copy path that reads the source object under its lock. + */ + +#ifdef HAVE_CONFIG_H + #include +#endif + +#include +#include + +#ifndef WOLFSSL_USER_SETTINGS + #include +#endif +#include +#include + +#ifndef WOLFPKCS11_USER_SETTINGS + #include +#endif +#include + +#ifndef HAVE_PKCS11_STATIC +#include +#endif + +#include "testdata.h" +#include "pkcs11_test_util.h" + +#define TEST_DIR "./store/copy_data_object_token_test" + +#ifndef WOLFPKCS11_NO_STORE + +static CK_SLOT_ID slot = 0; +static const char* soPin = "password123456"; +static const char* userPin = "wolfpkcs11-test"; +static const char tokenLabel[] = "copy-data-token"; + +static const byte valueData[] = "token-data-value-payload"; +static const byte idData[] = { 0x0A, 0x0B, 0x0C, 0x0D }; +static const char objLabel[] = "copy-data-token-object"; +static const byte objectIdData[] = { 0x51, 0x52, 0x53 }; + +static CK_RV token_setup(void) +{ + CK_C_INITIALIZE_ARGS args; + CK_SLOT_ID slotList[16]; + CK_ULONG slotCount = sizeof(slotList) / sizeof(slotList[0]); + CK_SESSION_HANDLE soSession = 0; + unsigned char label[32]; + CK_RV rv; + + XMEMSET(&args, 0, sizeof(args)); + args.flags = CKF_OS_LOCKING_OK; + rv = funcList->C_Initialize(&args); + if (rv != CKR_OK) + return rv; + rv = funcList->C_GetSlotList(CK_TRUE, slotList, &slotCount); + if (rv != CKR_OK) + return rv; + if (slotCount == 0) + return CKR_TOKEN_NOT_PRESENT; + slot = slotList[0]; + + XMEMSET(label, ' ', sizeof(label)); + XMEMCPY(label, tokenLabel, XSTRLEN(tokenLabel)); + rv = funcList->C_InitToken(slot, (CK_UTF8CHAR_PTR)soPin, + (CK_ULONG)XSTRLEN(soPin), label); + if (rv != CKR_OK) + return rv; + + rv = funcList->C_OpenSession(slot, CKF_SERIAL_SESSION | CKF_RW_SESSION, + NULL, NULL, &soSession); + if (rv != CKR_OK) + return rv; + rv = funcList->C_Login(soSession, CKU_SO, (CK_UTF8CHAR_PTR)soPin, + (CK_ULONG)XSTRLEN(soPin)); + if (rv == CKR_OK) { + rv = funcList->C_InitPIN(soSession, (CK_UTF8CHAR_PTR)userPin, + (CK_ULONG)XSTRLEN(userPin)); + } + funcList->C_Logout(soSession); + funcList->C_CloseSession(soSession); + return rv; +} + +static int check_attr(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE obj, + CK_ATTRIBUTE_TYPE type, const byte* expect, + CK_ULONG expectLen, const char* name) +{ + CK_RV rv; + byte buf[64]; + CK_ATTRIBUTE attr; + + attr.type = type; + attr.pValue = buf; + attr.ulValueLen = sizeof(buf); + rv = funcList->C_GetAttributeValue(session, obj, &attr, 1); + CHECK_RV(rv, name, CKR_OK); + if (rv != CKR_OK) + return -1; + CHECK_TRUE(attr.ulValueLen == expectLen && + XMEMCMP(buf, expect, expectLen) == 0, name); + return 0; +} + +static int run_test(void) +{ + CK_RV rv; + CK_SESSION_HANDLE session = 0; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_OBJECT_HANDLE copy = CK_INVALID_HANDLE; + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_BBOOL ckTrue = CK_TRUE; + CK_BBOOL ckFalse = CK_FALSE; + CK_ATTRIBUTE createTmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + { CKA_TOKEN, &ckTrue, sizeof(ckTrue) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, + { CKA_LABEL, (void*)objLabel, sizeof(objLabel) - 1 }, + { CKA_ID, (void*)idData, sizeof(idData) }, + { CKA_VALUE, (void*)valueData, sizeof(valueData) - 1 }, + { CKA_OBJECT_ID, (void*)objectIdData, sizeof(objectIdData) }, + }; + CK_ULONG createTmplCnt = sizeof(createTmpl) / sizeof(*createTmpl); + + rv = pkcs11_load(); + CHECK_RV(rv, "load library", CKR_OK); + if (rv != CKR_OK) + return -1; + + rv = token_setup(); + CHECK_RV(rv, "token setup", CKR_OK); + if (rv != CKR_OK) + goto out; + + rv = funcList->C_OpenSession(slot, CKF_SERIAL_SESSION | CKF_RW_SESSION, + NULL, NULL, &session); + CHECK_RV(rv, "open session", CKR_OK); + if (rv != CKR_OK) + goto out; + rv = funcList->C_Login(session, CKU_USER, (CK_UTF8CHAR_PTR)userPin, + (CK_ULONG)XSTRLEN(userPin)); + CHECK_RV(rv, "login user", CKR_OK); + if (rv != CKR_OK) + goto out; + + rv = funcList->C_CreateObject(session, createTmpl, createTmplCnt, &obj); + CHECK_RV(rv, "C_CreateObject(token data object)", CKR_OK); + if (rv != CKR_OK) + goto out; + + rv = funcList->C_CopyObject(session, obj, NULL, 0, ©); + CHECK_RV(rv, "C_CopyObject(token data object)", CKR_OK); + if (rv != CKR_OK) + goto out; + + check_attr(session, copy, CKA_LABEL, (const byte*)objLabel, + sizeof(objLabel) - 1, "copy preserves CKA_LABEL"); + check_attr(session, copy, CKA_ID, idData, sizeof(idData), + "copy preserves CKA_ID"); + check_attr(session, copy, CKA_VALUE, valueData, sizeof(valueData) - 1, + "copy preserves CKA_VALUE"); + check_attr(session, copy, CKA_OBJECT_ID, objectIdData, + sizeof(objectIdData), "copy preserves CKA_OBJECT_ID"); + +out: + if (copy != CK_INVALID_HANDLE) + funcList->C_DestroyObject(session, copy); + if (obj != CK_INVALID_HANDLE) + funcList->C_DestroyObject(session, obj); + if (session != 0) { + funcList->C_Logout(session); + funcList->C_CloseSession(session); + } + funcList->C_Finalize(NULL); + pkcs11_unload(); + return 0; +} +#endif /* !WOLFPKCS11_NO_STORE */ + +int main(int argc, char* argv[]) +{ + (void)argc; + (void)argv; + +#ifndef WOLFPKCS11_NO_ENV + XSETENV("WOLFPKCS11_TOKEN_PATH", TEST_DIR, 1); +#endif + + printf("=== wolfPKCS11 token data-object copy test ===\n"); +#ifndef WOLFPKCS11_NO_STORE + run_test(); +#else + printf("KeyStore not compiled in!\n"); +#endif + return pkcs11_test_summary(); +} diff --git a/tests/include.am b/tests/include.am index ce95e1d7..958c7ec4 100644 --- a/tests/include.am +++ b/tests/include.am @@ -260,6 +260,17 @@ noinst_PROGRAMS += tests/unique_id_test tests_unique_id_test_SOURCES = tests/unique_id_test.c tests_unique_id_test_LDADD = +check_PROGRAMS += tests/copy_data_object_test +noinst_PROGRAMS += tests/copy_data_object_test +tests_copy_data_object_test_SOURCES = tests/copy_data_object_test.c +tests_copy_data_object_test_LDADD = + + +check_PROGRAMS += tests/copy_data_object_token_test +noinst_PROGRAMS += tests/copy_data_object_token_test +tests_copy_data_object_token_test_SOURCES = tests/copy_data_object_token_test.c +tests_copy_data_object_token_test_LDADD = + if BUILD_STATIC tests_pkcs11test_LDADD += src/libwolfpkcs11.la tests_pkcs11mtt_LDADD += src/libwolfpkcs11.la @@ -313,6 +324,8 @@ tests_concurrent_destroy_object_test_LDADD += src/libwolfpkcs11.la tests_hbs_persistence_test_LDADD += src/libwolfpkcs11.la tests_hkdf_persistence_test_LDADD += src/libwolfpkcs11.la tests_unique_id_test_LDADD += src/libwolfpkcs11.la +tests_copy_data_object_test_LDADD += src/libwolfpkcs11.la +tests_copy_data_object_token_test_LDADD += src/libwolfpkcs11.la else tests_object_id_uniqueness_test_LDADD += src/libwolfpkcs11.la tests_empty_pin_store_test_LDADD += src/libwolfpkcs11.la @@ -358,6 +371,8 @@ tests_concurrent_destroy_object_test_LDADD += src/libwolfpkcs11.la tests_hbs_persistence_test_LDADD += src/libwolfpkcs11.la tests_hkdf_persistence_test_LDADD += src/libwolfpkcs11.la tests_unique_id_test_LDADD += src/libwolfpkcs11.la +tests_copy_data_object_test_LDADD += src/libwolfpkcs11.la +tests_copy_data_object_token_test_LDADD += src/libwolfpkcs11.la endif EXTRA_DIST += tests/unit.h \ From a807adde7ba994a2d8a204ede8fdb91aba612240 Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Thu, 17 Sep 2026 09:09:48 -0700 Subject: [PATCH 2/2] F-13240 - Preserve omitted attributes on data object updates --- src/crypto.c | 4 +- src/internal.c | 91 ++++++-------- tests/data_object_partial_update_test.c | 159 ++++++++++++++++++++++++ tests/include.am | 7 ++ wolfpkcs11/internal.h | 2 +- 5 files changed, 210 insertions(+), 53 deletions(-) create mode 100644 tests/data_object_partial_update_test.c diff --git a/src/crypto.c b/src/crypto.c index 4046b10c..08be7d30 100644 --- a/src/crypto.c +++ b/src/crypto.c @@ -888,6 +888,7 @@ static CK_RV SetAttributeValue(WP11_Session* session, WP11_Object* obj, int i, j; unsigned char* data[OBJ_MAX_PARAMS] = { 0, }; CK_ULONG len[OBJ_MAX_PARAMS] = { 0, }; + int present[OBJ_MAX_PARAMS] = { 0, }; CK_ATTRIBUTE_TYPE* attrs = NULL; int cnt; CK_BBOOL attrsFound = 0; @@ -1005,6 +1006,7 @@ static CK_RV SetAttributeValue(WP11_Session* session, WP11_Object* obj, for (j = 0; j < (int)ulCount; j++) { if (attrs[i] == pTemplate[j].type) { attrsFound = 1; + present[i] = 1; data[i] = (unsigned char*)pTemplate[j].pValue; if (data[i] == NULL) { /* For CKO_DATA, values can be NULL */ @@ -1027,7 +1029,7 @@ static CK_RV SetAttributeValue(WP11_Session* session, WP11_Object* obj, } #endif else if (objClass == CKO_DATA) { - ret = WP11_Object_DataObject(obj, data, len); + ret = WP11_Object_DataObject(obj, data, len, present); } else { /* Set the value and length of key specific attributes diff --git a/src/internal.c b/src/internal.c index 18d3c1e2..6655cfc1 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11435,69 +11435,58 @@ int WP11_Object_SetTrust(WP11_Object* object, unsigned char** data, } #endif -int WP11_Object_DataObject(WP11_Object* object, unsigned char** data, - CK_ULONG* len) +/* Update one generic-data field. An omitted attribute (present == 0) is left + * unchanged; a supplied attribute is replaced when it carries data, or cleared + * when it is empty. The old buffer may hold keying material, so it is zeroized + * before release. */ +static int wp11_SetGenericField(byte** field, word32* fieldLen, int present, + unsigned char* data, CK_ULONG len) { int ret = 0; - if (object->onToken) - WP11_Lock_LockRW(object->lock); + if (!present) + return 0; - if (data[0] != NULL && len[0] > 0) { - XFREE(object->data.genericData.data, NULL, DYNAMIC_TYPE_CERT); - object->data.genericData.data = - (byte*)XMALLOC(len[0], NULL, DYNAMIC_TYPE_CERT); - if (object->data.genericData.data == NULL) { - ret = MEMORY_E; - } - else { - XMEMCPY(object->data.genericData.data, data[0], len[0]); - object->data.genericData.dataLen = (word32)len[0]; - } - } - else if (data[0] == NULL) { - /* Clear data if not provided */ - XFREE(object->data.genericData.data, NULL, DYNAMIC_TYPE_CERT); - object->data.genericData.data = NULL; - object->data.genericData.dataLen = 0; + if (*field != NULL) { + wc_ForceZero(*field, *fieldLen); + XFREE(*field, NULL, DYNAMIC_TYPE_CERT); + *field = NULL; + *fieldLen = 0; } - if (ret == 0 && data[1] != NULL && len[1] > 0) { - XFREE(object->data.genericData.application, NULL, DYNAMIC_TYPE_CERT); - object->data.genericData.application = - (byte*)XMALLOC(len[1], NULL, DYNAMIC_TYPE_CERT); - if (object->data.genericData.application == NULL) { + if (data != NULL && len > 0) { + *field = (byte*)XMALLOC(len, NULL, DYNAMIC_TYPE_CERT); + if (*field == NULL) ret = MEMORY_E; - } else { - XMEMCPY(object->data.genericData.application, data[1], len[1]); - object->data.genericData.applicationLen = (word32)len[1]; + XMEMCPY(*field, data, len); + *fieldLen = (word32)len; } } - else if (ret == 0 && data[1] == NULL) { - /* Clear application if not provided */ - XFREE(object->data.genericData.application, NULL, DYNAMIC_TYPE_CERT); - object->data.genericData.application = NULL; - object->data.genericData.applicationLen = 0; - } - if (ret == 0 && data[2] != NULL && len[2] > 0) { - XFREE(object->data.genericData.objectId, NULL, DYNAMIC_TYPE_CERT); - object->data.genericData.objectId = - (byte*)XMALLOC(len[2], NULL, DYNAMIC_TYPE_CERT); - if (object->data.genericData.objectId == NULL) { - ret = MEMORY_E; - } - else { - XMEMCPY(object->data.genericData.objectId, data[2], len[2]); - object->data.genericData.objectIdLen = (word32)len[2]; - } + return ret; +} + +int WP11_Object_DataObject(WP11_Object* object, unsigned char** data, + CK_ULONG* len, int* present) +{ + int ret; + + if (object->onToken) + WP11_Lock_LockRW(object->lock); + + ret = wp11_SetGenericField(&object->data.genericData.data, + &object->data.genericData.dataLen, + present[0], data[0], len[0]); + if (ret == 0) { + ret = wp11_SetGenericField(&object->data.genericData.application, + &object->data.genericData.applicationLen, + present[1], data[1], len[1]); } - else if (ret == 0 && data[2] == NULL) { - /* Clear object ID if not provided */ - XFREE(object->data.genericData.objectId, NULL, DYNAMIC_TYPE_CERT); - object->data.genericData.objectId = NULL; - object->data.genericData.objectIdLen = 0; + if (ret == 0) { + ret = wp11_SetGenericField(&object->data.genericData.objectId, + &object->data.genericData.objectIdLen, + present[2], data[2], len[2]); } if (object->onToken) diff --git a/tests/data_object_partial_update_test.c b/tests/data_object_partial_update_test.c new file mode 100644 index 00000000..1f011dd0 --- /dev/null +++ b/tests/data_object_partial_update_test.c @@ -0,0 +1,159 @@ +/* data_object_partial_update_test.c + * + * Copyright (C) 2006-2025 wolfSSL Inc. + * + * This file is part of wolfPKCS11. + * + * wolfPKCS11 is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfPKCS11 is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA + * + * Updating one attribute of a data object must leave the attributes the + * template omits unchanged, while a supplied empty attribute clears that one. + */ + +#ifdef HAVE_CONFIG_H + #include +#endif + +#include +#include + +#ifndef WOLFSSL_USER_SETTINGS + #include +#endif +#include +#include + +#ifndef WOLFPKCS11_USER_SETTINGS + #include +#endif +#include + +#ifndef HAVE_PKCS11_STATIC +#include +#endif + +#include "testdata.h" +#include "pkcs11_test_util.h" + +#define TEST_DIR "./store/data_object_partial_update_test" + +static const byte valueData[] = "original-value"; +static const byte appData[] = "original-application"; +static const byte objectIdData[] = { 0x11, 0x22, 0x33, 0x44 }; +static const byte newValue[] = "updated-value-payload"; + +static int expect_attr(CK_SESSION_HANDLE session, CK_OBJECT_HANDLE obj, + CK_ATTRIBUTE_TYPE type, const byte* expect, + CK_ULONG expectLen, const char* name) +{ + CK_RV rv; + byte buf[64]; + CK_ATTRIBUTE attr; + + attr.type = type; + attr.pValue = buf; + attr.ulValueLen = sizeof(buf); + rv = funcList->C_GetAttributeValue(session, obj, &attr, 1); + CHECK_RV(rv, name, CKR_OK); + if (rv != CKR_OK) + return -1; + CHECK_TRUE(attr.ulValueLen == expectLen && + (expectLen == 0 || XMEMCMP(buf, expect, expectLen) == 0), name); + return 0; +} + +static int run_test(void) +{ + CK_RV rv; + CK_SESSION_HANDLE session = 0; + CK_OBJECT_HANDLE obj = CK_INVALID_HANDLE; + CK_OBJECT_CLASS dataClass = CKO_DATA; + CK_BBOOL ckFalse = CK_FALSE; + CK_ATTRIBUTE createTmpl[] = { + { CKA_CLASS, &dataClass, sizeof(dataClass) }, + { CKA_TOKEN, &ckFalse, sizeof(ckFalse) }, + { CKA_PRIVATE, &ckFalse, sizeof(ckFalse) }, + { CKA_VALUE, (void*)valueData, sizeof(valueData) - 1 }, + { CKA_APPLICATION, (void*)appData, sizeof(appData) - 1 }, + { CKA_OBJECT_ID, (void*)objectIdData, sizeof(objectIdData) }, + }; + CK_ULONG createTmplCnt = sizeof(createTmpl) / sizeof(*createTmpl); + CK_ATTRIBUTE setValueOnly[] = { + { CKA_VALUE, (void*)newValue, sizeof(newValue) - 1 }, + }; + CK_ATTRIBUTE clearApp[] = { + { CKA_APPLICATION, NULL, 0 }, + }; + + rv = pkcs11_load(); + CHECK_RV(rv, "load library", CKR_OK); + if (rv != CKR_OK) + return -1; + + rv = pkcs11_open_session(&session); + CHECK_RV(rv, "open session", CKR_OK); + if (rv != CKR_OK) + goto out; + + rv = funcList->C_CreateObject(session, createTmpl, createTmplCnt, &obj); + CHECK_RV(rv, "C_CreateObject(data object)", CKR_OK); + if (rv != CKR_OK) + goto out; + + /* Update only CKA_VALUE; the other two attributes must survive. */ + rv = funcList->C_SetAttributeValue(session, obj, setValueOnly, 1); + CHECK_RV(rv, "C_SetAttributeValue(CKA_VALUE only)", CKR_OK); + + expect_attr(session, obj, CKA_VALUE, newValue, sizeof(newValue) - 1, + "CKA_VALUE updated"); + expect_attr(session, obj, CKA_APPLICATION, appData, sizeof(appData) - 1, + "CKA_APPLICATION preserved"); + expect_attr(session, obj, CKA_OBJECT_ID, objectIdData, + sizeof(objectIdData), "CKA_OBJECT_ID preserved"); + + /* A supplied empty attribute clears only that attribute. */ + rv = funcList->C_SetAttributeValue(session, obj, clearApp, 1); + CHECK_RV(rv, "C_SetAttributeValue(CKA_APPLICATION empty)", CKR_OK); + + expect_attr(session, obj, CKA_APPLICATION, NULL, 0, + "CKA_APPLICATION cleared"); + expect_attr(session, obj, CKA_VALUE, newValue, sizeof(newValue) - 1, + "CKA_VALUE still present after clearing application"); + expect_attr(session, obj, CKA_OBJECT_ID, objectIdData, + sizeof(objectIdData), "CKA_OBJECT_ID still present"); + +out: + if (obj != CK_INVALID_HANDLE) + funcList->C_DestroyObject(session, obj); + if (session != 0) + funcList->C_CloseSession(session); + funcList->C_Finalize(NULL); + pkcs11_unload(); + return 0; +} + +int main(int argc, char* argv[]) +{ + (void)argc; + (void)argv; + +#ifndef WOLFPKCS11_NO_ENV + XSETENV("WOLFPKCS11_TOKEN_PATH", TEST_DIR, 1); +#endif + + printf("=== wolfPKCS11 data-object partial update test ===\n"); + run_test(); + return pkcs11_test_summary(); +} diff --git a/tests/include.am b/tests/include.am index 958c7ec4..72877459 100644 --- a/tests/include.am +++ b/tests/include.am @@ -271,6 +271,11 @@ noinst_PROGRAMS += tests/copy_data_object_token_test tests_copy_data_object_token_test_SOURCES = tests/copy_data_object_token_test.c tests_copy_data_object_token_test_LDADD = +check_PROGRAMS += tests/data_object_partial_update_test +noinst_PROGRAMS += tests/data_object_partial_update_test +tests_data_object_partial_update_test_SOURCES = tests/data_object_partial_update_test.c +tests_data_object_partial_update_test_LDADD = + if BUILD_STATIC tests_pkcs11test_LDADD += src/libwolfpkcs11.la tests_pkcs11mtt_LDADD += src/libwolfpkcs11.la @@ -326,6 +331,7 @@ tests_hkdf_persistence_test_LDADD += src/libwolfpkcs11.la tests_unique_id_test_LDADD += src/libwolfpkcs11.la tests_copy_data_object_test_LDADD += src/libwolfpkcs11.la tests_copy_data_object_token_test_LDADD += src/libwolfpkcs11.la +tests_data_object_partial_update_test_LDADD += src/libwolfpkcs11.la else tests_object_id_uniqueness_test_LDADD += src/libwolfpkcs11.la tests_empty_pin_store_test_LDADD += src/libwolfpkcs11.la @@ -373,6 +379,7 @@ tests_hkdf_persistence_test_LDADD += src/libwolfpkcs11.la tests_unique_id_test_LDADD += src/libwolfpkcs11.la tests_copy_data_object_test_LDADD += src/libwolfpkcs11.la tests_copy_data_object_token_test_LDADD += src/libwolfpkcs11.la +tests_data_object_partial_update_test_LDADD += src/libwolfpkcs11.la endif EXTRA_DIST += tests/unit.h \ diff --git a/wolfpkcs11/internal.h b/wolfpkcs11/internal.h index 0b67003d..c4f95fe4 100644 --- a/wolfpkcs11/internal.h +++ b/wolfpkcs11/internal.h @@ -522,7 +522,7 @@ WP11_LOCAL int WP11_Object_SetSecretKey(WP11_Object* object, unsigned char** dat WP11_LOCAL int WP11_Object_SetCert(WP11_Object* object, unsigned char** data, CK_ULONG* len); WP11_LOCAL int WP11_Object_DataObject(WP11_Object* object, unsigned char** data, - CK_ULONG* len); + CK_ULONG* len, int* present); WP11_LOCAL int WP11_Object_SetClass(WP11_Object* object, CK_OBJECT_CLASS objClass); WP11_LOCAL CK_OBJECT_CLASS WP11_Object_GetClass(WP11_Object* object);