> On May 30, 2018, 1:37 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Line 253 (original), 253-261 (patched) > > <https://reviews.apache.org/r/67357/diff/1/?file=2031573#file2031573line253> > > > > I suggest to exatract it into a funciton, say `compareConstantTime`. > > Alexander Rojas wrote: > `compareConstantTime` is not a good name, since in C++ _compare_ > functions always returns an integer which, if less than zero means left is > greater than right, greater than zero means right is greater than left and 0 > means equality, see > [std::comapre](http://www.cplusplus.com/reference/string/string/compare/) and > [memcmp](http://www.cplusplus.com/reference/cstring/memcmp/) for examples. > > I also don't think it makes a lot of sense to move this little snippet to > a new fuction given that is only used here. But I will do it anyway.
Good point about the name, Alexander. Regarding a separate function, I think it is much easier to spot this snippet if someone needs something similar in the future. Plus small doing-one-thing with no side effects routines simplify reading the code significantly. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67357/#review204048 ----------------------------------------------------------- On May 31, 2018, 9:02 a.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67357/ > ----------------------------------------------------------- > > (Updated May 31, 2018, 9:02 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/2/ > > > Testing > ------- > > ```sh > make check > ``` > > > Thanks, > > Alexander Rojas > >
