[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Issues regarding ruby-rack/CVE-2019-16782

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 <bam@debian.org> wrote:
Ola Lundqvist <ola@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:


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:


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 <bam@debian.org>

 --- Inguza Technology AB --- MSc in Information Technology ----
|  ola@inguza.com                    opal@debian.org            |
|  http://inguza.com/                Mobile: +46 (0)70-332 1551 |

Reply to: