Issue 14, OpenSSL RC4

[artulloss]

This creates a new parameter in the constructor to opt to not use OpenSSL and instead use a native PHP implementation.

[JanSlabon]

We had such fallback in place if OpenSSL was not installed at all - but it is simply slow as hell compared to OpenSSL. At some time we left this fallback, which worked for years now.

The problem you try to solve by adding this fallback is that you or your system admin had installed an incompatible version of OpenSSL to your PHP version. I would suggest to fix this at this place instead of silencing it by using a native slow fallback solution.

[artulloss]

@wpengine is shipping OpenSSL 3 on PHP 7.4 builds as part of their ongoing security updates. I brought it up to their engineering team that it's incompatible and breaking this library but they didn't seem to care. They comprise 51% of the managed WordPress hosting market so a significant number of websites probably have this problem with OpenSSL.

I did not notice a terrible delay in the time it took to generate the PDF, but even if this is the case, generating a protected PDF is likely not something going to be done frequently enough to be a performance bottleneck. While this is entirely situational, I think a fallback that can be manually enabled after receiving a warning is useful, but I understand your reasoning for not wanting to allow for that.

[JanSlabon]

They setup an environment which is incompatible (per documentation) and ignore this? Strange... At the end it's not only FPDI Protection that doesn't work but any other tool that relies on OpenSSL. The behavior is completely broken or undefined by doing this.

Would it be the right way to silence that your environment is broken?

We've currently no access to such a faulty environment but I would go for the fallback if:

The parameter $useArcfourFallback (please name it that way) is set and:
a) the algorithm is not supported (!in_array('rc4-40', openssl_get_cipher_methods(), true))
or
b) OpenSSL 3 is installed on PHP < 8.1

Otherwise we should throw the current exception (maybe with a more details message).

Please use following arcfour()-method (it is tested as it is shipped with our other PDF solutions for several years now):

public static function arcfour($key, $data)
{
    if ($this->opensslCipherSupported) {
        return openssl_encrypt($data, 'rc4-40', $key,  OPENSSL_RAW_DATA, '');
    }
    
    static $_lastRc4Key = null, $_lastRc4KeyValue = null;

    if ($_lastRc4Key !== $key) {
        $k = str_repeat($key, (int)(256 / strlen($key) + 1));
        $rc4 = range(0, 255);
        $j = 0;
        for ($i = 0; $i < 256; $i++) {
            $rc4[$i] = $rc4[$j = ($j + ($t = $rc4[$i]) + ord($k[$i])) % 256];
            $rc4[$j] = $t;
        }
        $_lastRc4Key = $key;
        $_lastRc4KeyValue = $rc4;

    } else {
        $rc4 = $_lastRc4KeyValue;
    }

    $len = strlen($data);
    $newData = '';
    $a = 0;
    $b = 0;
    for ($i = 0; $i < $len; $i++) {
        $b = ($b + ($t = $rc4[$a = ($a + 1) % 256])) % 256;
        $rc4[$a] = $rc4[$b];
        $rc4[$b] = $t;
        $newData .= chr(ord($data[$i]) ^ $rc4[($rc4[$a] + $rc4[$b]) % 256]);
    }

    return $newData;
}

Regarding speed: It will be noticeable if you process files with e.g. big images.

[JanSlabon]

Okay, thanks for the update. We just had an internal conversation about this and came to the conclusion that a silence fallback should not happen. So if the parameter is set to true, the fallback should be used throughout.

If it is set to false and "openssl is not installed" or the "rc4 cipher is not available" or "an incompatible version construct" is installed an exception should be thrown.

The current PHP version comparsion looks wrong, or?
Can you share a link with information about the OPENSSL_VERSION_NUMBER constant?

[JanSlabon]

This becomes more complex than I ever thought. Isn't there an easier way to check for the OpenSSL version?
From my experience the issue was only triggered with OpenSSL 3. We never ever had such problem before. So for me it would be okay, if you only check for this one specific version problem.

[artulloss]

Honestly that'd be fine. We could also ignore the version number checking entirely because the RC4 algorithm isn't supported in the bad environment. I believe this is because legacy providers are not loaded, however it wasn't throwing an error before, just output with a password to view that didn't align with the password that was set. Let me know what you'd like to go with.

[JanSlabon]

From my experience the faulty environment is "lying" about the supported cipher. That's why I think we need the version comparison. Can you confirm that? What's in_array('rc4-40', openssl_get_cipher_methods(), true) returning in a faulty environment?

[artulloss]

This was what my debugging data looked like on this last one. It is false, but I believe this may vary depending on if OpenSSL 3 legacy providers are loaded.

array(5) {
["!in_array('rc4-40', openssl_get_cipher_methods(), true)"]=> bool(false)
["!function_exists('openssl_encrypt')"]=> bool(false)
["PHP Version ID]=> int(70433)
["OpenSSL Version"]=> int(805306400)
["isOpensslCompatibleWithPhp7"]=> bool(false)
}

[JanSlabon]

Isn't this negated:

["!in_array('rc4-40', openssl_get_cipher_methods(), true)"]=> bool(false)

? Without negation it would be true which means that it is lying, or?

[artulloss]

Oh, you're right, it does appear to be lying. I think the version check is good then, and the latest one only applies to this specific issue.

[JanSlabon]

Such variable naming is strange. Please simply make a check for OpenSSL 3 + an incompatible PHP version and if $useArcFourFallback is set to false throw an Exception with an appropriate error message.

[JanSlabon]

Do not throw this exception if there's a version compatibility issue in OpenSSL 3 and PHP (just handle this earlier and separated)

[JanSlabon]

If an incompatible construct is recognized and $useArcfourFallback is set to false, we should throw an exception with an appropriated error message, to let the dev known that their environment is faulty (I already commented this earlier).

The later exception should only be thrown if OpenSSL is or the cipher is not installed and $useArcfourFallback is set to false.

[JanSlabon]

Isn't the "OpenSSL 3" simply incompatible to PHP versions below 8.1? Why a do you check for 8.2 and 7.1? See https://www.php.net/manual/en/openssl.requirements.php ?

Please also add a space: if (OPENSSL_....

[JanSlabon]

Wrong wording, see https://www.php.net/manual/en/openssl.requirements.php

[JanSlabon]

Add a space if (!function_...

[JanSlabon]

Add a space if ($useArcfourFallback) and use brackets around the return; statement.

[JanSlabon]

Sorry for the delayed feedback but I was ill.