nisse@lysator.liu.se (Niels Möller) writes:

...

...

Side-channel silent mem_equalp.

[...]

Difficulty is proper naming... Since it doesn't distinguish between lexically smaller and larger like memcmp, I don't think it should be named like *_memcmp. And we also need a name for the nettle header file.

See patch below. Header file "memops.h", and function called memeql_sec. Any comments or opinions before I merge to master?

Regards, /Niels

diff --git a/ChangeLog b/ChangeLog index b315011..09f41b0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2016-09-03 Niels Möller nisse@lysator.liu.se + + * testsuite/memeql-test.c (test_main): New test case. + (memeql_sec_for_test): Wrapper to get valgrind to check for + side-channel silence. + + * examples/rsa-decrypt.c (process_file): Use memeql_sec to check + the digest. + + * memeql-sec.c (memeql_sec): New public function, moved from... + * ccm.c (memeql_sec): ... previous location. + + * memops.h: New header file, generalizing memxor.h. + 2016-08-29 Niels Möller nisse@lysator.liu.se

* sexp-format.c (strlen_u8): New helper function. diff --git a/Makefile.in b/Makefile.in index 9d47552..135542f 100644 --- a/Makefile.in +++ b/Makefile.in @@ -109,7 +109,7 @@ nettle_SOURCES = aes-decrypt-internal.c aes-decrypt.c \ knuth-lfib.c \ md2.c md2-meta.c md4.c md4-meta.c \ md5.c md5-compress.c md5-compat.c md5-meta.c \ - memxor.c memxor3.c \ + memeql-sec.c memxor.c memxor3.c \ nettle-meta-aeads.c nettle-meta-armors.c \ nettle-meta-ciphers.c nettle-meta-hashes.c \ pbkdf2.c pbkdf2-hmac-sha1.c pbkdf2-hmac-sha256.c \ @@ -193,7 +193,7 @@ HEADERS = aes.h arcfour.h arctwo.h asn1.h blowfish.h \ macros.h \ md2.h md4.h \ md5.h md5-compat.h \ - memxor.h \ + memops.h memxor.h \ nettle-meta.h nettle-types.h \ pbkdf2.h \ pgp.h pkcs1.h realloc.h ripemd160.h rsa.h \ diff --git a/ccm.c b/ccm.c index b98bc9c..bdbd595 100644 --- a/ccm.c +++ b/ccm.c @@ -44,7 +44,7 @@ #include "ccm.h" #include "ctr.h"

-#include "memxor.h" +#include "memops.h" #include "nettle-internal.h" #include "macros.h"

@@ -246,19 +246,6 @@ ccm_encrypt_message(const void *cipher, nettle_cipher_func *f, ccm_digest(&ctx, cipher, f, tlength, tag); }

-/* FIXME: Should be made public, under some suitable name. */ -static int -memeql_sec (const void *a, const void *b, size_t n) -{ - volatile const unsigned char *ap = (const unsigned char *) a; - volatile const unsigned char *bp = (const unsigned char *) b; - volatile unsigned char d; - size_t i; - for (d = i = 0; i < n; i++) - d |= (ap[i] ^ bp[i]); - return d == 0; -} - int ccm_decrypt_message(const void *cipher, nettle_cipher_func *f, size_t nlength, const uint8_t *nonce, diff --git a/examples/rsa-decrypt.c b/examples/rsa-decrypt.c index 0168821..94c2219 100644 --- a/examples/rsa-decrypt.c +++ b/examples/rsa-decrypt.c @@ -49,6 +49,7 @@ #include "cbc.h" #include "hmac.h" #include "macros.h" +#include "memops.h" #include "rsa.h" #include "yarrow.h"

@@ -189,7 +190,7 @@ process_file(struct rsa_session *ctx, } } hmac_sha1_digest(&ctx->hmac, SHA1_DIGEST_SIZE, digest); - if (memcmp(digest, buffer + AES_BLOCK_SIZE, SHA1_DIGEST_SIZE) != 0) + if (!memeql_sec(digest, buffer + AES_BLOCK_SIZE, SHA1_DIGEST_SIZE)) { werror("Decryption failed: Invalid mac.\n"); return 0; diff --git a/memeql-sec.c b/memeql-sec.c new file mode 100644 index 0000000..b19052e --- /dev/null +++ b/memeql-sec.c @@ -0,0 +1,51 @@ +/* memeql-sec.c + + Copyright (C) 2016 Niels Möller + + This file is part of GNU Nettle. + + GNU Nettle is free software: you can redistribute it and/or + modify it under the terms of either: + + * the GNU Lesser General Public License as published by the Free + Software Foundation; either version 3 of the License, or (at your + option) any later version. + + or + + * the GNU General Public License as published by the Free + Software Foundation; either version 2 of the License, or (at your + option) any later version. + + or both in parallel, as here. + + GNU Nettle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received copies of the GNU General Public License and + the GNU Lesser General Public License along with this program. If + not, see http://www.gnu.org/licenses/. +*/ + +#if HAVE_CONFIG_H +# include "config.h" +#endif + +#include "memops.h" + +int +memeql_sec (const void *a, const void *b, size_t n) +{ + volatile const unsigned char *ap = (const unsigned char *) a; + volatile const unsigned char *bp = (const unsigned char *) b; + + volatile unsigned char diff; + size_t i; + + for (i = diff = 0; i < n; i++) + diff |= (ap[i] ^ bp[i]); + + return diff == 0; +} diff --git a/memops.h b/memops.h new file mode 100644 index 0000000..8e40594 --- /dev/null +++ b/memops.h @@ -0,0 +1,51 @@ +/* memops.h + + Copyright (C) 2016 Niels Möller + + This file is part of GNU Nettle. + + GNU Nettle is free software: you can redistribute it and/or + modify it under the terms of either: + + * the GNU Lesser General Public License as published by the Free + Software Foundation; either version 3 of the License, or (at your + option) any later version. + + or + + * the GNU General Public License as published by the Free + Software Foundation; either version 2 of the License, or (at your + option) any later version. + + or both in parallel, as here. + + GNU Nettle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received copies of the GNU General Public License and + the GNU Lesser General Public License along with this program. If + not, see http://www.gnu.org/licenses/. +*/ + +#ifndef NETTLE_MEMOPS_H_INCLUDED +#define NETTLE_MEMOPS_H_INCLUDED + +#include "memxor.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/* Name mangling */ +#define memeql_sec nettle_memeql_sec + +int +memeql_sec (const void *a, const void *b, size_t n); + +#ifdef __cplusplus +} +#endif + +#endif /* NETTLE_MEMOPS_H_INCLUDED */ diff --git a/testsuite/.test-rules.make b/testsuite/.test-rules.make index 3c74b88..b263e1f 100644 --- a/testsuite/.test-rules.make +++ b/testsuite/.test-rules.make @@ -46,6 +46,9 @@ md5-test$(EXEEXT): md5-test.$(OBJEXT) md5-compat-test$(EXEEXT): md5-compat-test.$(OBJEXT) $(LINK) md5-compat-test.$(OBJEXT) $(TEST_OBJS) -o md5-compat-test$(EXEEXT)

+memeql-test$(EXEEXT): memeql-test.$(OBJEXT) + $(LINK) memeql-test.$(OBJEXT) $(TEST_OBJS) -o memeql-test$(EXEEXT) + memxor-test$(EXEEXT): memxor-test.$(OBJEXT) $(LINK) memxor-test.$(OBJEXT) $(TEST_OBJS) -o memxor-test$(EXEEXT)

diff --git a/testsuite/Makefile.in b/testsuite/Makefile.in index 02c18fc..689d432 100644 --- a/testsuite/Makefile.in +++ b/testsuite/Makefile.in @@ -16,7 +16,7 @@ TS_NETTLE_SOURCES = aes-test.c arcfour-test.c arctwo-test.c \ camellia-test.c chacha-test.c \ des-test.c des3-test.c des-compat-test.c \ md2-test.c md4-test.c md5-test.c md5-compat-test.c \ - memxor-test.c gosthash94-test.c \ + memeql-test.c memxor-test.c gosthash94-test.c \ ripemd160-test.c \ salsa20-test.c \ sha1-test.c sha224-test.c sha256-test.c \ diff --git a/testsuite/memeql-test.c b/testsuite/memeql-test.c new file mode 100644 index 0000000..356671d --- /dev/null +++ b/testsuite/memeql-test.c @@ -0,0 +1,53 @@ +#include "testutils.h" +#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) +{ + int res; + + /* Makes valgrind trigger on any branches depending on the input + data. */ + VALGRIND_MAKE_MEM_UNDEFINED (a, n); + VALGRIND_MAKE_MEM_UNDEFINED (b, n); + + res = memeql_sec (a, b, n); + VALGRIND_MAKE_MEM_DEFINED (&res, sizeof(res)); + return res; +} +#else +#define memeql_sec_for_test memeql_sec +#endif + +#define MAX_SIZE 50 +void +test_main(void) +{ + uint8_t orig[MAX_SIZE]; + uint8_t a[MAX_SIZE]; + uint8_t b[MAX_SIZE]; + struct knuth_lfib_ctx random_ctx; + + knuth_lfib_init (&random_ctx, 11); + + size_t size; + for (size = 0; size < 50; size++) + { + size_t i; + uint8_t bit; + knuth_lfib_random (&random_ctx, size, orig); + memcpy (a, orig, size); + memcpy (b, orig, size); + ASSERT (memeql_sec_for_test (a, b, size)); + for (i = 0; i < size; i++) + for (bit = 0x80; bit; bit >>= 1) + { + b[i] = orig[i] ^ bit; + ASSERT (!memeql_sec_for_test (a, b, size)); + b[i] = orig[i]; + } + } +}