Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#52409 closed defect (bug) (fixed)

Upload method SSH2 shouldn't use hardwired ssh-rsa hostkey

Reported by: richybkreckel's profile richybkreckel Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.9.3 Priority: normal
Severity: normal Version:
Component: Filesystem API Keywords: fixed-major
Focuses: Cc:

Description

The constructor of class WP_Filesystem_SSH2 has 'ssh-rsa' hard-coded in wp-admin/includes/class-wp-filesystem-ssh2.php:91.

This breaks SSH2 uploads on modern systems where this public key signature algorithm is disabled by default. (This problem currently affects Fedora 33.)

Changing it to a supported algorithm makes it work again. (I used 'ssh-ed25519'.) I wonder if we need to set the algorithm at all? Can't we let SSH just negotiate one from it's set of supported algorithms?

Attachments (1)

wp-upgrade.patch (523 bytes) - added by richybkreckel 3 years ago.
patch

Download all attachments as: .zip

Change History (17)

#1 @johnbillion
3 years ago

  • Component changed from Upload to Filesystem API
  • Keywords dev-feedback added
  • Version 5.6 deleted

#2 @johnbillion
3 years ago

Thanks for the report @richybkreckel, and welcome.

#3 follow-up: @dd32
3 years ago

I wonder if we need to set the algorithm at all? Can't we let SSH just negotiate one from it's set of supported algorithms?

Looking at the docs, I don't see any requirement for it to be set, and I don't recall any need for it to be set, so removing it makes sense to me.

All the examples of using key based authentication with SSH include it, but looking at the latest source for the ssh2 extension, it looks like it's optional, it could potentially just be a hold-over from when ssh-dsa certificates were common and considered old.

If testing without it indicates that it still uses key authentication, then removal should be okay. If removal proves problematic, it could be updated to simply be "ssh-rsa,sh-ed25519" I think based on my reading of the libssh docs. Unsupported types by the libssh would be ignored.

Note: I encourage everyone using the built in SSH to consider keeping in mind the plugin which offers a pure-PHP implementation of it, as the PHP extension has been known to have incompatibilities from time-to-time.

#4 @richybkreckel
3 years ago

Any prospect this is going to be fixed soon? As it stands, update is broken on Fedora 33 and after fixing it manually every update breaks it again. I can confirm that a comma-separated list of hostkey algorithms works just fine.

#5 in reply to: ↑ 3 @richybkreckel
3 years ago

Replying to dd32:

If testing without it indicates that it still uses key authentication, then removal should be okay. If removal proves problematic, it could be updated to simply be "ssh-rsa,sh-ed25519" I think based on my reading of the libssh docs. Unsupported types by the libssh would be ignored.

Typo alert up there: it should be "ssh-rsa,ssh-ed25519", not "ssh-rsa,sh-ed25519".

@richybkreckel
3 years ago

patch

#6 @richybkreckel
2 years ago

Ping!
Upgrading WP on a growing number of Linux distros breaks the installation.
Please apply the patch attached above.

#7 @richybkreckel
2 years ago

Can this very simple patch above please be applied?
Still with 5.9.1 we have this broken upgrade.
Seriously, this is quite annoying!

#8 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 6.0

#9 @SergeyBiryukov
2 years ago

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

In 52807:

Filesystem API: Include the ssh-ed25519 public key signature algorithm as an alternative to ssh-rsa.

The ssh-rsa signature algorithm is disabled by default as of OpenSSH 8.8, which breaks SSH2 uploads in WordPress on modern systems. ssh-ed25519 is one of the suggested alternatives, supported since OpenSSH 6.5.

References:

Follow-up to [8865].

Props richybkreckel, dd32.
Fixes #52409.

#10 @SergeyBiryukov
2 years ago

Welcome to WordPress Trac, and thanks for the patch. Sorry it took so long to merge it.

It is merged now and will be a part of the WordPress 6.0 release. I would also be open to backporting this to the 5.9 branch if anyone feels strongly about including it in the next minor release.

#11 @richybkreckel
2 years ago

Thank you very much, Sergey!
(I would actually appreciate this being backported to the 5.9 branch.)

#12 @SergeyBiryukov
2 years ago

  • Keywords fixed-major added; dev-feedback removed
  • Milestone changed from 6.0 to 5.9.2
  • Resolution fixed deleted
  • Status changed from closed to reopened

Thanks! Reopening for 5.9.2 consideration.

#13 @audrasjb
2 years ago

  • Milestone changed from 5.9.2 to 5.9.3

Moving to milestone 5.9.3 since we're about to release 5.9.2.

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


2 years ago

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


2 years ago

#16 @audrasjb
2 years ago

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

In 53022:

Filesystem API: Include the ssh-ed25519 public key signature algorithm as an alternative to ssh-rsa.

The ssh-rsa signature algorithm is disabled by default as of OpenSSH 8.8, which breaks SSH2 uploads in WordPress on modern systems. ssh-ed25519 is one of the suggested alternatives, supported since OpenSSH 6.5.

References:

  • OpenSSH 8.2 release notes
  • OpenSSH 8.7 release notes
  • OpenSSH 8.8 release notes

Follow-up to [8865].

Props richybkreckel, dd32, SergeyBiryukov.
Merges [52807] to the 5.9 branch.
Fixes #52409.

Note: See TracTickets for help on using tickets.