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

introduce wrapped_openssl_seal() and wrapped_openssl_open() #35916

Closed
wants to merge 1 commit into from
Closed

introduce wrapped_openssl_seal() and wrapped_openssl_open() #35916

wants to merge 1 commit into from

Conversation

yahesh
Copy link
Member

@yahesh yahesh commented Dec 29, 2022

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
Contributor

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?

@yahesh
Copy link
Member Author

yahesh commented Dec 30, 2022

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 ?

@yahesh
Copy link
Member Author

yahesh commented Mar 3, 2023

@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