Why are cookies not being signed?

[ghost]

Session cookies are not cryptographically signed when I use this store. The cookies are signed if I switch back to cookie store.

[KartikKumarSahoo]

I'm also facing this problem. Can somebody suggest how to encrypt or sign the cookie?

[KartikKumarSahoo]

Solved the problem by using the following patch. Source: https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb

Created a "config/initializers/activerecord-session_store.rb" with following content

module ActionDispatch
  module Session
    class ActiveRecordStore < ActionDispatch::Session::AbstractStore
      private
      def extract_session_id(req)
        stale_session_check! do
          unpacked_cookie_data(req)
        end
      end

      def unpacked_cookie_data(req)
        req.fetch_header("action_dispatch.request.unsigned_session_cookie") do |k|
          v = stale_session_check! do
            get_cookie(req) || {}
          end
          req.set_header k, v
        end
      end

      def write_session(request, sid, session_data, options)
        logger.silence_logger do
          sid ||= get_cookie(request)
          record = get_session_model(request, sid)
          record.data = session_data
          return false unless record.save

          session_data = record.data
          if session_data && session_data.respond_to?(:each_value)
            session_data.each_value do |obj|
              obj.clear_association_cache if obj.respond_to?(:clear_association_cache)
            end
          end
          sid
        end
      end

      def set_cookie(request, session_id, cookie)
        cookie_jar(request)[@key] = cookie
      end

      def get_cookie(req)
        cookie_jar(req)[@key]
      end

      def cookie_jar(request)
        request.cookie_jar.signed_or_encrypted
      end
    end
  end
end

[twolfson]

This was very close to being correct but we were encountering issues when the cookie had never been originally set. It looks like we had to correct the line:

get_cookie(req) || {}

to

get_cookie(req) || ""

for a proper fallback as extract_session_id expects a string, not an object as its value

[PeterMozesMerl]

One of the reasons they aren’t encrypted or signed is that they don’t have to be.

What would be the purpose? Without signing it, it’s a random string that identifies a database record.

If it would be signed, it would be a random script that would identify a database record.

(I’ve never committed to this gem. It’s enough if you think it through.)

[rchekaluk]

I could be wrong, but here are a few cases where you might want the cookie to be signed and/or encrypted:

• If you must conform to a security scanner that requires all cookies to be signed or encrypted
• If you want to guard against tampering
• If you want to avoid disclosing that the session cookie is somehow different from other cookies
• For general consistency with the default :cookie_store (which signs and encrypts cookies)

[PeterMozesMerl]

You can sign a random string to convert it into another random string. That’s what will happen. It adds no security. If it adds any security, then the original string was not random.

The signing makes sense if the signed content carries information.

[twolfson]

@rchekaluk is right. For me specifically, signing is to prevent brute force guessing someone else's session identifier

For example attacker can guess aaaaa, aaaab, aaaac with different cookies

With signing, we'd throw out the request/invalidate the cookie if it's not signed for the corresponding identifier appropriately (i.e. request must be aaaaa.{hash-with-private-key-and-data-of-aaaaa})

[sikachu]

If my memory has served me right, this gem was extracted before signed cookie in Rails is a thing.

I would see no reason that we would reject a patch to make a configurable option to sign the session cookie while have it backward compatible with old cookie with no signature. Would any of you like to work on the patch?

[sikachu]

Oops, I just saw #140.

I'll be reviewing that patch then.

[PeterMozesMerl]

@rchekaluk is right. For me specifically, signing is to prevent brute force guessing someone else's session identifier

For example attacker can guess aaaaa, aaaab, aaaac with different cookies

With signing, we'd throw out the request/invalidate the cookie if it's not signed for the corresponding identifier appropriately (i.e. request must be aaaaa.{hash-with-private-key-and-data-of-aaaaa})

It is not true.

1. Signing does not prevent anyone from brute force.

2. The added security you imagine to be there is not because of the signing but because of the increased length of the identifier. The longer string takes more time to brute force.

If the identifiers are random, and the signed and the unsigned identifiers have the same length, there is no difference and no advantage of the signing in this case. It would also take the same time (on average) to brute force them.

[twolfson]

Ah, you're right. That's a good point. Now that I think about it, maybe I wanted to use signing to prevent timing attacks

With a normal database lookup (and no signing), we will short circuit via an index so the more matching characters, the longer the response takes and thus the timing attack can be applied

With signing, we compute the expected hash (time constant) and compare it to the actual given hash with a time constant comparison. If they're the same, then no issue and we do a db lookup

If they're not, then we throw out the request and the attacker much search the whole universe rather than get clues from the database as to how many of their first n characters are right

On Sun, Feb 24, 2019, 4:32 AM PeterMozesMerl notifications@github.com wrote:

@rchekaluk https://github.com/rchekaluk is right. For me specifically, signing is to prevent brute force guessing someone else's session identifier

For example attacker can guess aaaaa, aaaab, aaaac with different cookies

With signing, we'd throw out the request/invalidate the cookie if it's not signed for the corresponding identifier appropriately (i.e. request must be aaaaa.{hash-with-private-key-and-data-of-aaaaa})

It is not true.

1.

Signing does not prevent anyone from brute force. 2.

The added security you imagine to be there is not because of the signing but because of the increased length of the identifier. The longer string takes more time to brute force.

If the identifiers are random, and the signed and the unsigned identifiers have the same length, there is no difference and no advantage of the signing in this case. It would also take the same time (on average) to brute force them.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rails/activerecord-session_store/issues/48#issuecomment-466770033, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3FWHXCbgJP5P1ChTiPh4ld3HWGI-q3ks5vQoZsgaJpZM4FFio5 .

[PeterMozesMerl]

You are also right. One possible benefit of the signing is that you can use a longer string in the cookie and a shorter in the database.

The benefit can be serious.

In one of my production websites (social-like), the third largest table is the "browser_sessions". Under the same software but different domain, it’s the largest table. The larger of the two takes 10GB even though I clean it up every day. I delete records where (created_at = updated_at and updated_at < now() - '3 months'::interval) or (updated_at < now() - '12 months'::interval)

The first condition is for single hits who never came back (or didn’t make anything that was worth to keep).

I store almost nothing in it. It’s the user id and sometimes a message. Still, this table is larger than the images uploaded by the users that I also store in the database. (And which, unlike the sessions, I keep forever.)

So yes, it can make sense to use not crazy long random strings. But one has to be careful about the security vs. performance. The longer it takes your application to responds to a request, either because of the computing of the hash or due to an explicit delay due to the wrong hash, the more connections it has to keep up. This will use up the server resources especially in production where one wants to use long keep-alive connections. You can put it behind a reverse proxy. It might help but even that way it will use resources. Besides, native Apache/NGINX will not be able to check whether the hash or the signing is valid. It will hold one process or thread of Rails.

One option is using a rate limiter. CloudFlare has one. Although, it’s paid based on the number of the requests.

You can also try counting the failed attempts. It’s trickier than it is with username/password because you only have a key that either exists in your database or not and an IP address.

Probably the rate limiting is better if its configuration is clever. The asset requests should not count, and the limit should kick in only after N requests per X time. Otherwise, it slows down your site.

In general, one doesn’t want a web application to respond slower.

[twolfson]

I might be misunderstanding you but it sounds like you're implying hashing and time constant comparisons are somehow slower than other methods

Yes, rate limiting is great but timing attacks can still make brute force be much more efficient/take fewer guesses. In my opinion, I'd treat these as orthogonal problems

This means it's a performance competition between hashing + time constant comparison vs a database lookup. In the former case, it goes by super quick -- everything is done in memory, not too many cycles at all. In the latter case, we have to do a round trip with the database service which might possibly hit disk

[PeterMozesMerl]

Probably, I got lost. In the begining, I said only that signing a random string won’t make it more secure. Now I am not sure whether the goal is to slow down the responses or not.

Do you mean if the brute force is getting closer, the response time will differ?

I tested it on a production (live) database two minutes ago that has about 13 million browser sessions.

# explain analyze select * from browser_sessions where session_id = 'hello';
                                                                QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------
 Index Scan using browser_sessions_pkey on browser_sessions  (cost=0.56..8.58 rows=1 width=162) (actual time=0.035..0.044 rows=0 loops=1)
   Index Cond: ((session_id)::text = 'hello'::text)
 Planning time: 0.075 ms
 Execution time: 0.096 ms

2.6.0 :003 > start = Time.now; 1000.times { ActiveRecord::Base.connection.query("select * from browser_sessions where session_id = 'hello'") }; puts Time.now - start
0.625279684

2.6.0 :011 > start = Time.now; 1000.times { ActiveRecord::Base.connection.query("select * from browser_sessions where session_id = 'a_real_session_id[0..-3]'") }; puts Time.now - start
0.622380255

2.6.0 :010 > start = Time.now; 1000.times { ActiveRecord::Base.connection.query("select * from browser_sessions where session_id = 'a_real_session_id'") }; puts Time.now - start
0.628823533

I ran the irb on another server than the database to include the network time. That’s why 1000 selects took 0.6s. I tried with an absolutely fake string 'hello', a session_id that I got from the database but I removed the last two characters of it, and the same session_id as it was.

I could see no time difference between the three cases. Although, doing it 1000 times without going through the Rails app + web + browser would make the difference magnitudes larger than what an attacker could see.

[twolfson]

Yes, as more characters match, then the response time will increase -- this would allow the attacker to clue in that they have more correct starting characters

This is less about your specific server and more generally for any possible server. It's great that you have such wonderful performance from your server that a timing attack is negligible. I'm unconvinced that an underpowered server would be equally immune

As a public package, it'd be irresponsible to not fix a known vulnerability because it doesn't exist in a personal use case

[PeterMozesMerl]

I wish everyone would take their package’s quality as seriously as you do. Thanx.

[hibachrach]

Was directed here when looking into session IDs. This feels like a fairly important issue. Anything particularly blocking the PR which resolves this? If anything, the gem should default to signed and/or encrypted values.