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

php/mcrypt has been deprecated #8265

Closed
LorbusChris opened this issue Feb 8, 2018 · 12 comments
Closed

php/mcrypt has been deprecated #8265

LorbusChris opened this issue Feb 8, 2018 · 12 comments
Labels
1. to develop Accepted and waiting to be taken care of technical debt

Comments

@LorbusChris
Copy link
Contributor

LorbusChris commented Feb 8, 2018

Mcrypt is listed as a Recommended lib in the installation manual. https://docs.nextcloud.com/server/13/admin_manual/installation/source_installation.html

It has been deprecated in PHP upstream and should be removed.
http://us1.php.net/manual/en/book.mcrypt.php
https://wiki.php.net/rfc/mcrypt-viking-funeral

@nickvergessen
Copy link
Member

mcrypt is only used by php-seclib which we use:
phpseclib/phpseclib#1040

They seem to be aware of the issue and will mostlikely fix it (instead of surpressing the deprecation info) in a future version.

@terrafrost
Copy link

terrafrost commented Mar 10, 2018

@nickvergessen - This is not an issue with phpseclib.

Quoting phpseclib/phpseclib#1134,

phpseclib has used the error suppression operator on all mcrypt function calls since 1.0.4 / 2.0.4:

phpseclib/phpseclib@640f106

Those versions were released on October 3, 2016. My recommendation: upgrade to the latest version of phpseclib from either the 1.0 or 2.0 branches.

Removing mcrypt is not going to happen because, deprecated tho it may be, it's still a heck of a lot faster than the pure-PHP implementation. The fact that it's being used on your system suggests that OpenSSL isn't available so mcrypt is gonna be the fastest way to do encryption in PHP.

phpseclib (all branches) are unit tested on PHP 7.2:

https://travis-ci.org/phpseclib/phpseclib

They all pass in spite of using mcrypt. idk if you've ever used Travis CI / phpunit but an E_DEPRECATED notice will result in a failing unit test and yet the unit tests are all passing.

@Skywalker-11
Copy link

The code comments says that they use a custom mcrypt implementations if the the php module is not available which would explain why it still works with PHP 7.2:
https://github.com/phpseclib/phpseclib/search?p=1&q=mcrypt

@glaringgibbon
Copy link

glaringgibbon commented Oct 10, 2018

Hi,

I raised issue iusrepo/wishlist#209 with IUS that Carl just closed off above. He suggested I subscribe to this thread and I'm hoping you can be as helpful as he was.

I'm strictly an enthusiastic amateur so please bear with me. Apologies in advance for any n00bish behaviour, comments, practices, etc.

I am trying to get to the bottom of the mcrypt/sodium situation, what the implications are, where is it heading etc. This is for a VPS install on public internet so I'm sure you appreciate my concern.

Can someone explain in layman's terms what the implications are of running PHP7.2 on NC14 instance?

Is a temporary 7.1 or 7.0 solution preferable?

I understand mcrypt will be replaced by sodium. Is there an ETA?

Is there anything else you can point me to that would help?

I opened a thread on the NC installation forum about the route I'm trying to take which gives some background which may/may not help. I'm trying to get a safe, secure, up to date NC14 running that won't require a huge overhaul in a few months time.

Thanks in advance for your help.

@nextcloud-bot nextcloud-bot removed the stale Ticket or PR with no recent activity label Oct 10, 2018
@mlduber
Copy link

mlduber commented Oct 12, 2018

Documentation is misleading but the point is, official Nextcloud 14 (Docker image) + official SAML app just don't work out of the box.

@kesselb
Copy link
Contributor

kesselb commented Oct 12, 2018

@mlduber could you report this at https://github.com/nextcloud/docker?

@glaringgibbon
Copy link

glaringgibbon commented Oct 17, 2018

Hi

Am I in any danger of a response in the foreseeable future?

I was asked to post here from the NC forum as this is the place to ask apparently. Of the few responses I got there one starts out with this comment

Feeling a wall of silence from NC?

7 days after posting here I am beginning to see what they were talking about.

This is not a good look.

In the absence of any info from you guys I have been reading up on the subject and have come to the conclusion that there appears to be a PHP timebomb under NC. Ignoring the issue won't make it go away.

@mlduber
Copy link

mlduber commented Oct 17, 2018

Hi

Am I in any danger of a response in the foreseeable future?

Hi, I understand things are being done on the mcrypt front, according to this pull request. Not sure it answers your questions about sodium, ffmpeg and all, though.

@kesselb
Copy link
Contributor

kesselb commented Oct 17, 2018

I guess there is not ETA for this issue. On my personal nextcloud instance i use php7.2 (with nextcloud 13) without mcrypt.

@terrafrost explained here phpseclib/phpseclib#1134 (comment) that mcrypt is not required when openssl is available as php module. When mcrypt is not available a slower plain php implementation is used in some special cases.

@nickvergessen
Copy link
Member

Yeah as daniel already said, we are working on it as much as we can.
We removed all mcrypt dependencies from our source files. A last lib was removed in #11769

The only thing remaining in core is phpseclib, which as per above, also works without it, so I guess it should be totally fine?

mihailstoynov added a commit to mihailstoynov/install_nextcloud that referenced this issue Nov 25, 2018
php/mcrypt deprecated in 7.1, removed in 7.2
nextcloud should work fine without it:
nextcloud/server#8265
mihailstoynov added a commit to mihailstoynov/install_nextcloud that referenced this issue Nov 25, 2018
php/mcrypt deprecated in 7.1, removed in 7.2
nextcloud should work fine without it:
nextcloud/server#8265
mihailstoynov pushed a commit to mihailstoynov/install_nextcloud that referenced this issue Nov 25, 2018
    php/mcrypt deprecated in 7.1, removed in 7.2
    nextcloud should work fine without it:
    nextcloud/server#8265
mihailstoynov pushed a commit to mihailstoynov/install_nextcloud that referenced this issue Nov 25, 2018
php/mcrypt deprecated in 7.1, removed in 7.2
nextcloud should work fine without it:
nextcloud/server#8265
@skjnldsv skjnldsv added the 1. to develop Accepted and waiting to be taken care of label Jun 17, 2019
@k00ni
Copy link
Contributor

k00ni commented Oct 20, 2020

Hi, can anyone give me a short status update please?

@kesselb
Copy link
Contributor

kesselb commented Oct 20, 2020

As per https://docs.nextcloud.com/server/latest/admin_manual/installation/source_installation.html mcrypt is no longer a mandatory dependency.

@kesselb kesselb closed this as completed Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of technical debt
Projects
None yet
Development

No branches or pull requests

10 participants