Add support for Argon2 password hashing
authorMax Semenik <maxsem.wiki@gmail.com>
Tue, 4 Dec 2018 07:46:38 +0000 (23:46 -0800)
committerMax Semenik <maxsem.wiki@gmail.com>
Sun, 10 Feb 2019 10:20:52 +0000 (02:20 -0800)
So far, everything we had was vulnerable to newest advances in
GPU cracking and timing side-channel attacks. Argon2 was designed
specifically to address these problems.

Unfortunately, PHP support is lagging, with some builds missing
Argon2id or even Argon2i.

Change-Id: Ifdf648f5d8a734a663e630286724a6d0a87c7510

RELEASE-NOTES-1.33
autoload.php
includes/DefaultSettings.php
includes/password/Argon2Password.php [new file with mode: 0644]
includes/password/Password.php
tests/phpunit/includes/password/Argon2PasswordTest.php [new file with mode: 0644]
tests/phpunit/includes/password/EncryptedPasswordTest.php
tests/phpunit/includes/password/PasswordTest.php
tests/phpunit/includes/password/PasswordTestCase.php

index 299cd94..55787e7 100644 (file)
@@ -50,6 +50,9 @@ production.
   pages.
 * (T214706) LinksUpdate::getAddedExternalLinks() and
   LinksUpdate::getRemovedExternalLinks() were introduced.
+* Argon2 password hashing is now available, can be enabled via
+  $wgPasswordDefault = 'argon2'. It's designed to resist timing attacks
+  (requires PHP 7.2+) and GPU hacking (7.3+).
 
 === External library changes in 1.33 ===
 
index d577272..a13763e 100644 (file)
@@ -156,6 +156,7 @@ $wgAutoloadLocalClasses = [
        'ApiValidatePassword' => __DIR__ . '/includes/api/ApiValidatePassword.php',
        'ApiWatch' => __DIR__ . '/includes/api/ApiWatch.php',
        'ArchivedFile' => __DIR__ . '/includes/filerepo/file/ArchivedFile.php',
+       'Argon2Password' => __DIR__ . '/includes/password/Argon2Password.php',
        'ArrayDiffFormatter' => __DIR__ . '/includes/diff/ArrayDiffFormatter.php',
        'ArrayUtils' => __DIR__ . '/includes/libs/ArrayUtils.php',
        'Article' => __DIR__ . '/includes/page/Article.php',
index dc8f1e8..582826e 100644 (file)
@@ -4785,6 +4785,24 @@ $wgPasswordConfig = [
                'cost' => '30000',
                'length' => '64',
        ],
+       'argon2' => [
+               'class' => Argon2Password::class,
+
+               // Algorithm used:
+               // * 'argon2i' is optimized against side-channel attacks (PHP 7.2+)
+               // * 'argon2id' is optimized against both side-channel and GPU cracking (PHP 7.3+)
+               // * 'auto' to use best available algorithm. If you're using more than one server, be
+               //   careful when you're mixing PHP versions because newer PHP might generate hashes that
+               //   older versions might would not understand.
+               'algo' => 'auto',
+
+               // The parameters below are the same as options accepted by password_hash().
+               // Set them to override that function's defaults.
+               //
+               // 'memory_cost' => PASSWORD_ARGON2_DEFAULT_MEMORY_COST,
+               // 'time_cost' => PASSWORD_ARGON2_DEFAULT_TIME_COST,
+               // 'threads' => PASSWORD_ARGON2_DEFAULT_THREADS,
+       ],
 ];
 
 /**
diff --git a/includes/password/Argon2Password.php b/includes/password/Argon2Password.php
new file mode 100644 (file)
index 0000000..9138c33
--- /dev/null
@@ -0,0 +1,117 @@
+<?php
+
+use Wikimedia\Assert\Assert;
+
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Implements Argon2, a modern key derivation algorithm designed to resist GPU cracking and
+ * side-channel attacks.
+ *
+ * @see https://en.wikipedia.org/wiki/Argon2
+ */
+class Argon2Password extends Password {
+       /**
+        * @var null[] Array with known password_hash() option names as keys
+        */
+       private static $knownOptions = [
+               'memory_cost' => null,
+               'time_cost' => null,
+               'threads' => null,
+       ];
+
+       /**
+        * @inheritDoc
+        */
+       protected function isSupported() {
+               // It is actually possible to have a PHP build with Argon2i but not Argon2id
+               return defined( 'PASSWORD_ARGON2I' ) || defined( 'PASSWORD_ARGON2ID' );
+       }
+
+       /**
+        * @return mixed[] Array of 2nd and third parmeters to password_hash()
+        */
+       private function prepareParams() {
+               switch ( $this->config['algo'] ) {
+                       case 'argon2i':
+                               $algo = PASSWORD_ARGON2I;
+                               break;
+                       case 'argon2id':
+                               $algo = PASSWORD_ARGON2ID;
+                               break;
+                       case 'auto':
+                               $algo = defined( 'PASSWORD_ARGON2ID' ) ? PASSWORD_ARGON2ID : PASSWORD_ARGON2I;
+                               break;
+                       default:
+                               throw new LogicException( "Unexpected algo: {$this->config['algo']}" );
+
+               }
+
+               $params = array_intersect_key( $this->config, self::$knownOptions );
+
+               return [ $algo, $params ];
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function crypt( $password ) {
+               list( $algo, $params ) = $this->prepareParams();
+               $this->hash = password_hash( $password, $algo, $params );
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function equals( $other ) {
+               if ( is_string( $other ) ) {
+                       return $this->verify( $other );
+               }
+
+               // Argon2 key derivation is not deterministic, can't pass objects to equals()
+               return false;
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function verify( $password ) {
+               Assert::parameterType( 'string', $password, '$password' );
+
+               return password_verify( $password, $this->hash );
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function toString() {
+               $res = ":argon2:{$this->hash}";
+               $this->assertIsSafeSize( $res );
+               return $res;
+       }
+
+       /**
+        * @inheritDoc
+        */
+       public function needsUpdate() {
+               list( $algo, $params ) = $this->prepareParams();
+               return password_needs_rehash( $this->hash, $algo, $params );
+       }
+}
index f167f95..8f6cb3e 100644 (file)
@@ -101,8 +101,11 @@ abstract class Password {
         * @param string|null $hash The raw hash, including the type
         */
        final public function __construct( PasswordFactory $factory, array $config, $hash = null ) {
+               if ( !$this->isSupported() ) {
+                       throw new Exception( 'PHP support not found for ' . get_class( $this ) );
+               }
                if ( !isset( $config['type'] ) ) {
-                       throw new MWException( 'Password configuration must contain a type name.' );
+                       throw new Exception( 'Password configuration must contain a type name.' );
                }
                $this->config = $config;
                $this->factory = $factory;
@@ -125,6 +128,15 @@ abstract class Password {
                return $this->config['type'];
        }
 
+       /**
+        * Whether current password type is supported on this system.
+        *
+        * @return bool
+        */
+       protected function isSupported() {
+               return true;
+       }
+
        /**
         * Perform any parsing necessary on the hash to see if the hash is valid
         * and/or to perform logic for seeing if the hash needs updating.
@@ -169,9 +181,7 @@ abstract class Password {
         * @return bool
         */
        public function verify( $password ) {
-               Assert::parameter( is_string( $password ),
-                       '$password', 'must be string, actual: ' . gettype( $password )
-               );
+               Assert::parameterType( 'string', $password, '$password' );
 
                // No need to use the factory because we're definitely making
                // an object of the same type.
diff --git a/tests/phpunit/includes/password/Argon2PasswordTest.php b/tests/phpunit/includes/password/Argon2PasswordTest.php
new file mode 100644 (file)
index 0000000..b518040
--- /dev/null
@@ -0,0 +1,105 @@
+<?php
+
+/**
+ * @group large
+ * @covers Argon2Password
+ * @covers Password
+ * @covers ParameterizedPassword
+ *
+ * @phpcs:disable Generic.Files.LineLength
+ */
+class Argon2PasswordTest extends PasswordTestCase {
+
+       public function setUp() {
+               parent::setUp();
+               if ( !defined( 'PASSWORD_ARGON2I' ) ) {
+                       $this->markTestSkipped( 'Argon2 support not found' );
+               }
+       }
+
+       /**
+        * Return an array of configs to be used for this class's password type.
+        *
+        * @return array[]
+        */
+       protected function getTypeConfigs() {
+               return [
+                       'argon2' => [
+                               'class' => Argon2Password::class,
+                               'algo' => 'argon2i',
+                               'memory_cost' => 1024,
+                               'time_cost' => 2,
+                               'threads' => 2,
+                       ]
+               ];
+       }
+
+       /**
+        * @return array
+        */
+       public static function providePasswordTests() {
+               $result = [
+                       [
+                               true,
+                               ':argon2:$argon2i$v=19$m=1024,t=2,p=2$RHpGTXJPeFlSV2NDTEswNA$VeW7rumZY4pL8XO4KeQkKD43r5uX3eazVJRtrFN7lNc',
+                               'password',
+                       ],
+                       [
+                               true,
+                               ':argon2:$argon2i$v=19$m=2048,t=5,p=3$MHFKSnh6WWZEWkpKa09SUQ$vU92h/8hkByL5VKW1P9amCj054pZILGKznAvKWAivZE',
+                               'password',
+                       ],
+                       [
+                               true,
+                               ':argon2:$argon2i$v=19$m=1024,t=2,p=2$bFJ4TzM5RWh2T0VmeFhDTA$AHFUFZRh69aZYBqyxn6tpujpEcf2JP8wgRCPU3nw3W4',
+                               "pass\x00word",
+                       ],
+                       [
+                               false,
+                               ':argon2:$argon2i$v=19$m=1024,t=2,p=2$UGZqTWJRUkI1alVNTGRUbA$RcASw9XUWjCDO9WNnuVkGkEylURUW/CcNwSffdFwN74',
+                               'password',
+                       ]
+               ];
+
+               if ( defined( 'PASSWORD_ARGON2ID' ) ) {
+                       // @todo: Argon2id cases
+                       $result = array_merge( $result, [] );
+               }
+
+               return $result;
+       }
+
+       /**
+        * @dataProvider provideNeedsUpdate
+        */
+       public function testNeedsUpdate( $updateExpected, $hash ) {
+               $password = $this->passwordFactory->newFromCiphertext( $hash );
+               $this->assertSame( $updateExpected, $password->needsUpdate() );
+       }
+
+       public function provideNeedsUpdate() {
+               return [
+                       [ false, ':argon2:$argon2i$v=19$m=1024,t=2,p=2$bFJ4TzM5RWh2T0VmeFhDTA$AHFUFZRh69aZYBqyxn6tpujpEcf2JP8wgRCPU3nw3W4' ],
+                       [ false, ':argon2:$argon2i$v=19$m=1024,t=2,p=2$<whatever>' ],
+                       [ true, ':argon2:$argon2i$v=19$m=666,t=2,p=2$<whatever>' ],
+                       [ true, ':argon2:$argon2i$v=19$m=1024,t=666,p=2$<whatever>' ],
+                       [ true, ':argon2:$argon2i$v=19$m=1024,t=2,p=666$<whatever>' ],
+               ];
+       }
+
+       public function testPartialConfig() {
+               $factory = new PasswordFactory();
+               $factory->register( 'argon2', [
+                       'class' => Argon2Password::class,
+                       'algo' => 'argon2i',
+               ] );
+
+               $partialPassword = $factory->newFromType( 'argon2' );
+               $partialPassword->crypt( 'password' );
+               $fullPassword = $this->passwordFactory->newFromCiphertext( $partialPassword->toString() );
+
+               $this->assertFalse( $fullPassword->needsUpdate(),
+                       'Options not set for a password should fall back to defaults'
+               );
+       }
+}
index c5d397f..7384310 100644 (file)
@@ -78,6 +78,7 @@ class EncryptedPasswordTest extends PasswordTestCase {
                $this->assertRegExp( '/^:both:aes-256-cbc:1:/', $serialized );
                $fromNewHash = $this->passwordFactory->newFromCiphertext( $serialized );
                $fromPlaintext = $this->passwordFactory->newFromPlaintext( 'password', $fromNewHash );
-               $this->assertTrue( $fromHash->equals( $fromPlaintext ) );
+               $this->assertTrue( $fromPlaintext->verify( 'password' ) );
+               $this->assertTrue( $fromHash->verify( 'password' ) );
        }
 }
index 65c9199..61a5147 100644 (file)
  * @covers InvalidPassword
  */
 class PasswordTest extends MediaWikiTestCase {
-       public function testInvalidUnequalInvalid() {
-               $passwordFactory = new PasswordFactory();
-               $invalid1 = $passwordFactory->newFromCiphertext( null );
-               $invalid2 = $passwordFactory->newFromCiphertext( null );
-
-               $this->assertFalse( $invalid1->equals( $invalid2 ) );
-       }
-
        public function testInvalidPlaintext() {
                $passwordFactory = new PasswordFactory();
                $invalid = $passwordFactory->newFromPlaintext( null );
index 384db5f..d1e2578 100644 (file)
@@ -60,9 +60,8 @@ abstract class PasswordTestCase extends MediaWikiTestCase {
         * @dataProvider providePasswordTests
         */
        public function testHashing( $shouldMatch, $hash, $password ) {
-               $fromHash = $this->passwordFactory->newFromCiphertext( $hash );
-               $fromPassword = $this->passwordFactory->newFromPlaintext( $password, $fromHash );
-               $this->assertSame( $shouldMatch, $fromHash->equals( $fromPassword ) );
+               $passwordObj = $this->passwordFactory->newFromCiphertext( $hash );
+               $this->assertSame( $shouldMatch, $passwordObj->verify( $password ) );
        }
 
        /**
@@ -85,6 +84,7 @@ abstract class PasswordTestCase extends MediaWikiTestCase {
 
                $this->assertFalse( $invalid->equals( $normal ) );
                $this->assertFalse( $normal->equals( $invalid ) );
+               $this->assertFalse( $invalid->verify( $hash ) );
        }
 
        protected function getValidTypes() {
@@ -106,6 +106,13 @@ abstract class PasswordTestCase extends MediaWikiTestCase {
                $fromType = $this->passwordFactory->newFromType( $type );
                $fromType->crypt( 'password' );
                $fromPlaintext = $this->passwordFactory->newFromPlaintext( 'password', $fromType );
-               $this->assertTrue( $fromType->equals( $fromPlaintext ) );
+               $this->assertTrue( $fromType->verify( 'password' ) );
+               $this->assertTrue( $fromPlaintext->verify( 'password' ) );
+               $this->assertFalse( $fromType->verify( 'different password' ) );
+               $this->assertFalse( $fromPlaintext->verify( 'different password' ) );
+               $this->assertEquals( get_class( $fromType ),
+                       get_class( $fromPlaintext ),
+                       'newFromPlaintext() should produce instance of the same class as newFromType()'
+               );
        }
 }