Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace timing attack prone sample auth code #87

Merged
merged 1 commit into from
Apr 21, 2017

Conversation

jkraemer
Copy link
Contributor

It's just sample code but I guess everybody just copy'n'pastes this anyway, so it might as well be safe :)

@andyatkinson
Copy link
Collaborator

@jkraemer This looks straightforward to merge, however would you be able to amend the commit and put a link to some details on the timing attack and why this approach prevents it? Thanks.

@jkraemer
Copy link
Contributor Author

Hi, basically the problem is that == takes different time to execute depending on what strings are compared. The relevant Rails-related CVE that led to the introduction of the variable_size_secure_compare method is https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-7576

@litenup
Copy link
Contributor

litenup commented Apr 1, 2017

Here's an example with Devise (warden) for reference. Perhaps consider adding to readme too?

match "/delayed_job" => DelayedJobWeb, :anchor => false, via: [:get, :post], :constraints => lambda 
  { |request|
    request.env['warden'].authenticate! unless request.env['warden'].authenticated?
  }

or with rolify support.

match "/delayed_job" => DelayedJobWeb, :anchor => false, via: [:get, :post], :constraints => lambda 
  { |request|
    if request.env['warden'].authenticated?
      request.env['warden'].user.has_role? :admin
    else
      request.env['warden'].authenticate! 
    end
  }

@andyatkinson andyatkinson merged commit 14cd7e3 into ejschmitt:master Apr 21, 2017
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.

None yet

3 participants