> 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; > > ``` > > ?
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. - Benno ----------------------------------------------------------- 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 > >
