Skip to content

PHP 5.6 mcrypt_create_iv() improvement #579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

PHP 5.6 mcrypt_create_iv() improvement #579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6bef2a0
mcrypt_create_iv() improvement
Feb 3, 2014
3cb2f61
Add constant
Feb 3, 2014
0b64e73
Move enum upwards
Feb 3, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 35 additions & 13 deletions ext/mcrypt/mcrypt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ typedef struct _php_mcrypt {
zend_bool init;
} php_mcrypt;

typedef enum {
RANDOM = 0,
URANDOM,
RAND,
ARANDOM
} iv_source;

/* {{{ arginfo */
ZEND_BEGIN_ARG_INFO_EX(arginfo_mcrypt_module_open, 0, 0, 4)
ZEND_ARG_INFO(0, cipher)
Expand Down Expand Up @@ -425,9 +432,10 @@ static PHP_MINIT_FUNCTION(mcrypt) /* {{{ */
REGISTER_LONG_CONSTANT("MCRYPT_DECRYPT", 1, CONST_PERSISTENT);

/* sources for mcrypt_create_iv */
REGISTER_LONG_CONSTANT("MCRYPT_DEV_RANDOM", 0, CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("MCRYPT_DEV_URANDOM", 1, CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("MCRYPT_RAND", 2, CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("MCRYPT_DEV_RANDOM", RANDOM, CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("MCRYPT_DEV_URANDOM", URANDOM, CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("MCRYPT_RAND", RAND, CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("MCRYPT_DEV_ARANDOM", ARANDOM, CONST_PERSISTENT);

/* ciphers */
MCRYPT_ENTRY2_2_4(3DES, "tripledes");
Expand Down Expand Up @@ -536,11 +544,6 @@ PHP_MINFO_FUNCTION(mcrypt) /* {{{ */
}
/* }}} */

typedef enum {
RANDOM = 0,
URANDOM,
RAND
} iv_source;

/* {{{ proto resource mcrypt_module_open(string cipher, string cipher_directory, string mode, string mode_directory)
Opens the module of the algorithm and the mode to be used */
Expand Down Expand Up @@ -1372,7 +1375,7 @@ PHP_FUNCTION(mcrypt_ofb)
Create an initialization vector (IV) */
PHP_FUNCTION(mcrypt_create_iv)
{
char *iv;
char *iv, *random_source;
long source = RANDOM;
long size;
int n = 0;
Expand All @@ -1387,8 +1390,26 @@ PHP_FUNCTION(mcrypt_create_iv)
}

iv = ecalloc(size + 1, 1);

if (source == RANDOM || source == URANDOM) {

switch(source) {
case RAND:
random_source = NULL;
break;
case URANDOM:
random_source = "/dev/urandom";
break;
case ARANDOM:
random_source = "/dev/arandom";
break;
case RANDOM:
random_source = "/dev/random";
break;
default:
random_source = "/dev/random";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a default setting that may exhaust the entropy source? I would opt for either RAND or URANDOM as the fallback, the former giving two notices :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it may exhaust. The only reason why I coded like this is "it was the default".

IV should be unpredictable, but it's not a secret. I would rather make /dev/urandom a default. (Windows uses their own API) Since this is for PHP 5.6, it would better to be changed now.

php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid random source specified(%dl)", source);
}

if (random_source) {
#if PHP_WIN32
/* random/urandom equivalent on Windows */
BYTE *iv_b = (BYTE *) iv;
Expand All @@ -1402,10 +1423,10 @@ PHP_FUNCTION(mcrypt_create_iv)
int fd;
size_t read_bytes = 0;

fd = open(source == RANDOM ? "/dev/random" : "/dev/urandom", O_RDONLY);
fd = open(random_source, O_RDONLY);
if (fd < 0) {
efree(iv);
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot open source device");
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Cannot open source device(%s)", random_source);
RETURN_FALSE;
}
while (read_bytes < size) {
Expand All @@ -1428,6 +1449,7 @@ PHP_FUNCTION(mcrypt_create_iv)
while (size) {
iv[--size] = (char) (255.0 * php_rand(TSRMLS_C) / RAND_MAX);
}
php_error_docref(NULL TSRMLS_CC, E_NOTICE, "RAND is not safe");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we find this undesirable it should be deprecated, not eternally spit out E_NOTICE errors,

}
RETURN_STRINGL(iv, n, 0);
}
Expand Down
1 change: 1 addition & 0 deletions ext/mcrypt/tests/mcrypt_cbc.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ echo trim(mcrypt_cbc($cipher, $key, $enc_data, MCRYPT_DECRYPT, $iv)) . "\n";
mcrypt_cbc($cipher, $key, $enc_data, MCRYPT_DECRYPT);

--EXPECTF--
Notice: mcrypt_create_iv(): RAND is not safe in %s on line %d

Deprecated: Function mcrypt_cbc() is deprecated in %s on line %d

Expand Down
1 change: 1 addition & 0 deletions ext/mcrypt/tests/mcrypt_cfb.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ echo trim(mcrypt_cfb($cipher, $key, $enc_data, MCRYPT_DECRYPT, $iv)) . "\n";
mcrypt_cfb($cipher, $key, $enc_data, MCRYPT_DECRYPT);

--EXPECTF--
Notice: mcrypt_create_iv(): RAND is not safe in %s on line %d

Deprecated: Function mcrypt_cfb() is deprecated in %s on line %d

Expand Down
3 changes: 2 additions & 1 deletion ext/mcrypt/tests/mcrypt_create_iv.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ $iv3 = mcrypt_create_iv(mcrypt_get_iv_size(MCRYPT_RIJNDAEL_128, MCRYPT_MODE_ECB)
echo strlen($iv1) . "\n";
echo strlen($iv2) . "\n";
echo strlen($iv3) . "\n";
--EXPECT--
--EXPECTF--
Notice: mcrypt_create_iv(): RAND is not safe in %s on line %d
16
16
16
1 change: 1 addition & 0 deletions ext/mcrypt/tests/mcrypt_decrypt.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ mcrypt_decrypt($cipher, $key, $enc_data, MCRYPT_MODE_CBC);

var_dump(strpos(mcrypt_decrypt(MCRYPT_BLOWFISH, "FooBar", $enc_data, MCRYPT_MODE_CBC, $iv), "Testfest") !== false);
--EXPECTF--
Notice: mcrypt_create_iv(): RAND is not safe in %s on line %d
PHP Testfest 2008

Warning: mcrypt_decrypt(): Attempt to use an empty IV, which is NOT recommend in %s on line %d
Expand Down
1 change: 1 addition & 0 deletions ext/mcrypt/tests/mcrypt_ecb.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ echo trim(mcrypt_ecb($cipher, $key, $enc_data, MCRYPT_DECRYPT, $iv)) . "\n";
mcrypt_ecb($cipher, $key, $enc_data, MCRYPT_DECRYPT);

--EXPECTF--
Notice: mcrypt_create_iv(): RAND is not safe in %s on line %d

Deprecated: Function mcrypt_ecb() is deprecated in %s on line %d

Expand Down
1 change: 1 addition & 0 deletions ext/mcrypt/tests/mcrypt_ofb.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ echo trim(mcrypt_ofb($cipher, $key, $enc_data, MCRYPT_DECRYPT, $iv)) . "\n";
mcrypt_ofb($cipher, $key, $enc_data, MCRYPT_DECRYPT, $iv);

--EXPECTF--
Notice: mcrypt_create_iv(): RAND is not safe in %s on line %d

Deprecated: Function mcrypt_ofb() is deprecated in %s on line %d

Expand Down