From 6c6a58e930c5863ab1bd11f6a19cbf22aa2f20d4 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 13 Oct 2020 16:46:32 +0200 Subject: [PATCH] Allow passing $tag for non-authenticated encryption openssl_encrypt() currently throws a warning if the $tag out parameter is passed for a non-authenticated cipher. This violates the principle that a function should behave the same if a parameter is not passed, and if the default value is passed for the parameter. I believe this warning should simply be dropped and the $tag be populated with null, as is already the case. Otherwise, it is not possible to use openssl_encrypt() in generic wrapper APIs, that are compatible with both authenticated and non-authenticated encryption. Closes GH-6333. --- ext/openssl/openssl.c | 2 -- ext/openssl/tests/openssl_decrypt_basic.phpt | 7 +++++++ ext/openssl/tests/openssl_decrypt_error.phpt | 5 ----- ext/openssl/tests/openssl_encrypt_error.phpt | 6 ------ 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 84a74bd1ca804..8489d9bfddcad 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -6744,8 +6744,6 @@ PHP_OPENSSL_API zend_string* php_openssl_encrypt(char *data, size_t data_len, ch } } else if (tag) { ZEND_TRY_ASSIGN_REF_NULL(tag); - php_error_docref(NULL, E_WARNING, - "The authenticated tag cannot be provided for cipher that doesn not support AEAD"); } else if (mode.is_aead) { php_error_docref(NULL, E_WARNING, "A tag should be provided when using AEAD mode"); zend_string_release_ex(outbuf, 0); diff --git a/ext/openssl/tests/openssl_decrypt_basic.phpt b/ext/openssl/tests/openssl_decrypt_basic.phpt index 37d17150fb25e..4175e703d2bb8 100644 --- a/ext/openssl/tests/openssl_decrypt_basic.phpt +++ b/ext/openssl/tests/openssl_decrypt_basic.phpt @@ -28,9 +28,16 @@ var_dump(rtrim($output)); $encrypted = openssl_encrypt($data, "bf-ecb", $password, OPENSSL_DONT_ZERO_PAD_KEY); $output = openssl_decrypt($encrypted, "bf-ecb", $password, OPENSSL_DONT_ZERO_PAD_KEY); var_dump($output); + +// It's okay to pass $tag for a non-authenticated cipher. +// It will be populated with null in that case. +openssl_encrypt($data, $method, $password, 0, $iv, $tag); +var_dump($tag); + ?> --EXPECT-- string(45) "openssl_encrypt() and openssl_decrypt() tests" string(45) "openssl_encrypt() and openssl_decrypt() tests" string(45) "openssl_encrypt() and openssl_decrypt() tests" string(45) "openssl_encrypt() and openssl_decrypt() tests" +NULL diff --git a/ext/openssl/tests/openssl_decrypt_error.phpt b/ext/openssl/tests/openssl_decrypt_error.phpt index be40f8080f315..dfd451fe93fde 100644 --- a/ext/openssl/tests/openssl_decrypt_error.phpt +++ b/ext/openssl/tests/openssl_decrypt_error.phpt @@ -23,8 +23,6 @@ var_dump(openssl_decrypt(array(), $method, $password)); var_dump(openssl_decrypt($encrypted, array(), $password)); var_dump(openssl_decrypt($encrypted, $method, array())); -// invalid using of an authentication tag -var_dump(openssl_encrypt($data, $method, $password, 0, $iv, $wrong)); ?> --EXPECTF-- Warning: openssl_encrypt(): Using an empty Initialization Vector (iv) is potentially insecure and not recommended in %s on line %d @@ -53,6 +51,3 @@ NULL Warning: openssl_decrypt() expects parameter 3 to be string, array given in %s on line %d NULL - -Warning: openssl_encrypt(): The authenticated tag cannot be provided for cipher that doesn not support AEAD in %s on line %d -string(44) "yof6cPPH4mLee6TOc0YQSrh4dvywMqxGUyjp0lV6+aM=" diff --git a/ext/openssl/tests/openssl_encrypt_error.phpt b/ext/openssl/tests/openssl_encrypt_error.phpt index ea39bff604cf2..71e679ffffe3f 100644 --- a/ext/openssl/tests/openssl_encrypt_error.phpt +++ b/ext/openssl/tests/openssl_encrypt_error.phpt @@ -21,9 +21,6 @@ var_dump(openssl_encrypt($arr, $method, $object)); var_dump(openssl_encrypt($data, $arr, $object)); var_dump(openssl_encrypt($data, $method, $arr)); -// invalid using of an authentication tag -var_dump(openssl_encrypt($data, $method, $password, 0, $iv, $wrong)); - // padding of the key is disabled var_dump(openssl_encrypt($data, $method, $password, OPENSSL_DONT_ZERO_PAD_KEY, $iv)); ?> @@ -49,8 +46,5 @@ NULL Warning: openssl_encrypt() expects parameter 3 to be string, array given in %s on line %d NULL -Warning: openssl_encrypt(): The authenticated tag cannot be provided for cipher that doesn not support AEAD in %s on line %d -string(44) "iPR4HulskuaP5Z6me5uImk6BqVyJG73+63tkPauVZYk=" - Warning: openssl_encrypt(): Key length cannot be set for the cipher method in %s on line %d bool(false)