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 <[email protected]>
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
