Skip to content

Fix timing attack#45

Closed
cdman wants to merge 1 commit intodound:masterfrom
cdman:master
Closed

Fix timing attack#45
cdman wants to merge 1 commit intodound:masterfrom
cdman:master

Conversation

@cdman
Copy link
Copy Markdown

@cdman cdman commented Sep 13, 2013

Makes HMAC comparison to avoid potential timing attacks. A timing attack would allow an attacker to "craft" (trough a series of requests) a valid HMAC for any cookie. Here is a good introduction to them: http://codahale.com/a-lesson-in-timing-attacks/

Note: I was unable to exploit the flaw either locally or in a production setup (even tried using a high-quality datacenter network), but that doesn't mean that someone smarter couldn't ✨

Makes HMAC comparison to avoid potential timing attacks.
@mdornseif
Copy link
Copy Markdown
Contributor

I'm not sure if application code is the right place to fix something like timing attacks. So far it is extremely difficult to run timing attacks against the python VM. While __equals_slowly(a, b) is valid code we can only assume. what the underlying VM (CPython? PyPy) is doing to enable or mess with timing attacks. So we don't know what other timing attacks are in our code path - the CPython string implementation does some funny things.

@cdman
Copy link
Copy Markdown
Author

cdman commented Sep 24, 2013

  • In a sense application code is not the right place to fix timing attacks, rather we should use libraries which implement these primitives correctly (for example Python 3 has a compare_digest method to be used exactly in this situation). Unfortunately GAE doesn't run Python 3 (yet)
  • Theoretically results of the code can vary between CPython / PyPy / IronPython / etc however this project is written to be run on one VM only: the one provided by GAE 😄

Timing attacks are not some mysterious things but a rather simple concept: you can measure how many characters you guessed right by measuring the time it takes for the HTTP call to finish. It is only relevant to code which compares hashes / padding / etc. The gae-session code does a lot of things right (like checking HMAC before decrypting) and with this small change you would eliminate the possibility of exploiting this flaw.

@cdman
Copy link
Copy Markdown
Author

cdman commented Sep 24, 2013

Closing this since I resubmitted it as #47

@cdman cdman closed this Sep 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants