Skip to content
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.

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

Unexplained incompatibility #10

Closed
marcusjhdon opened this issue Dec 18, 2017 · 5 comments
Closed

Unexplained incompatibility #10

marcusjhdon opened this issue Dec 18, 2017 · 5 comments

Comments

@marcusjhdon
Copy link

marcusjhdon commented Dec 18, 2017

I've been trying to replace some legacy mcrypt functions with openssl equivalents, but even though the cipher and mode (MCRYPT_TRIPLEDES and MCRYPT_MODE_ECB) are apparently supported, I haven't been able to get it to work. So, I tried installing your shim/polyfill instead, but this also doesn't seem to work for this particular use case.

Here is some code to reproduce the problem. When run under PHP 5.6.30 with mcrypt enabled, it works fine, but if I disable mcrypt and use your polyfill it doesn't.

$string = 'Password:abc123';
$expected = base64_decode('UEFjUYBUmsVnm7WU1fBfGQ==');

$td = mcrypt_module_open(MCRYPT_TRIPLEDES, '', MCRYPT_MODE_ECB, '');
$iv = mcrypt_create_iv(mcrypt_enc_get_iv_size($td), MCRYPT_RAND);
mcrypt_generic_init($td, '123456789abcdefg', $iv);
$result = mcrypt_generic($td, $string);
mcrypt_generic_deinit($td);

echo $result == $expected ? "PASS\n" : "FAIL\n";
@terrafrost
Copy link
Member

I'll try to check it out this evening or within the next few days as time permits.

Thanks!

@terrafrost
Copy link
Member

Your code snippet doesn't define $expected. But that said, the issue is due to the fact that the key is 16 bytes long instead of 24 bytes long. mcrypt null pads the key, phpseclib, in this particular case, employees 2TDEA or "keying option 2" or whatever it's best called. wikipedia elaborates:

https://en.wikipedia.org/wiki/Triple_DES#Keying_options

@terrafrost
Copy link
Member

Anyway, I'll work on a fix - thanks for pointing this out!

@marcusjhdon
Copy link
Author

I've updated the code snippet with the missing declarations.

Thanks for the fast replies BTW!

@terrafrost
Copy link
Member

The 1.0.3 release should fix this.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants