Skip to content

fix: fixes attribute reading on private objects #805

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
nb-tech5 wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: fixes attribute reading on private objects #805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0425096
fix: fixes attribute reading on private objects
nb-tech5 Aug 1, 2025
d9abdbd
fix: fixes typo
nb-tech5 Aug 4, 2025
0663025
fix: unsused parameters
nb-tech5 Aug 6, 2025
2d49baa
chrore: adds regression unit test case
nb-tech5 Aug 18, 2025
70e47b0
fix: fixes private object attributes test
nb-tech5 Mar 4, 2026
ad5ca5b
test: adds test case to expose issue
nb-tech5 Oct 6, 2026
c9661db
applies antoine's suggested patch
nb-tech5 Oct 6, 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
71 changes: 39 additions & 32 deletions src/lib/P11Attributes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
{
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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
*****************************************/
Expand Down Expand Up @@ -1181,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

Expand All @@ -1191,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);
}

/*****************************************
Expand All @@ -1208,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

Expand All @@ -1218,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);
}

/*****************************************
Expand Down
21 changes: 17 additions & 4 deletions src/lib/P11Attributes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,26 @@ class P11Attribute
// Update the value if allowed
virtual CK_RV updateAttr(Token *token, bool isPrivate, CK_VOID_PTR pValue, CK_ULONG ulValueLen, int op);

// Retrieve the value if allowed
virtual CK_RV retrieveAttrByteString(Token *token, bool isPrivate, OSAttribute *attr, ByteString &value);

// Helper functions
bool isModifiable();
bool isSensitive();
bool isExtractable();
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
*****************************************/
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
37 changes: 30 additions & 7 deletions src/lib/test/ObjectTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1775,22 +1775,28 @@ 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,
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'}};
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) },
{ CKA_PRIVATE_EXPONENT, pD, sizeof(pD) }
{ CKA_PRIVATE_EXPONENT, pD, sizeof(pD) },
{ CKA_START_DATE, &startDate, sizeof(startDate) },
{ CKA_END_DATE, &endDate, sizeof(endDate)}
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Just make sure that we finalize any previous tests
Expand All @@ -1815,13 +1821,31 @@ 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);
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);
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, 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;
Expand Down Expand Up @@ -2433,4 +2457,3 @@ void ObjectTests::testCreateSecretKey()
rv = CRYPTOKI_F_PTR( C_DestroyObject(hSession,hObject) );
CPPUNIT_ASSERT_EQUAL(CKR_OK, rv);
}