ProcessWire 2.3 features

[ryan]

I tried to clear modules cache, but still no luck. PHP is 5.4 something.

That's interesting -- PHP 5.3 doesn't report this error. The methods it's complaining about are implemented. Here's the inheritance stack: Wire > WireData > SessionHandler > SessionHandlerDB. The __get() and __set() methods are implemented by WireData. The PHP 5.4 error doesn't appear to be correct, unless they've changed something fundamental about the way objects work. Is your PHP 5.4 version possibly a non-stable version? If not, I'm going to see if I can upgrade to 5.4.

Maybe the container width wider than 85%. Space is precious

Lots of tweaks to take care of still. I'll definitely lose the 85%. Right now it's responsive for the desktop browser and theoretically mobile responsive, but have not yet gotten to the point of testing with phones and tablets (will do that later this week). I expect a lot of tweaks once I'm actually testing with real mobile devices.

[apeisa]

It shouldn't be anything unstable, just what came from package manager: PHP Version 5.4.6-1ubuntu1

I was thinking that maybe I was messing with git, but other than that all the 2.3 features seem to work just fine. Does anyone else have 5.4 available to test dev branch?

I did follow the inheritance stack (just to be sure) and I don't understand why it is throwing that error. It does comes from WireData.

[Mats]

Great new features indeed!

After updating to latest dev-branch jquery fullcalendar and Somas Ajax search module stopped working on a local site. Cant find any errors in the console. Something totally unrelated to the update. Sorry for bugging you gentlemen.

[Soma]

Great new features indeed!

After updating to latest dev-branch jquery fullcalendar and Somas Ajax search module stopped working on a local site. Cant find any errors in the console.

:'(You made me curious, but my ajax search module still works with dev branch.

[Mats]

Thanks for checking Soma. Then i must be doing something else wrong...

[Peter]

Testing dev branch on MAMP (PHP 5.4.4) and on my VPS (PHP 5.4.8).

Getting the same error as apeisa on modules page:

Error Class SessionHandlerDB contains 2 abstract methods and must therefore be declared abstract or implement the remaining methods (ConfigurableModule::__get, ConfigurableModule::__set) (line 195 of /home/tucamed2/public_html/wire/modules/Session/SessionHandlerDB/SessionHandlerDB.module)

[onjegolders]

I posted in general forum but thought it may be more relevant here. Have been having a few issues with this playing nicely with Apeisa's crop module. Ryan's fix sorted out to an extent but upon trying to save the thumbnails, I get an empty image icon and the images in the template look stretched so not sure it's working properly.

Love the new additions though!

[ryan]

I'm really not sure what to think about the PHP 5.4 error message. If inheritance is no longer fully working in 5.4, it seems like we'd have problems all over the place. This really rings of a PHP 5.4 bug. But it's easy enough for me to duplicate those __get() and __set() methods back into the class directly in the SessionHandlerDB module, so I went ahead and did that (and it's now on GitHub). Hopefully we can remove this duplication later, but it'll be easy enough for us to work around for now.

[Martijn Geerts]

When installing 2.2.10 installer said: 2.3 (this evening)

a little notice on the the first installer pages:

( ! ) Notice: Undefined variable: config in /Users/martijn/Sites/www_domains/test/htdocs/wire/templates-admin/install-head.inc on line 26

But so far no other issues.

[ryan]

Thanks Martijn, this has just now been fixed in the latest commit.

[nik]

New password security updates

• ProcessWire now uses Blowfish hashing for passwords when you are on PHP 5.3 or newer. If your database were to ever be compromised, this provides better protection from someone attempting to reverse engineer passwords from the hashes. Beyond blowfish hashing, we still use double salting as before (with one salt on the file system, and other unique to each user in the database). For existing accounts, the blowfish hash doesn't actually take effect until you change your password, so it'll be a gradual transition for many of us. Though the admin does give you a reminder every now and then. However, once you go blowfish, you can't go back, so don't develop a site in PHP 5.3 and then launch it to a PHP 5.2 server, unless you don't mind changing your password(s).
  

We've encountered a problem with Blowfish-hashed passwords when using PW 2.3-to-be (dev-branch). Setting or changing a password breaks it so that the user is unable to login - not even right after otherwise successful installation. I've done a fair amount of reading, digging around and testing various things trying to understand this - and yet I'm not quite sure whether it's a PW bug that needs to be fixed (and how) or a PHP bug in certain versions that some of just have to get around in a way or another.

My apologies for the way too long post - just wanted to give all the information to whoever is able to grab this thing and check the facts for real.

Let's get to the details. What I found the problem to be is that PHP crypt() requires for the Blowfish salt to be at least 22 digits from the given set, but only when PHP version is around 5.3.2. As 5.3.0 introduced internal Blowfish implementation in the first place and 5.3.7 fixed a security bug in it, I'm assuming this could be the version range affected.

PW 2.3 creates salt for Blowfish by using only 21 digits + a '$', which seems to be just fine in most cases. However, according to http://php.net/manual/en/function.crypt.php, there should be one more digit from the given alphabet:

CRYPT_BLOWFISH - Blowfish hashing with a salt as follows: "$2a$", "$2x$" or "$2y$", a two digit cost parameter, "$", and 22 digits from the alphabet "./0-9A-Za-z". Using characters outside of this range in the salt will cause crypt() to return a zero-length string.

Then again, the answer at http://stackoverflow.com/questions/4683350/blowfish-salt-length-for-the-crypt-function does state that salt should be 21 chars + the trailing $ as currently implemented in PW 2.3. Dollar sign seems to be commonly used separator/terminator for salt strings in *NIX at least, but that PHP.net article doesn't say it should be used. I'm nowhere close to an expert on this, but according to my tests the $-terminator is not required.

Here's some more information on the matter to digest (feel free to dive in, but don't blame me if your brain melts during the process): http://stackoverflow.com/questions/2225720/why-does-crypt-blowfish-generate-the-same-hash-with-two-different-salts

Here's a little script I used to check for PHP version differences:

#!/usr/bin/php
<?php

$salt20 = '12345678901234567890';
$pw = 'test';

// see if Blowfish hashing is possible
var_dump(CRYPT_BLOWFISH);

echo "Hash with salt20: " . crypt($pw, '$2a$11$'    . $salt20) . "\n";
echo "Hash with salt21: " . crypt($pw, '$2a$11$x'   . $salt20) . "\n";
echo "Hash with salt22: " . crypt($pw, '$2a$11$xy'  . $salt20) . "\n";
echo "Hash with salt23: " . crypt($pw, '$2a$11$xyz' . $salt20) . "\n";

And here are the results for the few PHP versions I was able to test this on:

PHP 5.2.17 (cli) (built: Jul 4 2012 17:10:25)

int(0)

Hash with salt20: $2vU67iv49YBo

Hash with salt21: $2vU67iv49YBo

Hash with salt22: $2vU67iv49YBo

Hash with salt23: $2vU67iv49YBo

PHP 5.3.2-1ubuntu4.18 with Suhosin-Patch (cli) (built: Sep 12 2012 19:33:42)

int(1)

Hash with salt20: *0

Hash with salt21: *0

Hash with salt22: $2a$11$xy1234567890123456789uUGWvzprkfSZicG0PhBtSK5UzC3n7g/i

Hash with salt23: $2a$11$xyz123456789012345678uWEkwqDg/kw42gJyByIvxheqqUx8E4UO

PHP 5.3.14 (cli) (built: Jul 4 2012 17:24:17)

PHP 5.3.15 (cli) (built: Jul 31 2012 18:42:11)

PHP 5.4.4 (cli) (built: Jul 4 2012 17:28:56)

int(1)

Hash with salt20: $2a$11$12345678901234567890$.sTHQ7O4gHgO7alM/cri4fhJvojMP3v2

Hash with salt21: $2a$11$x12345678901234567890.2CYfZ3UUxkaqHY9XWQ.UEKooJv0TT7S

Hash with salt22: $2a$11$xy1234567890123456789uUGWvzprkfSZicG0PhBtSK5UzC3n7g/i

Hash with salt23: $2a$11$xyz123456789012345678uWEkwqDg/kw42gJyByIvxheqqUx8E4UO

PHP 5.2 had no Blowfish support.

PHP 5.3.14+ (at least) pads under 22 digit salt with extra $'s or takes the first 22 digits from a longer salt string.

PHP 5.3.2 does the same for longer strings but fails to return any valid hash for salt with less than 22 digits (this being the very problem we encountered).

Here are the changes I made to get things working for us. Generate 22 digits instead of 21 and separate 7 ($2a$11$) + 22 (random salt) = 29 digits of salt instead of 28 (29th would be the trailing $) from the resulting hash.

$ git diff wire/core/Password.php
diff --git a/wire/core/Password.php b/wire/core/Password.php
index 5ae8668..96a019d 100644
--- a/wire/core/Password.php
+++ b/wire/core/Password.php
@@ -103,7 +103,7 @@ class Password extends Wire {

               // if it's a blowfish hash, separate the salt from the hash
               if($this->isBlowfish($hash)) {
-                       $this->data['salt'] = substr($hash, 0, 28);
+                       $this->data['salt'] = substr($hash, 0, 29);
                       $this->data['hash'] = substr($hash, 29);
               } else {
                       $this->data['hash'] = $hash;
@@ -134,7 +134,7 @@ class Password extends Wire {
               $len = strlen($chars)-1;

               // generate a 21 character random blowfish salt
-               for($n = 0; $n < 21; $n++) $salt .= $chars[mt_rand(0, $len)]; 
+               for($n = 0; $n < 22; $n++) $salt .= $chars[mt_rand(0, $len)]; 
               $salt .= '$'; // plus trailing $

               return $salt;

The trailing $ doesn't seem to be required but does no harm either. The nasty thing here is that anyone having let the current version write Blowfish hashes to the database would be left with broken passwords if the salt generation was changed like this now.

We decided to fall back to old hashes until this has been solved properly (- make supportsBlowfish() return always false to have it this way). Better way for us would be updating PHP to a more recent version, but we're still using Ubuntu 10.04 LTS which only has PHP 5.3.2 by default. So we'd have to either upgrade the OS or tweak a little to get the current one running with PHP 5.3.10 for example.

Another associated thing with Blowfish crypted and salted passwords if that if one sets up a site using current PW 2.3 with PHP 5.3.0-5.3.6 installed, all the passwords will be corrupted when updating PHP to version 5.3.7 or above. See http://php.net/security/crypt_blowfish.php for details. The passwords can be fixed by replacing $2a$ with $2x$ for the old hashes, but there's no bullet-proof way for PW to correct this on its own. And there shouldn't be as those old hashes are a security risk after all. Newly created passwords would work perfectly though, so instructing users to request a new password might be one way to get over this scenario.

Phew. Over and out.

[ryan]

Nik, thanks for your research and testing on this. Admittedly I found a lot of ambiguity about the the salt rules. It's been awhile now, but if I recall correctly I was not getting the expected results when taking the PHP docs literally on this one. I found several other sources that indicated it should be 21 characters plus a trailing $, so that's what I went with. My first inclination here is to avoid the issue you brought up by making supportsBlowfish() return false when the PHP version is older than 5.3.7. After all, 5.3.19 and 5.4.9 are the current, so treating 5.3.6 and lower like 5.2 probably makes sense here. Based on your understanding of this, would this solve the issue you were running into? Or do you think revisiting the salt length is a necessity?

[nik]

I was just about to say "that sounds good, go for it". But then I decided to give it one more test, just a quick one. Yeah right, like one of those ever existed in the middle of the night. So now, a couple of hours and a few installed PHP versions later, I'm about to lose my mind.

I've tried all the PHP versions I was able to find pre-packaged (for that Ubuntu 10.04) and the only one I got working was the very latest 5.4.9. Both 5.3.6 and 5.3.10 (!) failed for me. Well, these results combined with the previous ones and I was about to say the problem was introduced somewhere between 5.3.10 and 5.3.14. But. A handy tool at http://3v4l.org/ comes in to "rescue" (allows to run a piece of PHP script in 80+ different PHP versions). I ran that script given in my previous post, just to get results like this:

After comparing all the results with each other, the only conclusion I'm able to come to is that there actually is no problem with a plain PHP. The problem does exists, but only when there's Suhosin-Patch around. I'm just not able to find out the exact pattern. I was able to test with three different versions of PHP with Suhosin-Patch: 5.3.2 (failed), 5.3.15 (ok), 5.4.9 (ok). So this few hours didn't give that much new information after all . And besides PHP version there's actually another parameter now: Suhosin-Patch version.

I did come up with a simple test though. If the next line run from the shell gives a result of "*0", it's run on a problematic combination of PHP and Suhosin:

php -r 'echo crypt("test", "$2a$11$12345")."\n";'

Then run these to get the version information for comparison:

php --version
php -r 'phpinfo();' | grep Suhosin

Got to go and get some sleep now. I think this will haunt me all night though...

[teppo]

Just ran nik's test script on couple of machines, all of which had same problem:

• PHP 5.3.18 (cli) (Suhosin Extension 0.9.33)
  
• PHP 5.3.3-7+squeeze14 with Suhosin-Patch (cli) (Suhosin Patch 0.9.9.1 + Suhosin Extension 0.9.32.1) (didn't know these two could co-exist, by the way, but so it claims..)
  
• PHP 5.3.2-1ubuntu4.18 with Suhosin-Patch (cli) (Suhosin Patch 0.9.9.1)
  

That makes three different PHP versions (5.3.x) using either Suhosin Patch 0.9.9.1 or Suhosin Extension 0.9.33 (or both.) Based on some quick research Suhosin does seem like a probable cause; it would seem that it makes multiple crypt-related changes. And if that's true, this problem also seems to exist on multiple versions of it..

By the way, you've probably seen this already, but there's also an open bug at bugs.php.net that might be somehow related to this: https://bugs.php.net/bug.php?id=55717.

[Adam Kiss]

Sorry to enter this conversation (as I am no pro), but wouldn't it be simplest just to set it to 22 digits instead of 21, which seems to work everywhere? (I may not fully understand the conversation, though

[nik]

No pro here either, just an overly enthusiastic developer banging his head against a PHP-wall . But I guess you're right adam, that would solve this for any new installation. The simplest approach just has this little drawback of blowing passwords for any existing 2.2.10 installation where Blowfish has already been utilized.

We've got a live site running that version but with Blowfish disabled so no harm would be done here. I've understood that Ryan's got his sites using the latest version so there may be some problems ahead there, possibly manageable though. But there might be someone else using the latest dev version as well.

So the safest bet would be to find the correct parameters and their values to check for and use Blowfish whenever we know it works with the current salt implementation. But I'm fine with changing the salt to be 22 digits as well. Still that may need some more testing as well.

[ferahl]

With regards to the error

"Failed to initialize storage module: memcache"

This happened when moving my project to remote hosting which is running php 5.3.10 and my local is running 5.3.13.

I tracked the bug down to sessions and saw that my remote host sets the temp session directory to some TCP ip address so thought it might be that. I tried just changing it to /tmp but the problem prevailed.

As a stab in the dark I added:

session_start();

at the top of index.php and all problems solved!

When I remove it though the problem is back.

I also cleared the session asset folder but it didn't make a difference either way.

[ryan]

If switching the salt to 22 digits fixes it, I'm all for that! I'll make this update. Thanks again for all of your research and testing there.

"Failed to initialize storage module: memcache"

This is interesting, it seems like your web host must have sessions configured to do something different than usual as PW has no memcache implementation for sessions yet. You might try installing the SessionHandlerDB module that comes on the dev branch to see if that resolves it. Your session_start(); at the top of index.php may be leading to other less apparent problems, so I would try to get it working without that if you can.

[Adam Kiss]

Whoa there, hold up! It was just my feeling from your conversation that 22 chars work everywhere. That however might not be true.

[nik]

Based on earlier test results that 22 digit salt should work just fine as there has only been problems with shorter ones so far. I see Ryan already updated this in the dev branch, so I'll do at least some testing later today and report back.

[netcarver]

Hmm, 22 chars is what ircmaxell's PHP5.5 compatibility library uses for bcrypt salts.

[apeisa]

I'm really not sure what to think about the PHP 5.4 error message. If inheritance is no longer fully working in 5.4, it seems like we'd have problems all over the place. This really rings of a PHP 5.4 bug. But it's easy enough for me to duplicate those __get() and __set() methods back into the class directly in the SessionHandlerDB module, so I went ahead and did that (and it's now on GitHub). Hopefully we can remove this duplication later, but it'll be easy enough for us to work around for now.

I did update to latest dev version and now it throws this error everywhere on admin:

2012-12-28 00:30:56 ? http://localhost:808...0/?/����Compile Error Cannot redeclare class SessionLoginThrottle (line 17 of /...path.../wire/modules/Session/SessionLoginThrottle/SessionLoginThrottle.module)

Ah, it seems Mint Linux did merge the files, instead of replacing the folder. It does work as expected now.

[apeisa]

Selector problem in dev branch, this works in master but throws an exception in dev:

<?php
if ($page->template == "organisation") $organisation = $page;
else $organisation = $page->parents()->filter("template=organisation")->first();
foreach ($page->parents as $p) {
 echo $p->title . $p->template; // This is just debugging to be sure there is at least one parent with template "organisation"
}
// Organisation not found, bad bad
if ( ! $organisation->id) throw new WireException("Organisation not found");

[ryan]

Thanks Antti, I'll get this fixed here early next week. For now, replace filter("template=organization") with filter("template.name=organization");

[nik]

Based on earlier test results that 22 digit salt should work just fine as there has only been problems with shorter ones so far. I see Ryan already updated this in the dev branch, so I'll do at least some testing later today and report back.

Umm, looks like I never actually reported back, sorry. So, no problems with the latest 22 digit version for me so far. I haven't done any systematic testing though, but it's looking good.

I compiled a table of the PHP tests run by me and Teppo if there ever happened to rise a need for the information gathered so far (hope not actually). This still remains somewhat a mystery to me, as there does not seem to be any one common factor involved - among the suspected ones that is. I don't actually think it has anything to do with Suhosin (Patch nor Extension), not alone anyway.