From ae8693db68581d4d0d3a25e317f4ca7cf55b128f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Mon, 18 Jul 2016 16:56:52 +0200 Subject: [PATCH] Backport cookie encryption from 4.6 branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use hash_hmac for MAC rather than plain SHA1 - Use different secret for MAC than encryption - Merge pmaServer and pmaPass cookies - Document 32 chars length for blowfish_secret Signed-off-by: Michal Čihař --- config.sample.inc.php | 2 +- doc/config.rst | 17 +- doc/setup.rst | 5 +- index.php | 24 +- libraries/core.lib.php | 26 ++ .../auth/AuthenticationCookie.class.php | 261 ++++++++++++------ setup/lib/index.lib.php | 6 +- 7 files changed, 244 insertions(+), 97 deletions(-) diff --git a/config.sample.inc.php b/config.sample.inc.php index 0c34ba5cca..e3d0f06982 100644 --- a/config.sample.inc.php +++ b/config.sample.inc.php @@ -12,7 +12,7 @@ /* * This is needed for cookie based authentication to encrypt password in - * cookie + * cookie. Needs to be 32 chars long. */ $cfg['blowfish_secret'] = 'a8b7c6d'; /* YOU MUST FILL IN THIS FOR COOKIE AUTH! */ diff --git a/doc/config.rst b/doc/config.rst index 963655a623..eef20540d1 100644 --- a/doc/config.rst +++ b/doc/config.rst @@ -1186,11 +1186,18 @@ Cookie authentication options :type: string :default: ``''`` - The "cookie" auth\_type uses blowfish algorithm to encrypt the - password. If you are using the "cookie" auth\_type, enter here a - random passphrase of your choice. It will be used internally by the - blowfish algorithm: you won’t be prompted for this passphrase. There - is no maximum length for this secret. + The "cookie" auth\_type uses AES algorithm to encrypt the password. If you + are using the "cookie" auth\_type, enter here a random passphrase of your + choice. It will be used internally by the AES algorithm: you won’t be + prompted for this passphrase. + + The secret should be 32 characters long. Using shorter will lead to weaker security + of encrypted cookies, using longer will cause no harm. + + .. note:: + + The configuration is called blowfish_secret for historical reasons as + Blowfish algorithm was originally used to do the encryption. .. versionchanged:: 3.1.0 Since version 3.1.0 phpMyAdmin can generate this on the fly, but it diff --git a/doc/setup.rst b/doc/setup.rst index 07eac693ac..512f29a2e2 100644 --- a/doc/setup.rst +++ b/doc/setup.rst @@ -60,8 +60,9 @@ simple configuration may look like this: $size) { + $key = str_pad(pack($pack, $algo($key)), $size, chr(0x00)); + } else { + $key = str_pad($key, $size, chr(0x00)); + } + + for ($i = 0; $i < strlen($key) - 1; $i++) { + $opad[$i] = $opad[$i] ^ $key[$i]; + $ipad[$i] = $ipad[$i] ^ $key[$i]; + } + + $output = $algo($opad.pack($pack, $algo($ipad.$data))); + + return ($raw_output) ? pack($pack, $output) : $output; + } +} /** * Sanitizes MySQL hostname diff --git a/libraries/plugins/auth/AuthenticationCookie.class.php b/libraries/plugins/auth/AuthenticationCookie.class.php index 13daf97dc8..b730ac562f 100644 --- a/libraries/plugins/auth/AuthenticationCookie.class.php +++ b/libraries/plugins/auth/AuthenticationCookie.class.php @@ -299,8 +299,7 @@ class AuthenticationCookie extends AuthenticationPlugin if (defined('PMA_CLEAR_COOKIES')) { foreach ($GLOBALS['cfg']['Servers'] as $key => $val) { - $GLOBALS['PMA_Config']->removeCookie('pmaPass-' . $key); - $GLOBALS['PMA_Config']->removeCookie('pmaServer-' . $key); + $GLOBALS['PMA_Config']->removeCookie('pmaAuth-' . $key); $GLOBALS['PMA_Config']->removeCookie('pmaUser-' . $key); } return false; @@ -317,17 +316,17 @@ class AuthenticationCookie extends AuthenticationPlugin // -> delete password cookie(s) if ($GLOBALS['cfg']['LoginCookieDeleteAll']) { foreach ($GLOBALS['cfg']['Servers'] as $key => $val) { - $GLOBALS['PMA_Config']->removeCookie('pmaPass-' . $key); - if (isset($_COOKIE['pmaPass-' . $key])) { - unset($_COOKIE['pmaPass-' . $key]); + $GLOBALS['PMA_Config']->removeCookie('pmaAuth-' . $key); + if (isset($_COOKIE['pmaAuth-' . $key])) { + unset($_COOKIE['pmaAuth-' . $key]); } } } else { $GLOBALS['PMA_Config']->removeCookie( - 'pmaPass-' . $GLOBALS['server'] + 'pmaAuth-' . $GLOBALS['server'] ); - if (isset($_COOKIE['pmaPass-' . $GLOBALS['server']])) { - unset($_COOKIE['pmaPass-' . $GLOBALS['server']]); + if (isset($_COOKIE['pmaAuth-' . $GLOBALS['server']])) { + unset($_COOKIE['pmaAuth-' . $GLOBALS['server']]); } } } @@ -349,22 +348,14 @@ class AuthenticationCookie extends AuthenticationPlugin // At the end, try to set the $GLOBALS['PHP_AUTH_USER'] // and $GLOBALS['PHP_AUTH_PW'] variables from cookies - // servername - if ($GLOBALS['cfg']['AllowArbitraryServer'] - && ! empty($_COOKIE['pmaServer-' . $GLOBALS['server']]) - ) { - $GLOBALS['pma_auth_server'] - = $_COOKIE['pmaServer-' . $GLOBALS['server']]; - } - - // username + // check cookies if (empty($_COOKIE['pmaUser-' . $GLOBALS['server']])) { return false; } - $GLOBALS['PHP_AUTH_USER'] = $this->blowfishDecrypt( + $GLOBALS['PHP_AUTH_USER'] = $this->cookieDecrypt( $_COOKIE['pmaUser-' . $GLOBALS['server']], - $this->_getBlowfishSecret() + $this->_getEncryptionSecret() ); // user was never logged in since session start @@ -386,18 +377,24 @@ class AuthenticationCookie extends AuthenticationPlugin exit; } - // password - if (empty($_COOKIE['pmaPass-' . $GLOBALS['server']])) { + // check password cookie + if (empty($_COOKIE['pmaAuth-' . $GLOBALS['server']])) { return false; } - $GLOBALS['PHP_AUTH_PW'] = $this->blowfishDecrypt( - $_COOKIE['pmaPass-' . $GLOBALS['server']], - $this->_getBlowfishSecret() + $auth_data = json_decode( + $this->cookieDecrypt( + $_COOKIE['pmaAuth-' . $GLOBALS['server']], + $this->_getSessionEncryptionSecret() + ), + true ); - - if ($GLOBALS['PHP_AUTH_PW'] == "\xff(blank)") { - $GLOBALS['PHP_AUTH_PW'] = ''; + if (! is_array($auth_data) || ! isset($auth_data['password'])) { + return false; + } + $GLOBALS['PHP_AUTH_PW'] = $auth_data['password']; + if ($GLOBALS['cfg']['AllowArbitraryServer'] && ! empty($auth_data['server'])) { + $GLOBALS['pma_auth_server'] = $auth_data['server']; } $GLOBALS['from_cookie'] = true; @@ -473,44 +470,13 @@ class AuthenticationCookie extends AuthenticationPlugin // Name and password cookies need to be refreshed each time // Duration = one month for username - $GLOBALS['PMA_Config']->setCookie( - 'pmaUser-' . $GLOBALS['server'], - $this->blowfishEncrypt( - $cfg['Server']['user'], - $this->_getBlowfishSecret() - ) - ); - + $this->storeUsernameCookie($cfg['Server']['user']); // Duration = as configured - $GLOBALS['PMA_Config']->setCookie( - 'pmaPass-' . $GLOBALS['server'], - $this->blowfishEncrypt( - ! empty($cfg['Server']['password']) - ? $cfg['Server']['password'] : "\xff(blank)", - $this->_getBlowfishSecret() - ), - null, - $GLOBALS['cfg']['LoginCookieStore'] - ); + $this->storePasswordCookie($cfg['Server']['password']); // Set server cookies if required (once per session) and, in this case, // force reload to ensure the client accepts cookies if (! $GLOBALS['from_cookie']) { - if ($GLOBALS['cfg']['AllowArbitraryServer']) { - if (! empty($GLOBALS['pma_auth_server'])) { - // Duration = one month for servername - $GLOBALS['PMA_Config']->setCookie( - 'pmaServer-' . $GLOBALS['server'], - $cfg['Server']['host'] - ); - } else { - // Delete servername cookie - $GLOBALS['PMA_Config']->removeCookie( - 'pmaServer-' . $GLOBALS['server'] - ); - } - } - // URL where to go: $redirect_url = $cfg['PmaAbsoluteUri'] . 'index.php'; @@ -544,7 +510,51 @@ class AuthenticationCookie extends AuthenticationPlugin } // end if return true; + } + /** + * Stores username in a cookie. + * + * @param string $username User name + * + * @return void + */ + public function storeUsernameCookie($username) + { + // Name and password cookies need to be refreshed each time + // Duration = one month for username + $GLOBALS['PMA_Config']->setCookie( + 'pmaUser-' . $GLOBALS['server'], + $this->cookieEncrypt( + $username, + $this->_getEncryptionSecret() + ) + ); + } + + /** + * Stores password in a cookie. + * + * @param string $password Password + * + * @return void + */ + public function storePasswordCookie($password) + { + $payload = array('password' => $password); + if ($GLOBALS['cfg']['AllowArbitraryServer'] && ! empty($GLOBALS['pma_auth_server'])) { + $payload['server'] = $GLOBALS['pma_auth_server']; + } + // Duration = as configured + $GLOBALS['PMA_Config']->setCookie( + 'pmaAuth-' . $GLOBALS['server'], + $this->cookieEncrypt( + json_encode($payload), + $this->_getSessionEncryptionSecret() + ), + null, + $GLOBALS['cfg']['LoginCookieStore'] + ); } /** @@ -563,7 +573,7 @@ class AuthenticationCookie extends AuthenticationPlugin global $conn_error; // Deletes password cookie and displays the login form - $GLOBALS['PMA_Config']->removeCookie('pmaPass-' . $GLOBALS['server']); + $GLOBALS['PMA_Config']->removeCookie('pmaAuth-' . $GLOBALS['server']); if (! empty($GLOBALS['login_without_password_is_forbidden'])) { $conn_error = __( @@ -596,31 +606,118 @@ class AuthenticationCookie extends AuthenticationPlugin * * @return string */ - private function _getBlowfishSecret() + private function _getEncryptionSecret() { if (empty($GLOBALS['cfg']['blowfish_secret'])) { - if (empty($_SESSION['auto_blowfish_secret'])) { - // this returns 32 characters - $_SESSION['auto_blowfish_secret'] = crypt_random_string(32); - } - return $_SESSION['auto_blowfish_secret']; + return $this->_getSessionEncryptionSecret(); } else { - // apply md5() to work around too long secrets (returns 32 characters) - return md5($GLOBALS['cfg']['blowfish_secret']); + return $GLOBALS['cfg']['blowfish_secret']; } } /** - * Encryption using blowfish algorithm (mcrypt) - * or phpseclib's AES if mcrypt not available + * Returns blowfish secret or generates one if needed. + * + * @return string + */ + private function _getSessionEncryptionSecret() + { + if (empty($_SESSION['encryption_key'])) { + if ($this->_useOpenSSL()) { + $_SESSION['encryption_key'] = openssl_random_pseudo_bytes(32); + } else { + $_SESSION['encryption_key'] = Crypt\Random::string(32); + } + } + return $_SESSION['encryption_key']; + } + + /** + * Checks whether we should use openssl for encryption. + * + * @return boolean + */ + private function _useOpenSSL() + { + return ( + function_exists('openssl_encrypt') + && function_exists('openssl_decrypt') + && function_exists('openssl_random_pseudo_bytes') + && PHP_VERSION_ID >= 50304 + ); + } + + /** + * Concatenates secret in order to make it 16 bytes log + * + * This doesn't add any security, just ensures the secret + * is long enough by copying it. + * + * @param string $secret Original secret + * + * @return string + */ + public function enlargeSecret($secret) + { + while (strlen($secret) < 16) { + $secret .= $secret; + } + return substr($secret, 0, 16); + } + + /** + * Derives MAC secret from encryption secret. + * + * @param string $secret the secret + * + * @return string the MAC secret + */ + public function getMACSecret($secret) + { + // Grab first part, up to 16 chars + // The MAC and AES secrets can overlap if original secret is short + $length = strlen($secret); + if ($length > 16) { + return substr($secret, 0, 16); + } + return $this->enlargeSecret( + $length == 1 ? $secret : substr($secret, 0, -1) + ); + } + + /** + * Derives AES secret from encryption secret. + * + * @param string $secret the secret + * + * @return string the AES secret + */ + public function getAESSecret($secret) + { + // Grab second part, up to 16 chars + // The MAC and AES secrets can overlap if original secret is short + $length = strlen($secret); + if ($length > 16) { + return substr($secret, -16); + } + return $this->enlargeSecret( + $length == 1 ? $secret : substr($secret, 1) + ); + } + + /** + * Encryption using openssl's AES or phpseclib's AES + * (phpseclib uses mcrypt when it is available) * * @param string $data original data * @param string $secret the secret * * @return string the encrypted result */ - public function blowfishEncrypt($data, $secret) + public function cookieEncrypt($data, $secret) { + $mac_secret = $this->getMACSecret($secret); + $aes_secret = $this->getAESSecret($secret); $iv = $this->createIV(); if (! function_exists('mcrypt_encrypt')) { /** @@ -631,7 +728,8 @@ class AuthenticationCookie extends AuthenticationPlugin */ include_once "./libraries/phpseclib/Crypt/AES.php"; $cipher = new Crypt_AES(CRYPT_AES_MODE_ECB); - $cipher->setKey($secret); + $cipher->setIV($iv); + $cipher->setKey($aes_secret); $result = base64_encode($cipher->encrypt($data)); } else { $result = base64_encode( @@ -644,10 +742,11 @@ class AuthenticationCookie extends AuthenticationPlugin ) ); } + $iv = base64_encode($iv); return json_encode( array( - 'iv' => base64_encode($iv), - 'mac' => sha1($result . $secret), + 'iv' => $iv, + 'mac' => hash_hmac('sha1', $iv . $result, $mac_secret), 'payload' => $result, ) ); @@ -662,15 +761,19 @@ class AuthenticationCookie extends AuthenticationPlugin * * @return string|bool original data, false on error */ - public function blowfishDecrypt($encdata, $secret) + public function cookieDecrypt($encdata, $secret) { $data = json_decode($encdata, true); - if (! is_array($data) || ! isset($data['mac']) || ! isset($data['iv']) || ! isset($data['payload'])) { + if (! is_array($data) || ! isset($data['mac']) || ! isset($data['iv']) || ! isset($data['payload']) + || ! is_string($data['mac']) || ! is_string($data['iv']) || ! is_string($data['payload']) + ) { return false; } - $newmac = sha1($data['payload'] . $secret); + $mac_secret = $this->getMACSecret($secret); + $aes_secret = $this->getAESSecret($secret); + $newmac = hash_hmac('sha1', $data['iv'] . $data['payload'], $mac_secret); if (! hash_equals($data['mac'], $newmac)) { return false; @@ -680,7 +783,7 @@ class AuthenticationCookie extends AuthenticationPlugin include_once "./libraries/phpseclib/Crypt/AES.php"; $cipher = new Crypt_AES(CRYPT_AES_MODE_ECB); $cipher->setIV(base64_decode($data['iv'])); - $cipher->setKey($secret); + $cipher->setKey($aes_secret); return $cipher->decrypt(base64_decode($data['payload'])); } else { $decrypted = mcrypt_decrypt( diff --git a/setup/lib/index.lib.php b/setup/lib/index.lib.php index ed2665f5a2..5d1072789c 100644 --- a/setup/lib/index.lib.php +++ b/setup/lib/index.lib.php @@ -330,7 +330,7 @@ function perform_config_checks() $server_name = htmlspecialchars($server_name); if ($cookie_auth_server && $blowfish_secret === null) { - $blowfish_secret = bin2hex(crypt_random_string(16)); + $blowfish_secret = bin2hex(crypt_random_string(32)); $blowfish_secret_set = true; $cf->set('blowfish_secret', $blowfish_secret); } @@ -419,9 +419,9 @@ function perform_config_checks() } else { $blowfish_warnings = array(); // check length - if (strlen($blowfish_secret) < 8) { + if (strlen($blowfish_secret) < 32) { // too short key - $blowfish_warnings[] = __('Key is too short, it should have at least 8 characters.'); + $blowfish_warnings[] = __('Key is too short, it should have at least 32 characters.'); } // check used characters $has_digits = (bool) preg_match('/\d/', $blowfish_secret);