> On May 31, 2018, 4:56 p.m., Benno Evers wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 205 (patched) > > <https://reviews.apache.org/r/67357/diff/2/?file=2033221#file2033221line205> > > > > To be more precise: "which compares either none of the content or the > > whole content of the strings". > > > > (but actually I'm not even sure a comment is needed at all, the > > function name kinda speaks for itself) > > Alexander Rojas wrote: > I agree but the shepherd requested it ;)
Hehe, can't argue with that :D > On May 31, 2018, 4:56 p.m., Benno Evers wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 215 (patched) > > <https://reviews.apache.org/r/67357/diff/2/?file=2033221#file2033221line215> > > > > I think its safer to use unsigned types for bit manipulation, otherwise > > it's really easy to accidentally trigger undefined behaviour. (i.e. even > > here I'm not sure its safe off the top of my head, since the xor could flip > > the sign bit of the chars) Hm, so `left[i] ^ right[i]` is still an xor of signed chars, but since its actually safe the way its used here, I guess its up to personal preference if you want to add an explicit cast. - Benno ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67357/#review204122 ----------------------------------------------------------- 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 > >