Skip to content

Commit

Permalink
Switch to HMAC + one tiny fix
Browse files Browse the repository at this point in the history
  • Loading branch information
judofyr committed Oct 27, 2008
1 parent bb5e3bf commit 6acd337
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions lib/camping/session.rb
Expand Up @@ -12,7 +12,7 @@
# For a basic tutorial, see the *Getting Started* section of the Camping::Session module.
#require 'camping'
require 'base64'
require 'digest/sha2'
require 'openssl'

module Camping
# The Camping::Session module is designed to be mixed into your application or into specific
Expand All @@ -33,6 +33,7 @@ module Camping
# * The session is stored in a cookie. Look in <tt>@cookies.identity</tt>.
# * Session data is only saved if it has changed.
module Session
DIGEST = OpenSSL::Digest::SHA1.new
# This <tt>service</tt> method, when mixed into controllers, intercepts requests
# and wraps them with code to start and close the session. If a session isn't found
# in the cookie it is created. The <tt>@state</tt> variable is set and if it changes,
Expand All @@ -47,7 +48,7 @@ def service(*a)
data = Marshal.restore(decoded_blob)
end

app = self.class.name.gsub(/^(\w+)::.+$/, '\1')
app = C.name
@state = (data[app] ||= Camping::H[])
hash_before = decoded_blob.hash
return super(*a)
Expand All @@ -65,7 +66,7 @@ def service(*a)
end

def secure_blob_hasher(data)
Digest::SHA256.hexdigest("#{state_secret}#{@env.REMOTE_ADDR}#{@env.HTTP_USER_AGENT}#{data}")
OpenSSL::HMAC.hexdigest(DIGEST, state_secret, "#{@env.REMOTE_ADDR}#{@env.HTTP_USER_AGENT}#{data}")
end

def state_secret; [__FILE__, File.mtime(__FILE__)].join(":") end
Expand Down

3 comments on commit 6acd337

@Bluebie
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m confused as to why this change is good. What is different about the hmac implementation in OpenSSL that makes it better?

Also please can we ditch @env.HTTP_USER_AGENT? It stops flash applets from being able to communicate with camping apps by killing their session, and presumably does the same to java. I’ve had to make my own http_user_agentless secure_blob_hasher to get flash to work with my camping webapp, where I use a flash applet to upload pictures, which was annoying and confusing. :/

@judofyr
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cryptology is pretty hard, and doing hash(key + data) has proved to be insecure: http://en.wikipedia.org/wiki/HMAC#Design_Principles

We’re not aiming for prefect security, but when can get a better solution for free I’m not complaining :-)

USER_AGENT is probably a good idea to remove, though!

@Bluebie
Copy link

@Bluebie Bluebie commented on 6acd337 Nov 2, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm… Fair enough! _

Doesn’t that algorithm require two different secret keys though? Do we know that the implementation in OpenSSL is strong? I would have thought everything in SSL would be incredibly outdated and ancient.

Please sign in to comment.