From 67aae9d2873bb56a7e7028709d2f2d4bd8897955 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Niels=20M=C3=B6ller?= <nisse@lysator.liu.se>
Date: Sat, 5 Aug 2023 21:05:49 +0200
Subject: [PATCH] Let make check test for side channel silence, if valgrind is
 available.

---
 ChangeLog                           | 22 +++++++++++++++++
 rsa-internal.h                      | 10 ++++++++
 rsa-sec-decrypt.c                   | 29 +++++++++++++++-------
 testsuite/Makefile.in               |  6 +++--
 testsuite/cnd-memcpy-test.c         | 15 ++++--------
 testsuite/gcm-test.c                | 23 +++++++-----------
 testsuite/memeql-test.c             | 11 +++------
 testsuite/pkcs1-sec-decrypt-test.c  | 15 ++++--------
 testsuite/rsa-sec-decrypt-test.c    | 37 ++++++++++++++---------------
 testsuite/sc-cnd-memcpy-test        |  6 +++++
 testsuite/sc-gcm-test               |  6 +++++
 testsuite/sc-memeql-test            |  6 +++++
 testsuite/sc-pkcs1-sec-decrypt-test |  6 +++++
 testsuite/sc-rsa-sec-decrypt-test   |  6 +++++
 testsuite/sc-valgrind.sh            |  7 ++++++
 testsuite/testutils.c               | 36 ++++++++++++++++++++++++++++
 testsuite/testutils.h               |  9 +++++++
 17 files changed, 177 insertions(+), 73 deletions(-)
 create mode 100755 testsuite/sc-cnd-memcpy-test
 create mode 100755 testsuite/sc-gcm-test
 create mode 100755 testsuite/sc-memeql-test
 create mode 100755 testsuite/sc-pkcs1-sec-decrypt-test
 create mode 100755 testsuite/sc-rsa-sec-decrypt-test
 create mode 100644 testsuite/sc-valgrind.sh

diff --git a/ChangeLog b/ChangeLog
index 136fc78f..5cc37901 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -49,6 +49,28 @@
 	* tools/sexp-conv-test: Likewise.
 	* tools/pkcs1-conv-test: Likewise.
 
+2023-08-05  Niels Möller  <nisse@lysator.liu.se>
+
+	* testsuite/testutils.c (mark_bytes_undefined)
+	(mark_bytes_defined): New functions. Update side-channel related
+	tests to use them.
+	(main): Check environment variable NETTLE_TEST_SIDE_CHANNEL.
+	(test_side_channel): New global variable.
+
+	* testsuite/sc-valgrind.sh (with_valgrind): New file, new shell
+	utility function.
+
+	* testsuite/sc-pkcs1-sec-decrypt-test: New test, for side channel
+	silence.
+	* testsuite/sc-memeql-test: Likewise.
+	* testsuite/sc-gcm-test: Likewise.
+	* testsuite/sc-cnd-memcpy-test: Likewise.
+	* testsuite/rsa-sec-decrypt-test: Likewise.
+
+	* rsa-sec-decrypt.c (_rsa_sec_decrypt): New internal function,
+	without input range checks.
+	(rsa_sec_decrypt): Use it.
+
 2023-08-02  Niels Möller  <nisse@lysator.liu.se>
 
 	* configure.ac: Replace obsoleted macros, require autoconf-2.69,
diff --git a/rsa-internal.h b/rsa-internal.h
index f66a7df0..ed4ebe88 100644
--- a/rsa-internal.h
+++ b/rsa-internal.h
@@ -44,6 +44,7 @@
 #define _rsa_sec_compute_root_itch _nettle_rsa_sec_compute_root_itch
 #define _rsa_sec_compute_root _nettle_rsa_sec_compute_root
 #define _rsa_sec_compute_root_tr _nettle_rsa_sec_compute_root_tr
+#define _rsa_sec_decrypt _nettle_rsa_sec_decrypt
 
 /* Internal functions. */
 int
@@ -85,4 +86,13 @@ _rsa_sec_compute_root_tr(const struct rsa_public_key *pub,
 			 void *random_ctx, nettle_random_func *random,
 			 mp_limb_t *x, const mp_limb_t *m);
 
+/* Variant without range check of the input, to ease testing for
+   side-channel silence. */
+int
+_rsa_sec_decrypt (const struct rsa_public_key *pub,
+		  const struct rsa_private_key *key,
+		  void *random_ctx, nettle_random_func *random,
+		  size_t length, uint8_t *message,
+		  const mpz_t gibberish);
+
 #endif /* NETTLE_RSA_INTERNAL_H_INCLUDED */
diff --git a/rsa-sec-decrypt.c b/rsa-sec-decrypt.c
index 4c98958d..e2f953e2 100644
--- a/rsa-sec-decrypt.c
+++ b/rsa-sec-decrypt.c
@@ -44,21 +44,19 @@
 
 #include "gmp-glue.h"
 
+/* Variant without range check of the input, to ease testing for
+   side-channel silence. */
 int
-rsa_sec_decrypt(const struct rsa_public_key *pub,
-	        const struct rsa_private_key *key,
-	        void *random_ctx, nettle_random_func *random,
-	        size_t length, uint8_t *message,
-	        const mpz_t gibberish)
+_rsa_sec_decrypt (const struct rsa_public_key *pub,
+		  const struct rsa_private_key *key,
+		  void *random_ctx, nettle_random_func *random,
+		  size_t length, uint8_t *message,
+		  const mpz_t gibberish)
 {
   TMP_GMP_DECL (m, mp_limb_t);
   TMP_GMP_DECL (em, uint8_t);
   int res;
 
-  /* First check that input is in range. */
-  if (mpz_sgn (gibberish) < 0 || mpz_cmp (gibberish, pub->n) >= 0)
-    return 0;
-
   TMP_GMP_ALLOC (m, mpz_size(pub->n));
   TMP_GMP_ALLOC (em, key->size);
 
@@ -78,3 +76,16 @@ rsa_sec_decrypt(const struct rsa_public_key *pub,
   return res;
 }
 
+int
+rsa_sec_decrypt (const struct rsa_public_key *pub,
+		 const struct rsa_private_key *key,
+		 void *random_ctx, nettle_random_func *random,
+		 size_t length, uint8_t *message,
+		 const mpz_t gibberish)
+{
+  /* First check that input is in range. */
+  if (mpz_sgn (gibberish) < 0 || mpz_cmp (gibberish, pub->n) >= 0)
+    return 0;
+
+  return _rsa_sec_decrypt (pub, key, random_ctx, random, length, message, gibberish);
+}
diff --git a/testsuite/Makefile.in b/testsuite/Makefile.in
index 2aa1dd81..ff491c4a 100644
--- a/testsuite/Makefile.in
+++ b/testsuite/Makefile.in
@@ -66,7 +66,9 @@ TS_HOGWEED = $(TS_HOGWEED_SOURCES:.c=$(EXEEXT))
 TS_C = $(TS_NETTLE) @IF_HOGWEED@ $(TS_HOGWEED)
 TS_CXX = @IF_CXX@ $(CXX_SOURCES:.cxx=$(EXEEXT))
 TARGETS = $(TS_C) $(TS_CXX)
-TS_SH = symbols-test
+TS_SC = sc-cnd-memcpy-test sc-gcm-test sc-memeql-test \
+	@IF_HOGWEED@ sc-pkcs1-sec-decrypt-test sc-rsa-sec-decrypt-test
+TS_SH = $(TS_SC) symbols-test
 TS_ALL = $(TARGETS) $(TS_SH) @IF_DLOPEN_TEST@ dlopen-test$(EXEEXT)
 
 TS_FAT = $(patsubst %, %$(EXEEXT), aes-test cbc-test \
@@ -127,7 +129,7 @@ $(TARGETS) $(EXTRA_TARGETS): testutils.$(OBJEXT) ../nettle-internal.$(OBJEXT) \
 # data.
 VALGRIND = valgrind --error-exitcode=1 --leak-check=full --show-reachable=yes @IF_ASM@ --partial-loads-ok=yes
 
-check: $(TS_ALL)
+check: $(TS_ALL) $(TS_ALL:sc-%=%)
 	TEST_SHLIB_DIR="$(TEST_SHLIB_DIR)" \
 	  srcdir="$(srcdir)" \
 	  EMULATOR="$(EMULATOR)" NM="$(NM)" EXEEXT="$(EXEEXT)" \
diff --git a/testsuite/cnd-memcpy-test.c b/testsuite/cnd-memcpy-test.c
index 6e5db341..466608d3 100644
--- a/testsuite/cnd-memcpy-test.c
+++ b/testsuite/cnd-memcpy-test.c
@@ -2,24 +2,19 @@
 #include "knuth-lfib.h"
 #include "memops.h"
 
-#if HAVE_VALGRIND_MEMCHECK_H
-# include <valgrind/memcheck.h>
 static void
 cnd_memcpy_for_test(int cnd, void *dst, const void *src, size_t n)
 {
   /* Makes valgrind trigger on any branches depending on the input
      data. */
-  VALGRIND_MAKE_MEM_UNDEFINED (dst, n);
-  VALGRIND_MAKE_MEM_UNDEFINED (src, n);
-  VALGRIND_MAKE_MEM_UNDEFINED (&cnd, sizeof(cnd));
+  mark_bytes_undefined (n, dst);
+  mark_bytes_undefined (n, src);
+  mark_bytes_undefined (sizeof(cnd), &cnd);
 
   cnd_memcpy (cnd, dst, src, n);
-  VALGRIND_MAKE_MEM_DEFINED (src, n);
-  VALGRIND_MAKE_MEM_DEFINED (dst, n);
+  mark_bytes_defined (n, src);
+  mark_bytes_defined (n, dst);
 }
-#else
-#define cnd_memcpy_for_test cnd_memcpy
-#endif
 
 #define MAX_SIZE 50
 void
diff --git a/testsuite/gcm-test.c b/testsuite/gcm-test.c
index bc555d60..023ff6f6 100644
--- a/testsuite/gcm-test.c
+++ b/testsuite/gcm-test.c
@@ -6,13 +6,6 @@
 #include "gcm.h"
 #include "ghash-internal.h"
 
-#if HAVE_VALGRIND_MEMCHECK_H
-# include <valgrind/memcheck.h>
-#else
-# define VALGRIND_MAKE_MEM_UNDEFINED(p, n)
-# define VALGRIND_MAKE_MEM_DEFINED(p, n)
-#endif
-
 static void
 test_gcm_hash (const struct tstring *msg, const struct tstring *ref)
 {
@@ -49,19 +42,19 @@ test_ghash_internal (const struct tstring *key,
   struct gcm_key gcm_key;
   union nettle_block16 state;
 
-  /* Use VALGRIND_MAKE_MEM_DEFINED to mark inputs as "undefined", to
-     get valgrind to warn about any branches or memory accesses
-     depending on secret data. */
+  /* Mark inputs as "undefined" to valgrind, to get warnings about any
+     branches or memory accesses depending on secret data. */
   memcpy (state.b, key->data, GCM_BLOCK_SIZE);
-  VALGRIND_MAKE_MEM_UNDEFINED (&state, sizeof(state));
+  mark_bytes_undefined (sizeof(state), &state);
   _ghash_set_key (&gcm_key, &state);
 
   memcpy (state.b, iv->data, GCM_BLOCK_SIZE);
-  VALGRIND_MAKE_MEM_UNDEFINED (&state, sizeof(state));
-  VALGRIND_MAKE_MEM_UNDEFINED (message->data, message->length);
+  mark_bytes_undefined (sizeof(state), &state);
+  mark_bytes_undefined (message->length, message->data);
   _ghash_update (&gcm_key, &state, message->length / GCM_BLOCK_SIZE, message->data);
-  VALGRIND_MAKE_MEM_DEFINED (&state, sizeof(state));
-  VALGRIND_MAKE_MEM_DEFINED (message->data, message->length);
+  mark_bytes_defined (sizeof(state), &state);
+  mark_bytes_defined (message->length, message->data);
+
   if (!MEMEQ(GCM_BLOCK_SIZE, state.b, digest->data))
     {
       fprintf (stderr, "gcm_hash (internal) failed\n");
diff --git a/testsuite/memeql-test.c b/testsuite/memeql-test.c
index 356671d6..98cd8a0c 100644
--- a/testsuite/memeql-test.c
+++ b/testsuite/memeql-test.c
@@ -2,8 +2,6 @@
 #include "knuth-lfib.h"
 #include "memops.h"
 
-#if HAVE_VALGRIND_MEMCHECK_H
-# include <valgrind/memcheck.h>
 static int
 memeql_sec_for_test(const void *a, const void *b, size_t n)
 {
@@ -11,16 +9,13 @@ memeql_sec_for_test(const void *a, const void *b, size_t n)
 
   /* Makes valgrind trigger on any branches depending on the input
      data. */
-  VALGRIND_MAKE_MEM_UNDEFINED (a, n);
-  VALGRIND_MAKE_MEM_UNDEFINED (b, n);
+  mark_bytes_undefined (n, a);
+  mark_bytes_undefined (n, b);
 
   res = memeql_sec (a, b, n);
-  VALGRIND_MAKE_MEM_DEFINED (&res, sizeof(res));
+  mark_bytes_defined (sizeof(res), &res);
   return res;
 }
-#else
-#define memeql_sec_for_test memeql_sec
-#endif
 
 #define MAX_SIZE 50
 void
diff --git a/testsuite/pkcs1-sec-decrypt-test.c b/testsuite/pkcs1-sec-decrypt-test.c
index c7fcdcb6..28189382 100644
--- a/testsuite/pkcs1-sec-decrypt-test.c
+++ b/testsuite/pkcs1-sec-decrypt-test.c
@@ -2,28 +2,23 @@
 
 #include "pkcs1-internal.h"
 
-#if HAVE_VALGRIND_MEMCHECK_H
-# include <valgrind/memcheck.h>
 static int
 pkcs1_decrypt_for_test(size_t msg_len, uint8_t *msg,
                        size_t pad_len, uint8_t *pad)
 {
   int ret;
 
-  VALGRIND_MAKE_MEM_UNDEFINED (msg, msg_len);
-  VALGRIND_MAKE_MEM_UNDEFINED (pad, pad_len);
+  mark_bytes_undefined (msg_len, msg);
+  mark_bytes_undefined (pad_len, pad);
 
   ret = _pkcs1_sec_decrypt (msg_len, msg, pad_len, pad);
 
-  VALGRIND_MAKE_MEM_DEFINED (msg, msg_len);
-  VALGRIND_MAKE_MEM_DEFINED (pad, pad_len);
-  VALGRIND_MAKE_MEM_DEFINED (&ret, sizeof (ret));
+  mark_bytes_defined (msg_len, msg);
+  mark_bytes_defined (pad_len, pad);
+  mark_bytes_defined (sizeof (ret), &ret);
 
   return ret;
 }
-#else
-#define pkcs1_decrypt_for_test _pkcs1_sec_decrypt
-#endif
 
 void
 test_main(void)
diff --git a/testsuite/rsa-sec-decrypt-test.c b/testsuite/rsa-sec-decrypt-test.c
index be7ab5fb..f257723b 100644
--- a/testsuite/rsa-sec-decrypt-test.c
+++ b/testsuite/rsa-sec-decrypt-test.c
@@ -1,17 +1,15 @@
 #include "testutils.h"
 
 #include "rsa.h"
+#include "rsa-internal.h"
 #include "knuth-lfib.h"
 
-#if HAVE_VALGRIND_MEMCHECK_H
-# include <valgrind/memcheck.h>
+#define MARK_MPZ_LIMBS_UNDEFINED(x) \
+  mark_bytes_undefined (mpz_size (x) * sizeof (mp_limb_t), mpz_limbs_read (x))
+
+#define MARK_MPZ_LIMBS_DEFINED(x) \
+  mark_bytes_defined (mpz_size (x) * sizeof (mp_limb_t), mpz_limbs_read (x))
 
-#define MARK_MPZ_LIMBS_UNDEFINED(parm) \
-  VALGRIND_MAKE_MEM_UNDEFINED (mpz_limbs_read (parm), \
-                               mpz_size (parm) * sizeof (mp_limb_t))
-#define MARK_MPZ_LIMBS_DEFINED(parm) \
-  VALGRIND_MAKE_MEM_DEFINED (mpz_limbs_read (parm), \
-                               mpz_size (parm) * sizeof (mp_limb_t))
 static int
 rsa_decrypt_for_test(const struct rsa_public_key *pub,
                      const struct rsa_private_key *key,
@@ -20,6 +18,9 @@ rsa_decrypt_for_test(const struct rsa_public_key *pub,
                      const mpz_t gibberish)
 {
   int ret;
+  if (!test_side_channel)
+    return rsa_sec_decrypt (pub, key, random_ctx, random, length, message, gibberish);
+
   /* Makes valgrind trigger on any branches depending on the input
      data. Except that (i) we have to allow rsa_sec_compute_root_tr to
      check that p and q are odd, (ii) mpn_sec_div_r may leak
@@ -27,20 +28,21 @@ rsa_decrypt_for_test(const struct rsa_public_key *pub,
      normalization check and table lookup in invert_limb, and (iii)
      mpn_sec_powm may leak information about the least significant
      bits of p and q, due to table lookup in binvert_limb. */
-  VALGRIND_MAKE_MEM_UNDEFINED (message, length);
+  mark_bytes_undefined (length, message);
   MARK_MPZ_LIMBS_UNDEFINED(gibberish);
   MARK_MPZ_LIMBS_UNDEFINED(key->a);
   MARK_MPZ_LIMBS_UNDEFINED(key->b);
   MARK_MPZ_LIMBS_UNDEFINED(key->c);
-  VALGRIND_MAKE_MEM_UNDEFINED(mpz_limbs_read (key->p) + 1,
-			      (mpz_size (key->p) - 3) * sizeof(mp_limb_t));
-  VALGRIND_MAKE_MEM_UNDEFINED(mpz_limbs_read (key->q) + 1,
-			      (mpz_size (key->q) - 3) * sizeof(mp_limb_t));
+  mark_bytes_undefined ((mpz_size (key->p) - 3) * sizeof(mp_limb_t),
+			mpz_limbs_read (key->p) + 1);
+  mark_bytes_undefined((mpz_size (key->q) - 3) * sizeof(mp_limb_t), 
+		       mpz_limbs_read (key->q) + 1);
 
-  ret = rsa_sec_decrypt (pub, key, random_ctx, random, length, message, gibberish);
+  /* Call variant not checking that 0 <= gibberish < n. */
+  ret = _rsa_sec_decrypt (pub, key, random_ctx, random, length, message, gibberish);
 
-  VALGRIND_MAKE_MEM_DEFINED (message, length);
-  VALGRIND_MAKE_MEM_DEFINED (&ret, sizeof(ret));
+  mark_bytes_defined (length, message);
+  mark_bytes_defined (sizeof(ret), &ret);
   MARK_MPZ_LIMBS_DEFINED(gibberish);
   MARK_MPZ_LIMBS_DEFINED(key->a);
   MARK_MPZ_LIMBS_DEFINED(key->b);
@@ -50,9 +52,6 @@ rsa_decrypt_for_test(const struct rsa_public_key *pub,
 
   return ret;
 }
-#else
-#define rsa_decrypt_for_test rsa_sec_decrypt
-#endif
 
 #define PAYLOAD_SIZE 50
 #define DECRYPTED_SIZE 256
diff --git a/testsuite/sc-cnd-memcpy-test b/testsuite/sc-cnd-memcpy-test
new file mode 100755
index 00000000..056bbdaa
--- /dev/null
+++ b/testsuite/sc-cnd-memcpy-test
@@ -0,0 +1,6 @@
+#! /bin/sh
+
+srcdir=`dirname $0`
+. "${srcdir}/sc-valgrind.sh"
+
+with_valgrind ./cnd-memcpy-test
diff --git a/testsuite/sc-gcm-test b/testsuite/sc-gcm-test
new file mode 100755
index 00000000..57e39511
--- /dev/null
+++ b/testsuite/sc-gcm-test
@@ -0,0 +1,6 @@
+#! /bin/sh
+
+srcdir=`dirname $0`
+. "${srcdir}/sc-valgrind.sh"
+
+with_valgrind ./gcm-test
diff --git a/testsuite/sc-memeql-test b/testsuite/sc-memeql-test
new file mode 100755
index 00000000..a6dfcbef
--- /dev/null
+++ b/testsuite/sc-memeql-test
@@ -0,0 +1,6 @@
+#! /bin/sh
+
+srcdir=`dirname $0`
+. "${srcdir}/sc-valgrind.sh"
+
+with_valgrind ./memeql-test
diff --git a/testsuite/sc-pkcs1-sec-decrypt-test b/testsuite/sc-pkcs1-sec-decrypt-test
new file mode 100755
index 00000000..2e0ddc34
--- /dev/null
+++ b/testsuite/sc-pkcs1-sec-decrypt-test
@@ -0,0 +1,6 @@
+#! /bin/sh
+
+srcdir=`dirname $0`
+. "${srcdir}/sc-valgrind.sh"
+
+with_valgrind ./pkcs1-sec-decrypt-test
diff --git a/testsuite/sc-rsa-sec-decrypt-test b/testsuite/sc-rsa-sec-decrypt-test
new file mode 100755
index 00000000..0453ce25
--- /dev/null
+++ b/testsuite/sc-rsa-sec-decrypt-test
@@ -0,0 +1,6 @@
+#! /bin/sh
+
+srcdir=`dirname $0`
+. "${srcdir}/sc-valgrind.sh"
+
+with_valgrind ./rsa-sec-decrypt-test
diff --git a/testsuite/sc-valgrind.sh b/testsuite/sc-valgrind.sh
new file mode 100644
index 00000000..39e2e941
--- /dev/null
+++ b/testsuite/sc-valgrind.sh
@@ -0,0 +1,7 @@
+# To setup a test to check for branches or memory accesses depending on secret data,
+# using valgrind.
+
+with_valgrind () {
+    type valgrind >/dev/null || exit 77
+    NETTLE_TEST_SIDE_CHANNEL=1 valgrind -q --error-exitcode=1 "$@"
+}
diff --git a/testsuite/testutils.c b/testsuite/testutils.c
index 3420ae9d..1a8d10a9 100644
--- a/testsuite/testutils.c
+++ b/testsuite/testutils.c
@@ -15,6 +15,11 @@
 #include <ctype.h>
 #include <sys/time.h>
 
+#if HAVE_VALGRIND_MEMCHECK_H
+# include <valgrind/memcheck.h>
+# include <valgrind/valgrind.h>
+#endif
+
 void
 die(const char *format, ...)
 {
@@ -119,6 +124,28 @@ print_hex(size_t length, const uint8_t *data)
 
 int verbose = 0;
 
+#if HAVE_VALGRIND_MEMCHECK_H
+int test_side_channel = 0;
+
+void
+mark_bytes_undefined (size_t size, const void *p)
+{
+  if (test_side_channel)
+    VALGRIND_MAKE_MEM_UNDEFINED(p, size);
+}
+void
+mark_bytes_defined (size_t size, const void *p)
+{
+  if (test_side_channel)
+    VALGRIND_MAKE_MEM_DEFINED(p, size);
+}
+#else
+void
+mark_bytes_undefined (size_t size, const void *p) {}
+void
+mark_bytes_defined (size_t size, const void *p) {}
+#endif
+
 int
 main(int argc, char **argv)
 {
@@ -134,6 +161,15 @@ main(int argc, char **argv)
 	}
     }
 
+  if (getenv("NETTLE_TEST_SIDE_CHANNEL"))
+    {
+#if HAVE_VALGRIND_MEMCHECK_H
+      if (RUNNING_ON_VALGRIND)
+	test_side_channel = 1;
+      else
+#endif
+	SKIP();
+    }
   test_main();
 
   tstring_clear();
diff --git a/testsuite/testutils.h b/testsuite/testutils.h
index 687bcd73..97710fc9 100644
--- a/testsuite/testutils.h
+++ b/testsuite/testutils.h
@@ -73,11 +73,20 @@ tstring_print_hex(const struct tstring *s);
 void
 print_hex(size_t length, const uint8_t *data);
 
+/* If side-channel tests are requested, attach valgrind annotations on
+   given memory area. */
+void
+mark_bytes_undefined (size_t size, const void *p);
+
+void
+mark_bytes_defined (size_t size, const void *p);
+
 /* The main program */
 void
 test_main(void);
 
 extern int verbose;
+extern int test_side_channel;
 
 typedef void
 nettle_encrypt_message_func(void *ctx,
-- 
GitLab