WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#7157 closed enhancement (fixed)

Disable APP and XMLRPC publishing by default

Reported by: westi Owned by: westi
Milestone: 2.6 Priority: high
Severity: normal Version:
Component: Security Keywords: has-patch
Focuses: Cc:

Description

In order to protect the majority of blogs which don't use these protocols against any possible security vulnerabilities we should disable them by default.

This is similar to the idea that registration is disabled by default.

Attachments (10)

disable_remote_publishing_by_default.diff (5.8 KB) - added by westi 7 years ago.
First pass patch. Still needs to actually stop APP working.
xmlrpc_disabled.diff (6.1 KB) - added by ryan 7 years ago.
xmlrpc.php.diff (4.1 KB) - added by josephscott 7 years ago.
wp-app.php.diff (699 bytes) - added by josephscott 7 years ago.
wp-admin--includes--schema.php.diff (1.5 KB) - added by josephscott 7 years ago.
wp-admin--includes--upgrade.php.diff (798 bytes) - added by josephscott 7 years ago.
wp-admin--install.php.diff (1.3 KB) - added by josephscott 7 years ago.
wp-app.php.2.diff (619 bytes) - added by josephscott 7 years ago.
wp-app.php.3.diff (647 bytes) - added by AlanJCastonguay 7 years ago.
not_allowed() requires an array
wp-app.php.4.diff (944 bytes) - added by AlanJCastonguay 7 years ago.
Should use 403 Forbidden instead.

Download all attachments as: .zip

Change History (38)

@westi7 years ago

First pass patch. Still needs to actually stop APP working.

comment:1 @westi7 years ago

  • Cc josephscott added

Patch adds the options, UI and lockdown for xmlrpc but needs the lockdown for APP implementing.

comment:2 @ryan7 years ago

(In [8136]) Disable remote publishing by default. Add options to turn them back on. Props josephscott. see #7157

comment:3 @ryan7 years ago

Leaving open for APP lockdown.

comment:4 @redsweater7 years ago

What a bummer. Terrible news for desktop clients. I hope the security enhancing tradeoff is very high because this will be a support burden on developers who are supporting users of desktop clients.

It's also worth noting that this will add friction to the process of using a remote client with WordPress, and therefore make other systems such as Blogger potentially more attractive to such users.

Daniel

comment:5 @westi7 years ago

We should of course consider making these options part of the 5 minute install like the privacy ones are.

Something like Enable Remote Client access - which would enable both types by default?

comment:6 @redsweater7 years ago

Adding the option to enable in the 5 minute install would help a lot.

comment:7 @westi7 years ago

(In [8139]) Allow enabling of remote publishing at install time. See #7157.

comment:8 @UseShots7 years ago

Will there be a way to find out whether the remote publishing is disabled or not?
I.e. some meaningful error message when you call the disabled XML-RPC methods or a special method that answers "disabled/enabled".

As a weblog client developer I would like to let the users know that why the "publishing" failed and tell them what they should do to fix the issue. So I need to distinguish this error from all other types of errors.

P.S. BTW adding the option to the "5 minute install" won't help much. People install blogs, then, after some time, they discover remote weblog clients. By that time they have already forgotten about those options they saw during the install.

comment:9 @redsweater7 years ago

Great points UseShots. I think you're right about the "deciding later" to use a remote client. All in all I think this change will be disappointing for remote editors, and their users, but I like your idea about at least providing a meaningful way of detecting this condition.

One of the major nuisances with Movable Type is that users are required to know about and then use a separate "web services" password. It would help a lot in their case if they just arranged for the authentication failure to explain what might be going wrong.

Is it at all worth considering a "secured mode" xmlrpc.php and app.php, that just returns an error stating that the user has not enabled access?

comment:10 follow-up: @matt7 years ago

"Enable remote publishing using the WordPress, Movable Type, MetaWeblog, Blogger and Atom publishing protocols for my blog."

If I'm a new blogger, or a first-time WordPress user, how does that make any sense? It's confusing enough that I bet people will check it simple because they have no idea what it does, and it can't cause any harm if it's a checkbox in installation, right?

I don't think this needs to be an option in setup.

comment:11 @Otto427 years ago

Something's wrong with this option. I was unable to enable it on the Options->Writing screen and have the change actually stick. I had to go into the options table and manually set the option to "1" to make it work.

comment:12 @ryan7 years ago

(In [8180]) Save enable_app and enable_xmlrpc settings. see #7157

@ryan7 years ago

comment:13 @ryan7 years ago

Here's an alternate approach that returns a descriptive error rather than the generic method not found error. The patch isn't complete.

comment:14 @redsweater7 years ago

I am still not a fan of this change, and I've decided to write some public debate to at least hopefully get some thoughts moving about the possibility of taking another approach:

http://www.red-sweater.com/blog/512/wordpress-to-disable-remote-access

comment:15 in reply to: ↑ 10 @lloydbudd7 years ago

Replying to matt:

I don't think this needs to be an option in setup.

I agree with Matt. This is a distracting option for setup and also currently written in Geek. A descriptive error message resolves all scenarios I can think of.

comment:16 @redsweater7 years ago

Assuming you decide to go ahead with this , I agree a descriptive error message will be very helpful.

comment:17 @ryan7 years ago

I can proceed with my patch and remove the install options while discussions continue.

@josephscott7 years ago

@josephscott7 years ago

comment:18 @josephscott7 years ago

New patches:

  • Provide helpful error message when XML-RPC is not enabled. Now done as part of the authentication check.
  • Provide helpful error message when AtomPub is not enabled.
  • Enable XML-RPC and AtomPub when doing upgrades.
  • Remove check box for enabling XML-RPC & AtomPub during install.

The only XML-RPC functions that don't attempt to authenticate users are:

  • demo.sayHello
  • demo.addTwoNumbers
  • mt.supportedMethods (which seems pretty useless in light of system.listMethods)
  • mt.supportedTextFilters
  • mt.getTrackbackPings
  • pingback.ping
  • pingback.extensions.getPingbacks

Turned XML-RPC and AtomPub back on for upgrades to reduce the amount of surprised existing users will have.

comment:19 @ryan7 years ago

(In [8202]) More informative error message when remote publishing is disabled. Don't disable if upgrading. Props josephscott. see #7157

comment:20 @DD327 years ago

(In [8202])

The atomPub errror message is not translated.

@josephscott7 years ago

comment:21 @josephscott7 years ago

Pass the AtomPub error message through the translation.

comment:22 @markjaquith7 years ago

(In [8234]) Pass AtomPub disabled messaged through translation. props josephscott. see #7157

comment:23 @AlanJCastonguay7 years ago

$allow passed to not_allowed() is expected to be an array, and joined into a comma-separated list of allowed values. If we're going to use not_allowed() to output this warning in the Allow: header, the content should be a single-element array rather than a string.

However, it may be better to use HTTP Status 403 instead, since Status 405 "MUST include an Allow header containing a list of valid methods for the requested resource", not an arbitrary user-oriented string. With Status 403, WordPress "SHOULD describe the reason for the refusal in the entity" body, not through the Accept: header.

@AlanJCastonguay7 years ago

not_allowed() requires an array

@AlanJCastonguay7 years ago

Should use 403 Forbidden instead.

comment:24 @ryan7 years ago

(In [8267]) Send 403 if publishing is disabled. Props AlanJCastonguay. see #7157

comment:25 @jacobsantos7 years ago

is this fixed?

comment:26 @Otto427 years ago

  • Milestone 2.9 deleted
  • Resolution set to fixed
  • Status changed from new to closed
  • Version 2.6 deleted

Yeah, this made it into 2.6. Closing.

comment:27 @jacobsantos7 years ago

Should be moved to 2.6 milestone, but it no longer exists.

comment:28 @DD327 years ago

  • Milestone set to 2.6
Note: See TracTickets for help on using tickets.