From 233f207c178642c5a1bc7184d0f4165f63b554cf Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Sun, 25 Sep 2011 17:24:52 -0700 Subject: [PATCH 1/9] Use "GMP_DESC" instead of "GPM_DESC" This seemed to be the only place in the code that was using this particular transposition. And, indeed, when compiling with "GMP_DESC", it looks like it is necessary to disable Diffie-Hellman. (Otherwise, the test fails for me.) --- src/headers/tomcrypt_custom.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/headers/tomcrypt_custom.h b/src/headers/tomcrypt_custom.h index 362403b..97de5de 100644 --- a/src/headers/tomcrypt_custom.h +++ b/src/headers/tomcrypt_custom.h @@ -306,8 +306,8 @@ /* #define LTC_RSA_BLINDING */ /* Include Diffie-Hellman support */ -#ifndef GPM_DESC -/* is_prime fails for GPM */ +#ifndef GMP_DESC +/* is_prime fails for GMP */ #define MDH /* Supported Key Sizes */ #define DH768 From d61c537a2aa365f8e1587eceb2bb6b06ab609cfd Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Mon, 26 Sep 2011 00:29:57 -0700 Subject: [PATCH 2/9] missing a comma --- src/math/gmp_desc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/math/gmp_desc.c b/src/math/gmp_desc.c index 9d7ff07..403211b 100644 --- a/src/math/gmp_desc.c +++ b/src/math/gmp_desc.c @@ -487,7 +487,7 @@ const ltc_math_descriptor gmp_desc = { NULL, #endif /* LTC_ECC_SHAMIR */ #else - NULL, NULL, NULL, NULL, NULL + NULL, NULL, NULL, NULL, NULL, #endif /* LTC_MECC */ #ifdef LTC_MRSA From 4a2b54a446baac1f760a0f9c2fe2e44fb71a1219 Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Sun, 25 Sep 2011 18:33:31 -0700 Subject: [PATCH 3/9] Changed "make clean" to not delete crypt.lof (which is checked into git) This line: rm -f `find . -type f | grep "[.]lo" | xargs` was deleting crypt.lof, which seemed undesirable. One solution would be to end the grep expression with "$", but it seemed more straightforward just to pass "-name" to "find", rather than piping through grep. --- makefile | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/makefile b/makefile index 901a3ef..70316a4 100644 --- a/makefile +++ b/makefile @@ -329,19 +329,19 @@ profile: #This rule cleans the source tree of all compiled code, not including the pdf #documentation. clean: - rm -f `find . -type f | grep "[.]o" | xargs` - rm -f `find . -type f | grep "[.]lo" | xargs` - rm -f `find . -type f | grep "[.]a" | xargs` - rm -f `find . -type f | grep "[.]la" | xargs` - rm -f `find . -type f | grep "[.]obj" | xargs` - rm -f `find . -type f | grep "[.]lib" | xargs` - rm -f `find . -type f | grep "[.]exe" | xargs` - rm -f `find . -type f | grep "[.]gcda" | xargs` - rm -f `find . -type f | grep "[.]gcno" | xargs` - rm -f `find . -type f | grep "[.]il" | xargs` - rm -f `find . -type f | grep "[.]dyn" | xargs` - rm -f `find . -type f | grep "[.]dpi" | xargs` - rm -rf `find . -type d | grep "[.]libs" | xargs` + rm -f `find . -type f -name "*.o" | xargs` + rm -f `find . -type f -name "*.lo" | xargs` + rm -f `find . -type f -name "*.a" | xargs` + rm -f `find . -type f -name "*.la" | xargs` + rm -f `find . -type f -name "*.obj" | xargs` + rm -f `find . -type f -name "*.lib" | xargs` + rm -f `find . -type f -name "*.exe" | xargs` + rm -f `find . -type f -name "*.gcda" | xargs` + rm -f `find . -type f -name "*.gcno" | xargs` + rm -f `find . -type f -name "*.il" | xargs` + rm -f `find . -type f -name "*.dyn" | xargs` + rm -f `find . -type f -name "*.dpi" | xargs` + rm -rf `find . -type d -name "*.libs" | xargs` rm -f crypt.aux crypt.dvi crypt.idx crypt.ilg crypt.ind crypt.log crypt.toc rm -f $(TV) $(PROF) $(SMALL) $(CRYPT) $(HASHSUM) $(MULTI) $(TIMING) $(TEST) rm -rf doc/doxygen From 9228cbbd1e0e88e23c73b0caca4587aed5dd9c91 Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Sun, 25 Sep 2011 18:40:52 -0700 Subject: [PATCH 4/9] don't delete doc/crypt.pdf in "make clean" "make clean" was deleting "doc/*.pdf", despite the fact that there were two comments (one above and one below) stating that it did not. Since doc/crypt.pdf is checked into git, running "make clean" made my git state dirty, which seems undesirable. I took sort of a compromise position and had "make clean" continue to delete any other .pdf files in doc (such as refman.pdf), but explicitly not delete crypt.pdf. --- makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/makefile b/makefile index 70316a4..53f80f4 100644 --- a/makefile +++ b/makefile @@ -345,7 +345,7 @@ clean: rm -f crypt.aux crypt.dvi crypt.idx crypt.ilg crypt.ind crypt.log crypt.toc rm -f $(TV) $(PROF) $(SMALL) $(CRYPT) $(HASHSUM) $(MULTI) $(TIMING) $(TEST) rm -rf doc/doxygen - rm -f doc/*.pdf + rm -f `find . -type f -name "*.pdf" | grep -FL crypt.pdf | xargs` rm -f *.txt #build the doxy files (requires Doxygen, tetex and patience) From cecbbb88fc585e5fd5691b741c8dc0d29655f96c Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Sun, 25 Sep 2011 18:04:05 -0700 Subject: [PATCH 5/9] When a test fails, print the algorithm that it failed on. As near as I can tell, LibTomCrypt doesn't provide any way to tell which cipher failed when it reports a cipher test failure. For example, I was getting: Algorithm failed test vectors. (5) cipher_hash_test.c:14:cipher_descriptor[x].test() But there's no way to tell what value x has, and even if there was, it would take a bit of digging to determine which algorithm that corresponds to. So, I added a variant of the DO() macro, DOX(), which takes an additional string argument which is displayed on failure. So now I get: Algorithm failed test vectors. (5) - camellia cipher_hash_test.c:14:cipher_descriptor[x].test() --- testprof/cipher_hash_test.c | 18 +++++++++--------- testprof/test_driver.c | 7 +++++-- testprof/tomcrypt_test.h | 8 +++++--- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/testprof/cipher_hash_test.c b/testprof/cipher_hash_test.c index 666d913..dba7d9a 100644 --- a/testprof/cipher_hash_test.c +++ b/testprof/cipher_hash_test.c @@ -11,25 +11,25 @@ int cipher_hash_test(void) /* test ciphers */ for (x = 0; cipher_descriptor[x].name != NULL; x++) { - DO(cipher_descriptor[x].test()); + DOX(cipher_descriptor[x].test(), cipher_descriptor[x].name); } /* test hashes */ for (x = 0; hash_descriptor[x].name != NULL; x++) { - DO(hash_descriptor[x].test()); + DOX(hash_descriptor[x].test(), hash_descriptor[x].name); } /* test prngs (test, import/export */ for (x = 0; prng_descriptor[x].name != NULL; x++) { - DO(prng_descriptor[x].test()); - DO(prng_descriptor[x].start(&nprng)); - DO(prng_descriptor[x].add_entropy((unsigned char *)"helloworld12", 12, &nprng)); - DO(prng_descriptor[x].ready(&nprng)); + DOX(prng_descriptor[x].test(), prng_descriptor[x].name); + DOX(prng_descriptor[x].start(&nprng), prng_descriptor[x].name); + DOX(prng_descriptor[x].add_entropy((unsigned char *)"helloworld12", 12, &nprng), prng_descriptor[x].name); + DOX(prng_descriptor[x].ready(&nprng), prng_descriptor[x].name); n = sizeof(buf); - DO(prng_descriptor[x].pexport(buf, &n, &nprng)); + DOX(prng_descriptor[x].pexport(buf, &n, &nprng), prng_descriptor[x].name); prng_descriptor[x].done(&nprng); - DO(prng_descriptor[x].pimport(buf, n, &nprng)); - DO(prng_descriptor[x].ready(&nprng)); + DOX(prng_descriptor[x].pimport(buf, n, &nprng), prng_descriptor[x].name); + DOX(prng_descriptor[x].ready(&nprng), prng_descriptor[x].name); if (prng_descriptor[x].read(buf, 100, &nprng) != 100) { fprintf(stderr, "Error reading from imported PRNG!\n"); exit(EXIT_FAILURE); diff --git a/testprof/test_driver.c b/testprof/test_driver.c index 6e54668..b728f91 100644 --- a/testprof/test_driver.c +++ b/testprof/test_driver.c @@ -1,9 +1,12 @@ #include -void run_cmd(int res, int line, char *file, char *cmd) +void run_cmd(int res, int line, char *file, char *cmd, const char *algorithm) { if (res != CRYPT_OK) { - fprintf(stderr, "%s (%d)\n%s:%d:%s\n", error_to_string(res), res, file, line, cmd); + fprintf(stderr, "%s (%d)%s%s\n%s:%d:%s\n", + error_to_string(res), res, + (algorithm ? " - " : ""), (algorithm ? algorithm : ""), + file, line, cmd); if (res != CRYPT_NOP) { exit(EXIT_FAILURE); } diff --git a/testprof/tomcrypt_test.h b/testprof/tomcrypt_test.h index 237eee3..84e173c 100644 --- a/testprof/tomcrypt_test.h +++ b/testprof/tomcrypt_test.h @@ -36,12 +36,14 @@ typedef struct { extern prng_state yarrow_prng; -void run_cmd(int res, int line, char *file, char *cmd); +void run_cmd(int res, int line, char *file, char *cmd, const char *algorithm); #ifdef LTC_VERBOSE -#define DO(x) do { fprintf(stderr, "%s:\n", #x); run_cmd((x), __LINE__, __FILE__, #x); } while (0); +#define DO(x) do { fprintf(stderr, "%s:\n", #x); run_cmd((x), __LINE__, __FILE__, #x, NULL); } while (0); +#define DOX(x, str) do { fprintf(stderr, "%s - %s:\n", #x, (str)); run_cmd((x), __LINE__, __FILE__, #x, (str)); } while (0); #else -#define DO(x) do { run_cmd((x), __LINE__, __FILE__, #x); } while (0); +#define DO(x) do { run_cmd((x), __LINE__, __FILE__, #x, NULL); } while (0); +#define DOX(x, str) do { run_cmd((x), __LINE__, __FILE__, #x, (str)); } while (0); #endif /* TESTS */ From ee7c031ddf27bc6a2a4b4a78b8a183b8630e7691 Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Sun, 25 Sep 2011 17:39:04 -0700 Subject: [PATCH 6/9] Added some code (commented out) to print details about Camellia test failure (and ditto for SEED) This is modeled after similar commented-out code in sober128_test(), but slightly fancier. --- src/ciphers/camellia.c | 15 +++++++++++++++ src/ciphers/kseed.c | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/ciphers/camellia.c b/src/ciphers/camellia.c index 0651012..e7b172f 100644 --- a/src/ciphers/camellia.c +++ b/src/ciphers/camellia.c @@ -686,6 +686,21 @@ int camellia_test(void) } camellia_done(&skey); if (XMEMCMP(tests[x].ct, buf[0], 16) || XMEMCMP(tests[x].pt, buf[1], 16)) { +#if 0 + int i, j; + printf ("\n\nLTC_CAMELLIA failed for x=%d, I got:\n", x); + for (i = 0; i < 2; i++) { + const unsigned char *expected, *actual; + expected = (i ? tests[x].pt : tests[x].ct); + actual = buf[i]; + printf ("expected actual (%s)\n", (i ? "plaintext" : "ciphertext")); + for (j = 0; j < 16; j++) { + const char *eq = (expected[j] == actual[j] ? "==" : "!="); + printf (" %02x %s %02x\n", expected[j], eq, actual[j]); + } + printf ("\n"); + } +#endif return CRYPT_FAIL_TESTVECTOR; } } diff --git a/src/ciphers/kseed.c b/src/ciphers/kseed.c index a163c95..1065d8c 100644 --- a/src/ciphers/kseed.c +++ b/src/ciphers/kseed.c @@ -346,6 +346,21 @@ int kseed_test(void) kseed_ecb_encrypt(tests[x].pt, buf[0], &skey); kseed_ecb_decrypt(buf[0], buf[1], &skey); if (XMEMCMP(buf[0], tests[x].ct, 16) || XMEMCMP(buf[1], tests[x].pt, 16)) { +#if 0 + int i, j; + printf ("\n\nLTC_KSEED failed for x=%d, I got:\n", x); + for (i = 0; i < 2; i++) { + const unsigned char *expected, *actual; + expected = (i ? tests[x].pt : tests[x].ct); + actual = buf[i]; + printf ("expected actual (%s)\n", (i ? "plaintext" : "ciphertext")); + for (j = 0; j < 16; j++) { + const char *eq = (expected[j] == actual[j] ? "==" : "!="); + printf (" %02x %s %02x\n", expected[j], eq, actual[j]); + } + printf ("\n"); + } +#endif return CRYPT_FAIL_TESTVECTOR; } } From cefff85550786ec869b39c0cb4a5904e88c84319 Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Sun, 25 Sep 2011 17:11:25 -0700 Subject: [PATCH 7/9] Add "memory" as a clobber for bswap inline assembly. This had been causing Camellia (the only cipher that uses these macros) to fail when compiling "out-of-the-box" with gcc version "4.3.3-5ubuntu4". I think because the compiler had no idea any memory access was going on in these macros. Adding "memory" as a clobber solves the problem, but is probably overkill. I suspect that if we specify the constraint for y differently, we could get rid of both "memory" and __volatile__, which would allow the compiler to optimize much more. Also, in gcc versions that support it, we should probably use the bswap builtins instead. --- src/headers/tomcrypt_macros.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/headers/tomcrypt_macros.h b/src/headers/tomcrypt_macros.h index 6e4d757..52a1719 100644 --- a/src/headers/tomcrypt_macros.h +++ b/src/headers/tomcrypt_macros.h @@ -105,13 +105,13 @@ asm __volatile__ ( \ "bswapq %0 \n\t" \ "movq %0,(%1)\n\t" \ "bswapq %0 \n\t" \ - ::"r"(x), "r"(y)); + ::"r"(x), "r"(y): "memory"); #define LOAD64H(x, y) \ asm __volatile__ ( \ "movq (%1),%0\n\t" \ "bswapq %0\n\t" \ - :"=r"(x): "r"(y)); + :"=r"(x): "r"(y): "memory"); #else From ad566e1b00fe6a83b4ef024e9945d24177f9310e Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Sun, 25 Sep 2011 20:18:26 -0700 Subject: [PATCH 8/9] Use __builtin_bswap64 if it is available This produces slightly better performance than the inline assembly, and has the added benefit that it should be portable to other systems that use gcc, not just x86-64. Here are the results on my "AMD Athlon(tm) 7450 Dual-Core Processor" with "gcc (Ubuntu 4.3.3-5ubuntu4) 4.3.3": with portable 64H macros: camellia : Schedule at 1659 camellia [ 23]: Encrypt at 431, Decrypt at 434 whirlpool : Process at 55 with inline assembly (with "memory clobber" for correctness): camellia : Schedule at 1380 camellia [ 23]: Encrypt at 406, Decrypt at 403 whirlpool : Process at 50 with __builtin_bswap64: camellia : Schedule at 1352 camellia [ 23]: Encrypt at 396, Decrypt at 391 whirlpool : Process at 46 --- src/headers/tomcrypt_macros.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/headers/tomcrypt_macros.h b/src/headers/tomcrypt_macros.h index 52a1719..732ec3c 100644 --- a/src/headers/tomcrypt_macros.h +++ b/src/headers/tomcrypt_macros.h @@ -96,9 +96,20 @@ asm __volatile__ ( \ #endif +/* gcc 4.3 and up has a bswap builtin */ +#if !defined(LTC_NO_BSWAP) && \ + (defined(__GNUC__) && (__GNUC__ * 100 + __GNUC_MINOR__ >= 403)) + +#define STORE64H(x, y) \ + { ulong64 __t = __builtin_bswap64 ((x)); \ + XMEMCPY ((y), &__t, 8); } + +#define LOAD64H(x, y) \ + { XMEMCPY (&(x), (y), 8); \ + (x) = __builtin_bswap64 ((x)); } /* x86_64 processor */ -#if !defined(LTC_NO_BSWAP) && (defined(__GNUC__) && defined(__x86_64__)) +#elif !defined(LTC_NO_BSWAP) && (defined(__GNUC__) && defined(__x86_64__)) #define STORE64H(x, y) \ asm __volatile__ ( \ From 382c9d4d85ba226e18d5254a2dc95ed4e4be3a0e Mon Sep 17 00:00:00 2001 From: Patrick Pelletier Date: Mon, 26 Sep 2011 00:39:19 -0700 Subject: [PATCH 9/9] Some fixes necessary to support the Clang compiler First of all, it had a failure in SEED: LTC_KSEED failed for x=0, I got: expected actual (ciphertext) 5e == 5e ba == ba c6 == c6 e0 == e0 05 != 00 4e != 00 16 != 00 68 != 00 19 == 19 af == af f1 == f1 cc == cc 6d != 00 34 != 00 6c != 00 db != 00 Since SEED uses the 32H macros, this is really analogous to the problem I saw with the 64H macros in Camellia with gcc. Not sure why gcc only had a problem with 64H and not 32H, but since this is an interaction with the optimizer, it's not going to happen every time the macro is used (hence why the store tests pass; only when you get into the complexity of a real cipher do you start having problems) and it makes sense it will vary from compiler to compiler. Anyway, I went ahead and added the ability to use __builtin_bswap32, in addition to __builtin_bswap64, which I already did in a previous commit. This solves the problem for clang, although I had to add new logic to detect the bswap builtins in clang, since it has a different way to detect them than gcc (see the comments in the code). The detection logic was complicated enough, and applied to both the 32H and 64H macros, so I factored out the detection logic into tomcrypt_cfg.h. --- src/headers/tomcrypt_cfg.h | 16 ++++++++++++++++ src/headers/tomcrypt_macros.h | 16 ++++++++++++---- src/misc/crypt/crypt.c | 4 +++- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/headers/tomcrypt_cfg.h b/src/headers/tomcrypt_cfg.h index f7ad3cc..cc3b6df 100644 --- a/src/headers/tomcrypt_cfg.h +++ b/src/headers/tomcrypt_cfg.h @@ -128,6 +128,22 @@ LTC_EXPORT int LTC_CALL XSTRCMP(const char *s1, const char *s2); #define ENDIAN_NEUTRAL #endif +/* gcc 4.3 and up has a bswap builtin; detect it by gcc version. + * clang also supports the bswap builtin, and although clang pretends + * to be gcc (macro-wise, anyway), clang pretends to be a version + * prior to gcc 4.3, so we can't detect bswap that way. Instead, + * clang has a __has_builtin mechanism that can be used to check + * for builtins: + * http://clang.llvm.org/docs/LanguageExtensions.html#feature_check */ +#ifndef __has_builtin + #define __has_builtin(x) 0 +#endif +#if !defined(LTC_NO_BSWAP) && defined(__GNUC__) && \ + ((__GNUC__ * 100 + __GNUC_MINOR__ >= 403) || \ + (__has_builtin(__builtin_bswap32) && __has_builtin(__builtin_bswap64))) + #define LTC_HAVE_BSWAP_BUILTIN +#endif + #endif diff --git a/src/headers/tomcrypt_macros.h b/src/headers/tomcrypt_macros.h index 732ec3c..86156cc 100644 --- a/src/headers/tomcrypt_macros.h +++ b/src/headers/tomcrypt_macros.h @@ -67,7 +67,17 @@ #ifdef ENDIAN_LITTLE -#if !defined(LTC_NO_BSWAP) && (defined(INTEL_CC) || (defined(__GNUC__) && (defined(__DJGPP__) || defined(__CYGWIN__) || defined(__MINGW32__) || defined(__i386__) || defined(__x86_64__)))) +#ifdef LTC_HAVE_BSWAP_BUILTIN + +#define STORE32H(x, y) \ + { ulong32 __t = __builtin_bswap32 ((x)); \ + XMEMCPY ((y), &__t, 4); } + +#define LOAD32H(x, y) \ + { XMEMCPY (&(x), (y), 4); \ + (x) = __builtin_bswap32 ((x)); } + +#elif !defined(LTC_NO_BSWAP) && (defined(INTEL_CC) || (defined(__GNUC__) && (defined(__DJGPP__) || defined(__CYGWIN__) || defined(__MINGW32__) || defined(__i386__) || defined(__x86_64__)))) #define STORE32H(x, y) \ asm __volatile__ ( \ @@ -96,9 +106,7 @@ asm __volatile__ ( \ #endif -/* gcc 4.3 and up has a bswap builtin */ -#if !defined(LTC_NO_BSWAP) && \ - (defined(__GNUC__) && (__GNUC__ * 100 + __GNUC_MINOR__ >= 403)) +#ifdef LTC_HAVE_BSWAP_BUILTIN #define STORE64H(x, y) \ { ulong64 __t = __builtin_bswap64 ((x)); \ diff --git a/src/misc/crypt/crypt.c b/src/misc/crypt/crypt.c index 1298397..e1b1ce0 100644 --- a/src/misc/crypt/crypt.c +++ b/src/misc/crypt/crypt.c @@ -286,7 +286,9 @@ const char *crypt_build_settings = #if defined(_MSC_VER) " MSVC compiler detected.\n" #endif -#if defined(__GNUC__) +#if defined(__clang_version__) + " Clang compiler " __clang_version__ ".\n" +#elif defined(__GNUC__) /* clang also defines __GNUC__ */ " GCC compiler detected.\n" #endif #if defined(INTEL_CC)