Sec Bug->Doc #75804 [Opn]: authenticated encryption tag is broken

Edit report at https://bugs.php.net/bug.php?id=75804&edit=1 ID: 75804 Updated by: bukka@php.net Reported by: sergiuthepenguin at gmail dot com Summary: authenticated encryption tag is broken Status: Open -Type: Security +Type: Documentation Problem Package: OpenSSL related Operating System: Windows and GNU/Linux PHP Version: 7.1.13 -Assigned To: +Assigned To: bukka Block user comment: N Private report: Y New Comment: This is known thing and that's how OpenSSL works as well. I agree that we should document this. Not sure about extra parameter as that's pretty easy to do in user land. Also we would need a default value which would break the cases that use shorter tag. But I guess this could be acceptable for 8.0. Might be a good idea though. Previous Comments: ------------------------------------------------------------------------ [2018-01-26 17:21:27] sergiuthepenguin at gmail dot com Thanks for confirming. IMHO, the documentation should be updated to warn developers about manually verifying the tag length using isset($tag[$chosenLength - 1]) or mb_strlen($tag, '8bit'). Speaking of which, is it not possible for openssl_decrypt to have a tag length argument for internal checks? ------------------------------------------------------------------------ [2018-01-26 16:23:03] ab@php.net Thanks for the shorter code. I can reproduce the issue with OpenSSL 1.0.2 and 1.1.0. The linked Ruby issue is actually same. Here's even a shorter reproducer $text = 'The quick brown fox jumps over the lazy dog.'; $key = random_bytes(32); $iv = openssl_random_pseudo_bytes(openssl_cipher_iv_length('aes-256-gcm')); $cipherText = openssl_encrypt($text, 'aes-256-gcm', $key, OPENSSL_RAW_DATA, $iv, $tag, 'test-aad', 16); $ret = openssl_decrypt($cipherText, 'aes-256-gcm', $key, OPENSSL_RAW_DATA, $iv, $tag[0], 'test-aad'); var_dump($ret); This prints the decrypted text, more logic were IMO bool(false). I'd say it's definitely not a good behavior. In how far it is a security issue, is another question. An application should assert the correct tag length in first place. However, this behavior definitely increases the security risk due to programmer mistakes. The documentation states the tag length accepted by openssl_encrypt is between 4 and 16, however the below comes through with the tag length of 3 $cipherText = openssl_encrypt($text, 'aes-256-gcm', $key, OPENSSL_RAW_DATA, $iv, $tag, 'test-aad', 3); We need to discuss further how this should be fixed. Perhaps raising the default tag length could be a solution, something like to 12 bytes as a default for AES. This of course doesn't eliminate the need to check this in the application. Thanks. ------------------------------------------------------------------------ [2018-01-25 22:53:57] sergiuthepenguin at gmail dot com Sorry for the slow reply. I have shortened the code as much as possible: https://gist.github.com/SergiuThePenguin/91ca9f9896882ede43684ab47555b203 If we remove all the bytes of the tag except the for the 1st one, the decryption is still successful. However, if we replace all bytes except the 1st one, the decryption fails as expected. I tested PHP 7.1.13 and 7.2.1 on Windows (outside of wamp, using the installer) as well as in GNU/Linux. The results were identical. ------------------------------------------------------------------------ [2018-01-20 19:34:33] ab@php.net Thanks for the report. Were it possible to reduce the reproduce script to max 20 lines? Also, PHP 7.2 uses OpenSSL 1.1.0, does it make a diff? Also note, that OpenSSL is provided as a DLL, the actual version can be overridden by an actual WAMP distribution. Preferable were a test with the versions provided by the official build. Thanks. ------------------------------------------------------------------------ [2018-01-17 18:39:09] sergiuthepenguin at gmail dot com Updated the test script: https://gist.github.com/SergiuThePenguin/55366527804b592ed86b29c8079b697a Also, Ruby has had a similar (if not identical) issue: https://github.com/ruby/openssl/issues/63 ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=75804 -- Edit this bug report at https://bugs.php.net/bug.php?id=75804&edit=1