Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add false as return type for encrypt/decrypt #1679

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kesselb
Copy link

@kesselb kesselb commented Jul 5, 2021

In some situations (especially a broken openssl configuration1) encrypt or decrypt may return false.

Returns the encrypted string on success or false on failure.

https://www.php.net/manual/en/function.openssl-encrypt.php

The decrypted string on success or false on failure.

https://www.php.net/manual/en/function.openssl-decrypt.php

1 nextcloud/server#26425 for some details about this broken openssl configuration.

@terrafrost
Copy link
Member

Ideally what it'd do in this scenario is detect that this is happening and then, transparently, switch to another mode.

I suppose to facilitate this a wrapper method could be created and all calls to openssl_encrypt could be updated to use that wrapper method. But what'd be better is if isValidEngineHelper in SymmetricKey.php could be updated to detect when this error happens and mark OpenSSL as being unavailable in this scenario.

In phpseclib 2.0 for BigInteger.php phpinfo() is checked to see if OpenSSL should be used or not:

https://github.com/phpseclib/phpseclib/blob/2.0.32/phpseclib/Math/BigInteger.php#L267

I wonder if something similar could be done for this.

What'd be really cool is a Docker container that could reproduce the error so that alternative fixes could be tested.

I'll mull on this some.

Thanks!

@terrafrost
Copy link
Member

terrafrost commented Jul 7, 2021

So in an attempt to reproduce this I downloaded FreeBSD-13.0-RELEASE-amd64-dvd1.iso and tried to run it in LiveCD mode in HyperV and... I don't have networking:

https://superuser.com/q/1661007/172193

Consequently I'm not able to install PHP.

The question will be eligible for a bounty in two days. If necessary I'll offer one but idk we'll see what we see...

I also thought I could try to install FreeBSD 13 in Hyper-V by installing VirtualBox in my WSL2 / Ubuntu 20.04 instance. That's not going super well either. To wit I posted this:

https://askubuntu.com/q/1350457/180177

@terrafrost
Copy link
Member

terrafrost commented Jul 20, 2021

So I was able to install FreeBSD 13.0-RELEASE and PHP 7.3.29 and OpenSSL 1.1.1k-freebsd and did not have issues with the following script:

<?php
$plaintext = 'this is a test';
$cipher = 'aes-128-ctr';
echo openssl_encrypt($plaintext, $cipher, str_repeat('z', 16), 0, str_repeat('z', 16));
echo "\n";

nextcloud/server#26425 (comment) mentions FreeBSD 12 so I'll try it next with that...

@terrafrost
Copy link
Member

terrafrost commented Jul 20, 2021

Yah I'm not able to reproduce the issue even on FreeBSD 12. My setup:

  • PHP 7.3.29
  • FreeBSD 12.2-RELEASE-p9
  • OpenSSL 1.1.1h-freebsd

nextcloud/server#26425 (comment) mentions OpenSSL 1.1K. idk how to get that installed on my system, short of compiling it. I tried freebsd-update fetch and then freebsd-update install and it worked just fine but I'm still running OpenSSL 1.1.1h-freebsd. It also mentions freebsd 12.2p4. I'm presumably doing p9. No sure how I can get p4...

I tried freebsd-update fetch again and got this:

src component not installed, skipped
Looking up update.FreeBSD.org mirrors... 2 mirrors found
Fetching metadata signature for 12.2-RELEASE from update2.freebsd.org... done.
Fetching metadata index... done.
Preparing to download files... done.

No updates needed to update system to 12.2-RELEASE-p9.

So idk what's up with that. Maybe I need to compile OpenSSL but that'll open up it's own can of worms...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants