Skip to content

Commit

Permalink
Always use phpseclib for cookie encryption
Browse files Browse the repository at this point in the history 
- it provides fallback in case mcrypt is not found
- we now use AES in both mcrypt and PHP code case
- cleanup the code by removing mcrypt conditials
- rename some methods and cookies so that they don't refer to
  implementation details
- switching encryption implementations no longer invalidates the
  credentials

Signed-off-by: Michal Čihař <michal@cihar.com>
  • Loading branch information
@nijel
nijel committed Jun 12, 2014
1 parent 0652ca9 commit 2064059
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 108 deletions.
101 changes: 35 additions & 66 deletions libraries/plugins/auth/AuthenticationCookie.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@
*/
require './libraries/plugins/auth/swekey/swekey.auth.lib.php';

/**
* phpseclib
*/
include_once PHPSECLIB_INC_DIR . '/Crypt/AES.php';
include_once PHPSECLIB_INC_DIR . '/Crypt/Random.php';

/**
* Handles the cookie authentication method
*
Expand All @@ -38,7 +44,7 @@ class AuthenticationCookie extends AuthenticationPlugin
/**
* IV for Blowfish.
*/
private $_blowfish_iv = null;
private $_cookie_iv = null;

/**
* Displays authentication form
Expand Down Expand Up @@ -443,12 +449,12 @@ public function authCheck()

// check cookies
if (empty($_COOKIE['pmaUser-' . $GLOBALS['server']])
|| empty($_COOKIE['pma_mcrypt_iv'])
|| empty($_COOKIE['pma_iv'])
) {
return false;
}

$GLOBALS['PHP_AUTH_USER'] = $this->blowfishDecrypt(
$GLOBALS['PHP_AUTH_USER'] = $this->cookieDecrypt(
$_COOKIE['pmaUser-' . $GLOBALS['server']],
$this->_getBlowfishSecret()
);
Expand Down Expand Up @@ -481,7 +487,7 @@ public function authCheck()
return false;
}

$GLOBALS['PHP_AUTH_PW'] = $this->blowfishDecrypt(
$GLOBALS['PHP_AUTH_PW'] = $this->cookieDecrypt(
$_COOKIE['pmaPass-' . $GLOBALS['server']],
$this->_getBlowfishSecret()
);
Expand Down Expand Up @@ -551,13 +557,13 @@ public function authSetUser()

$_SESSION['last_access_time'] = time();

$this->createBlowfishIV();
$this->createIV();

// Name and password cookies need to be refreshed each time
// Duration = one month for username
$GLOBALS['PMA_Config']->setCookie(
'pmaUser-' . $GLOBALS['server'],
$this->blowfishEncrypt(
$this->cookieEncrypt(
$cfg['Server']['user'],
$this->_getBlowfishSecret()
)
Expand All @@ -566,7 +572,7 @@ public function authSetUser()
// Duration = as configured
$GLOBALS['PMA_Config']->setCookie(
'pmaPass-' . $GLOBALS['server'],
$this->blowfishEncrypt(
$this->cookieEncrypt(
! empty($cfg['Server']['password'])
? $cfg['Server']['password'] : "\xff(blank)",
$this->_getBlowfishSecret()
Expand Down Expand Up @@ -688,30 +694,12 @@ private function _getBlowfishSecret()
*
* @return string the encrypted result
*/
public function blowfishEncrypt($data, $secret)
public function cookieEncrypt($data, $secret)
{
if (! function_exists('mcrypt_encrypt')) {
/**
* This library uses mcrypt when available, so
* we could always call it instead of having an
* if/then/else logic, however the include_once
* call is costly
*/
include_once PHPSECLIB_INC_DIR . '/Crypt/AES.php';
$cipher = new Crypt_AES(CRYPT_AES_MODE_ECB);
$cipher->setKey($secret);
return base64_encode($cipher->encrypt($data));
} else {
return base64_encode(
mcrypt_encrypt(
MCRYPT_BLOWFISH,
$secret,
$data,
MCRYPT_MODE_CBC,
$this->_blowfish_iv
)
);
}
$cipher = new Crypt_AES(CRYPT_AES_MODE_CBC);
$cipher->setIV($this->_cookie_iv);
$cipher->setKey($secret);
return base64_encode($cipher->encrypt($data));
}

/**
Expand All @@ -723,27 +711,16 @@ public function blowfishEncrypt($data, $secret)
*
* @return string original data
*/
public function blowfishDecrypt($encdata, $secret)
public function cookieDecrypt($encdata, $secret)
{
if (is_null($this->_blowfish_iv)) {
$this->_blowfish_iv = base64_decode($_COOKIE['pma_mcrypt_iv'], true);
}
if (! function_exists('mcrypt_encrypt')) {
include_once PHPSECLIB_INC_DIR . '/Crypt/AES.php';
$cipher = new Crypt_AES(CRYPT_AES_MODE_ECB);
$cipher->setKey($secret);
return $cipher->decrypt(base64_decode($encdata));
} else {
$data = base64_decode($encdata);
$decrypted = mcrypt_decrypt(
MCRYPT_BLOWFISH,
$secret,
$data,
MCRYPT_MODE_CBC,
$this->_blowfish_iv
);
return trim($decrypted);
if (is_null($this->_cookie_iv)) {
$this->_cookie_iv = base64_decode($_COOKIE['pma_iv'], true);
}

$cipher = new Crypt_AES(CRYPT_AES_MODE_CBC);
$cipher->setIV($this->_cookie_iv);
$cipher->setKey($secret);
return $cipher->decrypt(base64_decode($encdata));
}

/**
Expand All @@ -754,22 +731,14 @@ public function blowfishDecrypt($encdata, $secret)
*
* @return void
*/
public function createBlowfishIV()
public function createIV()
{
if (function_exists('mcrypt_encrypt')) {
$td = mcrypt_module_open(MCRYPT_BLOWFISH, '', MCRYPT_MODE_CBC, '');
if ($td === false) {
PMA_fatalError(__('Failed to use Blowfish from mcrypt!'));
}
$this->_blowfish_iv = mcrypt_create_iv(
mcrypt_enc_get_iv_size($td),
MCRYPT_DEV_URANDOM
);
$GLOBALS['PMA_Config']->setCookie(
'pma_mcrypt_iv',
base64_encode($this->_blowfish_iv)
);
}
$cipher = new Crypt_AES(CRYPT_AES_MODE_CBC);
$this->_cookie_iv = crypt_random_string($cipher->block_size);
$GLOBALS['PMA_Config']->setCookie(
'pma_iv',
base64_encode($this->_cookie_iv)
);
}

/**
Expand All @@ -779,9 +748,9 @@ public function createBlowfishIV()
*
* @return void
*/
public function setBlowfishIv($vector)
public function setIV($vector)
{
$this->_blowfish_iv = $vector;
$this->_cookie_iv = $vector;
}

/**
Expand Down
66 changes: 24 additions & 42 deletions test/classes/plugin/auth/PMA_AuthenticationCookie_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ public function testAuthCheck()
$GLOBALS['server'] = 1;
$_COOKIE['pmaServer-1'] = 'pmaServ1';
$_COOKIE['pmaUser-1'] = '';
$_COOKIE['pma_mcrypt_iv'] = base64_encode('testiv09');
$_COOKIE['pma_iv'] = base64_encode('testiv09');

$this->assertFalse(
$this->object->authCheck()
Expand All @@ -590,7 +590,7 @@ public function testAuthCheck()
$GLOBALS['server'] = 1;
$_COOKIE['pmaServer-1'] = 'pmaServ1';
$_COOKIE['pmaUser-1'] = 'pmaUser1';
$_COOKIE['pma_mcrypt_iv'] = base64_encode('testiv09');
$_COOKIE['pma_iv'] = base64_encode('testiv09');
$_COOKIE['pmaPass-1'] = '';
$GLOBALS['cfg']['blowfish_secret'] = 'secret';
$_SESSION['last_access_time'] = time() - 1000;
Expand Down Expand Up @@ -665,19 +665,19 @@ public function testAuthCheckBlowfishCase()
$_REQUEST['pma_username'] = '';
$_COOKIE['pmaServer-1'] = 'pmaServ1';
$_COOKIE['pmaUser-1'] = 'pmaUser1';
$_COOKIE['pma_mcrypt_iv'] = base64_encode('testiv09');
$_COOKIE['pma_iv'] = base64_encode('testiv09');
$GLOBALS['cfg']['blowfish_secret'] = 'secret';
$_SESSION['last_access_time'] = '';
$_SESSION['last_valid_captcha'] = true;

// mock for blowfish function
$this->object = $this->getMockBuilder('AuthenticationCookie')
->disableOriginalConstructor()
->setMethods(array('blowfishDecrypt'))
->setMethods(array('cookieDecrypt'))
->getMock();

$this->object->expects($this->once())
->method('blowfishDecrypt')
->method('cookieDecrypt')
->will($this->returnValue('testBF'));

$this->assertFalse(
Expand All @@ -704,7 +704,7 @@ public function testAuthCheckBlowfishCaseSecond()
$_COOKIE['pmaServer-1'] = 'pmaServ1';
$_COOKIE['pmaUser-1'] = 'pmaUser1';
$_COOKIE['pmaPass-1'] = 'pmaPass1';
$_COOKIE['pma_mcrypt_iv'] = base64_encode('testiv09');
$_COOKIE['pma_iv'] = base64_encode('testiv09');
$GLOBALS['cfg']['blowfish_secret'] = 'secret';
$_SESSION['last_valid_captcha'] = true;
$_SESSION['last_access_time'] = time() - 1000;
Expand All @@ -713,11 +713,11 @@ public function testAuthCheckBlowfishCaseSecond()
// mock for blowfish function
$this->object = $this->getMockBuilder('AuthenticationCookie')
->disableOriginalConstructor()
->setMethods(array('blowfishDecrypt'))
->setMethods(array('cookieDecrypt'))
->getMock();

$this->object->expects($this->at(1))
->method('blowfishDecrypt')
->method('cookieDecrypt')
->will($this->returnValue("\xff(blank)"));

$this->assertTrue(
Expand Down Expand Up @@ -748,7 +748,7 @@ public function testAuthCheckAuthFails()
$_REQUEST['pma_username'] = '';
$_COOKIE['pmaServer-1'] = 'pmaServ1';
$_COOKIE['pmaUser-1'] = 'pmaUser1';
$_COOKIE['pma_mcrypt_iv'] = base64_encode('testiv09');
$_COOKIE['pma_iv'] = base64_encode('testiv09');
$GLOBALS['cfg']['blowfish_secret'] = 'secret';
$_SESSION['last_access_time'] = 1;
$_SESSION['last_valid_captcha'] = true;
Expand Down Expand Up @@ -1049,52 +1049,34 @@ public function testGetBlowfishSecret()
}

/**
* Test for AuthenticationConfig::blowfishEncrypt
* Test for AuthenticationConfig::cookieEncrypt
*
* @return void
*/
public function testBlowfishEncrypt()
{
if (! function_exists('mcrypt_encrypt')) {
$this->assertEquals(
'/xytF/kXKuBx7zHzGexkFw==',
$this->object->blowfishEncrypt('data123', 'sec321')
);
} else {
//using our own iv for testing
$this->object->createBlowfishIV();
$this->object->setBlowfishIv('testiv09');
$this->assertEquals(
'x/2GwHKoPyc=',
$this->object->blowfishEncrypt('data123', 'sec321')
);
}
$this->object->setIV('testiv09');
$this->assertEquals(
'vzJVtW8Ujd4phw7Cxl2PcQ==',
$this->object->cookieEncrypt('data123', 'sec321')
);
}

/**
* Test for AuthenticationConfig::blowfishDecrypt
* Test for AuthenticationConfig::cookieDecrypt
*
* @return void
*/
public function testBlowfishDecrypt()
{
if (function_exists('mcrypt_encrypt')) {

//using our own iv for testing
$this->object->setBlowfishIv('testiv09');
$this->assertEquals(
'data123',
$this->object->blowfishDecrypt('x/2GwHKoPyc=', 'sec321')
);
} else {
$this->assertEquals(
'data123',
$this->object->blowfishDecrypt(
'/xytF/kXKuBx7zHzGexkFw==',
'sec321'
)
);
}
$this->object->setIV('testiv09');
$this->assertEquals(
'data123',
$this->object->cookieDecrypt(
'vzJVtW8Ujd4phw7Cxl2PcQ==',
'sec321'
)
);
}


Expand Down

0 comments on commit 2064059

Please sign in to comment.