From 20650115784bfc926174895f19114cfd55278a7e 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 937f043fb..82a0c4d77 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 6fd31f8d0..ba6bfbf8a 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 8e52c8832b7b899de514ec39b3c339636532931c 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 ba6bfbf8a..9dd9a5ae9 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 494525b292d625f94adf0c5582eeafe8cfac2d53 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 82a0c4d77..26da25805 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 4bb88ff43dfd912e4cf4647061a9c47e936a036c 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 b1d5fb7f69a801342833249cb2c2fb02c3db7b16 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) },