Skip to content

Conversation

@weizenspreu
Copy link
Member

This commit introduces wrapped_openssl_seal() and wrapped_openssl_open() with a custom implementation so that RC4 can be supported with OpenSSL v3 without having to reactivate legacy ciphers in the OpenSSL config. The wrapped functions could also be a basis to replace openssl_seal() and openssl_open() with something more modern that maybe uses OAEP padding as well as authenticated encryption.

This commit specifically fixes Nextcloud Server issue #32003.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

…ent RC4 problems with OpenSSL v3

Signed-off-by: Kevin Niehage <k.niehage@syseleven.de>
@kesselb
Copy link
Collaborator

kesselb commented Dec 30, 2022

Thanks for your pull request 👍

Sounds like a good plan to introduce a fallback when the cipher is not available.

I wonder if we could use some code from https://github.com/nextcloud/3rdparty/blob/master/phpseclib/phpseclib/phpseclib/Crypt/RC4.php?

@weizenspreu
Copy link
Member Author

I wonder if we could use some code from https://github.com/nextcloud/3rdparty/blob/master/phpseclib/phpseclib/phpseclib/Crypt/RC4.php?

If someone feels like it they are free to rewrite the code to use the phpseclib implementation instead.

@come-nc
Copy link
Contributor

come-nc commented Jan 2, 2023

I would prefer to switch to phpseclib implementation of RC4 to avoid running our own.
Also, I would always use the wrapped version of seal and remove the fallback.

I can look into that later this week.

@solracsf
Copy link
Member

solracsf commented Mar 2, 2023

Does this still make sense after #36173 ?

@weizenspreu
Copy link
Member Author

@solracsf No, #36173 is a modified version of this pull request here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants