[UtilitiesBundle] Cipher deprecations

[cv65kr]

Q	A
Bug fix?	yes
New feature?	yes
BC breaks?	yes
Deprecations?	yes
Fixed tickets	n/a

mcrypt_decrypt, mcrypt_encrypt - deprecated for PHP 7.1

[sandergo90]

@cv65kr When searching a bit on the internet, the library https://github.com/defuse/php-encryption is used in lots of places and even suggested by symfony. Maybe it's a good package to use for this purpose ?

[cv65kr]

@sandergo90, Yes this lib is great and is better choice than our simple crypto class.
The fundamental question is we want implement this lib as a service using only encrypt/decrypt function like we do now? If you look on doc https://github.com/defuse/php-encryption/blob/master/docs/classes/Crypto.md class have a little bit more potential and it would be good to use it.

Summarizing:
We using this convension:

Crypto::decryptWithPassword($ciphertext, $password, $raw_binary = false)
Crypto::decrypt($ciphertext, Key $key, $raw_binary = false)
// etc.

or we implement all method in our service like factory? WDYT?

[sandergo90]

@cv65kr I would keep our utility service and implement all methods in our service. That way we can still extend our service when there is the need to and we can still use the secret as parameter voor the new encrypter.

[sandergo90]

@cv65kr I also think that we shouldn't make the PR on the 4.0 branch. Php 7 will not be supported in 4.0 branch so we can just add the functionality to the master branch ?

[cv65kr]

@sandergo90, ok i will do rebase if PR will be done.

I need some idea because at this moment lib need secret key like this

def00000290a4b250a1b24c41f3076b5e3955e1a51d8535a5dbcf209d17f1eb8d772349cbd12af5dc8f4b05d43ca900489c0fb5aa5c4c5190ccffb5663ae4831e3022fc6

otherwise throws exceptions. The main purpose was to use Key::createNewRandomKey() and maybe saving this secret in DB, i guess. In the our case we need passed secret in __constructor. Have You any ideas how to bite?

---

Also, i created command for handling cipher.

[Numkil]

You could still use the secret in a useful way by using it as a salt when storing the asciisafe representation of a Key object in your value itself or in any other place with a different implementation.

so you could theoretically do in the constructor

    private $secret;
    private $ultimateSecret;
    /**
     * @param string $secret
     * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException
     * @throws \Defuse\Crypto\Exception\BadFormatException
     */
    public function __construct($secret)
    {
        Assert::stringNotEmpty($secret, 'You need to configure a Cipher secret in your parameters.yml before you can use this!');
       $key = Key::createNewRandomKey();
       $this->secret = $secret;
       $this->ultimateSecret = base64_encode($key->saveToAsciiSafeString() . $this->secret);
    }

    /**
     * Encrypt the given value to an unreadable string.
     *
     * @param string $value
     * @param bool $raw_binary
     * @return string
     * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException
     */
    public function encrypt($value, $raw_binary=false)
    {
        $value =  Crypto::encrypt($value, $this->secret, $raw_binary);
        return base64_encode($value . "_key_" . $this->ultimateSecret);
    }

    /**
     * Decrypt the given value so that it's readable again.
     *
     * @param string $value
     * @param bool $raw_binary
     * @return string
     * @throws \Defuse\Crypto\Exception\WrongKeyOrModifiedCiphertextException
     * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException
     */
    public function decrypt($value, $raw_binary=false)
    {
        $value = base64_decode($value);
        $parts = explode("_key_", $value);
        $encryptedValue = $parts[0];
        $key = str_replace($this->secret, "", base64_decode($parts[1]));
        return Crypto::decrypt($encryptedValue, Key:: loadFromAsciiSafeString($key), $raw_binary);
    }

Now everytime you encrypt a value you could append that ultimateSecret to the result and base64_encode it again to store it. This could solve both your problems of having no use for the secret parameter and for how to store the key to the encryption. the key would be stored in the result itself. And since you need to know the $secret value to be able to get the original encryption key back, its safe to store it in the value.

Disclaimer
I do not claim to be an expert in encryption and this way of storing it might be stupid.
I did not try out any of this code i just made a quick brain excercise
I am not convinced the bundles need their own implementation of a cipher object but since it's been already in here it would be too much of a deprecation to remove it. I just don't like it, it doesn't seem our responsibility to implement this.

[Numkil]

On the other hand the Crypto class has a method called encryptWithPassword() which skips the key all together and encrypts a value based on a password. the $secret in our case. That would be a lot simpler but probably also less secure than using a real generated random key for each value.

[cv65kr]

@Numkil I think this solution have a one simple mistake, if you have a hash, just use twice base64_decode and in result you have a secret and random bytes.

Maybe using a encryptWithPassword instead is some solution, let's wait for others opinions.

[cv65kr]

@Numkil, i decide changed to withPassword, can You review?

[Numkil]

@cv65kr thank you. this is a very necesary improvement.