Skip to content

Fix data object copy and partial update attribute handling #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aidangarske wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix data object copy and partial update attribute handling #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13a849e
F-13239 - Copy CKO_DATA payload attributes in C_CopyObject
aidangarske Sep 17, 2026
a807add
F-13240 - Preserve omitted attributes on data object updates
aidangarske Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand All @@ -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
Expand Down Expand Up @@ -1797,10 +1799,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) {
Expand Down
192 changes: 131 additions & 61 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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;
}
Expand All @@ -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 */
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -11354,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)
Expand Down
Loading
Loading