Clear static variables in X509 parser on error

In mbedtls_x509_parser.c there are some static arrays that are filled
during the integrity check and then read whenever an authentication
parameter is requested. However, they aren't cleared in case of an
integrity check failure, which can be problematic from a security
point of view. This patch clears these arrays in the case of failure.

Change-Id: I9d48f5bc71fa13e5a75d6c45b5e34796ef13aaa2
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>

diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c
index 1a6a9a75ef3629157d4c8aadd8d9b4a12bb28dea..73da9d1e7c4d8a264073a8305e12a37e97f410de 100644 (file)
--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c
+++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2017, ARM Limited and Contributors. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -36,6 +36,7 @@
  * extensions field, such as an image hash or a public key.
  */
 
+#include <arch_helpers.h>
 #include <assert.h>
 #include <img_parser_mod.h>
 #include <mbedtls_common.h>
@@ -63,6 +64,26 @@ static mbedtls_asn1_buf pk;
 static mbedtls_asn1_buf sig_alg;
 static mbedtls_asn1_buf signature;
 
+/*
+ * Clear all static temporary variables.
+ */
+static void clear_temp_vars(void)
+{
+#define ZERO_AND_CLEAN(x)                                      \
+       do {                                                    \
+               memset(&x, 0, sizeof(x));                       \
+               clean_dcache_range((uintptr_t)&x, sizeof(x));   \
+       } while (0);
+
+       ZERO_AND_CLEAN(tbs)
+       ZERO_AND_CLEAN(v3_ext);
+       ZERO_AND_CLEAN(pk);
+       ZERO_AND_CLEAN(sig_alg);
+       ZERO_AND_CLEAN(signature);
+
+#undef ZERO_AND_CLEAN
+}
+
 /*
  * Get X509v3 extension
  *
@@ -134,7 +155,12 @@ static int get_ext(const char *oid, void **ext, unsigned int *ext_len)
 
 /*
  * Check the integrity of the certificate ASN.1 structure.
+ *
  * Extract the relevant data that will be used later during authentication.
+ *
+ * This function doesn't clear the static variables located on the top of this
+ * file in case of an error. It is only called from check_integrity(), which
+ * performs the cleanup if necessary.
  */
 static int cert_parse(void *img, unsigned int img_len)
 {
@@ -398,9 +424,18 @@ static void init(void)
        mbedtls_init();
 }
 
+/*
+ * Wrapper for cert_parse() that clears the static variables used by it in case
+ * of an error.
+ */
 static int check_integrity(void *img, unsigned int img_len)
 {
-       return cert_parse(img, img_len);
+       int rc = cert_parse(img, img_len);
+
+       if (rc != IMG_PARSER_OK)
+               clear_temp_vars();
+
+       return rc;
 }
 
 /*