Hi Brian
Interesting. I think the upstream solution to search first for private id
and then for public id is good enough for Debian stable releases.
It should be extremely difficult (unless the computer is really slow) to
calculate the timing for the public id part since there are so many other
variables like link speed, process switching time and other that will cause
delays on the same magnitude.
For unstable, maybe a more secure solution is better.
What I think we need to do however is to make a correction without changing
to the more complex object to avoid breakage. Would that be a lot of work
to do that?
// Ola
On Tue, 11 Feb 2020 at 07:58, Brian May <b...@debian.org> wrote:
> Ola Lundqvist <o...@inguza.com> writes:
>
> > I have looked into this some but I have not been able to determine how
> long
> > the session ids were before the fix. Do anyone have that information?
> > Based on that we can rather easily determine how long time a timing
> attack
> > would take. My guess is a really long time.
>
> My understanding is increasing.
>
> The problem is when we lookup the session details from the database:
>
> @pool[sid]
>
> As sid is untrusted data, it means that can attacker can carefully
> control the value and find out information on existing sessions based on
> timing the query.
>
> This patch would change that database lookup to:
>
> @pool[sid.private_id]
>
> Where sid.private_id = "#{ID_VERSION}::#{hash_sid(public_id)}"
>
> This means the timing attack is harder (impossible?), because an
> attacker cannot create good test data for the database lookup.
>
> The problem with this approach however is we now have broken all
> existing sessions. Because we have already stored the public id, not the
> private id in the database. So the upstream solution is to use:
>
> @pool[sid.private_id] || @pool[sid.public_id]
>
> If the private_id lookup fails, the public_id lookup will be tried.
>
> In https://github.com/rack/rack/issues/1431 the issue is raised that the
> code still does the public_id lookup, which is what was responsible for
> the bug in the first place. However the counter claim is that we have
> already done the private_id lookup, and that is going to make timing
> attacks hard.
>
> If I understand this correctly, the goal will be to remove the public_id
> lookup eventually.
>
> Also, as upstream changed sid from a string to a more complicated
> object, this breaks the current API. There are other possible solutions,
> but as mentioned in
> https://github.com/rack/rack/issues/1432#issuecomment-571688819 these
> could lead to silent breakage, which is probably not a better outcome.
> --
> Brian May <b...@debian.org>
>
--
--- Inguza Technology AB --- MSc in Information Technology ----
| o...@inguza.com o...@debian.org |
| http://inguza.com/ Mobile: +46 (0)70-332 1551 |
---------------------------------------------------------------
