[PATCH 2/2] fips: Fix self-check compiled with clang

[Clemens Lang]

* src/fips.c [ENABLE_HMAC_BINARY_CHECK] (hmac256_check): Prevent
  constant propagation of a 0 HMAC value.

--

clang 13 assumes that the static const unsigned char[32]
hmac_for_the_implementation is zero, propagates this constant into its
use in hmac256_check and replaces with invocation of memcmp(3) with
assembly instructions that compare the computed digest with 0.

clang is able to make this assumption, because a 0-initialized static
const variable should never change its value, but this assumption is
invalid as soon as objcpy(1) changes the HMAC in the binary.

Add a volatile specifier to prevent this optimization. Note that this
requires casting away the volatile keyword in some invocations where
function signature does not support it.

Signed-off-by: Clemens Lang <cllang at redhat.com>
---
 src/fips.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/fips.c b/src/fips.c
index 193af36b..fabc5158 100644
--- a/src/fips.c
+++ b/src/fips.c
@@ -592,7 +592,10 @@ run_random_selftests (void)
 # endif
 #define HMAC_LEN 32
 
-static const unsigned char __attribute__ ((section (".rodata1")))
+/* Compilers can and will constant-propagate this as 0 when reading if it is
+ * not declared volatile. Since this value will be changed using objcopy(1)
+ * after compilation, this can cause the HMAC verification to fail. */
+static const volatile unsigned char __attribute__ ((section (".rodata1")))
 hmac_for_the_implementation[HMAC_LEN];
 
 /**
@@ -805,10 +808,16 @@ hmac256_check (const char *filename, const char *key)
     err = gpg_error (GPG_ERR_INV_HANDLE);
   else
     {
+      unsigned char hmac[HMAC_LEN];
       unsigned char *digest;
+      size_t idx;
+
+      // memcpy(3) does not accept volatile pointers
+      for (idx = 0; idx < HMAC_LEN; idx++)
+        hmac[idx] = hmac_for_the_implementation[idx];
 
       digest = _gcry_md_read (hd, 0);
-      if (!memcmp (digest, hmac_for_the_implementation, HMAC_LEN))
+      if (!memcmp (digest, hmac, HMAC_LEN))
         /* Success.  */
         err = 0;
       else
@@ -830,7 +839,7 @@ check_binary_integrity (void)
   Dl_info info;
   const char *key = KEY_FOR_BINARY_CHECK;
 
-  if (!dladdr (hmac_for_the_implementation, &info))
+  if (!dladdr ((const char *)hmac_for_the_implementation, &info))
     err = gpg_error_from_syserror ();
   else
     err = hmac256_check (info.dli_fname, key);
-- 
2.34.1