Skip to content

Comments

Improvements!#3

Open
ccutrer wants to merge 85 commits intoFooBarWidget:masterfrom
instructure:master
Open

Improvements!#3
ccutrer wants to merge 85 commits intoFooBarWidget:masterfrom
instructure:master

Conversation

@ccutrer
Copy link

@ccutrer ccutrer commented May 11, 2012

  • don't bother encrypting the IV (?!)
    • use the same encryption key for the encryption as for the HMAC
      (easy to configure, since it's the same as for normal CookieStore)
    • compress the session before encrypting it (optional, will skip
      compression if it actually makes it larger)
    • use AES 128 CBC (AES 256 is actually weaker, due to a known
      attack on it), which also reduces the cookie size due to a smaller
      IV and less padding
    • use URI-safe base64 encoding to not waste space percent-encoding
      characters in the cookie
    • use . as the separator between cookie fields (instead of -, which
      is now a part of the base64 encoding)
    • if :expire_after is provided, add a timestamp to the cookie that's
      included in the HMAC to allow for secure expiration of cookies
      (preventing session replay attacks after the expiration of the
      cookie)
    • don't re-set the cookie on every request if the session hasn't
      changed (unless :expire_after is in use, in which case do it every
      5 minutes by default in order to refresh the timestamp)

 * don't bother encrypting the IV (?!)
 * use the same encryption key for the encryption as for the HMAC
   (easy to configure, since it's the same as for normal CookieStore)
 * compress the session before encrypting it (optional, will skip
   compression if it actually makes it larger)
 * use AES 128 CBC (AES 256 is actually *weaker*, due to a known
   attack on it), which also reduces the cookie size due to a smaller
   IV and less padding
 * use URI-safe base64 encoding to not waste space percent-encoding
   characters in the cookie
 * use . as the separator between cookie fields (instead of -, which
   is now a part of the base64 encoding)
 * if :expire_after is provided, add a timestamp to the cookie that's
   included in the HMAC to allow for secure expiration of cookies
   (preventing session replay attacks after the expiration of the
   cookie)
 * don't re-set the cookie on every request if the session hasn't
   changed (unless :expire_after is in use, in which case do it every
   5 minutes by default in order to refresh the timestamp)
@FooBarWidget
Copy link
Owner

Very nice! But why not encrypt the IV? Is it supposed to be public information?

I'm opposed to removing the "It is prone to session replay attacks" sentence from the README. You are less prone now but not completely invulnerable. Attackers can still replay sessions within the timeout period.

@ccutrer
Copy link
Author

ccutrer commented May 11, 2012

Because it's a waste of CPU cycles, adds unnecessary complexity, and does not add any "real" security (an IV is completely random data anyway; encrypting the randomness doesn't make it any more random, nor increase the security of bruteforcing the encrypted data): http://stackoverflow.com/questions/6167114/protect-encrypt-initialization-vector

I'll adjust the readme re: session replay attacks.

ccutrer and others added 27 commits May 11, 2012 13:11
Include more details on still being vulnerable to session replay attacks, just less vulnerable.
test plan:
 * set :expire_after, but also set :expires to nil for session
   expiration of the real cookie
 * it shouldn't give a page error when the cookie expires
Options is now a class variable that you can
change during call time. This way you can set
attributes on the options hash. In this case
we ensure the options hash gets the option
to set :expire_after
Allow options to be changed.
the timestamp being used is refreshed every so often and doesn't
actually indicate the *start* of the session
the timestamp stored in the encrypted cookie already represents the
"session_refreshed_at" value. in 1.0.3 we incorrectly interpreted it as
a "expires_at" value, as if it already included the expire_after value
fix calculation of session_refreshed_at timestamp
update to work with rails 3.2
allow setting expire_after before unmarshalling
fix deprecation warnings in Ruby 2.1
catch exceptions unmarshalling data
ccutrer and others added 30 commits November 16, 2016 12:13
Ruby 2.4 validates the encryption key length is exactly the
correct size, instead of just long enough. but be careful, because
we still use the secret for the HMAC, which can take a key of
any length. so only truncate what goes to encryption, not what
goes to HMAC
fix encryption for Ruby 2.4
keys are strings; don't use a symbol for the timestamp key
fix refreshing the session on every request
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.

8 participants