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

[4.0] Remove the FTP Layer #21507

Closed
wants to merge 22 commits into from
Closed

Conversation

roland-d
Copy link
Contributor

@roland-d roland-d commented Aug 9, 2018

Summary of Changes

It is time to bring Joomla into the modern era, FTP layers should not be needed with modern webhosting. This PR removes the FTP layer from:

  • Global Configuration
  • Joomla Updater
  • Installer

Testing Instructions

Global Configuration

  1. Go to System -> Global Configuration
  2. Click on the Server Tab
  3. Check that the FTP options above the Proxy Settings is gone

Joomla Updater

  1. Go to Components -> Joomla! Updater
  2. Click on Upload & Update
  3. The option Installation method should be gone

Installer

  1. Go to Components -> Install
  2. Check that the FTP Login Details tab is gone
  3. Go to Update
  4. Check that the FTP Login Details block is gone
  5. Go to Manage
  6. Check that the FTP Login Details block is gone
  7. Go to Discover
  8. Check that the FTP Login Details block is gone

Installation testing

Joomla Updater

This is tricky because there needs to be a Joomla Update or you need to build your own update server.

  1. Go to Components -> Joomla! Updater

  2. Check for an update

  3. Install the available update

  4. See that the installation completes

  5. Go to Components -> Joomla! Updater

  6. Click on the Upload & Update tab

  7. Select a Joomla installation package

  8. Click on Upload & Install

  9. See that the installation completes

Installer

  1. Go to Components -> Install

  2. Select an extension to Install

  3. See that the installation completes

  4. Go to Update

  5. Check for Updates

  6. Select an extension to update

  7. Check that the installation completes

  8. Go to Manage

  9. Select an extension to uninstall

  10. Check that the installation completes

  11. Go to Discover

  12. Click on Discover to find an extension to install

  13. Select an extension to install

  14. Check that the installation completes

Expected result

There are no more FTP settings available

Documentation Changes Required

https://docs.joomla.org/Why_can%27t_you_install_any_extensions%3F
https://docs.joomla.org/How_to_use_the_filesystem_package
https://docs.joomla.org/Joomla_Core_Features
https://docs.joomla.org/J3.x:Joomla_Core_Features

Signed-off-by: Roland Dalmulder <contact@rolandd.com>
@roland-d roland-d added this to the Joomla 4.0 milestone Aug 9, 2018
@roland-d roland-d self-assigned this Aug 9, 2018
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Aug 9, 2018
@dgrammatiko
Copy link
Contributor

@roland-d you know you're gonna make few people unhappy, right?

@@ -747,16 +747,9 @@ public static function getStream($use_prefix = true, $use_network = true, $ua =

if ($use_prefix)
{
$FTPOptions = \JClientHelper::getCredentials('ftp');
Copy link
Contributor

Choose a reason for hiding this comment

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

This bit actually isn't bad to retain support for as it allows FTP streams, which is a separate thing from the file/folder classes having the FTP layer/fallback. Everything else being gone seems fair to me.

@brianteeman
Copy link
Contributor

Thanks for this.

I think there may be some extra steps to remove for the installation template but it is hard to check on my phone. I can check in the morning

@richard67
Copy link
Member

The testing instructions only describe to check if options have gone. They don’t tell to check that things work as before. But this has to be tested, especially by people with shared hosting.

@roland-d
Copy link
Contributor Author

@richard67 You are correct, I am going to update. I realized the same thing when I went to sleep :P

@HLeithner
Copy link
Member

You know that you remove an important security feature?

@roland-d
Copy link
Contributor Author

@HLeithner Care to explain?

@HLeithner
Copy link
Member

Sure,

normally you don't want that your software can manipulate it self. Yes we want it in some situations, f. ex. updates. The reset of the time we maybe want to write to some directories (image uploads, temp dir, cache dir, log dir). All of them doesn't have to execute code.

So if you do you server configuration right you don't want/have write access by CMS to any directory on the server, best case is it doesn't have write access at all.

In combination with restricting access to PHP files in write able directories makes it a bit hard to exploit a file upload exploit.
Something like this:

<Directory images>
     <FilesMatch "(?i)\.(php|phtml)$">
            Order Deny,Allow
            Deny from All
    </FilesMatch>
</Directory>

@@ -59,11 +58,8 @@ function admin_postinstall_eaccelerator_action()
// Set the configuration file path.
$file = JPATH_CONFIGURATION . '/configuration.php';

// Get the new FTP credentials.
$ftp = ClientHelper::getCredentials('ftp', true);

// Attempt to make the file writeable if using FTP.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment needs updating

@@ -79,7 +75,7 @@ function admin_postinstall_eaccelerator_action()
}

// Attempt to make the file unwriteable if using FTP.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment needs updatinng

@brianteeman
Copy link
Contributor

@nikosdion as this touches com_joomlaupdate can you check the changes please

@brianteeman
Copy link
Contributor

(note for testers - you can not use com_patchtester to test the changes in the /installation files)

@brianteeman
Copy link
Contributor

doesnt this section need updating

@brianteeman
Copy link
Contributor

@roland-d there is still a lot of stuff to remove from the installer or are you intending that Joomla could still be installed with ftp

@nikosdion
Copy link
Contributor

I disagree 100% with this PR. While philosophically I agree and in theory the idea behind it is solid, it is in disconnect with real world usage of Joomla (sadly) and what we've done to help users understand the security implications.

What we see is plenty of clients stuck on hosting which uses mod_php on Apache, itself running as www-data:www-data. This is still the default configuration of Ubuntu even in 18.04 LTS. As a result the use of the FTP layer is a one-way street for these users.

Of course it has very serious security implications. However, this is not a problem you can solve with technology alone, definitely not by simply removing this feature. It's primarily an education problem, first and foremost with (people who think they are) web hosts. They need to learn to use FastCGI or FPM. Then it's a matter of educating users to avoid hosts where the FTP layer is necessary.

I don't see us solving anything by removing the FTP layer from Joomla!. The people who need it will either stick with an old version of Joomla! or they will choose a different CMS which works on their host. Or both. They do not understand the security implications and we have done a really bad job educating them on that the last 15 years. If there is something we need to change it's educating users. Look at the results after we started warning users about old PHP versions: they are migrating to newer versions of PHP. Users are not suicidal idiots, they just don't know better unless those in the know (us) educate them.

What I would very much prefer is an approach where we educate users and ease them into removing this feature in the future.

For versions 3.x I would recommend adding a post-installation message if the FTP layer is installed informing the users about the dangers of the FTP layer.

In version 4.x we could double down on that, printing a BIG FAT RED WARNING below the header of the backend page with the same information.
people
The copy could read something along those lines:

WARNING! YOU ARE USING JOOMLA'S FTP LAYER
By necessity, this feature stores your FTP credentials in your site's configuration. This could have serious security implications. The FTP layer feature will be removed from Joomla! in version 5.0.
Try disabling this feature. If your site does not work properly after that please contact your host and ask them to use FastCGI or PHP-FPM to run PHP on their server instead of mod_php. This is a faster and more secure alternative.

I would even put the FTP layer controls in an area with a red background and a big warning above it "THESE OPTIONS MAY HAVE AN ADVERSE EFFECT ON YOUR SITE'S SECURITY".

Then in version 5.0 we can remove the FTP layer completely. Of course you could accelerate the timeline and remove the FTP layer in version 4.1. That's a decision that needs to be taken by the production department, not us random contributors on the issue tracker.

It goes without saying that the documentation would need to be updated to dissuade users from enabling the FTP layer at all.

@brianteeman
Copy link
Contributor

@nikosdion do you have any stats with akeeba/admintools usage with ftp?
The "education" process you suggest can definitely be done as well

@nikosdion
Copy link
Contributor

Since the use of FTP layer is in the ANGIE application (restoration script) we do not collect that information for privacy reasons. We know empirically that the stream of FTP-related issues hasn't decreased significantly in the last 5 years. I try to educate my users but I'm basically facing the argument "Joomla! comes with an FTP layer and they don't say it's insecure so you are obviously a lazy developer who doesn't want to support it".

@brianteeman
Copy link
Contributor

Well it was worth asking ;)

I guess security issues would be reduced if the user can not save their ftp username/password. Currently if they choose not to then they will be asked for it each time. So perhaps we should just remove the ability to save them and then each time they will need to enter them AND we can display an "education" message at the same time

@nikosdion
Copy link
Contributor

That would be the ideal way to handle this. I'd even go one step further to show a warning before people fill in their credentials if their site is not HTTPS. Credentials sent over plain HTTP can be trivially intercepted and are a major security risk. Credentials sent over HTTPS are reasonably secure. I mean that they are only stored in the session which temporarily places them in the database or the server memory (depending on the storage backend) and wiped after the operation completes.

@810
Copy link
Contributor

810 commented Aug 10, 2018

We can also move the warning to 3.9 and 3.10

If you want to use ftp layer, you can stay on that release scheme

@rdeutz
Copy link
Contributor

rdeutz commented Aug 10, 2018

I like the eduaction path, doesn't matter if we've it 5 year longer in our repo

@Bakual Bakual removed their request for review August 10, 2018 21:24
Signed-off-by: Roland Dalmulder <contact@rolandd.com>
# Conflicts:
#	administrator/components/com_admin/postinstall/eaccelerator.php
#	administrator/components/com_config/Controller/ApplicationController.php
#	administrator/components/com_config/Controller/ComponentController.php
#	administrator/components/com_config/tmpl/application/default_ftp.php
#	administrator/components/com_installer/Controller/DisplayController.php
#	administrator/components/com_installer/Model/InstallModel.php
#	administrator/components/com_joomlaupdate/Controller/DisplayController.php
#	administrator/components/com_joomlaupdate/Controller/UpdateController.php
#	administrator/components/com_joomlaupdate/Helper/Select.php
#	administrator/components/com_joomlaupdate/Model/UpdateModel.php
#	administrator/components/com_joomlaupdate/View/Joomlaupdate/HtmlView.php
#	administrator/components/com_users/Controller/ProfileController.php
#	components/com_config/Controller/ConfigController.php
#	components/com_config/Controller/TemplatesController.php
#	components/com_users/Controller/ProfileController.php
#	libraries/src/Filesystem/File.php
# Conflicts:
#	administrator/components/com_admin/postinstall/eaccelerator.php
#	administrator/components/com_config/Controller/ApplicationController.php
#	administrator/components/com_config/Controller/ComponentController.php
#	administrator/components/com_config/tmpl/application/default_ftp.php
#	administrator/components/com_installer/Controller/DisplayController.php
#	administrator/components/com_installer/Model/InstallModel.php
#	administrator/components/com_joomlaupdate/Controller/DisplayController.php
#	administrator/components/com_joomlaupdate/Controller/UpdateController.php
#	administrator/components/com_joomlaupdate/Helper/Select.php
#	administrator/components/com_joomlaupdate/Model/UpdateModel.php
#	administrator/components/com_joomlaupdate/View/Joomlaupdate/HtmlView.php
#	administrator/components/com_users/Controller/ProfileController.php
#	components/com_config/Controller/ConfigController.php
#	components/com_config/Controller/TemplatesController.php
#	components/com_users/Controller/ProfileController.php
#	libraries/src/Filesystem/File.php
…p-layer

# Conflicts:
#	administrator/components/com_config/tmpl/application/default_ftp.php
# Conflicts:
#	administrator/components/com_installer/Controller/DisplayController.php
#	administrator/components/com_joomlaupdate/Controller/DisplayController.php
# Conflicts:
#	administrator/components/com_installer/Controller/DisplayController.php
#	administrator/components/com_joomlaupdate/Controller/DisplayController.php
#	installation/src/Model/ConfigurationModel.php
#	libraries/src/Filesystem/Folder.php
# Conflicts:
#	installation/configuration.php-dist
# Conflicts:
#	installation/configuration.php-dist
#	installation/src/Model/ConfigurationModel.php
#	libraries/src/Filesystem/Folder.php
…p-layer

# Conflicts:
#	installation/configuration.php-dist
# Conflicts:
#	language/en-GB/en-GB.com_media.ini
@wilsonge
Copy link
Contributor

wilsonge commented Mar 3, 2019

I'm willing to work on an education thing for looking to remove this in J5 or something if we can show clearly the use case is going down. But right now with all the objections raised I don't see the value in removing this given it give or take works

@wilsonge wilsonge closed this Mar 3, 2019
@roland-d roland-d deleted the remove-ftp-layer branch March 3, 2019 14:38
@roland-d
Copy link
Contributor Author

roland-d commented Mar 3, 2019

Thanks, a decision is better than no decision.

@dgrammatiko dgrammatiko mentioned this pull request Apr 9, 2021
@brianteeman
Copy link
Contributor

I wonder if the people who had string objections to the removal of the ftp layer (which doesnt work anyway) in August 2018 still have those strong objections three years later

@richard67
Copy link
Member

According to my experience shared hosting has improved meanwhile, so for up to date hosting it should not be necessary anymore. But I have no idea if there are still cases where it could be necessary.
So I can't really say I have a strong opinion in the one or the other direction.

@dgrammatiko
Copy link
Contributor

But I have no idea if there are still cases where it could be necessary.

These won't work for other reasons: outdated PHP, Mysql, etc. Honestly, FTP was a thing 15 years ago not anymore...
That said I remember back in 2015-16 we were talking that J4 had to support at least IE8. The conclusion here is that there are many devs that are totally disconnected from the real WEB world...

@brianteeman
Copy link
Contributor

The conclusion here is that there are many devs that are totally disconnected from the real WEB world...

That's a bit harsh holding them to a comment they made three years ago.

FWIW I couldnt get joomla 3.9.26 to work with J3

@richard67
Copy link
Member

FWIW I couldnt get joomla 3.9.26 to work with J3

"... work with J3"? Or "... work with FTP"?

@brianteeman
Copy link
Contributor

oops
work with ftp #33079

@richard67
Copy link
Member

Unfortunately our statistics don't tell us if FTP is used, do they?

I wish we had a kind of global poll where we just could outreach people and ask them.

@brianteeman
Copy link
Contributor

No they dont. But then @PhilETaylor did volunteer that not one of 67,000 sites he provides services to use it

@roland-d
Copy link
Contributor Author

Even browsers are now removing FTP support.

@PhilETaylor

This comment was marked as abuse.

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

Successfully merging this pull request may close these issues.

None yet