Use random_bytes instead of self-rolled random function

[IljaN]

Description

Replace Secure::generate with random_bytes to strengthen the IV.
This creates a new format for the ecrypted-string with a v2| prefix. The modified encryption
code encrypts the session on disk

Related Issue

• Fixes SecureRandom::generate should use bytes instead of ASCII characters encryption#214

Motivation and Context

How Has This Been Tested?

• test environment:
• test case 1:
• test case 2:
• ...

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[IljaN]

CI Fail. The issue is that the random values are outside of the UTF8 range and thus can't be written to the database. This is probably the reason why the weak "random-text" method was used to generate the IV. Or maybe the code predates random_bytes.

The correct implementation would be to use base64Encode before writing the cipher-material to disk or in to the database.

[C0rby]

How about this?

$iv = \random_bytes($this->ivLength);
$this->cipher->setIV($iv);
$iv = \bin2hex($iv);
...
return $ciphertext.'|'.$iv.'|'.$hmac;

And when one needs the iv again it's just:

$iv = \hex2bin($iv);
$this->cipher-setIV($iv);

[lynn-stephenson]

@C0rby That's the correct way of doing it. You can encode the output to ASCII hexadecimal before storing the data, or base64ing it. Your choice.

[C0rby]

Looks good to me! 👍

[DeepDiver1975]

Please add unit tests

[DeepDiver1975]

V1 test ?

[IljaN]

@DeepDiver1975 V1 is already tested here:

core/tests/lib/Security/CryptoTest.php

Line 49 in 816bc5b

public function testLaterDecryption() {

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
No Duplication information