From 0425096974e26f524aa458aa7299337d7110812a Mon Sep 17 00:00:00 2001 From: nb-tech5 <115477424+nb-tech5@users.noreply.github.com> Date: Fri, 1 Aug 2025 11:44:47 +0100 Subject: [PATCH 1/7] fix: fixes attribute reading on private objects According to the PKCS#11 spec, all key objects have defined a semantic for CKA_START_DATE and CKA_END_DATE, although, for key objects that are private (e.g. CKO_PRIVATE_KEY, CKO_SECRET_KEY), the retrieval of their values fail. This is happening when an attribute with a specialized class, e.g. P11AttrStartDate, gets added to a private object, its value is always written in clear, due to the updateAttr method overload, although, upon retrieving the value, due to the retrieve method not being symmetrically overloaded, and because the object is private, the attribute value is decrypted and fails. This change adds protected virtual method `retrieveAttrByteString` to allow overloading the default behavior, namely for public attributes (written in clear), from private object. Also add a base class `P11NonPrivateAttribute` to be extended by public attributes specializations. --- src/lib/P11Attributes.cpp | 59 +++++++++++++++++++++++---------------- src/lib/P11Attributes.h | 21 +++++++++++--- 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/lib/P11Attributes.cpp b/src/lib/P11Attributes.cpp index e2fd8117f..3efcbe8af 100644 --- a/src/lib/P11Attributes.cpp +++ b/src/lib/P11Attributes.cpp @@ -69,6 +69,23 @@ CK_RV P11Attribute::updateAttr(Token *token, bool isPrivate, CK_VOID_PTR pValue, return CKR_OK; } +CK_RV P11Attribute::retrieveAttrByteString(Token *token, bool isPrivate, OSAttribute *attr, ByteString &value) +{ + if (isPrivate && attr->getByteStringValue().size() != 0) + { + if (!token->decrypt(attr->getByteStringValue(), value)) + { + ERROR_MSG("Internal error: failed to decrypt private attribute value"); + return CKR_GENERAL_ERROR; + } + } + else + { + value = attr->getByteStringValue(); + } + return CKR_OK; +} + bool P11Attribute::isModifiable() { // Get the CKA_MODIFIABLE attribute, when the attribute is @@ -273,18 +290,13 @@ CK_RV P11Attribute::retrieve(Token *token, bool isPrivate, CK_VOID_PTR pValue, C // Lower level attribute has to be variable sized. if (attr.isByteStringAttribute()) { - if (isPrivate && attr.getByteStringValue().size() != 0) + ByteString value; + CK_RV res = retrieveAttrByteString(token, isPrivate, &attr, value); + if (res != CKR_OK) { - ByteString value; - if (!token->decrypt(attr.getByteStringValue(),value)) - { - ERROR_MSG("Internal error: failed to decrypt private attribute value"); - return CKR_GENERAL_ERROR; - } - attrSize = value.size(); + return res; } - else - attrSize = attr.getByteStringValue().size(); + attrSize = value.size(); } else if (attr.isMechanismTypeSetAttribute()) { @@ -332,22 +344,15 @@ CK_RV P11Attribute::retrieve(Token *token, bool isPrivate, CK_VOID_PTR pValue, C } else if (attr.isByteStringAttribute()) { - if (isPrivate && attr.getByteStringValue().size() != 0) + ByteString value; + CK_RV res = retrieveAttrByteString(token, isPrivate, &attr, value); + if (res != CKR_OK) { - ByteString value; - if (!token->decrypt(attr.getByteStringValue(),value)) - { - ERROR_MSG("Internal error: failed to decrypt private attribute value"); - return CKR_GENERAL_ERROR; - } - if (value.size() != 0) { - const unsigned char* attrPtr = value.const_byte_str(); - memcpy(pValue,attrPtr,attrSize); - } + return res; } - else if (attr.getByteStringValue().size() != 0) - { - const unsigned char* attrPtr = attr.getByteStringValue().const_byte_str(); + + if (value.size() != 0) { + const unsigned char* attrPtr = value.const_byte_str(); memcpy(pValue,attrPtr,attrSize); } } @@ -495,6 +500,12 @@ CK_RV P11Attribute::update(Token* token, bool isPrivate, CK_VOID_PTR pValue, CK_ return CKR_ATTRIBUTE_READ_ONLY; } +CK_RV P11NonPrivateAttribute::retrieveAttrByteString(Token *token, bool isPrivate, OSAttribute *attr, ByteString &value) +{ + value = attr->getByteStringValue(); + return CKR_OK; +} + /***************************************** * CKA_CLASS *****************************************/ diff --git a/src/lib/P11Attributes.h b/src/lib/P11Attributes.h index 00460df0a..69f5495e4 100644 --- a/src/lib/P11Attributes.h +++ b/src/lib/P11Attributes.h @@ -123,6 +123,9 @@ class P11Attribute // Update the value if allowed virtual CK_RV updateAttr(Token *token, bool isPrivate, CK_VOID_PTR pValue, CK_ULONG ulValueLen, int op); + // Retrie the value if allowed + virtual CK_RV retrieveAttrByteString(Token *token, bool isPrivate, OSAttribute *attr, ByteString &value); + // Helper functions bool isModifiable(); bool isSensitive(); @@ -130,6 +133,16 @@ class P11Attribute bool isTrusted(); }; +class P11NonPrivateAttribute : public P11Attribute +{ +protected: + // Constructor + P11NonPrivateAttribute(OSObject* inobject) : P11Attribute(inobject) {}; + + // Retrie the value if allowed + virtual CK_RV retrieveAttrByteString(Token *token, bool isPrivate, OSAttribute *attr, ByteString &value); +}; + /***************************************** * CKA_CLASS *****************************************/ @@ -456,11 +469,11 @@ class P11AttrCertificateCategory : public P11Attribute * CKA_START_DATE *****************************************/ -class P11AttrStartDate : public P11Attribute +class P11AttrStartDate : public P11NonPrivateAttribute { public: // Constructor - P11AttrStartDate(OSObject* inobject, CK_ULONG inchecks) : P11Attribute(inobject) { type = CKA_START_DATE; checks = inchecks; } + P11AttrStartDate(OSObject* inobject, CK_ULONG inchecks) : P11NonPrivateAttribute(inobject) { type = CKA_START_DATE; checks = inchecks; } protected: // Set the default value of the attribute @@ -474,11 +487,11 @@ class P11AttrStartDate : public P11Attribute * CKA_END_DATE *****************************************/ -class P11AttrEndDate : public P11Attribute +class P11AttrEndDate : public P11NonPrivateAttribute { public: // Constructor - P11AttrEndDate(OSObject* inobject, CK_ULONG inchecks) : P11Attribute(inobject) { type = CKA_END_DATE; checks = inchecks; } + P11AttrEndDate(OSObject* inobject, CK_ULONG inchecks) : P11NonPrivateAttribute(inobject) { type = CKA_END_DATE; checks = inchecks; } protected: // Set the default value of the attribute From d9abdbdd3385b37a0480fbc5acb3a139d2f61e17 Mon Sep 17 00:00:00 2001 From: nb-tech5 <115477424+nb-tech5@users.noreply.github.com> Date: Mon, 4 Aug 2025 10:16:11 +0100 Subject: [PATCH 2/7] fix: fixes typo --- src/lib/P11Attributes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/P11Attributes.h b/src/lib/P11Attributes.h index 69f5495e4..a3630b058 100644 --- a/src/lib/P11Attributes.h +++ b/src/lib/P11Attributes.h @@ -123,7 +123,7 @@ class P11Attribute // Update the value if allowed virtual CK_RV updateAttr(Token *token, bool isPrivate, CK_VOID_PTR pValue, CK_ULONG ulValueLen, int op); - // Retrie the value if allowed + // Retrieve the value if allowed virtual CK_RV retrieveAttrByteString(Token *token, bool isPrivate, OSAttribute *attr, ByteString &value); // Helper functions From 0663025dfa8c4f97dbb58e813732cdb5446f7ddf Mon Sep 17 00:00:00 2001 From: nb-tech5 <115477424+nb-tech5@users.noreply.github.com> Date: Wed, 6 Aug 2025 09:43:59 +0100 Subject: [PATCH 3/7] fix: unsused parameters --- src/lib/P11Attributes.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/P11Attributes.cpp b/src/lib/P11Attributes.cpp index 3efcbe8af..0680d1eb9 100644 --- a/src/lib/P11Attributes.cpp +++ b/src/lib/P11Attributes.cpp @@ -500,7 +500,7 @@ CK_RV P11Attribute::update(Token* token, bool isPrivate, CK_VOID_PTR pValue, CK_ return CKR_ATTRIBUTE_READ_ONLY; } -CK_RV P11NonPrivateAttribute::retrieveAttrByteString(Token *token, bool isPrivate, OSAttribute *attr, ByteString &value) +CK_RV P11NonPrivateAttribute::retrieveAttrByteString(Token* /*token*/, bool /*isPrivate*/, OSAttribute *attr, ByteString &value) { value = attr->getByteStringValue(); return CKR_OK; From 2d49baa2a36a10717a307710168b923b46247ed9 Mon Sep 17 00:00:00 2001 From: nb-tech5 <115477424+nb-tech5@users.noreply.github.com> Date: Mon, 18 Aug 2025 12:30:52 +0100 Subject: [PATCH 4/7] chrore: adds regression unit test case --- src/lib/test/ObjectTests.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lib/test/ObjectTests.cpp b/src/lib/test/ObjectTests.cpp index c4248b291..2275e643a 100644 --- a/src/lib/test/ObjectTests.cpp +++ b/src/lib/test/ObjectTests.cpp @@ -1782,7 +1782,8 @@ void ObjectTests::testDefaultRSAPrivAttributes() 0x30, 0xD1, 0x4D, 0x3C, 0x60, 0x33, 0xB5, 0xED, 0x4C, 0x39, 0xDA, 0x68, 0x78, 0xF9, 0x6B, 0x4F, 0x47, 0x55, 0xB2, 0x02, 0x00, 0x7E, 0x9C, 0x05 }; - CK_DATE emptyDate; + CK_DATE startDate = {{'2', '0', '0', '0'}, {'0', '1'}, {'0', '2'}}; + CK_DATE endDate = {{'2', '0', '1', '0'}, {'0', '1'}, {'0', '2'}}; // Make the key non-sensitive and extractable so that we can test it. CK_ATTRIBUTE objTemplate[] = { { CKA_CLASS, &objClass, sizeof(objClass) }, @@ -1790,7 +1791,9 @@ void ObjectTests::testDefaultRSAPrivAttributes() { CKA_SENSITIVE, &bFalse, sizeof(bFalse) }, { CKA_EXTRACTABLE, &bTrue, sizeof(bTrue) }, { CKA_MODULUS, pN, sizeof(pN) }, - { CKA_PRIVATE_EXPONENT, pD, sizeof(pD) } + { CKA_PRIVATE_EXPONENT, pD, sizeof(pD) }, + { CKA_START_DATE, &startDate, sizeof(startDate) }, + { CKA_END_DATE, &endDate, sizeof(endDate)} }; // Just make sure that we finalize any previous tests @@ -1815,8 +1818,7 @@ void ObjectTests::testDefaultRSAPrivAttributes() // Check attributes in RSA public key object checkCommonObjectAttributes(hSession, hObject, objClass); checkCommonStorageObjectAttributes(hSession, hObject, CK_FALSE, CK_TRUE, CK_TRUE, NULL_PTR, 0, CK_TRUE, CK_TRUE); - memset(&emptyDate, 0, sizeof(emptyDate)); - checkCommonKeyAttributes(hSession, hObject, objType, NULL_PTR, 0, emptyDate, 0, emptyDate, 0, CK_FALSE, CK_FALSE, CK_UNAVAILABLE_INFORMATION, NULL_PTR, 0); + checkCommonKeyAttributes(hSession, hObject, objType, NULL_PTR, 0, startDate, sizeof(startDate), endDate, sizeof(endDate), CK_FALSE, CK_FALSE, CK_UNAVAILABLE_INFORMATION, NULL_PTR, 0); checkCommonPrivateKeyAttributes(hSession, hObject, NULL_PTR, 0, CK_FALSE, CK_TRUE, CK_TRUE, CK_TRUE, CK_TRUE, CK_TRUE, CK_FALSE, CK_FALSE, CK_FALSE, NULL_PTR, 0, CK_FALSE); checkCommonRSAPrivateKeyAttributes(hSession, hObject, pN, sizeof(pN), NULL_PTR, 0, pD, sizeof(pD), NULL_PTR, 0, NULL_PTR, 0, NULL_PTR, 0, NULL_PTR, 0, NULL_PTR, 0); checkToTrueAttributes(hSession, hObject); From 70e47b0886828bf89870cfdb00cac5c0900d69e9 Mon Sep 17 00:00:00 2001 From: Nelson Branco Date: Wed, 4 Mar 2026 15:18:11 +0000 Subject: [PATCH 5/7] fix: fixes private object attributes test --- src/lib/test/ObjectTests.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/test/ObjectTests.cpp b/src/lib/test/ObjectTests.cpp index 2275e643a..6c73b6d6b 100644 --- a/src/lib/test/ObjectTests.cpp +++ b/src/lib/test/ObjectTests.cpp @@ -1775,7 +1775,7 @@ void ObjectTests::testDefaultRSAPrivAttributes() 0x4B, 0x3F, 0x9D, 0x4E, 0xCB, 0x78, 0x12, 0xA9, 0x42, 0xAD, 0x51, 0x1F, 0x3B, 0xBD, 0x3D, 0x6A, 0xE5, 0x38, 0xB7, 0x45, 0x65, 0x50, 0x30, 0x35 }; - CK_BYTE pD[] = { 0x6D, 0x94, 0x6B, 0xEB, 0xFF, 0xDC, 0x03, 0x80, 0x7B, 0x0A, + CK_BYTE pD[] = { 0x6D, 0x94, 0x6B, 0xEB, 0xFF, 0xDC, 0x03, 0x80, 0x7B, 0x0A, 0x4F, 0x0A, 0x98, 0x6C, 0xA3, 0x2A, 0x8A, 0xE4, 0xAA, 0x18, 0x44, 0xA4, 0xA5, 0x39, 0x37, 0x0A, 0x2C, 0xFC, 0x5F, 0xD1, 0x44, 0x6E, 0xCE, 0x25, 0x9B, 0xE5, 0xD1, 0x51, 0xAF, 0xA8, @@ -1788,6 +1788,7 @@ void ObjectTests::testDefaultRSAPrivAttributes() CK_ATTRIBUTE objTemplate[] = { { CKA_CLASS, &objClass, sizeof(objClass) }, { CKA_KEY_TYPE, &objType, sizeof(objType) }, + { CKA_PRIVATE, &bTrue, sizeof(bTrue) }, { CKA_SENSITIVE, &bFalse, sizeof(bFalse) }, { CKA_EXTRACTABLE, &bTrue, sizeof(bTrue) }, { CKA_MODULUS, pN, sizeof(pN) }, From ad5ca5b2899518c3482c41ee678c48f097b316a0 Mon Sep 17 00:00:00 2001 From: Nelson Branco Date: Tue, 6 Oct 2026 17:11:21 +0100 Subject: [PATCH 6/7] test: adds test case to expose issue --- src/lib/test/ObjectTests.cpp | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/lib/test/ObjectTests.cpp b/src/lib/test/ObjectTests.cpp index 6c73b6d6b..11572ed1b 100644 --- a/src/lib/test/ObjectTests.cpp +++ b/src/lib/test/ObjectTests.cpp @@ -1784,11 +1784,13 @@ void ObjectTests::testDefaultRSAPrivAttributes() 0x00, 0x7E, 0x9C, 0x05 }; CK_DATE startDate = {{'2', '0', '0', '0'}, {'0', '1'}, {'0', '2'}}; CK_DATE endDate = {{'2', '0', '1', '0'}, {'0', '1'}, {'0', '2'}}; + CK_BYTE pSubject[] = "RSA private key subject"; // Make the key non-sensitive and extractable so that we can test it. CK_ATTRIBUTE objTemplate[] = { { CKA_CLASS, &objClass, sizeof(objClass) }, { CKA_KEY_TYPE, &objType, sizeof(objType) }, { CKA_PRIVATE, &bTrue, sizeof(bTrue) }, + { CKA_SUBJECT, pSubject, sizeof(pSubject)-1 }, { CKA_SENSITIVE, &bFalse, sizeof(bFalse) }, { CKA_EXTRACTABLE, &bTrue, sizeof(bTrue) }, { CKA_MODULUS, pN, sizeof(pN) }, @@ -1820,11 +1822,30 @@ void ObjectTests::testDefaultRSAPrivAttributes() checkCommonObjectAttributes(hSession, hObject, objClass); checkCommonStorageObjectAttributes(hSession, hObject, CK_FALSE, CK_TRUE, CK_TRUE, NULL_PTR, 0, CK_TRUE, CK_TRUE); checkCommonKeyAttributes(hSession, hObject, objType, NULL_PTR, 0, startDate, sizeof(startDate), endDate, sizeof(endDate), CK_FALSE, CK_FALSE, CK_UNAVAILABLE_INFORMATION, NULL_PTR, 0); - checkCommonPrivateKeyAttributes(hSession, hObject, NULL_PTR, 0, CK_FALSE, CK_TRUE, CK_TRUE, CK_TRUE, CK_TRUE, CK_TRUE, CK_FALSE, CK_FALSE, CK_FALSE, NULL_PTR, 0, CK_FALSE); + checkCommonPrivateKeyAttributes(hSession, hObject, pSubject, sizeof(pSubject)-1, CK_FALSE, CK_TRUE, CK_TRUE, CK_TRUE, CK_TRUE, CK_TRUE, CK_FALSE, CK_FALSE, CK_FALSE, NULL_PTR, 0, CK_FALSE); checkCommonRSAPrivateKeyAttributes(hSession, hObject, pN, sizeof(pN), NULL_PTR, 0, pD, sizeof(pD), NULL_PTR, 0, NULL_PTR, 0, NULL_PTR, 0, NULL_PTR, 0, NULL_PTR, 0); checkToTrueAttributes(hSession, hObject); + + // The private key must be findable by its subject and dates + CK_ATTRIBUTE findTemplate[] = { + { CKA_CLASS, &objClass, sizeof(objClass) }, + { CKA_SUBJECT, pSubject, sizeof(pSubject)-1 }, + { CKA_START_DATE, &startDate, sizeof(startDate) }, + { CKA_END_DATE, &endDate, sizeof(endDate) } + }; + CK_OBJECT_HANDLE hFound[2]; + CK_ULONG ulFoundCount = 0; + rv = CRYPTOKI_F_PTR( C_FindObjectsInit(hSession, findTemplate, sizeof(findTemplate)/sizeof(CK_ATTRIBUTE)) ); + CPPUNIT_ASSERT_EQUAL(CKR_OK, rv); + rv = CRYPTOKI_F_PTR( C_FindObjects(hSession, hFound, 2, &ulFoundCount) ); + CPPUNIT_ASSERT_EQUAL(CKR_OK, rv); + CPPUNIT_ASSERT_EQUAL((CK_ULONG)1, ulFoundCount); + CPPUNIT_ASSERT_EQUAL(hObject, hFound[0]); + rv = CRYPTOKI_F_PTR( C_FindObjectsFinal(hSession) ); + CPPUNIT_ASSERT_EQUAL(CKR_OK, rv); } + void ObjectTests::testAlwaysNeverAttribute() { CK_RV rv; @@ -2436,4 +2457,3 @@ void ObjectTests::testCreateSecretKey() rv = CRYPTOKI_F_PTR( C_DestroyObject(hSession,hObject) ); CPPUNIT_ASSERT_EQUAL(CKR_OK, rv); } - From c9661db09bbc03e3a8576dfcdb2e6cd83e08db32 Mon Sep 17 00:00:00 2001 From: Nelson Branco Date: Tue, 6 Oct 2026 17:37:40 +0100 Subject: [PATCH 7/7] applies antoine's suggested patch --- src/lib/P11Attributes.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/lib/P11Attributes.cpp b/src/lib/P11Attributes.cpp index 0680d1eb9..8f97f6b2c 100644 --- a/src/lib/P11Attributes.cpp +++ b/src/lib/P11Attributes.cpp @@ -1192,7 +1192,7 @@ bool P11AttrStartDate::setDefault() } // Update the value if allowed -CK_RV P11AttrStartDate::updateAttr(Token* /*token*/, bool /*isPrivate*/, CK_VOID_PTR pValue, CK_ULONG ulValueLen, int /*op*/) +CK_RV P11AttrStartDate::updateAttr(Token* token, bool isPrivate, CK_VOID_PTR pValue, CK_ULONG ulValueLen, int op) { // Attribute specific checks @@ -1202,9 +1202,7 @@ CK_RV P11AttrStartDate::updateAttr(Token* /*token*/, bool /*isPrivate*/, CK_VOID } // Store data - osobject->setAttribute(type, ByteString((unsigned char*)pValue, ulValueLen)); - - return CKR_OK; + return P11Attribute::updateAttr(token, isPrivate, pValue, ulValueLen, op); } /***************************************** @@ -1219,7 +1217,7 @@ bool P11AttrEndDate::setDefault() } // Update the value if allowed -CK_RV P11AttrEndDate::updateAttr(Token* /*token*/, bool /*isPrivate*/, CK_VOID_PTR pValue, CK_ULONG ulValueLen, int /*op*/) +CK_RV P11AttrEndDate::updateAttr(Token* token, bool isPrivate, CK_VOID_PTR pValue, CK_ULONG ulValueLen, int op) { // Attribute specific checks @@ -1229,9 +1227,7 @@ CK_RV P11AttrEndDate::updateAttr(Token* /*token*/, bool /*isPrivate*/, CK_VOID_P } // Store data - osobject->setAttribute(type, ByteString((unsigned char*)pValue, ulValueLen)); - - return CKR_OK; + return P11Attribute::updateAttr(token, isPrivate, pValue, ulValueLen, op); } /*****************************************