Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Sessions generated in 2.8.5 are no longer valid in 2.8.6 #121

Closed
roelvanduijnhoven opened this issue Aug 16, 2019 · 8 comments
Closed

Sessions generated in 2.8.5 are no longer valid in 2.8.6 #121

roelvanduijnhoven opened this issue Aug 16, 2019 · 8 comments

Comments

@roelvanduijnhoven
Copy link

That is due to #114. The length of the ID becomes longer after this update for us (PHP 7.2). And Zend Sessions' validation fails.

As there is no system to refresh failed session, we had to chose another cookie name to resolve this issue fully!

I do not exactly what changed, but I think the length of session names should remain identical when doing this minor upgrade, or the old session names should remains valid.

@michalbundyra
Copy link
Member

michalbundyra commented Aug 16, 2019

@roelvanduijnhoven Thanks for reporting the issue. I agree, it shouldn't break between any minor release. Please note that Validator\Id was not working properly on PHP 7.1+. Everything depends on your PHP configuration. Can you check what is you PHP setting for session.sid_bits_per_character, please?

@roelvanduijnhoven
Copy link
Author

It is 4 on the new host. And was 5 on the old host!

@michalbundyra
Copy link
Member

@roelvanduijnhoven how do you mean "old" and "new" host?

What PHP versions are used on both?
Are you using shared session between two hosts?

Would you be able provide failing test case, so we can investigate and provide a fix, please?

@roelvanduijnhoven
Copy link
Author

@webimpress Sorry for the confusion.. we migrated our platform to a new host. But.... that has nothing to do with it ... so forget my previous post! 😅

So: the system is using 7.2.21 and has session.sid_bits_per_character set to 4. We updated this repo from 2.8.5 to 2.8.6, and that caused this issue. Apparently the new generated IDs are longer than the previous ones, and the old IDs are no longer valid.

For everybody reading this, we resolved this by changing the cookie name:

return [
    'session_config' => [
        'name' => 'new-session-name',
    ],

@michalbundyra
Copy link
Member

michalbundyra commented Aug 23, 2019

Hi @roelvanduijnhoven !

Sorry for my late response.
I've checked it again, and still I can't find the issue.
Validator\Id has been fixed, but not the method how session id is generated (unless here you are using some custom script to generate session ids?).

I had closer look on validator and here what we have fixed is to use proper value from configuration (session.sid_bits_per_character). Before it was ignored on PHP 7.1+, so your validator before used that pattern:

$pattern = '#^[0-9a-v]*$#';

and now, after the fix it uses:

$pattern = '#^[0-9a-f]*$#';

So it is more restrictive in case of allowed characters, but nothing to do with the length.

For reference see:

// Get the session id bits per character INI setting, using 5 if unavailable
$bitsPerCharacter = PHP_VERSION_ID >= 70100
? 'session.sid_bits_per_character'
: 'session.hash_bits_per_character';
$hashBitsPerChar = ini_get($bitsPerCharacter) ?: 5;
switch ($hashBitsPerChar) {
case 4:
$pattern = '#^[0-9a-f]*$#';
break;
case 6:
$pattern = '#^[0-9a-zA-Z-,]*$#';
break;
case 5:
// intentionally fall-through
default:
$pattern = '#^[0-9a-v]*$#';
break;
}
return (bool) preg_match($pattern, $id);

As you can see, these patter do have any length, so doesn't matter how long is the session id. The only thing this is checking is allowed characters.

Can you tell me please how you generate session id? Would you be able to xdebug the code and check what is the source of the issue? If it is something we can fix in the library, we definitely do! Thanks.

@roelvanduijnhoven
Copy link
Author

I will be on vacation the next 3 weeks. Will try to get back to you after that!

@roelvanduijnhoven
Copy link
Author

@webimpress I know why this happened.

We had a migration prior to this issue were the new PHP configuration had sid_bits_per_character set to 4, instead of the previous 5. And that did not cause any problem at all at that time.

However some time later after the update of this repo to 2.8.6 this suddenly gave problems!

Why?

  1. In 2.8.6 the variable $hashBitsPerChar defaulted to 5 for PHP 7.1 installations.
  2. In the new version this variable was lowered to 4, as it now looked at sid_bits_per_character.
  3. Thus the sessions that were previously generated using 5 bits per char were now suddenly deemed invalid.

This could have been prevented by us by migrating sid_bits_per_character from our old PHP properly to our new PHP stack. Especially because php.net advises 5 bits over 4.


So in theory the 2.8.6 release was a major BC break. But it is very, very specific. My suggestion would be to close this issue. And add a line to the release notes of 2.8.6 as follows:

In some very specific situations this release can lead to a BC break. See issue #121.

@michalbundyra
Copy link
Member

@roelvanduijnhoven Thanks for getting back to me.

I've updated CHANGELOG as you've suggested. I see what has happened and it looks like very rare and edge case. We haven't seen any other reports about this issue. Unfortunately sometimes bugfix can cause BC Break, as the code behaviour was incorrect.

michalbundyra added a commit that referenced this issue Sep 19, 2019
Adding notes about edge case issue with previously generated sessions
- issue #121.
michalbundyra added a commit that referenced this issue Sep 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants