Improvements! #3

Open
wants to merge 67 commits into
from

Conversation

Projects
None yet
6 participants

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)
@ccutrer ccutrer improvements and gemify!
 * 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)
0329d67
Owner

FooBarWidget commented May 11, 2012

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 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 some commits May 11, 2012

@ccutrer ccutrer Update README
Include more details on still being vulnerable to session replay attacks, just less vulnerable.
abda410
@ccutrer ccutrer fix bug with expiring cookie
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
1f1ed9d
@fourcolors fourcolors Make options a class variable
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
5a181ea
@ccutrer ccutrer Merge pull request #1 from fourcolors/master
Allow options to be changed.
a06560b
@ccutrer ccutrer version 1.0.2 705a5f0
@lukfugl lukfugl record session_started 00f5e86
@lukfugl lukfugl rename session_started -> session_refreshed_at
the timestamp being used is refreshed every so often and doesn't
actually indicate the *start* of the session
c733335
@ccutrer ccutrer Merge pull request #2 from lukfugl/session_started
record session_refreshed_at
6aeaa44
@ccutrer ccutrer bump to 1.0.3 2c6f041
@lukfugl lukfugl fix calculation of session_refreshed_at timestamp
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
bcffe64
@canadaduane canadaduane Merge pull request #3 from lukfugl/session_started
fix calculation of session_refreshed_at timestamp
02e1853
@canadaduane canadaduane bump to 1.0.4 adde6ed
@canadaduane canadaduane update gem date e89056e
@maneframe maneframe update to work with rails 3.2 8577985
@maneframe maneframe update date and authors 1f14985
@maneframe maneframe Merge pull request #4 from maneframe/rails3b
update to work with rails 3.2
5d3e105
@maneframe maneframe allow setting expire_after before unmarshalling eb63be8
@lukfugl lukfugl Merge pull request #6 from maneframe/expire_after
allow setting expire_after before unmarshalling
ac385d1
@lukfugl lukfugl bump version 9f6fd8f
@maneframe maneframe use unsigned cookies in cookie_jar for rails 2 interoperability e9b13a0
@ccutrer ccutrer bump 7e517d7
@ccutrer ccutrer fix deprecation warnings in Ruby 2.1 7429080
@maneframe maneframe Merge pull request #8 from ccutrer/master
fix deprecation warnings in Ruby 2.1
ec0cdd3
@ccutrer ccutrer bump version 3497eb2
@ccutrer ccutrer catch exceptions unmarshalling data 6dfe35c
@maneframe maneframe Merge pull request #9 from ccutrer/unmarshal_fix
catch exceptions unmarshalling data
400b114
@ccutrer ccutrer bump version 357c8d8
@maneframe maneframe log unmarshalling errors a036de9
@maneframe maneframe bump version a2ace2a
@ccutrer ccutrer allow rails 4.0 and 4.1 d2965c9
@ccutrer ccutrer Merge pull request #11 from ccutrer/rails4
allow rails 4.0 and 4.1
80c99fd
@ccutrer ccutrer bump version ee3397e
@ccutrer ccutrer make load_session public
for rails4 compatibility
860f7bf
@maneframe maneframe Merge pull request #12 from ccutrer/rails4
make load_session public
1616d81
@ccutrer ccutrer bump version eb2142e
@ccutrer ccutrer include the IV in the HMAC
and optionally still allow HMACs that didn't have it
d5b6f5a
@ccutrer ccutrer extract hmac into helper function
and fix bug where we passed iv twice
6a6b858
@ccutrer ccutrer Merge pull request #13 from ccutrer/iv_in_hmac
include the IV in the HMAC
803e0f3
@ccutrer ccutrer bump version 4010858
@lukfugl @lukfugl lukfugl remove accidental '@' in option key c50a83a
@ccutrer ccutrer Merge pull request #14 from lukfugl/master
remove accidental '@' in option key
07162bd
@lukfugl lukfugl bump version 86fd888
@maneframe maneframe fix destroy_session for rails 4 40eae12
@maneframe maneframe bump version d778e83
@maneframe maneframe allow rails 4.2 and bump version 6ebe1b1
@ccutrer ccutrer upgrade rspec, lock to rails 4.2, fix spec c58b766
@ccutrer ccutrer drop legacy HMAC support c3f02b7
@ccutrer ccutrer remove shim for legacy OpenSSL 75603d7
@ccutrer ccutrer refactor for newer rails' set_cookie/get_cookie methods ee5d926
@ccutrer ccutrer don't use a unique env key for the cookie
then we don't need to override destroy_session at all
3b0e3f4
@ccutrer ccutrer assume rails 4+ 5abfb13
@ccutrer ccutrer don't use a separate option key for per-request altering of expire_after 84972d1
@ccutrer ccutrer rails 5/rack 2 support 3c7a413
@ccutrer ccutrer Merge pull request #15 from ccutrer/rails5
Rails 5
fc8e687
@ccutrer ccutrer bump version b4c2fd1
@ccutrer ccutrer oops, still compatible with 4.2 cf1956d
@ccutrer ccutrer bump version 896d056
@ccutrer ccutrer fix enforcing expire_after 9992333
@ccutrer ccutrer bump version c77776f
@ccutrer ccutrer stop using deprecated constant 3b34667
@ccutrer ccutrer bump version eb2eacf
@ccutrer ccutrer fix encryption for Ruby 2.4
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
323436c
@ccutrer ccutrer Merge pull request #16 from ccutrer/keylength24
fix encryption for Ruby 2.4
7463290
@ccutrer ccutrer bump version dd2c79b
@ccutrer ccutrer allow rails 5.1 fda332d
@ccutrer ccutrer bump version acdde99
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment