From 24f3ab666ad34dae95d79f48eae62e9fbe65d6b8 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/5] 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 c9603bf9c..33e57ce2f 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 ed8bc05fc..23ae4177c 100644 --- a/src/lib/P11Attributes.h +++ b/src/lib/P11Attributes.h @@ -122,6 +122,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(); @@ -129,6 +132,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 *****************************************/ @@ -455,11 +468,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 @@ -473,11 +486,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 4066010156035efa17f6511c7a541d176f851a78 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/5] 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 23ae4177c..f2a354cd3 100644 --- a/src/lib/P11Attributes.h +++ b/src/lib/P11Attributes.h @@ -122,7 +122,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 4559d03cfbab582a0d6f0f6450282e7a71cd5c3d 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/5] 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 33e57ce2f..0650a8b3a 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 13772a74703278a121b990c4e848f2d5a3d2dc4e 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/5] 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 9b6d44cd93e088b571b14d5e4da8f314c242b27c Mon Sep 17 00:00:00 2001 From: Nelson Branco Date: Wed, 4 Mar 2026 15:18:11 +0000 Subject: [PATCH 5/5] 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) },