Commit a2a5a945 authored by Manish V Badarkhe's avatar Manish V Badarkhe
Browse files

fix(driver/auth): avoid NV counter upgrade without certificate validation



Platform NV counter get updated (if cert NV counter > plat NV counter)
before authenticating the certificate if the platform specifies NV
counter method before signature authentication in its CoT, and this
provides an opportunity for a tempered certificate to upgrade the
platform NV counter. This is theoretical issue, as in practice none
of the standard CoT (TBBR, dualroot) or upstream platforms ones (NXP)
exercised this issue.

To fix this issue, modified the auth_nvctr method to do only NV
counter check, and flags if the NV counter upgrade is needed or not.
Then ensured that the platform NV counter gets upgraded with the NV
counter value from the certificate only after that certificate gets
authenticated.

This change is verified manually by modifying the CoT that specifies
certificate with:
1. NV counter authentication before signature authentication
   method
2. NV counter authentication method only

Change-Id: I1ad17f1a911fb1035a1a60976cc26b2965b05166
Signed-off-by: default avatarManish V Badarkhe <Manish.Badarkhe@arm.com>
parent d3555651
...@@ -222,19 +222,25 @@ static int auth_signature(const auth_method_param_sig_t *param, ...@@ -222,19 +222,25 @@ static int auth_signature(const auth_method_param_sig_t *param,
* To protect the system against rollback, the platform includes a non-volatile * To protect the system against rollback, the platform includes a non-volatile
* counter whose value can only be increased. All certificates include a counter * counter whose value can only be increased. All certificates include a counter
* value that should not be lower than the value stored in the platform. If the * value that should not be lower than the value stored in the platform. If the
* value is larger, the counter in the platform must be updated to the new * value is larger, the counter in the platform must be updated to the new value
* value. * (provided it has been authenticated).
* *
* Return: 0 = success, Otherwise = error * Return: 0 = success, Otherwise = error
* Returns additionally,
* cert_nv_ctr -> NV counter value present in the certificate
* need_nv_ctr_upgrade = 0 -> platform NV counter upgrade is not needed
* need_nv_ctr_upgrade = 1 -> platform NV counter upgrade is needed
*/ */
static int auth_nvctr(const auth_method_param_nv_ctr_t *param, static int auth_nvctr(const auth_method_param_nv_ctr_t *param,
const auth_img_desc_t *img_desc, const auth_img_desc_t *img_desc,
void *img, unsigned int img_len) void *img, unsigned int img_len,
unsigned int *cert_nv_ctr,
bool *need_nv_ctr_upgrade)
{ {
char *p; char *p;
void *data_ptr = NULL; void *data_ptr = NULL;
unsigned int data_len, len, i; unsigned int data_len, len, i;
unsigned int cert_nv_ctr, plat_nv_ctr; unsigned int plat_nv_ctr;
int rc = 0; int rc = 0;
/* Get the counter value from current image. The AM expects the IPM /* Get the counter value from current image. The AM expects the IPM
...@@ -265,22 +271,20 @@ static int auth_nvctr(const auth_method_param_nv_ctr_t *param, ...@@ -265,22 +271,20 @@ static int auth_nvctr(const auth_method_param_nv_ctr_t *param,
} }
/* Convert to unsigned int. This code is for a little-endian CPU */ /* Convert to unsigned int. This code is for a little-endian CPU */
cert_nv_ctr = 0; *cert_nv_ctr = 0;
for (i = 0; i < len; i++) { for (i = 0; i < len; i++) {
cert_nv_ctr = (cert_nv_ctr << 8) | *p++; *cert_nv_ctr = (*cert_nv_ctr << 8) | *p++;
} }
/* Get the counter from the platform */ /* Get the counter from the platform */
rc = plat_get_nv_ctr(param->plat_nv_ctr->cookie, &plat_nv_ctr); rc = plat_get_nv_ctr(param->plat_nv_ctr->cookie, &plat_nv_ctr);
return_if_error(rc); return_if_error(rc);
if (cert_nv_ctr < plat_nv_ctr) { if (*cert_nv_ctr < plat_nv_ctr) {
/* Invalid NV-counter */ /* Invalid NV-counter */
return 1; return 1;
} else if (cert_nv_ctr > plat_nv_ctr) { } else if (*cert_nv_ctr > plat_nv_ctr) {
rc = plat_set_nv_ctr2(param->plat_nv_ctr->cookie, *need_nv_ctr_upgrade = true;
img_desc, cert_nv_ctr);
return_if_error(rc);
} }
return 0; return 0;
...@@ -351,6 +355,10 @@ int auth_mod_verify_img(unsigned int img_id, ...@@ -351,6 +355,10 @@ int auth_mod_verify_img(unsigned int img_id,
void *param_ptr; void *param_ptr;
unsigned int param_len; unsigned int param_len;
int rc, i; int rc, i;
unsigned int cert_nv_ctr = 0;
bool need_nv_ctr_upgrade = false;
bool sig_auth_done = false;
const auth_method_param_nv_ctr_t *nv_ctr_param = NULL;
/* Get the image descriptor from the chain of trust */ /* Get the image descriptor from the chain of trust */
img_desc = FCONF_GET_PROPERTY(tbbr, cot, img_id); img_desc = FCONF_GET_PROPERTY(tbbr, cot, img_id);
...@@ -376,10 +384,13 @@ int auth_mod_verify_img(unsigned int img_id, ...@@ -376,10 +384,13 @@ int auth_mod_verify_img(unsigned int img_id,
case AUTH_METHOD_SIG: case AUTH_METHOD_SIG:
rc = auth_signature(&auth_method->param.sig, rc = auth_signature(&auth_method->param.sig,
img_desc, img_ptr, img_len); img_desc, img_ptr, img_len);
sig_auth_done = true;
break; break;
case AUTH_METHOD_NV_CTR: case AUTH_METHOD_NV_CTR:
rc = auth_nvctr(&auth_method->param.nv_ctr, nv_ctr_param = &auth_method->param.nv_ctr;
img_desc, img_ptr, img_len); rc = auth_nvctr(nv_ctr_param,
img_desc, img_ptr, img_len,
&cert_nv_ctr, &need_nv_ctr_upgrade);
break; break;
default: default:
/* Unknown authentication method */ /* Unknown authentication method */
...@@ -389,6 +400,16 @@ int auth_mod_verify_img(unsigned int img_id, ...@@ -389,6 +400,16 @@ int auth_mod_verify_img(unsigned int img_id,
return_if_error(rc); return_if_error(rc);
} }
/*
* Do platform NV counter upgrade only if the certificate gets
* authenticated, and platform NV-counter upgrade is needed.
*/
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);
}
/* Extract the parameters indicated in the image descriptor to /* Extract the parameters indicated in the image descriptor to
* authenticate the children images. */ * authenticate the children images. */
if (img_desc->authenticated_data != NULL) { if (img_desc->authenticated_data != NULL) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment