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

Use OpenSSL functions if mcrypt is not available #6826

Merged
merged 24 commits into from
Mar 12, 2018

Conversation

jessp01
Copy link

@jessp01 jessp01 commented Feb 28, 2018

This is the server side follow up of kaltura/clients-generator#278

Starting from PHP 7.1, the Mcrypt extension is deprecated.
There is a good underlying reason for that, to wit: libmcrypt's last update was back in 2007. This is bad for any piece of code but especially troubling for security sensitive ones.

The pulls above will ensure compatibility with KS strings generated using Mcrypt because we set the OPENSSL_ZERO_PADDING bit and do our own padding thus making it compatible with mcrypt's zero padding, as opposed to OpenSSL's default which is PKCS#7.

More info about this can be found here:
http://thefsb.tumblr.com/post/110749271235/using-opensslendecrypt-in-php-instead-of
and in particular under the "Encoding, padding and OPENSSL_RAW_DATA vs.
OPENSSL_ZERO_PADDING" section.

Jess Portnoy added 20 commits February 28, 2018 11:57
Introduce a helper class called KCryptoWrapper that will determine
whether to use OpenSSL or Mcrypt based on the value of the php_crypto_ext
"kConf" directive
- add whitespaces to adhere to the general coding style
- as requested by Erans, avoid use of kConf to determine which extension to use, if mcrypt is
loaded use it, otherwise, use openssl
attempt to use the KCryptoWrapper class fails
const AES_METHOD = 'AES-128-CBC';
const DES3_METHOD = 'DES-EDE3';

public static function random_pseudo_bytes($bytes)
Copy link
Contributor

Choose a reason for hiding this comment

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

rename bytes to length or size, since it may be confusing.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think it's confusing but renamed it to $length anyhow since it's not worth arguing about.

@jessp01 jessp01 changed the base branch from Mercury-13.15.0 to Mercury-13.16.0 March 12, 2018 09:51
@jessp01 jessp01 merged commit e82ab3b into Mercury-13.16.0 Mar 12, 2018
@MosheMaorKaltura MosheMaorKaltura deleted the openssl-fallback branch April 30, 2018 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants