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.

Author: nb-tech5

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
  *****************************************/

src/lib/P11Attributes.h

@@ -122,13 +122,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);
 
+	// Retrie 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
  *****************************************/
@@ -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