Drupal won't work with sha1 session ids

The other solution to this is to set :-

ini_set('session.hash_bits_per_character', 5);

in the settings.php file. If SHA-1 is used the 160bits are then fitted into 32characters. I tested this and it works fine on PHP5.1.4 and doesn't require a database change.

regards,
--AjK

Maybe we should add a comment?

We could add a comment to make it more clear. The main problem is that PHP's default bits per char is 4. Might have made more sense to make it 5 but hey, what more can you say.

I've attached a system.install patch here also which changes the VARCHAR from 32 to 40 (as noted above) if this is the prefered method/solution.

I wasn't sure if this alteration required an update_ as it seems to me that if the problem exists then an old site just won't be working anyway! So, only possible future versions really need 40 chars.

But a retrospective change by a site admin would render the login system dead so adding a note is a good idea.

regards,
--AjK

There should still be an update for this. I might have a 4.7 site and not use this functionality yet, but I may in the future when I upgrade to 5.0, for example.

having spoke with killes on IRC it seems (so far) the prefered fix is to set the hash_bits_per_character correctly as per my first patch (and with some added comments in there regarding this issue?)

I guuess this one is "for the powers that be" to decide on the proper end solution.

regards,
--AjK

Ok, in that case, patch still needs comments saying why you're setting the bits to 5. :)

How reasonable would be to document session.hash_bits_per_character 5 necessity if sha1 is used and not bother with code? What happens if you set an ini setting which does not exist in the relevant php version?

Sorry, yet another patch to apply the "fix" in a more suitable way.

This patch adds the two PHP INI settings to the .htaccess file as there exists in there a way to set these depending upon the PHP version (the PHP version test already exists in .htaccess).

There's little in the way of comments in .htaccess so if this needs documenting then I'd be happy to add a doc page in the handbook that explains what these two settings do and implications of altering them (and how to alter them safely).

regards,
--AjK

The best solution is to grow the size of the database field. .htaccess solutions are limited to Apache and not portable to other web servers. We should have a good index/key on the field so it shouldn't really matter for performance (I think).

Here is a patch which updates system_install and adds system_update_1013. I tested the query for the mysql update manually. I tested a clean install of HEAD on mysql 4.1. The pgsql should be examined by someone who can vouch for it. I can still login and my sessions seems to work fine.

Why not just set the field to VARCHAR(255) so we don't need to update again should an even longer session id pop up. Storage wise it really doesn't matter in the MySQL world at least.

http://dev.mysql.com/doc/refman/5.0/en/char.html#char

Also since the sid key is the primary key I think it defaults to using the first 96 chars for the index. My bet is that the index file will be just as large, if not bigger, as the data file with the current indexes.

The patch above is exactly was was originally asked for.
I tested on mysql 4 and it works as expected.
postgresql untested.

If it is felt that the field should be increased to 255 chars, the patch can easily be updated (even manually) to reflect this.

Committed to CVS HEAD. Thanks.

FYI: patch committed has varchar(64)
http://drupal.org/cvs?commit=41667

To add this to 4.7 I'd need a patch that adds the correct db update number. However, I am not really in favour of adding db updates to maintenance releases and I dont consider this to be really critical.

if you are savvy enough to set sha1 then you are savvy enoguh to raise session.hash_bits_per_character integer too. While it's OK in HEAD because no pain (and imo no gain but no lose either) it's not important to disturb the peace of 4.7.

No, not all. Only at that host. You are free to complain at the host and if there is no act, vote with your feet.

The HEAD patch misses the sid field in the access_log table [statistics module].

IMO, omitting this patch for 4.7 is very negligent. I will try to submit a patch for it soon.

Cheers,
-K

Correction, the committed patch:

1. does not address the sid in the accesslog table.
2. removes the NOT NULL and DEFAULT attributes of these fields.
3. does not use db_change_column for the pgSQL updates. [Unsure if this has changed in HEAD].

I will submit a backport for 4.7 once this has been sorted. Changing version to CVS.

Thanks,
-K

Try this...

Note: I did not add another update to correct the mistake in the previous update because we do not support HEAD -> HEAD updates.

That capitalized VARCHAR was annoying me. ;)

Committed to CVS HEAD. Thanks.

What update number should I use for a backport?

Thanks
-K

Looks like 183.

Actually, for 4.7.x there is http://drupal.org/node/64049 which was posted before this one. So that's a better place for discussion. Marking this bug as 5.x-dev and fixed.