On 09/12/15 23:13, Benjamin Kaduk wrote: > On 12/09/2015 05:04 PM, Matt Caswell wrote: >> >> On 09/12/15 11:44, Jayalakshmi bhat wrote: >>> Hi Matt, >>> >>> I could build and execute the constant_time_test. I have attached the .c >>> file and test results. 34 tests have failed. All failures are >>> around constant_time_eq_8. This is the function I had mentioned in the >>> earlier mails. >> Not quite all. There is also a failure right at the beginning of your >> log in constant_time_is_zero_8. Although it looks very similar to the >> constant_time_eq_8 failure. >> >> As to the failure it is very strange. This is the function doing the test: >> >> int test_binary_op_8(unsigned >> char (*op) (unsigned int a, unsigned int b), >> const char *op_name, unsigned int a, >> unsigned int b, int is_true) >> { >> unsigned char c = op(a, b); >> if (is_true && c != CONSTTIME_TRUE_8) { >> printf( "Test failed for %s(%du, %du): expected %u " >> "(TRUE), got %u at line %d\n", op_name, a, b, >> CONSTTIME_TRUE_8, c,__LINE__); >> return 1; >> } else if (!is_true && c != CONSTTIME_FALSE_8) { >> printf( "Test failed for %s(%du, %du): expected %u " >> "(FALSE), got %u at line %d\n", op_name, a, b, >> CONSTTIME_FALSE_8, c,__LINE__); >> return 1; >> } >> printf( "Test passed for %s(%du, %du): expected %u got %u at line %d >> with %s\n", op_name, a, b, CONSTTIME_TRUE_8, >> c,__LINE__,is_true?"TRUE":"FALSE"); >> return 0; >> } >> >> >> and the output we see in the log file is: >> >> Test failed for constant_time_eq_8(0u, 0u): expected 255 (TRUE), got >> 4294967295 at line 85 >> >> That big number in the output is actually 0x7FFFFFFF in hex. The >> variable that it is printing here is "c" which is declared as an >> "unsigned char". >> >> Please someone correct me if I'm wrong but doesn't the C spec guarantee >> that a "char" is 8 bits? In which case how can the value of "c" be >> greater than 255????? > > C does not make such a guarantee, though recent-ish POSIX does. (This > system is a windows one, thought, right?) > > In any case, due to C's type promotion rules, it's very difficult to > actually use types narrower than 'int', since integers get auto-promoted > to int at integer conversion time. This has extra-fun interactions with > varargs functions, depending on the platform ABI in use. (Always cast > NULL to a pointer type when passing to a varargs function; this does > cause real bugs.) Since c is unsigned, it is odd to see it get promoted > to (int)-1, since C type conversions are supposed to be > value-preserving, but it is certainly possible that the windows ABI is > doing something I don't expect. Adjusting things so that the format > specifier and the type passed to printf match (whether by casting c to > int or qualifying the format specifier) might help.
Thanks Ben. It's not 100% clear to me that we are dealing with a system where a char has more than 8 bits, but it certainly seems like a plausible explanation for what is going on. Especially when you look at the implementation of constant_time_eq_8: static inline unsigned char constant_time_eq_8(unsigned int a, unsigned int b) { return (unsigned char)(constant_time_eq(a, b)); } The function "constant_time_eq" here returns an "unsigned int". The whole purpose of "constant_time_eq_8" is to provide a convenience function to create an 8 bit mask. If the number of bits in an unsigned char > 8 then this code is going to fail! Jaya - please could you try the attached patch to see if that resolves the problem. Please try re-executing both your SSL/TLS tests and the constant_time test. Let me know how you get on. Thanks Matt
From 9649f5fac40438608f010d1cd25d0d553e2c0fae Mon Sep 17 00:00:00 2001 From: Matt Caswell <m...@openssl.org> Date: Thu, 10 Dec 2015 09:33:24 +0000 Subject: [PATCH] Fix constant time assumption that char is 8 bits The constant time code assumes that a char is 8 bits. In reality the C standard does not guarantee this, so it could be longer than this. There was a reported issue due to this on WinCE. --- crypto/constant_time_locl.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crypto/constant_time_locl.h b/crypto/constant_time_locl.h index c786aea..08f4647 100644 --- a/crypto/constant_time_locl.h +++ b/crypto/constant_time_locl.h @@ -49,6 +49,8 @@ # include "e_os.h" /* For 'inline' */ +# define CONSTTIME_8BITMASK 0xff + #ifdef __cplusplus extern "C" { #endif @@ -142,7 +144,7 @@ static inline unsigned int constant_time_lt(unsigned int a, unsigned int b) static inline unsigned char constant_time_lt_8(unsigned int a, unsigned int b) { - return (unsigned char)(constant_time_lt(a, b)); + return (unsigned char)(constant_time_lt(a, b) & CONSTTIME_8BITMASK); } static inline unsigned int constant_time_ge(unsigned int a, unsigned int b) @@ -152,7 +154,7 @@ static inline unsigned int constant_time_ge(unsigned int a, unsigned int b) static inline unsigned char constant_time_ge_8(unsigned int a, unsigned int b) { - return (unsigned char)(constant_time_ge(a, b)); + return (unsigned char)(constant_time_ge(a, b) & CONSTTIME_8BITMASK); } static inline unsigned int constant_time_is_zero(unsigned int a) @@ -162,7 +164,7 @@ static inline unsigned int constant_time_is_zero(unsigned int a) static inline unsigned char constant_time_is_zero_8(unsigned int a) { - return (unsigned char)(constant_time_is_zero(a)); + return (unsigned char)(constant_time_is_zero(a) & CONSTTIME_8BITMASK); } static inline unsigned int constant_time_eq(unsigned int a, unsigned int b) @@ -172,7 +174,7 @@ static inline unsigned int constant_time_eq(unsigned int a, unsigned int b) static inline unsigned char constant_time_eq_8(unsigned int a, unsigned int b) { - return (unsigned char)(constant_time_eq(a, b)); + return (unsigned char)(constant_time_eq(a, b) & CONSTTIME_8BITMASK); } static inline unsigned int constant_time_eq_int(int a, int b) @@ -196,7 +198,8 @@ static inline unsigned char constant_time_select_8(unsigned char mask, unsigned char a, unsigned char b) { - return (unsigned char)(constant_time_select(mask, a, b)); + return (unsigned char)(constant_time_select(mask, a, b) + & CONSTTIME_8BITMASK); } static inline int constant_time_select_int(unsigned int mask, int a, int b) -- 2.5.0
_______________________________________________ openssl-users mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users