php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #81502 $tag argument of openssl_decrypt() should accept null/empty string
Submitted: 2021-10-04 17:12 UTC Modified: 2021-10-08 07:49 UTC
From: alec at alec dot pl Assigned:
Status: Closed Package: OpenSSL related
PHP Version: 8.1.0RC3 OS:
Private report: No CVE-ID: None
 [2021-10-04 17:12 UTC] alec at alec dot pl
Description:
------------
If I do:

$tag = null;
openssl_decrypt($cipher, $method, $ckey, $opts, $iv, $tag);

I get:

PHP Deprecated:  openssl_decrypt(): Passing null to parameter #6 ($tag) of type string is deprecated.

But if I do:

$tag = '';
openssl_decrypt($cipher, $method, $ckey, $opts, $iv, $tag);

I get:

PHP Warning:  openssl_decrypt(): The tag cannot be used because the cipher algorithm does not support AEAD.

I understand the warnings, but this is not convenient if my code is supposed to support AEAD and non-AEAD cipher methods. Do I have to do two code paths depending on the cipher method is used?

What's more. The default value for the $tag argument in openssl_encrypt() is null, which sounds kind of inconsistent.


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-10-05 15:17 UTC] nikic@php.net
-Status: Open +Status: Feedback
 [2021-10-05 15:17 UTC] nikic@php.net
I can't reproduce this. Empty tag for non-AEAD cipher does not produce a warning for me and also matches my reading of the code (see https://github.com/php/php-src/blob/f313854c98e79fa5f4ec5ef9bf8755a15290b1a5/ext/openssl/openssl.c#L7333).

Can you please provide a complete reproducer that demonstrates the issue?
 [2021-10-05 17:08 UTC] alec at alec dot pl
```
<?php
ini_set('error_reporting', E_ALL);
ini_set('display_errors', 'on');

function encrypt($clear)
{
    $key    = 'key';
    $ckey   = '123456789012345678901234';
    $method = 'DES-EDE3-CBC';
    $opts   = OPENSSL_RAW_DATA;
    $iv     = random_bytes(openssl_cipher_iv_length($method));

    $cipher = openssl_encrypt($clear, $method, $ckey, $opts, $iv, $tag);

    $cipher = $iv . $cipher;

    return $cipher;
}

function decrypt($cipher)
{
    $key     = 'key';
    $ckey    = '123456789012345678901234';
    $method  = 'DES-EDE3-CBC';
    $opts    = OPENSSL_RAW_DATA;
    $iv_size = openssl_cipher_iv_length($method);
    $tag     = null;
    
    $iv     = substr($cipher, 0, $iv_size);
    $cipher = substr($cipher, $iv_size);

    return openssl_decrypt($cipher, $method, $ckey, $opts, $iv, $tag);
}

echo decrypt(encrypt('test')) === 'test';
```

One correction, I'm using PHP 8.1.0RC2 on Ubuntu 18.04.
 [2021-10-07 06:29 UTC] alec at alec dot pl
Again, to make it clear. I want openssl_decrypt() to support null or empty string in $tag argument (as not existing) for non-AEAD ciphers.

My real code is here: https://github.com/roundcube/roundcubemail/blob/318d6d08597bbfe481e01bf989150faa8c0403e5/program/lib/Roundcube/rcube.php#L894

With php8.0 I can have ($tag is null):

openssl_decrypt($cipher, $method, $ckey, OPENSSL_RAW_DATA, $iv, $tag);

with php8.1 it throws a deprecation warning, I cannot use this:

openssl_decrypt($cipher, $method, $ckey, OPENSSL_RAW_DATA, $iv, (string) $tag);

because it throws the other warning. I have to use:

if ($tag === null) {
    openssl_decrypt($cipher, $method, $ckey, OPENSSL_RAW_DATA, $iv);
} else {
    openssl_decrypt($cipher, $method, $ckey, OPENSSL_RAW_DATA, $iv, $tag);
}

which I don't like.
 [2021-10-08 05:28 UTC] alec at alec dot pl
-Summary: Problem with $tag argument of openssl_decrypt() +Summary: $tag argument of openssl_decrypt() should accept null/empty string -Status: Feedback +Status: Open
 [2021-10-08 05:28 UTC] alec at alec dot pl
Summary updated
 [2021-10-08 07:44 UTC] nikic@php.net
I still can't reproduce this: If I change your code to use "$tag = '';" instead of "$tag = null;" then I don't get a warning on PHP 8.1. I also checked older versions in case that's recent behavior, but those also don't throw a warning for an empty tag string.
 [2021-10-08 07:49 UTC] nikic@php.net
But anyway, I do agree that we should accept null $tag here. I believe openssl_encrypt() will set it to null for non-aead ciphers, so it only makes sense to also accept this as an input.
 [2021-10-08 12:09 UTC] git@php.net
Automatic comment on behalf of nikic
Revision: https://github.com/php/php-src/commit/7f0d3f5413dfbd989ffe34a417c61210441763f3
Log: Fixed bug #81502
 [2021-10-08 12:09 UTC] git@php.net
-Status: Open +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 08:01:29 2024 UTC