ta: pkcs11: Handle optional attributes without default values

In the current implementation all optional attributes of an object
if not specified in the template while creating object, are assigned
empty value by default. This works fine for the attributes where
specification mentions that default value is empty or the
attribute is modifiable later.

However for attributes like CKA_ALLOWED_MECHANISM, adding an empty
default value results in a failure later in crypto operations when
attribute of the object are checked against the mechanism.

To avoid such errors, the optional attributes array are split in 2 parts,
one with the default empty value and ones which don't require a
default value. All attributes in the specification which either should
have default empty value or are allowed to be modified later by call to
C_SetAttributes() or C_CopyObjects() would fall in the former category
and be initialized with empty/NULL value.

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Signed-off-by: Ruchika Gupta <ruchika.gupta@linaro.org>

1 files changed, 114 insertions, 79 deletions

diff --git a/ta/pkcs11/src/pkcs11_attributes.c b/ta/pkcs11/src/pkcs11_attributes.c
index 3335b6dc..36d14696 100644
--- a/ta/pkcs11/src/pkcs11_attributes.c
+++ b/ta/pkcs11/src/pkcs11_attributes.c
@@ -245,10 +245,11 @@ static enum pkcs11_rc get_default_value(enum pkcs11_attr_id id, void **value,
 	return PKCS11_CKR_OK;
 }
 
-static enum pkcs11_rc set_optional_attributes(struct obj_attrs **out,
-					      struct obj_attrs *temp,
-					      uint32_t const *bp,
-					      size_t bp_count)
+static enum pkcs11_rc set_optional_attributes_with_def(struct obj_attrs **out,
+						       struct obj_attrs *temp,
+						       uint32_t const *bp,
+						       size_t bp_count,
+						       bool default_to_null)
 {
 	enum pkcs11_rc rc = PKCS11_CKR_OK;
 	size_t n = 0;
@@ -258,8 +259,14 @@ static enum pkcs11_rc set_optional_attributes(struct obj_attrs **out,
 		void *value = NULL;
 
 		rc = get_attribute_ptr(temp, bp[n], &value, &size);
-		if (rc == PKCS11_RV_NOT_FOUND)
-			rc = get_default_value(bp[n], &value, &size);
+		if (rc == PKCS11_RV_NOT_FOUND) {
+			if (default_to_null) {
+				rc = get_default_value(bp[n], &value, &size);
+			} else {
+				rc = PKCS11_CKR_OK;
+				continue;
+			}
+		}
 		if (rc)
 			return rc;
 
@@ -271,6 +278,22 @@ static enum pkcs11_rc set_optional_attributes(struct obj_attrs **out,
 	return rc;
 }
 
+static enum pkcs11_rc set_attributes_opt_or_null(struct obj_attrs **out,
+						 struct obj_attrs *temp,
+						 uint32_t const *bp,
+						 size_t bp_count)
+{
+	return set_optional_attributes_with_def(out, temp, bp, bp_count, true);
+}
+
+static enum pkcs11_rc set_optional_attributes(struct obj_attrs **out,
+					      struct obj_attrs *temp,
+					      uint32_t const *bp,
+					      size_t bp_count)
+{
+	return set_optional_attributes_with_def(out, temp, bp, bp_count, false);
+}
+
 /*
  * Below are listed the mandated or optional expected attributes for
  * PKCS#11 storage objects.
@@ -281,33 +304,36 @@ static enum pkcs11_rc set_optional_attributes(struct obj_attrs **out,
  */
 
 /* PKCS#11 specification for any object (session/token) of the storage */
-static const uint32_t pkcs11_any_object_boolprops[] = {
+static const uint32_t any_object_boolprops[] = {
 	PKCS11_CKA_TOKEN, PKCS11_CKA_PRIVATE,
 	PKCS11_CKA_MODIFIABLE, PKCS11_CKA_COPYABLE, PKCS11_CKA_DESTROYABLE,
 };
 
-static const uint32_t pkcs11_any_object_optional[] = {
+static const uint32_t any_object_opt_or_null[] = {
 	PKCS11_CKA_LABEL,
 };
 
-/* PKCS#11 specification for raw data object (+pkcs11_any_object_xxx) */
-const uint32_t pkcs11_raw_data_optional[] = {
+/* PKCS#11 specification for raw data object (+any_object_xxx) */
+const uint32_t raw_data_opt_or_null[] = {
 	PKCS11_CKA_OBJECT_ID, PKCS11_CKA_APPLICATION, PKCS11_CKA_VALUE,
 };
 
-/* PKCS#11 specification for any key object (+pkcs11_any_object_xxx) */
-static const uint32_t pkcs11_any_key_boolprops[] = {
+/* PKCS#11 specification for any key object (+any_object_xxx) */
+static const uint32_t any_key_boolprops[] = {
 	PKCS11_CKA_DERIVE,
 };
 
-static const uint32_t pkcs11_any_key_optional[] = {
+static const uint32_t any_key_opt_or_null[] = {
 	PKCS11_CKA_ID,
 	PKCS11_CKA_START_DATE, PKCS11_CKA_END_DATE,
+};
+
+static const uint32_t any_key_optional[] = {
 	PKCS11_CKA_ALLOWED_MECHANISMS,
 };
 
-/* PKCS#11 specification for any symmetric key (+pkcs11_any_key_xxx) */
-static const uint32_t pkcs11_symm_key_boolprops[] = {
+/* PKCS#11 specification for any symmetric key (+any_key_xxx) */
+static const uint32_t symm_key_boolprops[] = {
 	PKCS11_CKA_ENCRYPT, PKCS11_CKA_DECRYPT,
 	PKCS11_CKA_SIGN, PKCS11_CKA_VERIFY,
 	PKCS11_CKA_WRAP, PKCS11_CKA_UNWRAP,
@@ -315,73 +341,73 @@ static const uint32_t pkcs11_symm_key_boolprops[] = {
 	PKCS11_CKA_WRAP_WITH_TRUSTED, PKCS11_CKA_TRUSTED,
 };
 
-static const uint32_t pkcs11_symm_key_optional[] = {
+static const uint32_t symm_key_opt_or_null[] = {
 	PKCS11_CKA_WRAP_TEMPLATE, PKCS11_CKA_UNWRAP_TEMPLATE,
 	PKCS11_CKA_DERIVE_TEMPLATE,
 	PKCS11_CKA_VALUE, PKCS11_CKA_VALUE_LEN,
 };
 
-/* PKCS#11 specification for any asymmetric public key (+pkcs11_any_key_xxx) */
-static const uint32_t pkcs11_public_key_boolprops[] = {
+/* PKCS#11 specification for any asymmetric public key (+any_key_xxx) */
+static const uint32_t public_key_boolprops[] = {
 	PKCS11_CKA_ENCRYPT, PKCS11_CKA_VERIFY, PKCS11_CKA_VERIFY_RECOVER,
 	PKCS11_CKA_WRAP,
 	PKCS11_CKA_TRUSTED,
 };
 
-static const uint32_t pkcs11_public_key_mandated[] = {
+static const uint32_t public_key_mandated[] = {
 	PKCS11_CKA_SUBJECT
 };
 
-static const uint32_t pkcs11_public_key_optional[] = {
+static const uint32_t public_key_opt_or_null[] = {
 	PKCS11_CKA_WRAP_TEMPLATE, PKCS11_CKA_PUBLIC_KEY_INFO,
 };
 
-/* PKCS#11 specification for any asymmetric private key (+pkcs11_any_key_xxx) */
-static const uint32_t pkcs11_private_key_boolprops[] = {
+/* PKCS#11 specification for any asymmetric private key (+any_key_xxx) */
+static const uint32_t private_key_boolprops[] = {
 	PKCS11_CKA_DECRYPT, PKCS11_CKA_SIGN, PKCS11_CKA_SIGN_RECOVER,
 	PKCS11_CKA_UNWRAP,
 	PKCS11_CKA_SENSITIVE, PKCS11_CKA_EXTRACTABLE,
 	PKCS11_CKA_WRAP_WITH_TRUSTED, PKCS11_CKA_ALWAYS_AUTHENTICATE,
 };
 
-static const uint32_t pkcs11_private_key_mandated[] = {
+static const uint32_t private_key_mandated[] = {
 	PKCS11_CKA_SUBJECT
 };
 
-static const uint32_t pkcs11_private_key_optional[] = {
+static const uint32_t private_key_opt_or_null[] = {
 	PKCS11_CKA_UNWRAP_TEMPLATE, PKCS11_CKA_PUBLIC_KEY_INFO,
 };
 
-/* PKCS#11 specification for any RSA key (+pkcs11_public/private_key_xxx) */
-static const uint32_t pkcs11_rsa_public_key_mandated[] = {
+/* PKCS#11 specification for any RSA key (+public/private_key_xxx) */
+static const uint32_t rsa_public_key_mandated[] = {
 	PKCS11_CKA_MODULUS_BITS,
 };
 
-static const uint32_t pkcs11_rsa_public_key_optional[] = {
+static const uint32_t rsa_public_key_opt_or_null[] = {
 	PKCS11_CKA_MODULUS, PKCS11_CKA_PUBLIC_EXPONENT,
 };
 
-static const uint32_t pkcs11_rsa_private_key_optional[] = {
+static const uint32_t rsa_private_key_opt_or_null[] = {
 	PKCS11_CKA_MODULUS, PKCS11_CKA_PUBLIC_EXPONENT,
 	PKCS11_CKA_PRIVATE_EXPONENT,
 	PKCS11_CKA_PRIME_1, PKCS11_CKA_PRIME_2,
 	PKCS11_CKA_EXPONENT_1, PKCS11_CKA_EXPONENT_2, PKCS11_CKA_COEFFICIENT,
 };
 
-/* PKCS#11 specification for any EC key (+pkcs11_public/private_key_xxx) */
-static const uint32_t pkcs11_ec_public_key_mandated[] = {
+/* PKCS#11 specification for any EC key (+public/private_key_xxx) */
+static const uint32_t ec_public_key_mandated[] = {
 	PKCS11_CKA_EC_PARAMS,
 };
 
-static const uint32_t pkcs11_ec_public_key_optional[] = {
+static const uint32_t ec_public_key_opt_or_null[] = {
 	PKCS11_CKA_EC_POINT,
 };
 
-static const uint32_t pkcs11_ec_private_key_mandated[] = {
+static const uint32_t ec_private_key_mandated[] = {
 	PKCS11_CKA_EC_PARAMS,
 };
 
-static const uint32_t pkcs11_ec_private_key_optional[] = {
+static const uint32_t ec_private_key_opt_or_null[] = {
 	PKCS11_CKA_VALUE,
 };
 
@@ -406,13 +432,13 @@ static enum pkcs11_rc create_storage_attributes(struct obj_attrs **out,
 	if (rc)
 		return rc;
 
-	rc = set_mandatory_boolprops(out, temp, pkcs11_any_object_boolprops,
-				     ARRAY_SIZE(pkcs11_any_object_boolprops));
+	rc = set_mandatory_boolprops(out, temp, any_object_boolprops,
+				     ARRAY_SIZE(any_object_boolprops));
 	if (rc)
 		return rc;
 
-	return set_optional_attributes(out, temp, pkcs11_any_object_optional,
-				       ARRAY_SIZE(pkcs11_any_object_optional));
+	return set_attributes_opt_or_null(out, temp, any_object_opt_or_null,
+					  ARRAY_SIZE(any_object_opt_or_null));
 }
 
 static enum pkcs11_rc create_genkey_attributes(struct obj_attrs **out,
@@ -435,13 +461,19 @@ static enum pkcs11_rc create_genkey_attributes(struct obj_attrs **out,
 	if (rc)
 		return rc;
 
-	rc = set_mandatory_boolprops(out, temp, pkcs11_any_key_boolprops,
-				     ARRAY_SIZE(pkcs11_any_key_boolprops));
+	rc = set_mandatory_boolprops(out, temp, any_key_boolprops,
+				     ARRAY_SIZE(any_key_boolprops));
 	if (rc)
 		return rc;
 
-	return set_optional_attributes(out, temp, pkcs11_any_key_optional,
-				       ARRAY_SIZE(pkcs11_any_key_optional));
+	rc = set_attributes_opt_or_null(out, temp, any_key_opt_or_null,
+					ARRAY_SIZE(any_key_opt_or_null));
+	if (rc)
+		return rc;
+
+	return set_optional_attributes(out, temp, any_key_optional,
+				       ARRAY_SIZE(any_key_optional));
+
 }
 
 static enum pkcs11_rc create_symm_key_attributes(struct obj_attrs **out,
@@ -474,13 +506,13 @@ static enum pkcs11_rc create_symm_key_attributes(struct obj_attrs **out,
 		return PKCS11_CKR_TEMPLATE_INCONSISTENT;
 	}
 
-	rc = set_mandatory_boolprops(out, temp, pkcs11_symm_key_boolprops,
-				     ARRAY_SIZE(pkcs11_symm_key_boolprops));
+	rc = set_mandatory_boolprops(out, temp, symm_key_boolprops,
+				     ARRAY_SIZE(symm_key_boolprops));
 	if (rc)
 		return rc;
 
-	return set_optional_attributes(out, temp, pkcs11_symm_key_optional,
-				       ARRAY_SIZE(pkcs11_symm_key_optional));
+	return set_attributes_opt_or_null(out, temp, symm_key_opt_or_null,
+					  ARRAY_SIZE(symm_key_opt_or_null));
 }
 
 static enum pkcs11_rc create_data_attributes(struct obj_attrs **out,
@@ -496,17 +528,17 @@ static enum pkcs11_rc create_data_attributes(struct obj_attrs **out,
 
 	assert(get_class(*out) == PKCS11_CKO_DATA);
 
-	return set_optional_attributes(out, temp, pkcs11_raw_data_optional,
-				       ARRAY_SIZE(pkcs11_raw_data_optional));
+	return set_attributes_opt_or_null(out, temp, raw_data_opt_or_null,
+					  ARRAY_SIZE(raw_data_opt_or_null));
 }
 
 static enum pkcs11_rc create_pub_key_attributes(struct obj_attrs **out,
 						struct obj_attrs *temp)
 {
 	uint32_t const *mandated = NULL;
-	uint32_t const *optional = NULL;
+	uint32_t const *opt_or_null = NULL;
 	size_t mandated_count = 0;
-	size_t optional_count = 0;
+	size_t opt_or_null_count = 0;
 	enum pkcs11_rc rc = PKCS11_CKR_OK;
 
 	assert(get_class(temp) == PKCS11_CKO_PUBLIC_KEY);
@@ -517,33 +549,34 @@ static enum pkcs11_rc create_pub_key_attributes(struct obj_attrs **out,
 
 	assert(get_class(*out) == PKCS11_CKO_PUBLIC_KEY);
 
-	rc = set_mandatory_boolprops(out, temp, pkcs11_public_key_boolprops,
-				     ARRAY_SIZE(pkcs11_public_key_boolprops));
+	rc = set_mandatory_boolprops(out, temp, public_key_boolprops,
+				     ARRAY_SIZE(public_key_boolprops));
 	if (rc)
 		return rc;
 
-	rc = set_mandatory_attributes(out, temp, pkcs11_public_key_mandated,
-				      ARRAY_SIZE(pkcs11_public_key_mandated));
+	rc = set_mandatory_attributes(out, temp, public_key_mandated,
+				      ARRAY_SIZE(public_key_mandated));
 	if (rc)
 		return rc;
 
-	rc = set_optional_attributes(out, temp, pkcs11_public_key_optional,
-				     ARRAY_SIZE(pkcs11_public_key_optional));
+	rc = set_attributes_opt_or_null(out, temp,
+					public_key_opt_or_null,
+					ARRAY_SIZE(public_key_opt_or_null));
 	if (rc)
 		return rc;
 
 	switch (get_key_type(*out)) {
 	case PKCS11_CKK_RSA:
-		mandated = pkcs11_rsa_public_key_mandated;
-		optional = pkcs11_rsa_public_key_optional;
-		mandated_count = ARRAY_SIZE(pkcs11_rsa_public_key_mandated);
-		optional_count = ARRAY_SIZE(pkcs11_rsa_public_key_optional);
+		mandated = rsa_public_key_mandated;
+		opt_or_null = rsa_public_key_opt_or_null;
+		mandated_count = ARRAY_SIZE(rsa_public_key_mandated);
+		opt_or_null_count = ARRAY_SIZE(rsa_public_key_opt_or_null);
 		break;
 	case PKCS11_CKK_EC:
-		mandated = pkcs11_ec_public_key_mandated;
-		optional = pkcs11_ec_public_key_optional;
-		mandated_count = ARRAY_SIZE(pkcs11_ec_public_key_mandated);
-		optional_count = ARRAY_SIZE(pkcs11_ec_public_key_optional);
+		mandated = ec_public_key_mandated;
+		opt_or_null = ec_public_key_opt_or_null;
+		mandated_count = ARRAY_SIZE(ec_public_key_mandated);
+		opt_or_null_count = ARRAY_SIZE(ec_public_key_opt_or_null);
 		break;
 	default:
 		EMSG("Invalid key type %#"PRIx32"/%s",
@@ -556,16 +589,17 @@ static enum pkcs11_rc create_pub_key_attributes(struct obj_attrs **out,
 	if (rc)
 		return rc;
 
-	return set_optional_attributes(out, temp, optional, optional_count);
+	return set_attributes_opt_or_null(out, temp, opt_or_null,
+					  opt_or_null_count);
 }
 
 static enum pkcs11_rc create_priv_key_attributes(struct obj_attrs **out,
 						 struct obj_attrs *temp)
 {
 	uint32_t const *mandated = NULL;
-	uint32_t const *optional = NULL;
+	uint32_t const *opt_or_null = NULL;
 	size_t mandated_count = 0;
-	size_t optional_count = 0;
+	size_t opt_or_null_count = 0;
 	enum pkcs11_rc rc = PKCS11_CKR_OK;
 
 	assert(get_class(temp) == PKCS11_CKO_PRIVATE_KEY);
@@ -576,31 +610,31 @@ static enum pkcs11_rc create_priv_key_attributes(struct obj_attrs **out,
 
 	assert(get_class(*out) == PKCS11_CKO_PRIVATE_KEY);
 
-	rc = set_mandatory_boolprops(out, temp, pkcs11_private_key_boolprops,
-				     ARRAY_SIZE(pkcs11_private_key_boolprops));
+	rc = set_mandatory_boolprops(out, temp, private_key_boolprops,
+				     ARRAY_SIZE(private_key_boolprops));
 	if (rc)
 		return rc;
 
-	rc = set_mandatory_attributes(out, temp, pkcs11_private_key_mandated,
-				      ARRAY_SIZE(pkcs11_private_key_mandated));
+	rc = set_mandatory_attributes(out, temp, private_key_mandated,
+				      ARRAY_SIZE(private_key_mandated));
 	if (rc)
 		return rc;
 
-	rc = set_optional_attributes(out, temp, pkcs11_private_key_optional,
-				     ARRAY_SIZE(pkcs11_private_key_optional));
+	rc = set_attributes_opt_or_null(out, temp, private_key_opt_or_null,
+					ARRAY_SIZE(private_key_opt_or_null));
 	if (rc)
 		return rc;
 
 	switch (get_key_type(*out)) {
 	case PKCS11_CKK_RSA:
-		optional = pkcs11_rsa_private_key_optional;
-		optional_count = ARRAY_SIZE(pkcs11_rsa_private_key_optional);
+		opt_or_null = rsa_private_key_opt_or_null;
+		opt_or_null_count = ARRAY_SIZE(rsa_private_key_opt_or_null);
 		break;
 	case PKCS11_CKK_EC:
-		mandated = pkcs11_ec_private_key_mandated;
-		optional = pkcs11_ec_private_key_optional;
-		mandated_count = ARRAY_SIZE(pkcs11_ec_private_key_mandated);
-		optional_count = ARRAY_SIZE(pkcs11_ec_private_key_optional);
+		mandated = ec_private_key_mandated;
+		opt_or_null = ec_private_key_opt_or_null;
+		mandated_count = ARRAY_SIZE(ec_private_key_mandated);
+		opt_or_null_count = ARRAY_SIZE(ec_private_key_opt_or_null);
 		break;
 	default:
 		EMSG("Invalid key type %#"PRIx32"/%s",
@@ -613,7 +647,8 @@ static enum pkcs11_rc create_priv_key_attributes(struct obj_attrs **out,
 	if (rc)
 		return rc;
 
-	return set_optional_attributes(out, temp, optional, optional_count);
+	return set_attributes_opt_or_null(out, temp, opt_or_null,
+					  opt_or_null_count);
 }
 
 /*