heroku / identity

[DEPRECATED] Login and OAuth management service for Heroku
https://id.heroku.com/
MIT License
246 stars 20 forks source link

Bump `fernet` to 2.2 #205

Closed owenthereal closed 8 years ago

owenthereal commented 8 years ago

fernet 1.5 throws exceptions when decoding with the same key but different Fernet instances:

NoMethodError: undefined method `stacktrace' for #<NoMethodError: undefined method `tr' for nil:NilClass>
"/opt/boxen/rbenv/versions/2.2.3/lib/ruby/2.2.0/base64.rb:89:in `urlsafe_decode64'",
 "/opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet/verifier.rb:46:in `deconstruct'",
 "/opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet/verifier.rb:19:in `verify_token'",
 "/opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet.rb:25:in `block in verifier'",
 "/opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet.rb:24:in `tap'",

The issue is gone with the latest fernet.

owenthereal commented 8 years ago

The latest fernet accepts a string instead of a hash, so we can't do something like generator.data = { "session" => data }.

rwz commented 8 years ago

Upgrading fernet means that we won't be able to decode sessions encoded with the older version, which mean that all existing sessions will be lost, right?

owenthereal commented 8 years ago

Upgrading fernet means that we won't be able to decode sessions encoded with the older version, which mean that all existing sessions will be lost, right?

Unfortunately yes. All logged in users will be logged out...

rwz commented 8 years ago

Unfortunately yes. All logged in users will be logged out...

e1cd10ca8ee140d0a82694be1c1b46d0bc2aaa3d303ee303cd68ededf2f3c0df

owenthereal commented 8 years ago

The issue with 1.5 is it doesn't play well with URL encoded string and cookies are URL encoded: https://github.com/heroku/identity/blob/root_domain_cookie/test/auth_test.rb#L463-L464

owenthereal commented 8 years ago

For example, a cookie like this:

ACnwDetHT8jJ2K3M4qH5roGjYBDU2yPXttSHb7LLugz-GOPYngdCa29E4ZNZMIjsxQ9ohL31WXaD01P5VxVue8naMmRwGg2CmqOxrYnqoUIjpOvy03GF5aIuyjVReH9lUU0e7vdVNOlr6dgaRAHSzb2kACWwFtBTQN31HX0QQ-_OKlCPVUu8c6z8RqhHXbDvj9xVTZZ30m360VnjI7sTA%3D%3D%7Cl5zfcTVMluczxkjWNickwQ%3D%3D%7C24242a64d6bac707b099cca85b1e7fcc10f608b37e77fd489152e4ab8c68a30b

will trigger this exception:

NoMethodError: undefined method `tr' for nil:NilClass
    /opt/boxen/rbenv/versions/2.2.3/lib/ruby/2.2.0/base64.rb:89:in `urlsafe_decode64'
    /opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet/verifier.rb:46:in `deconstruct'
    /opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet/verifier.rb:19:in `verify_token'
    /opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet.rb:25:in `block in verifier'
    /opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet.rb:24:in `tap'
    /opt/boxen/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/fernet-1.5/lib/fernet.rb:24:in `verifier'
    /Users/jou/src/heroku/identity/lib/identity/cookie_coder.rb:51:in `decode_with_key'
    /Users/jou/src/heroku/identity/lib/identity/cookie_coder.rb:27:in `block in decode'
    /Users/jou/src/heroku/identity/lib/identity/cookie_coder.rb:25:in `each'
    /Users/jou/src/heroku/identity/lib/identity/cookie_coder.rb:25:in `decode'
    test/auth_test.rb:468:in `block (3 levels) in <main>

%3D is not valid base64 character. I believe that's why we need to catch all errors: https://github.com/heroku/identity/blob/master/lib/identity/fernet_cookie_coder.rb#L30-L34.

A CGI unescaped version works for fernet though:

A6LobFwQ1YSvOvGZryRhfBzl3Rn_H-8UXmOOchE7UqEylMUYuPtvtg0VF4hf-nvfOU0lL6xWgfKMs9mmku4saI7BNP4z16xjfWKdB30997vsjDY8jdn75LAGWRD73uK4zhNU9j50lUiwJX7Hr0NFdAwdPM2rdfERzJWvb-LAFWLBDRXIqYbiKs1JRXThV4erRA43uek7OVfMg3e1rKJW5A==|Stj0M80GZzPpzZIAoJK8tA==|10db02238f442df3b258f2675e67b2951d791e57b4244ce221e9fb683b366e20
owenthereal commented 8 years ago

This error did happen in production: https://splunk.herokai.com/en-US/app/search/search?sid=1452534869.650917.

rwz commented 8 years ago

Seems like we could use legacy-fernet to retain support for existing sessions and encode all new ones with new version.

owenthereal commented 8 years ago

@rwz:

Seems like we could use legacy-fernet to retain support for existing sessions and encode all new ones with new version.

legacy-fernet is targeting at fernet 1.6. And we need 1.5 for identity. I forked fernet 1.5 here with necessary namespace changes: https://github.com/heroku/fernet-rb.

rwz commented 8 years ago

@jingweno aren't 1.5 and 1.6 compatible?

owenthereal commented 8 years ago

@rwz I'm not sure. Just to be safe let's make sure we use the same version 1.5.