Skip to content

Use OpenSSL instead of Mcrypt in the examples #7934

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 2 commits into from
Closed

Use OpenSSL instead of Mcrypt in the examples #7934

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5be25c
Use OpenSSL instead of Mcrypt in the examples
javiereguiluz May 21, 2017
452d064
Updated the example to use php-encryption library
javiereguiluz Jul 9, 2017
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
4 changes: 2 additions & 2 deletions session/proxy_examples.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@ and decrypt the session as required::
{
$data = parent::read($id);

return mcrypt_decrypt(\MCRYPT_3DES, $this->key, $data);
return openssl_decrypt($data, \OPENSSL_ALGO_SHA1, $this->key);
Copy link
Member

Choose a reason for hiding this comment

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

SHA1 is a Hash algorithm, you cannont use it to encrypt/decrypt purpose

Copy link
Member Author

@javiereguiluz javiereguiluz May 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Which algo should we use instead?

Copy link
Contributor

@sstok sstok May 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I would actually recommend to use LibSodium instead of OpenSSL, encryption is a specialty that is hard to get right, LibSodium makes this as easy as possible. Alternatively you can use https://github.com/defuse/php-encryption which provide a developer friendly (and very secure) API on-top of OpenSSL.

https://paragonie.com/blog/2015/05/if-you-re-typing-word-mcrypt-into-your-code-you-re-doing-it-wrong
https://paragonie.com/blog/2015/05/using-encryption-and-authentication-correctly

Copy link
Member Author

Choose a reason for hiding this comment

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

@sstok I'm not sure we can do that, because this should work out-of-the-box in all/mosts PHP versions, so we can't use external libs.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you can't use LibSodium I would recommend https://github.com/defuse/php-encryption (uses OpenSSL internally), yes it's PHP 5.4, and Symfony 2.7 still supports PHP 5.3, but from the point of security using an old unmaintained PHP version is already a horrible idea (TM).

It all depends on the level of security you want to provide, 3DES is horrible insecure (for many years). Using OpenSSL without proper padding, authentication (crypto authentication) or IV generating, your data is still encrypted. But it becomes easier to break the encryption and leaking sensitive information, and potentially making it possible change the underlying data https://paragonie.com/blog/2015/05/using-encryption-and-authentication-correctly#title.2.1 and thus compromising the trust level of your security.

As this is only used for session data (server-side) the risk smaller, but still existent.
Not to mention using this example and assuming it's safe 🙀.

For the purpose of an example I think it's best to provide a simple example (using OpenSSL) with a big warning that this is only meant as an example and it should not be used for an actual implementation, and providing links with information on how to do this safely 👍

Copy link
Member

Choose a reason for hiding this comment

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

@sstok Thanks for navigating this with us :). So PHP deprecated mcrypt "in favour of OpenSSL"... but there's really no simple way to actually use OpenSSL? If so, that sucks!

If https://github.com/defuse/php-encryption is "just as good" as LibSodium, I'd rather show it being used. I don't think we need to cover its installation, just show how the end-code would look and link to it (and say it's one encryption example). After all, if you're trying to secure your session data, you obviously care about security and are ok with doing some work to get things setup properly. As long as we show a secure example (even if it takes some setup), we're on solid ground.

Copy link
Member Author

@javiereguiluz javiereguiluz May 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Don't we really have a choice to solve this problem using pure PHP code (no libraries)? Let's remember that we're "just" encrypting some "silly" session data, not the nuclear launch codes. Thanks!

Copy link

@mvrhov mvrhov May 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Just an example.

$iv = openssl_random_pseudo_bytes(openssl_cipher_iv_length('AES-256-CFB'), true);
return [
    'iv' => $iv,
    'encrypted' => openssl_encrypt($data, 'AES-256-CFB', $key, true, $iv)
];

Copy link
Contributor

Choose a reason for hiding this comment

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

Good enough as an example or session.

But not failure proof for corruption. Because "changing" the iv could produce a garbage/altered result and you would never notice, that's why authenticated encryption is so important, even if the risk is minimal here. You would still want to have a simple MAC to prevent (file-system) corruptions from breaking the application.

}

public function write($id, $data)
{
$data = mcrypt_encrypt(\MCRYPT_3DES, $this->key, $data);
$data = openssl_encrypt($data, \OPENSSL_ALGO_SHA1, $this->key);

return parent::write($id, $data);
}
Expand Down