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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: alec at alec dot pl
New email:
PHP Version: OS:

 

 [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 10:01:29 2024 UTC