[openssl-users] CBC ciphers + TLS 1.0 protocol does not work in OpenSSL 1.0.2d

[Jayalakshmi bhat]

Hi Matt,

Thanks for the patch. Unfortunately patch did not work. I continued
debugging and found that issue was in constant_time_msb.

static inline unsigned int constant_time_msb(unsigned int a) {
-    *return 0 - (a >> (sizeof(a) * 8 - 1));*
+ return (((unsigned)((int)(a) >> (sizeof(int) * 8 - 1))));
}

Changed constant_time_msb implementation as shown above. All the tests
passed. I have attached the dis-assembly of the code for both successful
case and failure case.  This was requested by Jakob.

Regards
Jaya

On Thu, Dec 10, 2015 at 2:48 AM, Matt Caswell <matt at openssl.org> wrote:

>
>
> 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
>
>
> _______________________________________________
> openssl-users mailing list
> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mta.openssl.org/pipermail/openssl-users/attachments/20151210/e88b8498/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: changes.7z
Type: application/octet-stream
Size: 4540 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-users/attachments/20151210/e88b8498/attachment-0001.obj>