Session collisions on rails 3.1rc4 (authlogic, omniauth, memcache store, passenger)

Session collisions on rails 3.1rc4 (authlogic, omniauth, memcache store, passenger)

[Neil]

While it's entirely possible that this issue is caused by some other
factor, but we are getting session collisions as well as an issue
where one user is getting another user's session.  This is clearly
bad, but I cannot for the life of me figure out how this could even
happen in the first place.  The code looks thread safe to me, and a
quick discussion on #ruby-lang seems to support that.

Thoughts:
1.  Session IDs are being generated in the same sequence (uses
securerandom -> openssl which does not have a static seed)
2.  Threads.  Looks good to me.
3.  Maybe memcached is returning something other than "STORED/
NOT_STORED" for @pool.add(sid, session), but the operation still
succeeded?
4.  Gnomes.

Any input is GREATLY appreciated.  Please don't say "it's an RC, what
do you expect?" :)


From https://github.com/rack/rack/blob/master/lib/rack/session/memcache.rb
      def generate_sid
        loop do
          sid = super
          break sid unless @pool.get(sid, true)
        end
      end

      def get_session(env, sid)
        with_lock(env, [nil, {}]) do
          unless sid and session = @pool.get(sid)
            sid, session = generate_sid, {}
            unless /^STORED/ =~ @pool.add(sid, session)
              raise "Session collision on '#{sid.inspect}'"
            end
          end
          [sid, session]
        end
      end

      def set_session(env, session_id, new_session, options)
        expiry = options[:expire_after]
        expiry = expiry.nil? ? 0 : expiry + 1

        with_lock(env, false) do
          @pool.set session_id, new_session, expiry
          session_id
        end
      end

Re: Session collisions on rails 3.1rc4 (authlogic, omniauth, memcache store, passenger)

[Joshua Ballanco]

[-- Attachment #1: Type: text/plain, Size: 2383 bytes --]

Have you considered the possibility that memcache might be recycling keys on you?

We had an issue a while back where we were using memcache for both fragment caching and session storage. Occasionally, we would get exceptions in retrieving a session, and looking at the error message it was clear that what was retrieved from memcache was a fragment and not a session. Unfortunately, we moved back to a cookie-based session store before we had a chance to look deeper into the issue. What I can say is that based on the key generation scheme we were using for sessions and cache keys, there was effectively 0 chance that we were duplicating keys. Instead, it seemed like memcache was re-using slots for different keys when we started exhausting free slots.

Hope that helps!

- Joshua Ballanco 


On Monday, July 25, 2011 at 2:22 PM, Neil wrote:

> While it's entirely possible that this issue is caused by some other
> factor, but we are getting session collisions as well as an issue
> where one user is getting another user's session. This is clearly
> bad, but I cannot for the life of me figure out how this could even
> happen in the first place. The code looks thread safe to me, and a
> quick discussion on #ruby-lang seems to support that.
> 
> Thoughts:
> 1. Session IDs are being generated in the same sequence (uses
> securerandom -> openssl which does not have a static seed)
> 2. Threads. Looks good to me.
> 3. Maybe memcached is returning something other than "STORED/
> NOT_STORED" for @pool.add(sid, session), but the operation still
> succeeded?
> 4. Gnomes.
> 
> Any input is GREATLY appreciated. Please don't say "it's an RC, what
> do you expect?" :)
> 
> 
> From https://github.com/rack/rack/blob/master/lib/rack/session/memcache.rb
>  def generate_sid
>  loop do
>  sid = super
>  break sid unless @pool.get(sid, true)
>  end
>  end
> 
>  def get_session(env, sid)
>  with_lock(env, [nil, {}]) do
>  unless sid and session = @pool.get(sid)
>  sid, session = generate_sid, {}
>  unless /^STORED/ =~ @pool.add(sid, session)
>  raise "Session collision on '#{sid.inspect}'"
>  end
>  end
>  [sid, session]
>  end
>  end
> 
>  def set_session(env, session_id, new_session, options)
>  expiry = options[:expire_after]
>  expiry = expiry.nil? ? 0 : expiry + 1
> 
>  with_lock(env, false) do
>  @pool.set session_id, new_session, expiry
>  session_id
>  end
>  end


[-- Attachment #2: Type: text/html, Size: 3309 bytes --]

Re: Session collisions on rails 3.1rc4 (authlogic, omniauth, memcache store, passenger)

[Neil Matatall]

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

Josh,

Thank you, I will investigate and report back.

Neil

On Jul 25, 2011, at 1:23 PM, Joshua Ballanco wrote:

> Have you considered the possibility that memcache might be recycling keys on you?
> 
> We had an issue a while back where we were using memcache for both fragment caching and session storage. Occasionally, we would get exceptions in retrieving a session, and looking at the error message it was clear that what was retrieved from memcache was a fragment and not a session. Unfortunately, we moved back to a cookie-based session store before we had a chance to look deeper into the issue. What I can say is that based on the key generation scheme we were using for sessions and cache keys, there was effectively 0 chance that we were duplicating keys. Instead, it seemed like memcache was re-using slots for different keys when we started exhausting free slots.
> 
> Hope that helps!
> 
> - Joshua Ballanco
> On Monday, July 25, 2011 at 2:22 PM, Neil wrote:
> 
>> While it's entirely possible that this issue is caused by some other
>> factor, but we are getting session collisions as well as an issue
>> where one user is getting another user's session. This is clearly
>> bad, but I cannot for the life of me figure out how this could even
>> happen in the first place. The code looks thread safe to me, and a
>> quick discussion on #ruby-lang seems to support that.
>> 
>> Thoughts:
>> 1. Session IDs are being generated in the same sequence (uses
>> securerandom -> openssl which does not have a static seed)
>> 2. Threads. Looks good to me.
>> 3. Maybe memcached is returning something other than "STORED/
>> NOT_STORED" for @pool.add(sid, session), but the operation still
>> succeeded?
>> 4. Gnomes.
>> 
>> Any input is GREATLY appreciated. Please don't say "it's an RC, what
>> do you expect?" :)
>> 
>> 
>> From https://github.com/rack/rack/blob/master/lib/rack/session/memcache.rb
>> def generate_sid
>> loop do
>> sid = super
>> break sid unless @pool.get(sid, true)
>> end
>> end
>> 
>> def get_session(env, sid)
>> with_lock(env, [nil, {}]) do
>> unless sid and session = @pool.get(sid)
>> sid, session = generate_sid, {}
>> unless /^STORED/ =~ @pool.add(sid, session)
>> raise "Session collision on '#{sid.inspect}'"
>> end
>> end
>> [sid, session]
>> end
>> end
>> 
>> def set_session(env, session_id, new_session, options)
>> expiry = options[:expire_after]
>> expiry = expiry.nil? ? 0 : expiry + 1
>> 
>> with_lock(env, false) do
>> @pool.set session_id, new_session, expiry
>> session_id
>> end
>> end
> 


[-- Attachment #2: Type: text/html, Size: 3682 bytes --]

Re: Session collisions on rails 3.1rc4 (authlogic, omniauth, memcache store, passenger)

[Neil Matatall]

Joshua,

I think what you described was indeed our issue.  The call to @pool.add
actually returned "END" which is a sign of a get_multikey

We were using memcache-client and the author informed me that the
project was no longer supported and said that this can occur when the
same memcache instance handles sessions and data.  He recommended
splitting the caches as well as switching to Dalli instead.

So...not a rack issue!

Thanks!
Neil

On 7/25/11 1:23 PM, Joshua Ballanco wrote:
> Have you considered the possibility that memcache might be recycling
> keys on you?
>
> We had an issue a while back where we were using memcache for both
> fragment caching and session storage. Occasionally, we would get
> exceptions in retrieving a session, and looking at the error message
> it was clear that what was retrieved from memcache was a fragment and
> not a session. Unfortunately, we moved back to a cookie-based session
> store before we had a chance to look deeper into the issue. What I can
> say is that based on the key generation scheme we were using for
> sessions and cache keys, there was effectively 0 chance that we were
> duplicating keys. Instead, it seemed like memcache was re-using slots
> for different keys when we started exhausting free slots.
>
> Hope that helps!
>
> - Joshua Ballanco
>
> On Monday, July 25, 2011 at 2:22 PM, Neil wrote:
>
>> While it's entirely possible that this issue is caused by some other
>> factor, but we are getting session collisions as well as an issue
>> where one user is getting another user's session. This is clearly
>> bad, but I cannot for the life of me figure out how this could even
>> happen in the first place. The code looks thread safe to me, and a
>> quick discussion on #ruby-lang seems to support that.
>>
>> Thoughts:
>> 1. Session IDs are being generated in the same sequence (uses
>> securerandom -> openssl which does not have a static seed)
>> 2. Threads. Looks good to me.
>> 3. Maybe memcached is returning something other than "STORED/
>> NOT_STORED" for @pool.add(sid, session), but the operation still
>> succeeded?
>> 4. Gnomes.
>>
>> Any input is GREATLY appreciated. Please don't say "it's an RC, what
>> do you expect?" :)
>>
>>
>> From
>> https://github.com/rack/rack/blob/master/lib/rack/session/memcache.rb
>> def generate_sid
>> loop do
>> sid = super
>> break sid unless @pool.get(sid, true)
>> end
>> end
>>
>> def get_session(env, sid)
>> with_lock(env, [nil, {}]) do
>> unless sid and session = @pool.get(sid)
>> sid, session = generate_sid, {}
>> unless /^STORED/ =~ @pool.add(sid, session)
>> raise "Session collision on '#{sid.inspect}'"
>> end
>> end
>> [sid, session]
>> end
>> end
>>
>> def set_session(env, session_id, new_session, options)
>> expiry = options[:expire_after]
>> expiry = expiry.nil? ? 0 : expiry + 1
>>
>> with_lock(env, false) do
>> @pool.set session_id, new_session, expiry
>> session_id
>> end
>> end
>

Re: Session collisions on rails 3.1rc4 (authlogic, omniauth, memcache store, passenger)

[Joshua Ballanco]

[-- Attachment #1: Type: text/plain, Size: 3384 bytes --]

Hi Neil,

Good to hear you figured that out. I, too, had heard mention of Dalli as a replacement for memcache-client. Perhaps we should update Rack's gemspec?

- Josh 


On Thursday, July 28, 2011 at 10:54 PM, Neil Matatall wrote:

> Joshua,
> 
> I think what you described was indeed our issue. The call to @pool.add
> actually returned "END" which is a sign of a get_multikey
> 
> We were using memcache-client and the author informed me that the
> project was no longer supported and said that this can occur when the
> same memcache instance handles sessions and data. He recommended
> splitting the caches as well as switching to Dalli instead.
> 
> So...not a rack issue!
> 
> Thanks!
> Neil
> 
> On 7/25/11 1:23 PM, Joshua Ballanco wrote:
> > Have you considered the possibility that memcache might be recycling
> > keys on you?
> > 
> > We had an issue a while back where we were using memcache for both
> > fragment caching and session storage. Occasionally, we would get
> > exceptions in retrieving a session, and looking at the error message
> > it was clear that what was retrieved from memcache was a fragment and
> > not a session. Unfortunately, we moved back to a cookie-based session
> > store before we had a chance to look deeper into the issue. What I can
> > say is that based on the key generation scheme we were using for
> > sessions and cache keys, there was effectively 0 chance that we were
> > duplicating keys. Instead, it seemed like memcache was re-using slots
> > for different keys when we started exhausting free slots.
> > 
> > Hope that helps!
> > 
> > - Joshua Ballanco
> > 
> > On Monday, July 25, 2011 at 2:22 PM, Neil wrote:
> > 
> > > While it's entirely possible that this issue is caused by some other
> > > factor, but we are getting session collisions as well as an issue
> > > where one user is getting another user's session. This is clearly
> > > bad, but I cannot for the life of me figure out how this could even
> > > happen in the first place. The code looks thread safe to me, and a
> > > quick discussion on #ruby-lang seems to support that.
> > > 
> > > Thoughts:
> > > 1. Session IDs are being generated in the same sequence (uses
> > > securerandom -> openssl which does not have a static seed)
> > > 2. Threads. Looks good to me.
> > > 3. Maybe memcached is returning something other than "STORED/
> > > NOT_STORED" for @pool.add(sid, session), but the operation still
> > > succeeded?
> > > 4. Gnomes.
> > > 
> > > Any input is GREATLY appreciated. Please don't say "it's an RC, what
> > > do you expect?" :)
> > > 
> > > 
> > > From
> > > https://github.com/rack/rack/blob/master/lib/rack/session/memcache.rb
> > > def generate_sid
> > > loop do
> > > sid = super
> > > break sid unless @pool.get(sid, true)
> > > end
> > > end
> > > 
> > > def get_session(env, sid)
> > > with_lock(env, [nil, {}]) do
> > > unless sid and session = @pool.get(sid)
> > > sid, session = generate_sid, {}
> > > unless /^STORED/ =~ @pool.add(sid, session)
> > > raise "Session collision on '#{sid.inspect}'"
> > > end
> > > end
> > > [sid, session]
> > > end
> > > end
> > > 
> > > def set_session(env, session_id, new_session, options)
> > > expiry = options[:expire_after]
> > > expiry = expiry.nil? ? 0 : expiry + 1
> > > 
> > > with_lock(env, false) do
> > > @pool.set session_id, new_session, expiry
> > > session_id
> > > end
> > > end


[-- Attachment #2: Type: text/html, Size: 3978 bytes --]

Re: Session collisions on rails 3.1rc4 (authlogic, omniauth, memcache store, passenger)

[James Tucker]

[-- Attachment #1: Type: text/plain, Size: 3564 bytes --]

Maybe once I've had time to do a proper review.

On Aug 3, 2011, at 3:32 AM, Joshua Ballanco wrote:

> Hi Neil,
> 
> Good to hear you figured that out. I, too, had heard mention of Dalli as a replacement for memcache-client. Perhaps we should update Rack's gemspec?
> 
> - Josh
> On Thursday, July 28, 2011 at 10:54 PM, Neil Matatall wrote:
> 
>> Joshua,
>> 
>> I think what you described was indeed our issue. The call to @pool.add
>> actually returned "END" which is a sign of a get_multikey
>> 
>> We were using memcache-client and the author informed me that the
>> project was no longer supported and said that this can occur when the
>> same memcache instance handles sessions and data. He recommended
>> splitting the caches as well as switching to Dalli instead.
>> 
>> So...not a rack issue!
>> 
>> Thanks!
>> Neil
>> 
>> On 7/25/11 1:23 PM, Joshua Ballanco wrote:
>>> Have you considered the possibility that memcache might be recycling
>>> keys on you?
>>> 
>>> We had an issue a while back where we were using memcache for both
>>> fragment caching and session storage. Occasionally, we would get
>>> exceptions in retrieving a session, and looking at the error message
>>> it was clear that what was retrieved from memcache was a fragment and
>>> not a session. Unfortunately, we moved back to a cookie-based session
>>> store before we had a chance to look deeper into the issue. What I can
>>> say is that based on the key generation scheme we were using for
>>> sessions and cache keys, there was effectively 0 chance that we were
>>> duplicating keys. Instead, it seemed like memcache was re-using slots
>>> for different keys when we started exhausting free slots.
>>> 
>>> Hope that helps!
>>> 
>>> - Joshua Ballanco
>>> 
>>> On Monday, July 25, 2011 at 2:22 PM, Neil wrote:
>>> 
>>>> While it's entirely possible that this issue is caused by some other
>>>> factor, but we are getting session collisions as well as an issue
>>>> where one user is getting another user's session. This is clearly
>>>> bad, but I cannot for the life of me figure out how this could even
>>>> happen in the first place. The code looks thread safe to me, and a
>>>> quick discussion on #ruby-lang seems to support that.
>>>> 
>>>> Thoughts:
>>>> 1. Session IDs are being generated in the same sequence (uses
>>>> securerandom -> openssl which does not have a static seed)
>>>> 2. Threads. Looks good to me.
>>>> 3. Maybe memcached is returning something other than "STORED/
>>>> NOT_STORED" for @pool.add(sid, session), but the operation still
>>>> succeeded?
>>>> 4. Gnomes.
>>>> 
>>>> Any input is GREATLY appreciated. Please don't say "it's an RC, what
>>>> do you expect?" :)
>>>> 
>>>> 
>>>> From
>>>> https://github.com/rack/rack/blob/master/lib/rack/session/memcache.rb
>>>> def generate_sid
>>>> loop do
>>>> sid = super
>>>> break sid unless @pool.get(sid, true)
>>>> end
>>>> end
>>>> 
>>>> def get_session(env, sid)
>>>> with_lock(env, [nil, {}]) do
>>>> unless sid and session = @pool.get(sid)
>>>> sid, session = generate_sid, {}
>>>> unless /^STORED/ =~ @pool.add(sid, session)
>>>> raise "Session collision on '#{sid.inspect}'"
>>>> end
>>>> end
>>>> [sid, session]
>>>> end
>>>> end
>>>> 
>>>> def set_session(env, session_id, new_session, options)
>>>> expiry = options[:expire_after]
>>>> expiry = expiry.nil? ? 0 : expiry + 1
>>>> 
>>>> with_lock(env, false) do
>>>> @pool.set session_id, new_session, expiry
>>>> session_id
>>>> end
>>>> end
> 


[-- Attachment #2: Type: text/html, Size: 4290 bytes --]