Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#32345 closed defect (bug) (fixed)

ssh2/sftp doesn't work in chrooted environments, FTP_BASE, FTP_CONTENT_DIR, FTP_PLUGIN_DIR not available

Reported by: aberbenni's profile aberbenni Owned by: dd32's profile dd32
Milestone: 4.3 Priority: normal
Severity: normal Version: 2.7
Component: Filesystem API Keywords:
Focuses: administration Cc:

Description (last modified by dd32)

The SSH2 extension doesn't work with chrooted environments.
Previously:


FTP_BASE, FTP_CONTENT_DIR, FTP_PLUGIN_DIR are not considered when using built in ssh2/sftp. They are necessary in chrooted environments.

A simple solution could be to modify

find_folder( $folder )

function from

if ( stripos($this->method, 'ftp') !== false ) {

to

if ( stripos($this->method, 'ftp') !== false || stripos($this->method, 'ssh2') !== false ) {

Change History (28)

#1 @dd32
9 years ago

  • Keywords reporter-feedback added

They shouldn't be needed in chroot'd environments, but do exist for times where the FTP code fails to locate the correct directory.

Can you explain a bit more about why you need it with ssh2/sftp / or explain a bit about the server configuration you're using?

#2 @aberbenni
9 years ago

I have sftp/scp access to the main wp folder with a chrooted user. Every time I try to update I receive the error:

Unable to locate WordPress Content directory (wp-content)

Digging into the code I found that wp was looking for /var/www./www.mysite.com/htdocs/wp-content

Adding this configuration should solve the problem:
define('FTP_BASE', '/');
define('FTP_CONTENT_DIR', '/wp-content/');
define('FTP_PLUGIN_DIR', '/wp-content/plugins/');

But this works only for FTP.

The configuration of the server is explained here: https://wiki.archlinux.org/index.php/SFTP_chroot

#3 @bondnono
9 years ago

Same issue here, I did set up a proftpd SFTP server with chrooted users and had to do the same modification as @aberbenni (on the latest Wordpress version). Jailing the users is a basic security feature, I don't get why it is not supported.

#4 @bondnono
9 years ago

Just @aberbenni you forgot to mention the file to edit.
It is wp-admin/includes/class-wp-filesystem-base.php line 178 (using WP 4.2.2)

#5 @dd32
9 years ago

Jailing the users is a basic security feature, I don't get why it is not supported.

Jailing users is supported, and definitely works on FTP, Feels like there's probably a bug or conflict in the SSH2 extension in use. Perhaps try using the PHP implementation - https://wordpress.org/plugins/ssh-sftp-updater-support/ (rather than the PHP SSH extension)

#6 follow-up: @bondnono
9 years ago

Jailing users is supported, and definitely works on FTP

Yes indeed, but who uses FTP anymore?! It is insecure and there are way better alternatives nowaday (you could argue that FTP could be faster, but really, we now have computers fast enough not to even notice the difference).
I haven't tried the plugin, but it is not really the point of this ticket, is it?

From my perspective, it is not a bug nor a conflict, it is more an omission. The issue is that there is a string comparison trying to find "ftp" in the protocol name before considering the FTP_BASE, FTP_CONTENT_DIR and FTP_PLUGIN_DIR constants.

So I think that the solution would be to either rename the ssh2 protocol name to "sftp" or changing the string comparison to what @aberbenni suggested.

#7 in reply to: ↑ 6 @dd32
9 years ago

  • Component changed from Upgrade/Install to Filesystem API

Replying to bondnono:

Jailing users is supported, and definitely works on FTP

Yes indeed, but who uses FTP anymore?! It is insecure and there are way better alternatives nowaday (you could argue that FTP could be faster, but really, we now have computers fast enough not to even notice the difference).

The important part that I missed from my reply, was the underlying transport of FTP or SSH doesn't matter here - the code to find the directories doesn't even know if it's FTP or sFTP, using the Plugin allows us to find what is faulty - a) the code to locate the directories, b) the SSH transport / the SSH extension, orc) something server-side in the ssh server

I haven't tried the plugin, but it is not really the point of this ticket, is it?

It'll rule out my suspected issue of it being caused by a faulty PHP SSH extension, which happens all too often - often enough that I've suggested removing the feature from WordPress all together into a separate plugin - #16925 (although in a perfect world everyone using the ssh extension would switch to the before mentioned plugin instead)

From my perspective, it is not a bug nor a conflict, it is more an omission. The issue is that there is a string comparison trying to find "ftp" in the protocol name before considering the FTP_BASE, FTP_CONTENT_DIR and FTP_PLUGIN_DIR constants.

That is also a legitimate bug, but I was taking it from the aspect of "these constants exist as a all-else-fails scenario, the first step is to determine why they're needed on a platform" - and historically so far no-one using the sFTP(SSH) transport has needed them, as it more-or-less works as a direct filesystem connection. Unlike FTP which is notoriously bad with different servers representing the filesystem in new and amazing ways.

#8 @dd32
9 years ago

  • Description modified (diff)
  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 4.3
  • Summary changed from FTP_BASE, FTP_CONTENT_DIR, FTP_PLUGIN_DIR are not considered when using built in ssh2/sftp to ssh2/sftp doesn't work in chrooted environments, FTP_BASE, FTP_CONTENT_DIR, FTP_PLUGIN_DIR not available
  • Version changed from 4.2.2 to 2.7

Turns out that the SSH2 transport definately does not work on chroot'd SSH configurations at present.

This is because it relies on the ability to run shell commands, however that's not possible in a chrooted/sftp-only environment - This service allows sftp connections only..
The following methods require shell access at present:

  • pwd() - Needed to locate directory
  • chdir() - Not used by the SSH transport (but used by FTP systems)
  • chgrp() - Not used
  • chmod() - Used, but it's okay if it fails, especially on a SSH connection (It's far more expected the files will be created with the correct permissions IMHO)
  • chown() - Not used

Based on that, it looks like we can fix pwd() to use ssh2_sftp_realpath( $this->sftp_link, '.' ); instead of shell_exec( 'pwd' ), which should fix SSH2 w/ chrooted environments.
This should avoid the need to make the FTP_* constants work for SSH2.

Last edited 9 years ago by dd32 (previous) (diff)

#9 @dd32
9 years ago

In 32726:

SSH2 Upgrade transport: Use ssh2_sftp_realpath() instead of shell commands to determine the current directory on the remote server. This should allow it to be used on chrooted SSH sessions.
See #32345

#10 follow-up: @dd32
9 years ago

@bondnono - Can you please try the above patch and see if that fixes the problems you're seeing?

#11 in reply to: ↑ 10 ; follow-up: @bondnono
9 years ago

Replying to dd32:

@bondnono - Can you please try the above patch and see if that fixes the problems you're seeing?

First of all, thanks a lot for the time you are spending trying to help me with this issue.
Unfortunately, your solution does not fix the problem, I still get the "Unable to locate WordPress Content directory (wp-content)." error.

$this->run_command('pwd');                   // returns false
ssh2_sftp_realpath( $this->sftp_link, '.' ); // returns '/'

But it doesn't really surprises me as the whole concept of jailing a user is to make it unaware of the environment it is in. So I don't know how PHP could manage to get that information. Anyway, from the above tests, the ssh2_sftp_realpath does seem to be a better solution than the pwd (at least in my env).

A solution that does work is the one proposed in the initial post (changing the ftp detection in the class-wp-filesystem-base). The main reason for that is that the ABSPATH constant is "wrong" if we are working in a chrooted env, that's why I don't think that there is a simple logic that would make the chrooted and non-chrooted work. Instead, we require the chrooted users to use additional constants to specify the BASE, CONTENT_DIR and PLUGIN_DIR of the user's env and the FTP constant do exactly that.

Thanks again.

#12 in reply to: ↑ 11 @dd32
9 years ago

Replying to bondnono:

Unfortunately, your solution does not fix the problem, I still get the "Unable to locate WordPress Content directory (wp-content)." error.

Can you enable some extra debug for me to get some information about what WordPress is seeing? I'd appreciate it so that hopefully WordPress will just work for the next person who encounters your configuration

  • Can you set WP_Filesystem_Base::$verbose to true in wp-admin/includes/class-wp-filesystem-base.php, and post the extra debug that is then shown when attempting the update?
  • Can you send me the debug data from this plugin? https://dd32.id.au/files/wp-filesystem-tester.zip (dion [at] wordpress.org, or the other email it will give you in the plugin)

But it doesn't really surprises me as the whole concept of jailing a user is to make it unaware of the environment it is in.

That's a problem we face on almost every single WordPress installation which uses FTP, it's a solved problem, that's why I've focused on fixing it instead.
Almost no FTP server is setup so as to present the entire filesystem, just a small slice of it - exactly the same thing that jailing a SSH user achieves, One angle has been fixed, but I guess there's a second aspect to it.

#13 @obenland
9 years ago

  • Owner set to dd32
  • Status changed from new to assigned

#14 @obenland
9 years ago

@dd32, is there more that needs to be done here?

#15 follow-up: @dd32
9 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

Marking as resolved for 4.3.

I acknowledge that @bondnono is still likely affected by a bug here though. @bondnono, if you can provide extra details in this ticket I'll look into fixing that.

After considering it, I don't want to add extra handling to cause the FTP_* constants to work for SSH connections, just as I wouldn't want them to affect the direct methods. Adding the constants has the possibility that people who have used ftp in the past, but switched to SSH will then have incorrect paths returned.

#16 in reply to: ↑ 15 @aberbenni
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to dd32:

Marking as resolved for 4.3.

Please add SSH_* constants if you don not want to add extra handling to cause the FTP_* constants to work for SSH connections.

#17 @dd32
9 years ago

@aberbenni Please add SSH_* constants if you don not want to add extra handling to cause the FTP_* constants to work for SSH connections.

Provide an example of a setup which results in the requirement of it, and it'll be done.

#18 @aberbenni
9 years ago

I think this is the simplest solution for ssh2 chrooted environments. Using a similar approach to ftp is easy and understandable for everybody.
The setup is the same explained in #comment:2.

#19 follow-up: @dd32
9 years ago

The example in comment:2 has been fixed with a chrooted environment in 4.3 with the changes on this ticket. Unfortunately pointing to that doesn't help diagnose anything at all.
If you could (using 4.3) follow the instructions in comment:12 to shed light onto the underlying structure of the setup, that'd be helpful.

#20 @aberbenni
9 years ago

Ok, I will test 4.3.
Thanks.

This ticket was mentioned in Slack in #core by obenland. View the logs.


9 years ago

#22 @obenland
9 years ago

@dd32, can we get this resolved for 4.3 before beta4? Either close as fixed or revert and try again in a future release?

#23 @dd32
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Marking as resolved in 4.3.

If you've got feedback about other bugs mentioned in this ticket, you can still comment and we'll create new tickets as need be.

#24 in reply to: ↑ 19 @aberbenni
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to dd32:

The example in comment:2 has been fixed with a chrooted environment in 4.3 with the changes on this ticket. Unfortunately pointing to that doesn't help diagnose anything at all.
If you could (using 4.3) follow the instructions in comment:12 to shed light onto the underlying structure of the setup, that'd be helpful.

Tested on 4.3 beta 3 and nothing changed.

Here is extra debug from WP_Filesystem_Base::$verbose

Looking for /var/www/www.mysite.com/htdocs/wp-content in /
Looking for /var/www/www.mysite.com/htdocs/wp-content/languages in /

and output from wp-filesystem-tester

Connection Method	ssh2

ABSPATH	/var/www/www.mysite.com/htdocs/

PLUGINDIR	wp-content/plugins

FS Errors	None

FS CWD	/

FS WordPress Locator	Looking for /var/www/www.mysite.com/htdocs in /

FS WordPress Location	/

FS0 WordPress Locator (Old code)	Changing to /
Found /wp-settings.php

FS0 WordPress Location (Old code)	/

Plugin location:	/var/www/www.mysite.com/htdocs/wp-content/plugins/wp-filesystem-tester.php (Local)
/wp-content/plugins/wp-filesystem-tester.php (FTP)

Plugin Locations	
hello.php	/wp-content/plugins/	Delete file: /wp-content/plugins/hello.php
akismet/akismet.php	/wp-content/plugins/akismet/	Delete entire folder: /wp-content/plugins/akismet/

File IO Errors	Error: Could not create file /wp-content/plugins/super-long-name-not-to-conflict.php on server

Downloading a zip	Downloading http://downloads.wordpress.org/plugin/akismet.zip... Suceeded

Extracting Zip	Suceeded

Zip Contents	
akismet/	0	ok	binary
akismet/wrapper.php	6427	ok	<?php

global $wpcom_api_key, $akismet_api_host, $...
akismet/index.php	26	ok	<?php
# Silence is golden....
akismet/class.akismet-widget.php	2719	ok	<?php
/**
 * @package Akismet
 */
class Akismet_Wi...
akismet/readme.txt	11447	ok	=== Akismet ===
Contributors: matt, ryan, andy, md...
akismet/.htaccess	616	ok	binary
akismet/_inc/	0	ok	binary
akismet/_inc/akismet.css	6003	ok	binary
akismet/_inc/img/	0	ok	binary
akismet/_inc/img/logo-full-2x.png	4970	ok	binary
akismet/_inc/akismet.js	5402	ok	binary
akismet/_inc/form.js	700	ok	binary
akismet/class.akismet-admin.php	39105	ok	<?php

class Akismet_Admin {
	const NONCE = 'akism...
akismet/tmp-akismet.pot	18656	ok	binary
akismet/akismet.php	2421	ok	<?php
/**
 * @package Akismet
 */
/*
Plugin Name: ...
akismet/views/	0	ok	binary
akismet/views/config.php	10537	ok	<div class="wrap">

	<h2><?php esc_html_e( 'Akisme...
akismet/views/start.php	6538	ok	<div class="no-key config-wrap"><?php
	if ( $akism...
akismet/views/stats.php	551	ok	<div class="wrap">
	<h2><?php esc_html_e( 'Akismet...
akismet/views/strict.php	830	ok	<tr valign="top">
	<th scope="row"><?php esc_html_...
akismet/views/notice.php	10631	ok	<?php if ( $type == 'plugin' ) :?>
<div class="upd...
akismet/views/get.php	505	ok	<form name="akismet_activate" action="https://akis...
akismet/class.akismet.php	42614	ok	<?php

class Akismet {
	const API_HOST = 'rest.aki...

#25 follow-up: @dd32
9 years ago

@aberbenni - In the debug you've provided, WordPress can now successfully locate the WordPress files via ssh2, however, it's failing to create new files.

Is the error you're seeing in the UI the same? (Unable to locate WordPress Content directory (wp-content))

#26 in reply to: ↑ 25 @aberbenni
9 years ago

Replying to dd32:

@aberbenni - In the debug you've provided, WordPress can now successfully locate the WordPress files via ssh2, however, it's failing to create new files.

Is the error you're seeing in the UI the same? (Unable to locate WordPress Content directory (wp-content))

Yes, the error during upgrade is the same.

And trying to delete a plugin I receive Unable to locate WordPress Plugin directory error.

This ticket was mentioned in Slack in #core by obenland. View the logs.


9 years ago

#28 @obenland
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Closing based on conversations linked to above.

Note: See TracTickets for help on using tickets.