Add optional serialization_type and hmac_key param values to RESTBagOStuff

T233537 made RESTBagOStuff work with the Kask external session
storage service, but broke backward compatibility. Add optional
values to the RESTBagOStuff $params constructor parameter to
support communicating with Kask, and to allow using HMAC. The
new values are:
  serialization_type: legacy (default), PHP, or JSON
  hmac_key: HMAC key to use for protecting the serialized blob
If these new values are not specified, behavior remains
unchanged (PHP serialization with no HMAC protection).

Bug: T233963
Change-Id: Ia2625c04e08cfe9616569500f1d613be73c170a2
(cherry picked from commit 2ed69af15cb8971e35aafc2c08397ed38c480488)

diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34
index 71d1a41..e98af0a 100644 (file)
--- a/RELEASE-NOTES-1.34
+++ b/RELEASE-NOTES-1.34
@@ -604,6 +604,7 @@ because of Phabricator reports.
 * ApiQueryBase::showHiddenUsersAddBlockInfo() is deprecated. Use
   ApiQueryBlockInfoTrait instead.
 * PasswordReset is now a service, its direct instantiation is deprecated.
+* RESTBagOStuff users should specify either "JSON" or "PHP" serialization type.
 * Language::getLocalisationCache() is deprecated. Use MediaWikiServices
   instead.
 

diff --git a/includes/libs/objectcache/RESTBagOStuff.php b/includes/libs/objectcache/RESTBagOStuff.php
index 82b5ac0..aa79b37 100644 (file)
--- a/includes/libs/objectcache/RESTBagOStuff.php
+++ b/includes/libs/objectcache/RESTBagOStuff.php
@@ -38,6 +38,7 @@ use Psr\Log\LoggerInterface;
  *             'writeHeaders' => [ 'content-type' => 'application/octet-stream' ],
  *             'deleteHeaders' => [],
  *             'writeMethod' => 'POST',
+ *             'serialization_type' => 'JSON',
  *     ],
  *     'extendedErrorBodyFields' => [ 'type', 'title', 'detail', 'instance' ]
  * );
@@ -73,6 +74,22 @@ class RESTBagOStuff extends MediumSpecificBagOStuff {
         */
        private $httpParams;
 
+       /**
+        * Optional serialization type to use. Allowed values: "PHP", "JSON", or "legacy".
+        * "legacy" is PHP serialization with no serialization type tagging or hmac protection.
+        * @var string
+        * @deprecated since 1.34, the "legacy" value will be removed in 1.35.
+        *   Use either "PHP" or "JSON".
+        */
+       private $serializationType;
+
+       /**
+        * Optional HMAC Key for protecting the serialized blob. If omitted, or if serializationType
+        * is "legacy", then no protection is done
+        * @var string
+        */
+       private $hmacKey;
+
        /**
         * @var array additional body fields to log on error, if possible
         */
@@ -105,6 +122,8 @@ class RESTBagOStuff extends MediumSpecificBagOStuff {
                $this->httpParams['writeHeaders'] = $params['httpParams']['writeHeaders'] ?? [];
                $this->httpParams['deleteHeaders'] = $params['httpParams']['deleteHeaders'] ?? [];
                $this->extendedErrorBodyFields = $params['extendedErrorBodyFields'] ?? [];
+               $this->serializationType = $params['serialization_type'] ?? 'legacy';
+               $this->hmacKey = $params['hmac_key'] ?? '';
 
                // The parent constructor calls setLogger() which sets the logger in $this->client
                parent::__construct( $params );
@@ -211,22 +230,76 @@ class RESTBagOStuff extends MediumSpecificBagOStuff {
         * @return mixed|bool the processed body, or false on error
         */
        private function decodeBody( $body ) {
-               $value = json_decode( $body, true );
-               return ( json_last_error() === JSON_ERROR_NONE ) ? $value : false;
+               if ( $this->serializationType === 'legacy' ) {
+                       $serialized = $body;
+               } else {
+                       $pieces = explode( '.', $body, 3 );
+                       if ( count( $pieces ) !== 3 || $pieces[0] !== $this->serializationType ) {
+                               return false;
+                       }
+                       list( , $hmac, $serialized ) = $pieces;
+                       if ( $this->hmacKey !== '' ) {
+                               $checkHmac = hash_hmac( 'sha256', $serialized, $this->hmacKey, true );
+                               if ( !hash_equals( $checkHmac, base64_decode( $hmac ) ) ) {
+                                       return false;
+                               }
+                       }
+               }
+
+               switch ( $this->serializationType ) {
+                       case 'JSON':
+                               $value = json_decode( $serialized, true );
+                               return ( json_last_error() === JSON_ERROR_NONE ) ? $value : false;
+
+                       case 'PHP':
+                       case 'legacy':
+                               return unserialize( $serialized );
+
+                       default:
+                               throw new \DomainException(
+                                       "Unknown serialization type: $this->serializationType"
+                               );
+               }
        }
 
        /**
         * Prepares the request body (the "value" portion of our key/value store) for transmission.
         *
         * @param string $body request body to prepare
-        * @return string the prepared body, or an empty string on error
+        * @return string the prepared body
         * @throws LogicException
         */
        private function encodeBody( $body ) {
-               $value = json_encode( $body );
-               if ( $value === false ) {
-                       throw new InvalidArgumentException( __METHOD__ . ": body could not be encoded." );
+               switch ( $this->serializationType ) {
+                       case 'JSON':
+                               $value = json_encode( $body );
+                               if ( $value === false ) {
+                                       throw new InvalidArgumentException( __METHOD__ . ": body could not be encoded." );
+                               }
+                               break;
+
+                       case 'PHP':
+                       case "legacy":
+                               $value = serialize( $body );
+                               break;
+
+                       default:
+                               throw new \DomainException(
+                                       "Unknown serialization type: $this->serializationType"
+                               );
+               }
+
+               if ( $this->serializationType !== 'legacy' ) {
+                       if ( $this->hmacKey !== '' ) {
+                               $hmac = base64_encode(
+                                       hash_hmac( 'sha256', $value, $this->hmacKey, true )
+                               );
+                       } else {
+                               $hmac = '';
+                       }
+                       $value = $this->serializationType . '.' . $hmac . '.' . $value;
                }
+
                return $value;
        }
 

diff --git a/tests/phpunit/unit/includes/objectcache/RESTBagOStuffTest.php b/tests/phpunit/unit/includes/objectcache/RESTBagOStuffTest.php
index 459e3ee..689ac21 100644 (file)
--- a/tests/phpunit/unit/includes/objectcache/RESTBagOStuffTest.php
+++ b/tests/phpunit/unit/includes/objectcache/RESTBagOStuffTest.php
@@ -25,17 +25,41 @@ class RESTBagOStuffTest extends \MediaWikiUnitTestCase {
                $this->bag = new RESTBagOStuff( [ 'client' => $this->client, 'url' => 'http://test/rest/' ] );
        }
 
-       public function testGet() {
+       /**
+        * @dataProvider dataGet
+        */
+       public function testGet( $serializationType, $hmacKey, $data ) {
+               $classReflect = new ReflectionClass( RESTBagOStuff::class );
+
+               $serializationTypeReflect = $classReflect->getProperty( 'serializationType' );
+               $serializationTypeReflect->setAccessible( true );
+               $serializationTypeReflect->setValue( $this->bag, $serializationType );
+
+               $hmacKeyReflect = $classReflect->getProperty( 'hmacKey' );
+               $hmacKeyReflect->setAccessible( true );
+               $hmacKeyReflect->setValue( $this->bag, $hmacKey );
+
                $this->client->expects( $this->once() )->method( 'run' )->with( [
                        'method' => 'GET',
                        'url' => 'http://test/rest/42xyz42',
                        'headers' => []
                        // list( $rcode, $rdesc, $rhdrs, $rbody, $rerr )
-               ] )->willReturn( [ 200, 'OK', [], '"somedata"', 0 ] );
+               ] )->willReturn( [ 200, 'OK', [], $data, 0 ] );
                $result = $this->bag->get( '42xyz42' );
                $this->assertEquals( 'somedata', $result );
        }
 
+       public static function dataGet() {
+               // Make sure the defaults are last, so the $bag is left as expected for the next test
+               return [
+                       [ 'JSON', '12345', 'JSON.Us1wli82zEJ6DNQnCG//w+MShOFrdx9wCdfTUhPPA2w=."somedata"' ],
+                       [ 'JSON', '', 'JSON.."somedata"' ],
+                       [ 'PHP', '12345', 'PHP.t2EKhUF4l65kZqWhoAnKW8ZPzekDYfrDxTkQcVmGsuM=.s:8:"somedata";' ],
+                       [ 'PHP', '', 'PHP..s:8:"somedata";' ],
+                       [ 'legacy', '', 's:8:"somedata";' ],
+               ];
+       }
+
        public function testGetNotExist() {
                $this->client->expects( $this->once() )->method( 'run' )->with( [
                        'method' => 'GET',
@@ -71,18 +95,42 @@ class RESTBagOStuffTest extends \MediaWikiUnitTestCase {
                $this->assertEquals( BagOStuff::ERR_UNEXPECTED, $this->bag->getLastError() );
        }
 
-       public function testPut() {
+       /**
+        * @dataProvider dataPut
+        */
+       public function testPut( $serializationType, $hmacKey, $data ) {
+               $classReflect = new ReflectionClass( RESTBagOStuff::class );
+
+               $serializationTypeReflect = $classReflect->getProperty( 'serializationType' );
+               $serializationTypeReflect->setAccessible( true );
+               $serializationTypeReflect->setValue( $this->bag, $serializationType );
+
+               $hmacKeyReflect = $classReflect->getProperty( 'hmacKey' );
+               $hmacKeyReflect->setAccessible( true );
+               $hmacKeyReflect->setValue( $this->bag, $hmacKey );
+
                $this->client->expects( $this->once() )->method( 'run' )->with( [
                        'method' => 'PUT',
                        'url' => 'http://test/rest/42xyz42',
-                       'body' => '"postdata"',
+                       'body' => $data,
                        'headers' => []
                        // list( $rcode, $rdesc, $rhdrs, $rbody, $rerr )
                ] )->willReturn( [ 200, 'OK', [], 'Done', 0 ] );
-               $result = $this->bag->set( '42xyz42', 'postdata' );
+               $result = $this->bag->set( '42xyz42', 'somedata' );
                $this->assertTrue( $result );
        }
 
+       public static function dataPut() {
+               // Make sure the defaults are last, so the $bag is left as expected for the next test
+               return [
+                       [ 'JSON', '12345', 'JSON.Us1wli82zEJ6DNQnCG//w+MShOFrdx9wCdfTUhPPA2w=."somedata"' ],
+                       [ 'JSON', '', 'JSON.."somedata"' ],
+                       [ 'PHP', '12345', 'PHP.t2EKhUF4l65kZqWhoAnKW8ZPzekDYfrDxTkQcVmGsuM=.s:8:"somedata";' ],
+                       [ 'PHP', '', 'PHP..s:8:"somedata";' ],
+                       [ 'legacy', '', 's:8:"somedata";' ],
+               ];
+       }
+
        public function testDelete() {
                $this->client->expects( $this->once() )->method( 'run' )->with( [
                        'method' => 'DELETE',