WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#7690 closed enhancement (fixed)

Using SSH2 to upload files/wordpress

Reported by: ShaneF Owned by:
Milestone: 2.7 Priority: normal
Severity: normal Version: 2.7
Component: Administration Keywords: ssh upgrade dev-testing has-patch
Focuses: Cc:

Description

This will you to choose SSH2 as your transferring scheme instead of using FTP. At least for me, I disable FTP on our server and only use SSH2 transfer.

  • New Class File for SSH2 transferring protocols
  • Won't show options for SSH2 if the user does not have the libssh2.so module loaded with php5+

Attachments (20)

wordpress_ssh2_transfer_2.7.x.patch (10.5 KB) - added by ShaneF 7 years ago.
this is the patch file for the current files.
class-wp-filesystem-ssh2.php (12.3 KB) - added by ShaneF 7 years ago.
This is the SSH2 Transfer Class -- UNTESTED -- Needs testing on Unix/Linux boxes with libssh2.so enabled and working
7690.diff (1.5 KB) - added by ryan 7 years ago.
7690.2.diff (5.3 KB) - added by ryan 7 years ago.
connection type radios
7690.3.diff (2.2 KB) - added by ShaneF 7 years ago.
small update
7690.4.diff (2.2 KB) - added by ShaneF 7 years ago.
added instrcutions to the class file for installation
7690.5.diff (8.0 KB) - added by DD32 7 years ago.
See Note in comments
7690-ports.diff (856 bytes) - added by technosailor 7 years ago.
Allows FTP_PORT to be defined and adds UI for port definition in update screen
7690.6.diff (15.8 KB) - added by ShaneF 7 years ago.
see note above
7690.7.diff (19.9 KB) - added by DD32 7 years ago.
7690.8.diff (21.9 KB) - added by ShaneF 7 years ago.
7690.9.diff (2.0 KB) - added by DD32 7 years ago.
7690.10.diff (10.9 KB) - added by ShaneF 7 years ago.
allows you to set SSH public/private keys for auth method for ssh mode. includes 7690.9. patch from DD32
7690.11.diff (595 bytes) - added by DD32 7 years ago.
fixes incorrect if nesting
7690.12.diff (5.5 KB) - added by ShaneF 7 years ago.
7690.13.diff (3.4 KB) - added by DD32 7 years ago.
7690.14.diff (4.7 KB) - added by DD32 7 years ago.
as as #13, but uses API and fixes directory creation
7690.15.diff (8.1 KB) - added by DD32 7 years ago.
7690.16.diff (9.2 KB) - added by DD32 7 years ago.
mainly some JS improvements over the last patch.
7690.17.diff (1.2 KB) - added by ShaneF 7 years ago.
minor html fix

Download all attachments as: .zip

Change History (63)

comment:1 @ShaneF7 years ago

I am trying to shoot for a 2.7 release. Might have to bump this to 2.7.1

comment:2 @ryan7 years ago

This would be sweet for 2.7. We have over a month until feature freeze.

@ShaneF7 years ago

this is the patch file for the current files.

@ShaneF7 years ago

This is the SSH2 Transfer Class -- UNTESTED -- Needs testing on Unix/Linux boxes with libssh2.so enabled and working

comment:3 @ShaneF7 years ago

  • Keywords dev-testing added

I have only tested that it sets the SSH2 class, but have been unable to test the class itself as I can not get libssh2.so working.

comment:4 @ShaneF7 years ago

Oh. I used the FTP class as a base, but had to change some thing since some functions are not functions.

comment:5 @technosailor7 years ago

Will happily test. Is the second patch the one needed or are both needed.

comment:6 @ryan7 years ago

You don't need separate SSH2 stuff, just use FTP. Add an FTP_SFTP option that can be checked in the UI and you don't need anything else, it seems.

comment:7 follow-up: @ShaneF7 years ago

Ah. Yeah. Good point. i'll make those changes in the patch file.

@technosailor: Right now you need to put the class file in the wp-admin/includes/ directory.

The patch file you just well.... apply, but I gotta make a few changes.

@ryan: Are we talking about the base (WP_Filesystem_Base) class?

@ryan7 years ago

comment:8 @ryan7 years ago

Something like that. The sftp and ssl checkboxes could be joined into a "Connection Type" radio button.

comment:9 in reply to: ↑ 7 @ryan7 years ago

Replying to ShaneF:

@ryan: Are we talking about the base (WP_Filesystem_Base) class?

All of wordpress_ssh2_transfer_2.7.x.patch seems unnecessary. Use FTP nomenclature everywhere with an option for the connection type.

comment:10 @DD327 years ago

  • Version set to 2.7

This sounds good to me, I've recently moved to a VPS which doesnt use suExec, and doesnt have FTP available, I was going to look into some form of SSH based connection - i wasnt aware there was a php ssh module though.

I'll join in with some testing this weekend.

Changing the FTP Credentials page to select between them if Direct filesystem access isnt available would be my suggestion, Something like this at the top of the page perhaps?:

 o FTP  o FTPS  o SSH  o ..anyother methods..
 Hostname: ..
 Username: ..
 Password: ..

FTPS would be the Secure form of FTP, and only available transports would be shown(which might require a few changes to other parts of the current code)

comment:11 follow-up: @technosailor7 years ago

  • Cc aaron@… added

Will wait on unified patch, but looking forward to running this.

@ryan7 years ago

connection type radios

comment:12 @ryan7 years ago

There's a start on connection type selection. Barely tested.

comment:13 @ryan7 years ago

Well, this slipped in with [8811]. I shouldn't commit at night. Let's just leave it in and start fixing.

comment:14 @ryan7 years ago

(In [8812]) SSH2 filesystem. Props ShaneF. see #7690

comment:15 in reply to: ↑ 11 @ShaneF7 years ago

Yeah. Here is a unified diff. The only error I am getting is two forms for username and password. No idea how to fix it and I can't test my own class until I get libssh2.so working correctly.

@ShaneF7 years ago

small update

@ShaneF7 years ago

added instrcutions to the class file for installation

@DD327 years ago

See Note in comments

comment:16 @DD327 years ago

attachment 7690.5.diff added.
See Note in comments

& of course HTML Preview would die :)

Notes:

  • Doesnt use ssh2_sftp_* functions, uses ssh2_exec() instead, Can probably be changed over at some point.
    • shouldnt be too hard.. just add $this->sftp_link = ssh2_sftp($this->link) and pass that around instead of $this->link in the affected functions.
  • includes changes from ShaneF in 7690.4.diff but NOT 7690.3.diff
  • A few changes relating to spacing and quotation types
  • It was working (in terms it could connect, create a file, delete a file, etc) before making this patch, but i have not yet run a plugin upgrade with it.
  • Will need @ re-added in the future at some point, Best to leave them out for debugging.. Else you'll be forever chasing the errors..

comment:17 @DD327 years ago

In that patch there was some inverse logic in some functions on the output of run_command()

It turns out ssh2_exec() returns an empty string on success, and a string on failure, which is the oppisite of what its expecting.

It looks to me like inverting that value in the run_command() function, and removing the invertions i added in the other functions should probably make it work nicely.

However, SSH is VERY slow for me, And it times out after 120seconds after about 15 commands..

Also: Due to those logic inversions i mentioned, Plugin upgrades might do funky things to your filesystem, for example, chmoding /home/ to 755, and ~ to 755, So dont go testing on a live system (like me) unless you're sure of what your doing (I hate to say that, but remember it)

comment:18 @ShaneF7 years ago

From my testing so far, I think you were using 0.18 hacked. PHP only really works for 0.14 and when using the ssh2* functions it does work correctly. Just a matter of getting the update scheme to work correctly.

comment:19 @technosailor7 years ago

We need port options. Not everyone comes in on Port 22 so it should be assumed, but overrideable.

@technosailor7 years ago

Allows FTP_PORT to be defined and adds UI for port definition in update screen

comment:20 @DD327 years ago

Yes, i was using 0.18 modified, but switched to 0.14, They're just as slow as one another and both fail just as often.

ssh2.sftp:// appears to be faster than the send/recv() functions, but are only available with PHP5+

Currently it can use 120+seconds to just connect and attempt creating a directory.
Of course, That could just be my SSH server being slow..

I've made some optimizations to the unzip_file() code to reduce the number of operations needed, but its still not fast enough for use IMO.

I may also know why its so slow..

  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND
11905 www-data  20   0  106m  37m  20m R  101 14.5   9:06.99 php5-cgi

Thats the PHP process thats upgrading the plugin via SSH/SFTP, Looks like its using 101% of the cpu constantly for the last 10minutes while trying to upgrade.. (I removed the 120second time limit) - It looks like the extension is just looping somewhere badly..

comment:21 @ShaneF7 years ago

Yeah. I found the loop also and fixed. I disappeared yesterday yesterday without posting my changes. I made some changes to the file.php and update.php files as well as they were not working. I made small changes, hoping that they do not effect the FTP upgrade.

The only thing I am having trouble doing now is deleting files/folders, transferring (creating) files. copy.

comment:22 @ShaneF7 years ago

Ok. In my patch this is what's going on.

  • We now run the command, but we always run an echo to make sure the loop checks to see if the command is finished running as SSH could hang there. A problem I discovered while testing. It will remove the echo once it breaks from the loop. No change to the logic though.
  • The functions "not" working above are still not working. deleting folders (unsure about files), transferring (creating) files, and copy. put_contents does not work at all when going through the files.
  • changes to file.php are part of the unzip process which I don't think worked at all (?) and I can not seem to go to work. I have marked in my patch where it's failing. I commented what I changed for ssh2 to work. Unsure if we will have to create a special switch just for ssh processing. patch also includes technosailor patch of 7690-ports.diff for port processing. Also added the places where it removes the trailing / as for SSH you can't have that slash at the end.
  • changes to update.php are just cosmetic issues including a message to indicate to the user that we are updating the core. there was no difference, however I noted in the function and placed an "exit" where ssh was failing for testing purposes. The debug function will not work when FTP is selected as debug doesn't exist in there.

It does do...

  • Downloads the zip file and deletes it
  • Creates the folders in the right order
  • Check to see if the folders are valid or not valid.

@ShaneF7 years ago

see note above

comment:23 @ShaneF7 years ago

It says the wrapper is 4.3.0+

http://us3.php.net/manual/en/wrappers.ssh2.php

I just re-wrote the put_contents with a wrapper and it does work.

comment:24 @DD327 years ago

It says the wrapper is 4.3.0+

But then check the functions which can utilise the wrapper, They all say they support that style wrapper in PHP5+, I think fopen() is probably the exception to that.

I actually have the entire thing working now, Completely seperate to your code, just with the rather large time being used (10+minutes)

Assuming the wrapper is available in all installs of the module, I say use that, Its supposably faster than the pure api calls (for some odd reason) and it'll mean no temporary files need to be created

(Note: _send/_recv() functions expect a local FILE NAME, not a file Handle, thats why its failing for you)

@DD327 years ago

comment:25 @DD327 years ago

attachment 7690.7.diff added.

Merged shaneF's patch 7690.6.diff with my lot, Plugin upgrades now work seemingly well, I tried a core update, But it just kept redirecting me back to the credentials page.
Oh: It looks like i missed the changes ShaneF made to the update.php file (wording changes mainly)

As for the trailing slashes: If the SSH functions cannot deal with a slashed input, they should call untrailingslashit() or rtrim($s, '/') rather than the main code being changed, Part of the abstraction is to allow quirks of the filesystem method to be dealt with within the classes.

The same goes with a few functions in ShaneF's patch, I changed some params to keep it in line with the rest of the classes.

I also optimized unzip_file() a bit, Yes, The loops probably look hard to read at first, But put simply:
Rather than starting at / and working through to /home/username/ to check if the folders exist (Time consuming), it starts at /...../wordpress/wp-content/plugins/folder/ and works backwards to wp-content/plugins, then wp-content, etc. Once it finds a folder that exists, it works back up towards where the folder is wanted and creates them. SSH can do a recursive create, however the FTP/direct classes cannot, so its best just to leave that functionality out of it and do it via PHP.

I removed the Port specification from the credentials page though, Not many people need to set that, and having to change it when switching to FTP was a pain, Instead, I changed it to allow the hostname to be set like 'dd32.id.au:9984' for port 9984.

Theres a bit of an issue of the connection_type radio selection not sticking.. It keeps reverting to ftp.

comment:26 @DD327 years ago

Should probably mention that i've been testing with:

bash --version
GNU bash, version 3.1.17(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2005 Free Software Foundation, Inc.

uname -a
Linux 209-20-84-250.slicehost.net 2.6.24-19-xen #1 SMP Wed Aug 20 21:08:51 UTC 2008 x86_64 GNU/Linux

(Debian 4 Etch)

I somewhat bet there'll be issues with other OS's, as AFAIK, ssh2_exec() probably runs its commands via the default shell?

comment:27 @ShaneF7 years ago

I approve DD32's patch file. I am able to get core-updates to work -- to a point. Downloading the files works correctly. Working on the copy right now.

comment:28 @ShaneF7 years ago

A few tweaks later and we get this:

@ShaneF7 years ago

comment:29 @ShaneF7 years ago

  • Keywords has-patch added

comment:30 @azaozz7 years ago

(In [8852]) SSH2 filesystem improvements, props ShaneF, see #7690

comment:31 @ShaneF7 years ago

Any more improvements after this post deal with changeset [8852]+

comment:32 follow-up: @Viper007Bond7 years ago

"Note: No not leave the directory yet!"

Does that line make no sense to anyone else?

comment:33 in reply to: ↑ 32 @DD327 years ago

Replying to Viper007Bond:

"Note: No not leave the directory yet!"

Does that line make no sense to anyone else?

Yes. Do not leave the current directory in the CLI session you're running at the time.

Any more improvements after this post deal with changeset [8852]+

Just about to attach a patch which doesnt do a whole lot.. Might fix some directory creation problems though.

@DD327 years ago

comment:34 @ShaneF7 years ago

New fields for location of your public/private keys. If entered into the system, they are not stored in the database. A password is not needed, if the private key is not encrypted. http://us3.php.net/manual/en/function.ssh2-auth-pubkey-file.php

@ShaneF7 years ago

allows you to set SSH public/private keys for auth method for ssh mode. includes 7690.9. patch from DD32

comment:35 @ShaneF7 years ago

I don't know if anyone else is having this problem, but without forcing the values in defines in the wp-config.php file, I can not get the settings to save at all. I tried going from FTP to SSH to FTP(SSL) to SSH and it never updated and stayed at FTP.

So I think we need to looking into this.

comment:36 @ryan7 years ago

(In [8865]) Handle ssh keys in ssf2 fs. Props ShaneF. see #7690

@DD327 years ago

fixes incorrect if nesting

comment:37 @ShaneF7 years ago

Patch #9 fails the upgrade. #12 fixes it. Anyway, the problem is $_POST from the form is not going through the function "request_filesystem_credentials" which sets the creds fields.

Only if it's set in DEFINE does it work.

Also in the patch #12 fixes that creds do not get set from the settings. They would change if the $_POST is passed through the function, but it doesn't. I been trying a fix, but I can't seem to find the cause why it will not. So that's the deal.

@ShaneF7 years ago

@DD327 years ago

comment:38 @DD327 years ago

attachment 7690.12.diff added.

Appears to include some changes which i felt not needed, and didnt actually solve the root problem.

attachment 7690.13.diff added.

should fix it.

@DD327 years ago

as as #13, but uses API and fixes directory creation

comment:39 @DD327 years ago

attachment 7690.14.diff added.

Has a few typo's... I'm working on some other related code, so i'll post a better patch later.

@DD327 years ago

@DD327 years ago

mainly some JS improvements over the last patch.

comment:40 @ryan7 years ago

(In [8880]) ssh2 fs fixes from ShaneF and DD32. see #7690

comment:41 @ShaneF7 years ago

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

I consider this now 100% fully working. I like DD32 option, but I think this is closed. FTP/FTPS hasn't really been tested and same with SSH Keys, but it works.

#5560 patch 3, fixes a password post problem that needs to be addressed.

@ShaneF7 years ago

minor html fix

comment:42 @ShaneF7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:43 @ShaneF7 years ago

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

nvm. Fixed. :)

Note: See TracTickets for help on using tickets.