Anomie has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/296925

Change subject: Session: Improvements to encryption functionality
......................................................................

Session: Improvements to encryption functionality

* Use CBC mode if CTR is unavailable, since the older method should be
  more commonly supported.
* Apply PKCS7 padding manually when using mcrypt, since mcrypt zero-pads
  instead. This didn't matter for CTR because the effective blocksize is
  1, but it does for CBC. OpenSSL uses PKCS7 padding for CBC mode by
  default, so we don't have to worry about it there.

Bug: T136587
Change-Id: I7290b1a7aa64df70f4ab10eee2080141528c4788
---
M includes/session/Session.php
1 file changed, 49 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/25/296925/1

diff --git a/includes/session/Session.php b/includes/session/Session.php
index 719f905..8fa212e 100644
--- a/includes/session/Session.php
+++ b/includes/session/Session.php
@@ -46,6 +46,9 @@
  * @since 1.27
  */
 final class Session implements \Countable, \Iterator, \ArrayAccess {
+       /** @var null|string[] Encryption algorithm to use */
+       private static $encryptionAlgorithm = null;
+
        /** @var SessionBackend Session backend */
        private $backend;
 
@@ -409,24 +412,42 @@
         * Decide what type of encryption to use, based on system capabilities.
         * @return array
         */
-       private function getEncryptionAlgorithm() {
+       private static function getEncryptionAlgorithm() {
                global $wgSessionInsecureSecrets;
 
-               if (
-                       function_exists( 'openssl_encrypt' )
-                       && in_array( 'aes-256-ctr', 
openssl_get_cipher_methods(), true )
-               ) {
-                       return [ 'openssl', 'aes-256-ctr' ];
-               } elseif (
-                       function_exists( 'mcrypt_encrypt' )
-                       && in_array( 'rijndael-128', mcrypt_list_algorithms(), 
true )
-                       && in_array( 'ctr', mcrypt_list_modes(), true )
-               ) {
-                       return [ 'mcrypt', 'rijndael-128', 'ctr' ];
-               } elseif ( $wgSessionInsecureSecrets ) {
-                       // @todo: import a pure-PHP library for AES instead of 
this
-                       return [ 'insecure' ];
-               } else {
+               if ( self::$encryptionAlgorithm === null ) {
+                       if ( function_exists( 'openssl_encrypt' ) ) {
+                               $methods = openssl_get_cipher_methods();
+                               if ( in_array( 'aes-256-ctr', $methods, true ) 
) {
+                                       self::$encryptionAlgorithm = [ 
'openssl', 'aes-256-ctr' ];
+                                       return self::$encryptionAlgorithm;
+                               }
+                               if ( in_array( 'aes-256-cbc', $methods, true ) 
) {
+                                       self::$encryptionAlgorithm = [ 
'openssl', 'aes-256-cbc' ];
+                                       return self::$encryptionAlgorithm;
+                               }
+                       }
+
+                       if ( function_exists( 'mcrypt_encrypt' )
+                               && in_array( 'rijndael-128', 
mcrypt_list_algorithms(), true )
+                       ) {
+                               $modes = mcrypt_list_modes();
+                               if ( in_array( 'ctr', $modes, true ) ) {
+                                       self::$encryptionAlgorithm = [ 
'mcrypt', 'rijndael-128', 'ctr' ];
+                                       return self::$encryptionAlgorithm;
+                               }
+                               if ( in_array( 'cbc', $modes, true ) ) {
+                                       self::$encryptionAlgorithm = [ 
'mcrypt', 'rijndael-128', 'cbc' ];
+                                       return self::$encryptionAlgorithm;
+                               }
+                       }
+
+                       if ( $wgSessionInsecureSecrets ) {
+                               // @todo: import a pure-PHP library for AES 
instead of this
+                               self::$encryptionAlgorithm = [ 'insecure' ];
+                               return self::$encryptionAlgorithm;
+                       }
+
                        throw new \BadMethodCallException(
                                'Encryption is not available. You really should 
install the PHP OpenSSL extension, ' .
                                'or failing that the mcrypt extension. But if 
you really can\'t and you\'re willing ' .
@@ -435,6 +456,7 @@
                        );
                }
 
+               return self::$encryptionAlgorithm;
        }
 
        /**
@@ -455,7 +477,7 @@
                // Encrypt
                // @todo: import a pure-PHP library for AES instead of doing 
$wgSessionInsecureSecrets
                $iv = \MWCryptRand::generate( 16, true );
-               $algorithm = $this->getEncryptionAlgorithm();
+               $algorithm = self::getEncryptionAlgorithm();
                switch ( $algorithm[0] ) {
                        case 'openssl':
                                $ciphertext = openssl_encrypt( $serialized, 
$algorithm[1], $encKey, OPENSSL_RAW_DATA, $iv );
@@ -464,6 +486,11 @@
                                }
                                break;
                        case 'mcrypt':
+                               // PKCS7 padding
+                               $blocksize = mcrypt_get_block_size( 
$algorithm[1], $algorithm[2] );
+                               $pad = $blocksize - ( strlen( $serialized ) % 
$blocksize );
+                               $serialized .= str_repeat( chr( $pad ), $pad );
+
                                $ciphertext = mcrypt_encrypt( $algorithm[1], 
$encKey, $serialized, $algorithm[2], $iv );
                                if ( $ciphertext === false ) {
                                        throw new \UnexpectedValueException( 
'Encryption failed' );
@@ -521,7 +548,7 @@
                }
 
                // Decrypt
-               $algorithm = $this->getEncryptionAlgorithm();
+               $algorithm = self::getEncryptionAlgorithm();
                switch ( $algorithm[0] ) {
                        case 'openssl':
                                $serialized = openssl_decrypt( base64_decode( 
$ciphertext ), $algorithm[1], $encKey,
@@ -540,6 +567,10 @@
                                        $this->logger->debug( 
$ex->getMessage(), [ 'exception' => $ex ] );
                                        return $default;
                                }
+
+                               // Remove PKCS7 padding
+                               $pad = ord( substr( $serialized, -1 ) );
+                               $serialized = substr( $serialized, 0, -$pad );
                                break;
                        case 'insecure':
                                $ex = new \Exception(

-- 
To view, visit https://gerrit.wikimedia.org/r/296925
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7290b1a7aa64df70f4ab10eee2080141528c4788
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to