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

Deprecate & Remove authorize.php entirely #3208

Open
quicksketch opened this issue Jul 22, 2018 · 15 comments
Open

Deprecate & Remove authorize.php entirely #3208

quicksketch opened this issue Jul 22, 2018 · 15 comments

Comments

@quicksketch
Copy link
Member

quicksketch commented Jul 22, 2018

Describe your issue or idea

The Purpose of authorize.php

The authorize.php file is intended to make it possible for Backdrop files to be owned by one user and the web server to run as another user. It is expected then that the web server user cannot write files to disk (other than the "files" directory). authorize.php then allows a user to enter in their FTP credentials so when a module is updated, Backdrop temporarily moves the files as if it were that FTP user, leaving the files on disk unwritable by the web server.

Implementation Problems

However, in Backdrop we do NOT use authorize.php when installing a module via the project browser (Installer module). We have an issue to fix that: #2271 (though if we remove authorize.php, we can close that issue).

A bigger problem with authorize.php is that it is fundamentally incompatible with automatic updates. As automatic updates do not require user-intervention, any attempt to update the site automatically would be blocked by the requirement that they enter in their FTP credentials.

Suggested Solution

We remove authorize.php entirely, and require if you want to install or update modules/themes/layouts via the UI, you MUST make the web server directory writable by the web server user. Installer module should check that the file system is writable at all times, and if it is not, throw an error in the status report. Sites that do not want a writable file system can disable Installer module, which in turn will turn off the Project browser (which doesn't work without a writable file system anyway) and ultimately automatic-updates once that is built into Installer as well.


PR by @quicksketch: backdrop/backdrop#2305

@docwilmot
Copy link
Contributor

This is not within my expertise sorry.

@serundeputy
Copy link
Member

I am for the removal of authorize.php.

@jlfranklin
Copy link
Member

jlfranklin commented Jul 22, 2018

I am in favor of removing authorize.php, but we should bring the issue up with Pantheon and some of the larger shared hosting environments for their opinion.

For shared hosting providers, the auto-update features and the hosting rules required to support it open a lot of risk to them and their customers.

A shared hosting environment may set a rule: A directory may either (a) be writable by a PHP process OR (b) include executable PHP files, but not both. This rule prevents a large class of hacks against PHP sites where the site code is tricked into writing malicious scripts in executable directories. The files directory itself exists to provide a dedicated writable directory in compliance with such rules.

The authorize.php method provides a compromise where a user can perform updates to his site via the web UI, but it requires additional credentials and uses the normal upload method (which is logged) to do so. This preserves the security model and provides the remote upgrade path.

The Backdrop project installer and auto-update features, while ensuring patched code is deployed in a timely manner even for abandoned sites, requires opening up a significant potential attack vector.

@opi
Copy link

opi commented Jul 22, 2018

Never used the authorize.php file in 8 years of Drupal, and didn't heard anybody ever use it.

@klonos
Copy link
Member

klonos commented Jul 23, 2018

I am for removing it, but as @jlfranklin said, I would hate to see that effectively make the usage of the project installer impossible in major hosting provider platforms. If we can ensure that the removal does not effect that (which is on of our bigger selling points), then lets go for it.

@serundeputy
Copy link
Member

I would hate to see that effectively make the usage of the project installer impossible in major hosting provider platforms.

project installer does not use authorize.php now. see OP, i.e project browser already has the requirement that to work the webserver user has to have write access to the files in question.

@quicksketch
Copy link
Member Author

Almost all shared hosting providers I have used run the web server and the FTP access as the same user. It's been a while, but I believe this was the case for GoDaddy, Bluehost, A2 Hosting, and Namecheap.

Speculation on why this is: Shared hosting typically has multiple users on the same machine, with each user being jailed to their home directory. There isn't a single "web user" because that user would potentially be able to access the sites of other users on the same machine, so both to restrict users to their own directories and reduce permissions problems, shared hosts just make the web user and the FTP user the same thing.

Pantheon is interesting in that it doesn't allow any modification of any files unless you put the site into "FTP mode". While in FTP module, the web server and the FTP user are the same. This allows things like Project Installer to work while on Pantheon. And in fact it makes it so authorize.php is never used, because either you have FTP mode on and the users are the same, or FTP mode is off and you can't enter in any credentials that would work because all file writing is locked. They only allow FTP mode on the dev environment in a dev/stage/production workflow. Which means on Pantheon, auto-updating would only work on the dev environment, but then you could manually promote it to stage/live after the update takes place. Though on Pantheon you'd probably also use the Backdrop upstream instead of the auto-updating.

@herbdool
Copy link

I'm for getting rid of the file

@stpaultim
Copy link
Member

Just listening to discussion about authorize.php at the meeting today. Just for the record, I've never used it on Drupal or Backdrop. Not sure about any other implications, but I'm pretty sure that it does not serve a majority of users.

@klonos
Copy link
Member

klonos commented Aug 3, 2018

I have occasionally seen the "enter FTP credentials" dialog come up in D7 instances in simplytest.me (when trying to install additional modules after the instance was spun up), but that's all.

@laryn
Copy link
Contributor

laryn commented Aug 7, 2018

@quicksketch Just a note that I updated a module via the UI just now and found myself on core/authorize.php (page view screenshot below):

screen shot 2018-08-07 at 10 52 11 am

@klonos
Copy link
Member

klonos commented Aug 7, 2018

As I have also mentioned in #3245, the "Your projects have been downloaded and updated." task is not a task, and all this can be skipped, redirecting to /core/update.php I believe.

@jenlampton jenlampton changed the title Remove authorize.php entirely Deprecate & Remove authorize.php entirely Aug 30, 2018
@quicksketch
Copy link
Member Author

I filed an initial PR for deprecating authorize.php and avoid all core use of it in backdrop/backdrop#2305. Tests are probably not passing and we need to provide more explicit instructions for setting permissions properly within Backdrop. @serundeputy and I have been working on documentation at https://backdropcms.org/file-permissions-and-ownership (currently unpublished).

The PR helps bring some consistency to update vs. install processes, though they are still pretty different from each other.

@herbdool
Copy link

Looks like all the tests passed

@quicksketch
Copy link
Member Author

Yeah, good news seems like all the previous functionality continues passing tests.

Looking deeper into authorize.php, it seems that the entire FileTransfer collection of classes and hook_filetransfer_info() are also all exclusively used by authorize.php. Seems like we could also remove that entire system and replace with simpler direct filesystem calls.

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

9 participants