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

[NFR]: crypt->decode throws Warning #15879

Closed
destyk opened this issue Jan 27, 2022 · 5 comments · Fixed by #15895
Closed

[NFR]: crypt->decode throws Warning #15879

destyk opened this issue Jan 27, 2022 · 5 comments · Fixed by #15895
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release new feature request Planned Feature or New Feature Request

Comments

@destyk
Copy link

destyk commented Jan 27, 2022

The current issue is related to the Phalcon\Encryption\Crypt module. It lies in the fact that if any random string that is less than 16 bytes long is passed to the decryption function, then PHP throws a Warning: openssl_decrypt(): the passed IV is only 2 bytes long, cipher expects an IV exactly 16 bytes long.

Example:

// service.php
$di->set('crypt', function () {
    $crypt = new Crypt();
    $crypt
        ->setKey('secretKey')
        ->useSigning(true);

    return $crypt;
}, true);
// ExampleController.php
// ...

$encryptStr = $this->crypt->encryptBase64('plain text');

// throws Warning
$input = $this->crypt->decryptBase64('123');

This problem is, of course, solved by disabling the display of warnings in the production environment. However, I would like to see in the future a function to check the encoded string for the number of bytes required by the cipher in use.

Example:

// ExampleController.php
// ...

$input = 'str data';
if ($this->crypt->isValidInputLength($input)) {
    echo 'input length is valid. You can try to decode';
} else {
    echo 'input length is NOT valid. Don\'t even try to decode';
}

Many thanks in advance and have a nice day! :)

@destyk destyk added the new feature request Planned Feature or New Feature Request label Jan 27, 2022
@niden niden added 5.0 The issues we want to solve in the 5.0 release bug A bug report status: medium Medium and removed new feature request Planned Feature or New Feature Request labels Jan 27, 2022
@niden niden self-assigned this Jan 27, 2022
@niden
Copy link
Sponsor Member

niden commented Jan 27, 2022

@destyk Thank you for reporting this. I will sort it out shortly.

@niden niden added this to Working on it in Phalcon Roadmap Jan 27, 2022
@niden niden mentioned this issue Feb 10, 2022
5 tasks
@niden niden linked a pull request Feb 10, 2022 that will close this issue
5 tasks
@niden niden added new feature request Planned Feature or New Feature Request and removed bug A bug report status: medium Medium labels Feb 10, 2022
@noone-silent
Copy link

Why using the complex error handler stuff? Because openssl_cipher_iv_length() raises a warning? Why not simply check with openssl_get_cipher_methods() if the cipher is supported?

@niden
Copy link
Sponsor Member

niden commented Feb 10, 2022

Why using the complex error handler stuff? Because openssl_cipher_iv_length() raises a warning? Why not simply check with openssl_get_cipher_methods() if the cipher is supported?

Yes that was the intent. Your solution is better and faster :)

@niden
Copy link
Sponsor Member

niden commented Feb 10, 2022

Why using the complex error handler stuff? Because openssl_cipher_iv_length() raises a warning? Why not simply check with openssl_get_cipher_methods() if the cipher is supported?

Actually looking at the code - and I forgot about this - the cipher is checked against the available ciphers so it will always be valid. I have simplified the code. Thank you @noone-silent

@niden
Copy link
Sponsor Member

niden commented Feb 10, 2022

Resolved in #15895

Thank you @destyk and @noone-silent

@niden niden closed this as completed Feb 10, 2022
Phalcon Roadmap automation moved this from Working on it to Implemented Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release new feature request Planned Feature or New Feature Request
Projects
Archived in project
Phalcon Roadmap
  
Implemented
Development

Successfully merging a pull request may close this issue.

3 participants