mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 67357: Added constant time comparison of JWT signatures.
Date Wed, 06 Jun 2018 12:12:39 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message