> On June 6, 2018, 11:58 a.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 213 (patched) > > <https://reviews.apache.org/r/67357/diff/3/?file=2036012#file2036012line213> > > > > To avoid conversions and integral promotions, does it make sense to say > > here > > ``` > > string::value_type valid = 0; > > ``` > > ? > > Benno Evers wrote: > I would recommend against that, as it would turn `valid` back into a > signed type and even obfuscate that fact. As I said before, it's good > practice to only do bitwise operation on unsigned types. > > For example (admittedly, its a bit far-fetched, but bear with me), > imagine a system using ones complement. Two different characters could xor to > produce a negative zero, and implementations are permitted to convert that to > positive zero, producing a false positive.
Discussed this offline with Benno and Alexander. If `char ^ char` produces `0`, during integer promotion it is padded with `0`s and `OR`d with unsigned correctly. If `char ^ char` does not produce `0`, it can be padded with either `1`s or `0`s, but we don't really care since the result of `OR` with unsigned will be non-zero. The code in the current version seems to be correct. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67357/#review204378 ----------------------------------------------------------- On June 6, 2018, 8:54 a.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67357/ > ----------------------------------------------------------- > > (Updated June 6, 2018, 8:54 a.m.) > > > Review request for Alexander Rukletsov. > > > Repository: mesos > > > Description > ------- > > A vulnerability in our JWT implementation allows an unauthenticated > remote attacker to execute to execute timing attacks [1]. > > This patch removes the vulnerability by adding a constant time > comparison of hashes, where the whole message is visited during > the comparison instead of returning at the first failure. > > [1] https://codahale.com/a-lesson-in-timing-attacks/ > > > Diffs > ----- > > 3rdparty/libprocess/src/jwt.cpp 4477ddd17dede2b924a47e33942b39244f10316f > > > Diff: https://reviews.apache.org/r/67357/diff/3/ > > > Testing > ------- > > ```sh > make check > ``` > > > Thanks, > > Alexander Rojas > >
