[Bug] Crypt encrypt != decrypt

[bluntik]

that something strange happens when using the encryption service.

$encryptField = 'kutuzov';
$salt = 'asdh32*6dfgg3dsf';
$encrypt = $this->crypt->encrypt($encryptField, $salt);
$decrypt = $this->crypt->decrypt($encrypt, $salt);
$isTrue = $encryptField== $decrypt;

//in debugger 
$decrypt = 'kutuzov\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000'

[ghost]

This is not a bug but a feature, as this is how the block ciphers work. When the size of the data to encrypt is not a multiple of the cipher's block size, the data are padded with binary zeros.

So, in your case, you use a cipher with the block size of 256 bits (32 bytes), but strlen('kutuzov') is only 7 bytes. Therefore mcrypt_encrypt() adds 25 binary zeros and encrypts the data. This behavior is documented,

When the encrypted data are passed to mcrypt_decrypt(), it decrypts them intact — and therefore leaves those padded zeros in place.

While you may want to rtrim($result, "\0"), this is probably not the safest solution in case you need to deal with binary data which may have trailing zeros. The recommended solution is to store the lenght of the data somewhere.

Hope that helps.

[bluntik]

Thanks for the detailed answer. in this case, of course not very nice to me to use rtrim. Why then do I use this service to encrypt the password? It is easier to me to use sha1 md5 salt.
Can be cleaned with null characters on the side of the framework, so that the output will immediately receive what we need?

[ghost]

This is probably how it should be implemented: http://www.php.net/manual/en/function.mcrypt-encrypt.php#105173

@phalcon if you think this solution is OK, I can prepare a pull request.

@bluntik

Why then do I use this service to encrypt the password?

I would not encrypt passwords unless I plan to reuse them (ie, need the cleartext password for some operation), I would hash them.

SHA1/MD5 aren't safe these days; you may want to look at this and this.

[bluntik]

Thanks, I will choose one of the solutions. I think it is possible to add one of them in the Phalcon Crypt in the future?

[phalcon]

I'd recommend using blowfish for passwords: http://docs.phalconphp.com/en/latest/reference/security.html#password-hashing

[phalcon]

I think is not expected remove the trailing zeros, developers that may need the complete padding will not able to recover the padding if we remove it.

[bluntik]

I use a security service for password encryption and it is working fine.

$user->password = $this->security->hash($password);

[ghost]

@phalcon For 2.3.0 we probablt can implement PKCS-7 padding, probably as separate methods.

@bluntik http://www.php.net/security/crypt_blowfish.php
$2a$ variant is used by Phalcon

[phalcon]

@sjinks right

Closing by now