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

Get rid of openssl_seal in encryption code #2182

Closed
LukasReschke opened this issue Nov 17, 2016 · 7 comments
Closed

Get rid of openssl_seal in encryption code #2182

LukasReschke opened this issue Nov 17, 2016 · 7 comments

Comments

@LukasReschke
Copy link
Member

We should get rid of openssl_seal in the encryption app as that uses the RC4 cipher which considered out-of-date. At the moment it is used for encrypting multi-keys to file recipients.

We should replace the openssl_seal part with an approach using openssl_public_encrypt (OPENSSL_PKCS1_OAEP_PADDING).

cc @schiessle @oparoz @karlitschek

@karlitschek
Copy link
Member

agreed. 👍 Please keep backward compatibility in mind.

@schiessle
Copy link
Member

schiessle commented Nov 29, 2016

I would suggest to make this changes in a new encryption module, there are some other stuff we can improve as well. The advantage of doing this in a second encryption module is that backward compatibility is easy and we don't face any limitations because of backward compatibility for the new module:
Admin can just enable the new module along the old one and set the new one to "default". Every file newly encrypted will be encrypted with the new module and old files can still be loaded with the old encryption module. I think a good time for a new module would be if we can also chose a Cipher which takes care about all the signing stuff.

@LukasReschke LukasReschke self-assigned this Dec 8, 2016
@LukasReschke
Copy link
Member Author

Ok. Good point. Let's take a look at that for 12. Going this approach we should take the less error-prone approach and use the OpenSSL AEAD support in PHP 7.1. So basically a new encryption module that requires at least PHP 7.1. I checked with @oparoz on that and this would be ok for this customer.

@nextcloud-bot nextcloud-bot added the stale Ticket or PR with no recent activity label Jun 20, 2018
@skjnldsv skjnldsv added the 1. to develop Accepted and waiting to be taken care of label Jun 12, 2019
@ghost ghost removed the stale Ticket or PR with no recent activity label Jun 12, 2019
@J0WI
Copy link
Contributor

J0WI commented Mar 26, 2020

See also #20146 (comment)

@yahesh
Copy link
Member

yahesh commented Dec 30, 2022

Getting rid of RC4 isn't dependent on replacing openssl_seal() and openssl_open() nowadays as the functions got an argument to switch the cipher algorithm. However, the functions still don't support AEADs as they don't return the corresponding tags.

With #35916 I introduced wrapped_openssl_seal() and wrapped_openssl_open() which contains a reimplementation of openssl_seal() and openssl_open(). These functions therefore contain everything that's needed to replace the standard functions with something individual which could also properly handle AEAD tags.

@yahesh
Copy link
Member

yahesh commented Mar 2, 2023

Preparations have been done in #36173.

@yahesh
Copy link
Member

yahesh commented Apr 25, 2023

Implemented in #37243.

@yahesh yahesh closed this as completed Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants