refactor(auth): remove return_if_error() macro
The usage of this macro hinders the accuracy of code coverage
data. Lines of code calling this macro always appear as covered because
the test condition within it always gets executed; however, the branch
is not necessarily taken. Consequently, we lose branch coverage
information on these error code paths.
Besides, it is debatable whether such a simple macro really improves
code readability or on the contrary obfuscates the code...
For these reasons, this patch inlines the macro code everywhere it was
called.
It also adds some error messages in all these places to help narrowing
down authentication failures. These messages only get displayed and
compiled into the binaries when building TF-A with 'LOG_VERBOSE' level
of verbosity. We use the same message string everywhere in order to
limit the memory footprint increase for 'LOG_VERBOSE' builds.
Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
Change-Id: I461078bb8c6fd6811d2cbefbe3614e17e83796f2
diff --git a/drivers/auth/auth_mod.c b/drivers/auth/auth_mod.c
index 14c3172..608866c 100644
--- a/drivers/auth/auth_mod.c
+++ b/drivers/auth/auth_mod.c
@@ -25,13 +25,6 @@
/* ASN.1 tags */
#define ASN1_INTEGER 0x02
-#define return_if_error(rc) \
- do { \
- if (rc != 0) { \
- return rc; \
- } \
- } while (0)
-
#pragma weak plat_set_nv_ctr2
static int cmp_auth_param_type_desc(const auth_param_type_desc_t *a,
@@ -99,24 +92,37 @@
{
void *data_ptr, *hash_der_ptr;
unsigned int data_len, hash_der_len;
- int rc = 0;
+ int rc;
/* Get the hash from the parent image. This hash will be DER encoded
* and contain the hash algorithm */
rc = auth_get_param(param->hash, img_desc->parent,
&hash_der_ptr, &hash_der_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/* Get the data to be hashed from the current image */
rc = img_parser_get_auth_param(img_desc->img_type, param->data,
img, img_len, &data_ptr, &data_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/* Ask the crypto module to verify this hash */
rc = crypto_mod_verify_hash(data_ptr, data_len,
hash_der_ptr, hash_der_len);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
- return rc;
+ return 0;
}
/*
@@ -153,22 +159,34 @@
void *data_ptr, *pk_ptr, *cnv_pk_ptr, *pk_plat_ptr, *sig_ptr, *sig_alg_ptr, *pk_oid;
unsigned int data_len, pk_len, cnv_pk_len, pk_plat_len, sig_len, sig_alg_len;
unsigned int flags = 0;
- int rc = 0;
+ int rc;
/* Get the data to be signed from current image */
rc = img_parser_get_auth_param(img_desc->img_type, param->data,
img, img_len, &data_ptr, &data_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/* Get the signature from current image */
rc = img_parser_get_auth_param(img_desc->img_type, param->sig,
img, img_len, &sig_ptr, &sig_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/* Get the signature algorithm from current image */
rc = img_parser_get_auth_param(img_desc->img_type, param->alg,
img, img_len, &sig_alg_ptr, &sig_alg_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/* Get the public key from the parent. If there is no parent (NULL),
* the certificate has been signed with the ROTPK, so we have to get
@@ -176,7 +194,11 @@
if (img_desc->parent != NULL) {
rc = auth_get_param(param->pk, img_desc->parent,
&pk_ptr, &pk_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
} else {
/*
* Root certificates are signed with the ROTPK, so we have to
@@ -184,7 +206,11 @@
*/
rc = plat_get_rotpk_info(param->pk->cookie, &pk_plat_ptr,
&pk_plat_len, &flags);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
assert(is_rotpk_flags_valid(flags));
@@ -192,7 +218,11 @@
rc = img_parser_get_auth_param(img_desc->img_type,
param->pk, img, img_len,
&pk_ptr, &pk_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/*
* Validate the certificate's key against the platform ROTPK.
@@ -211,7 +241,11 @@
* suffixed or modified pk
*/
rc = crypto_mod_convert_pk(pk_ptr, pk_len, &cnv_pk_ptr, &cnv_pk_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/*
* The hash of the certificate's public key must match
@@ -219,7 +253,11 @@
*/
rc = crypto_mod_verify_hash(cnv_pk_ptr, cnv_pk_len,
pk_plat_ptr, pk_plat_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
} else {
/* Platform supports full ROTPK */
if ((pk_len != pk_plat_len) ||
@@ -245,7 +283,8 @@
*/
rc = plat_mboot_measure_key(pk_oid, pk_ptr, pk_len);
if (rc != 0) {
- WARN("Public Key measurement failure = %d\n", rc);
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
}
}
@@ -254,8 +293,13 @@
sig_ptr, sig_len,
sig_alg_ptr, sig_alg_len,
pk_ptr, pk_len);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
- return rc;
+ return 0;
}
/*
@@ -283,14 +327,18 @@
void *data_ptr = NULL;
unsigned int data_len, len, i;
unsigned int plat_nv_ctr;
- int rc = 0;
+ int rc;
bool is_trial_run = false;
/* Get the counter value from current image. The AM expects the IPM
* to return the counter value as a DER encoded integer */
rc = img_parser_get_auth_param(img_desc->img_type, param->cert_nv_ctr,
img, img_len, &data_ptr, &data_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/* Parse the DER encoded integer */
assert(data_ptr);
@@ -329,7 +377,11 @@
/* Get the counter from the platform */
rc = plat_get_nv_ctr(param->plat_nv_ctr->cookie, &plat_nv_ctr);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
if (*cert_nv_ctr < plat_nv_ctr) {
/* Invalid NV-counter */
@@ -417,7 +469,11 @@
/* Ask the parser to check the image integrity */
rc = img_parser_check_integrity(img_desc->img_type, img_ptr, img_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/* Authenticate the image using the methods indicated in the image
* descriptor. */
@@ -449,7 +505,11 @@
rc = 1;
break;
}
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
- return_if_error(rc);
}
/*
@@ -459,7 +519,11 @@
if (need_nv_ctr_upgrade && sig_auth_done) {
rc = plat_set_nv_ctr2(nv_ctr_param->plat_nv_ctr->cookie,
img_desc, cert_nv_ctr);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
}
/* Extract the parameters indicated in the image descriptor to
@@ -474,7 +538,11 @@
rc = img_parser_get_auth_param(img_desc->img_type,
img_desc->authenticated_data[i].type_desc,
img_ptr, img_len, ¶m_ptr, ¶m_len);
- return_if_error(rc);
+ if (rc != 0) {
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
+ return rc;
+ }
/* Check parameter size */
if (param_len > img_desc->authenticated_data[i].data.len) {
@@ -495,8 +563,8 @@
param_ptr,
param_len);
if (rc != 0) {
- WARN("Public Key measurement "
- "failure = %d\n", rc);
+ VERBOSE("[TBB] %s():%d failed with error code %d.\n",
+ __func__, __LINE__, rc);
}
}
}