[PATCH 01/25] memcmp: Introduce and use consttime_memcmp

Ian Jackson ijackson at chiark.greenend.org.uk
Sat Jul 20 00:38:45 BST 2013


We need to use a constant-time memcmp in MAC checking, to avoid
leaking (to an adversary) how much of the MAC is right.  (This would
be especially dangerous if our MAC was outside the encryption, which
thankfully it isn't.)

Signed-off-by: Ian Jackson <ijackson at chiark.greenend.org.uk>
---
 site.c      |    2 +-
 transform.c |    2 +-
 util.c      |   23 +++++++++++++++++++++++
 util.h      |    2 ++
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/site.c b/site.c
index db65bd8..e96d6f4 100644
--- a/site.c
+++ b/site.c
@@ -463,7 +463,7 @@ static bool_t check_msg(struct site *st, uint32_t type, struct msg *m,
 	return False;
     }
     if (type==LABEL_MSG2) return True;
-    if (memcmp(m->nR,st->remoteN,NONCELEN)!=0) {
+    if (consttime_memcmp(m->nR,st->remoteN,NONCELEN)!=0) {
 	*error="wrong remotely-generated nonce";
 	return False;
     }
diff --git a/transform.c b/transform.c
index 289b02e..893f41c 100644
--- a/transform.c
+++ b/transform.c
@@ -220,7 +220,7 @@ static uint32_t transform_reverse(void *sst, struct buffer_if *buf,
 	serpent_encrypt(&ti->mackey,macplain,macacc);
     }
     serpent_encrypt(&ti->mackey,macacc,macacc);
-    if (memcmp(macexpected,macacc,16)!=0) {
+    if (consttime_memcmp(macexpected,macacc,16)!=0) {
 	*errmsg="invalid MAC";
 	return 1;
     }
diff --git a/util.c b/util.c
index f5f3d75..e50218c 100644
--- a/util.c
+++ b/util.c
@@ -363,7 +363,30 @@ static list_t *buffer_apply(closure_t *self, struct cloc loc, dict_t *context,
     return new_closure(&st->cl);
 }
 
+static FILE *devnull;
+
+int consttime_memcmp(const void *s1in, const void *s2in, size_t n)
+{
+    const uint8_t *s1=s1in, *s2=s2in;
+    size_t accumulator=0;
+
+    assert(n <= SIZE_MAX/255);
+
+    while (n-- > 0) {
+	accumulator += (*s1++ ^ *s2++);
+    }
+    /* This forces the compiler to actually compute all of the bits of
+     * accumulator. Hopefully that's enough to stop it shortcutting
+     * the loop. */
+    fwrite(&accumulator,1,sizeof(accumulator),devnull);
+
+    return !!accumulator;
+}
+
 void util_module(dict_t *dict)
 {
+    devnull=fopen("/dev/null","wb");
+    if (!devnull) fatal_perror("could not open /dev/null (for util)");
+
     add_closure(dict,"sysbuffer",buffer_apply);
 }
diff --git a/util.h b/util.h
index af19363..2491fd5 100644
--- a/util.h
+++ b/util.h
@@ -39,4 +39,6 @@ extern int32_t write_mpbin(MP_INT *a, uint8_t *buffer, int32_t buflen);
 
 extern struct log_if *init_log(list_t *loglist);
 
+extern int consttime_memcmp(const void *s1, const void *s2, size_t n);
+
 #endif /* util_h */
-- 
1.7.2.5




More information about the sgo-software-discuss mailing list