php / php-src

The PHP Interpreter
https://www.php.net
Other
37.87k stars 7.72k forks source link

Change to openssl_private_decrypt Error Handling in 8.1.27 #13173

Open JaggedJax opened 7 months ago

JaggedJax commented 7 months ago

Description

The following code:

<?php

$res = openssl_pkey_new();
openssl_pkey_export($res, $privkey);
$pubkey = openssl_pkey_get_details($res)['key'] ?? null;

// Test String
$data = 'non-encrypted data';

// Encrypt Test String
openssl_public_encrypt($data, $encrypted_data, $pubkey, OPENSSL_PKCS1_PADDING);
// Decrypt Encrypted Test String
openssl_private_decrypt($encrypted_data, $decrypted_data, $privkey, OPENSSL_PKCS1_PADDING);

echo "Starting Data: $data".PHP_EOL;
echo "Decrypted Data: $decrypted_data".PHP_EOL;

// Decrypt Unencrypted Test String
if (!openssl_private_decrypt($data, $decrypted_unencrypted_data, $privkey, OPENSSL_PKCS1_PADDING)){
    echo "Could not decrypt invalid data".PHP_EOL;
}
else {
    echo "Decrypted Unencrypted Data: $decrypted_unencrypted_data" . PHP_EOL;
}

Resulted in this output:

Starting Data: non-encrypted data
Decrypted Data: non-encrypted data
Decrypted Unencrypted Data: |~�
�|{[22��E�_Mr�ѝ?�lv�D2�TrKF�_+t��Z�c)
                                     ����9���`����gP���XK
                                                         ����%����RfC��u��L��$Z&��%��%�CLR�����/Z��dB��ލ���T� |��`l=��_�^m��j6�����J�UU…q�N<�v�C�S�AL?��G:

But I expected this output instead:

Starting Data: non-encrypted data
Decrypted Data: non-encrypted data
Could not decrypt invalid data

PHP Version

PHP 8.1.27

Operating System

Rocky Linux release 8.9 (Green Obsidian)

JaggedJax commented 7 months ago

This change in response started in 8.1.27 and I don't think it's intentional, but I could be wrong about that.

bukka commented 7 months ago

I just tested it with latest 8.2 (using OpenSSL 1.1.1 and OpenSSL 3.0) as well as latest 8.1 (with OpenSSL 3.1.5) which is out of active support btw and I'm not able to re-create this locally. What OpenSSL version do you use (output of php --ri openssl)? Do you see that only on Rocky Linux?

JaggedJax commented 7 months ago

openssl

OpenSSL support => enabled OpenSSL Library Version => OpenSSL 1.1.1k FIPS 25 Mar 2021 OpenSSL Header Version => OpenSSL 1.1.1k FIPS 25 Mar 2021 Openssl default config => /etc/pki/tls/openssl.cnf

Rocky 8.9 has Active Support until May 31, 2024 and EOL on May 31, 2029

I didn't even think about the fact that openssl may have upgraded at the same time. Looking at my dnf history, I can see that openssl did have a version update at the same time as I performed the php update:

Upgrade openssl-1:1.1.1k-12.el8_9.x86_64 @baseos Upgraded openssl-1:1.1.1k-9.el8_7.x86_64 @@System

JEDIBC commented 6 months ago

Hi, I just came accross the exact same problem on Ubuntu 22.04 with php 8.2.15 :

OpenSSL support => enabled OpenSSL Library Version => OpenSSL 3.0.2 15 Mar 2022 OpenSSL Header Version => OpenSSL 3.0.2 15 Mar 2022 Openssl default config => /usr/lib/ssl/openssl.cnf

Regards

ranvis commented 6 months ago

fyi: https://github.com/openssl/openssl/commit/7fc67e0a33102aa47bbaa56533eeecb98c0450f7 man: https://www.openssl.org/docs/manmaster/man3/EVP_PKEY_decrypt.html

Since version 3.2.0, the EVP_PKEY_decrypt() method when used with PKCS#1 v1.5 padding doesn't return an error in case it detects an error in padding, instead it returns a pseudo-randomly generated message, removing the need of side-channel secure code from applications using OpenSSL.

backports:

JaggedJax commented 5 months ago

That's very helpful information @ranvis, thanks.

Is there any way to check if encrypted data is properly formatted/valid before calling openssl_private_decrypt? I don't see another function that can be used for that.

I was previously lazily using the result of openssl_private_decrypt to check that input data was valid. Now I'm checking if the output is valid UFT-8 data, but that's a bit hacky (and sometimes the random data returned will be valid UTF-8).

bukka commented 5 months ago

It is not possible. Best solution is to use OPENSSL_PKCS1_OAEP_PADDING. We will be soon updating docs with more info.

ranvis commented 5 months ago

I believe message integrity cannot be validated with padding. OAEP is better though, still an extra MAC is preferred IMO.