rdbms: add "secret" parameter to ChronologyProtector to use HMAC client IDs

Also make $posIndex mandatory and clean up some IDE warnings in LBFactory.

Change-Id: I9e686b670bc86eb377f14ca57a94e1aa3fd901d5

diff --git a/includes/libs/rdbms/ChronologyProtector.php b/includes/libs/rdbms/ChronologyProtector.php
index 62a2968..fa454c8 100644 (file)
--- a/includes/libs/rdbms/ChronologyProtector.php
+++ b/includes/libs/rdbms/ChronologyProtector.php
@@ -73,13 +73,19 @@ class ChronologyProtector implements LoggerAwareInterface {
        /**
         * @param BagOStuff $store
         * @param array $client Map of (ip: <IP>, agent: <user-agent> [, clientId: <hash>] )
-        * @param int|null $posIndex Write counter index [optional]
+        * @param int|null $posIndex Write counter index
+        * @param string $secret Secret string for HMAC hashing [optional]
         * @since 1.27
         */
-       public function __construct( BagOStuff $store, array $client, $posIndex = null ) {
+       public function __construct( BagOStuff $store, array $client, $posIndex, $secret = '' ) {
                $this->store = $store;
-               $this->clientId = $client['clientId'] ??
-                       md5( $client['ip'] . "\n" . $client['agent'] );
+               if ( isset( $client['clientId'] ) ) {
+                       $this->clientId = $client['clientId'];
+               } else {
+                       $this->clientId = strlen( $secret )
+                               ? hash_hmac( 'md5', $client['ip'] . "\n" . $client['agent'], $secret )
+                               : md5( $client['ip'] . "\n" . $client['agent'] );
+               }
                $this->key = $store->makeGlobalKey( __CLASS__, $this->clientId, 'v2' );
                $this->waitForPosIndex = $posIndex;
 

diff --git a/includes/libs/rdbms/lbfactory/ILBFactory.php b/includes/libs/rdbms/lbfactory/ILBFactory.php
index cb8be21..682561b 100644 (file)
--- a/includes/libs/rdbms/lbfactory/ILBFactory.php
+++ b/includes/libs/rdbms/lbfactory/ILBFactory.php
@@ -57,6 +57,7 @@ interface ILBFactory {
         *  - perfLogger: PSR-3 logger instance. [optional]
         *  - errorLogger: Callback that takes an Exception and logs it. [optional]
         *  - deprecationLogger: Callback to log a deprecation warning. [optional]
+        *  - secret: Secret string to use for HMAC hashing [optional]
         * @throws InvalidArgumentException
         */
        public function __construct( array $conf );

diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php
index 4ea2eb9..fc0606d 100644 (file)
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -24,6 +24,7 @@
 namespace Wikimedia\Rdbms;
 
 use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 use Wikimedia\ScopedCallback;
 use BagOStuff;
 use EmptyBagOStuff;
@@ -74,6 +75,8 @@ abstract class LBFactory implements ILBFactory {
        private $cliMode;
        /** @var string Agent name for query profiling */
        private $agent;
+       /** @var string Secret string for HMAC hashing */
+       private $secret;
 
        /** @var array[] $aliases Map of (table => (dbname, schema, prefix) map) */
        private $tableAliases = [];
@@ -123,7 +126,7 @@ abstract class LBFactory implements ILBFactory {
                $this->wanCache = $conf['wanCache'] ?? WANObjectCache::newEmpty();
 
                foreach ( self::$loggerFields as $key ) {
-                       $this->$key = $conf[$key] ?? new \Psr\Log\NullLogger();
+                       $this->$key = $conf[$key] ?? new NullLogger();
                }
                $this->errorLogger = $conf['errorLogger'] ?? function ( Exception $e ) {
                        trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
@@ -148,6 +151,7 @@ abstract class LBFactory implements ILBFactory {
                $this->hostname = $conf['hostname'] ?? gethostname();
                $this->agent = $conf['agent'] ?? '';
                $this->defaultGroup = $conf['defaultGroup'] ?? null;
+               $this->secret = $conf['secret'] ?? '';
 
                $this->ticket = mt_rand();
        }
@@ -463,7 +467,7 @@ abstract class LBFactory implements ILBFactory {
                        $this->perfLogger->error( __METHOD__ . ": $fname does not have outer scope.\n" .
                                ( new RuntimeException() )->getTraceAsString() );
 
-                       return;
+                       return false;
                }
 
                // The transaction owner and any caller with the empty transaction ticket can commit
@@ -482,6 +486,7 @@ abstract class LBFactory implements ILBFactory {
                if ( $fnameEffective !== $fname ) {
                        $this->beginMasterChanges( $fnameEffective );
                }
+
                return $waitSucceeded;
        }
 
@@ -508,7 +513,8 @@ abstract class LBFactory implements ILBFactory {
                                'agent' => $this->requestInfo['UserAgent'],
                                'clientId' => $this->requestInfo['ChronologyClientId']
                        ],
-                       $this->requestInfo['ChronologyPositionIndex']
+                       $this->requestInfo['ChronologyPositionIndex'],
+                       $this->secret
                );
                $this->chronProt->setLogger( $this->replLogger );
 

diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php
index 7d13ac6..b79cdf3 100644 (file)
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -347,7 +347,8 @@ class LBFactoryTest extends MediaWikiTestCase {
                        [
                                'ip' => '127.0.0.1',
                                'agent' => "Totally-Not-FireFox"
-                       ]
+                       ],
+                       null
                );
 
                $mockDB1->expects( $this->exactly( 1 ) )->method( 'writesOrCallbacksPending' );