Ticket #7157 (new enhancement)

Opened 2 months ago

Last modified 2 months ago

Disable APP and XMLRPC publishing by default

Reported by: westi Assigned to: westi
Priority: high Milestone: 2.9
Component: Security Version: 2.6
Severity: normal Keywords: has-patch
Cc: josephscott

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

disable_remote_publishing_by_default.diff (5.8 kB) - added by westi on 06/20/08 09:23:33.
First pass patch. Still needs to actually stop APP working.
xmlrpc_disabled.diff (6.1 kB) - added by ryan on 06/24/08 00:59:08.
xmlrpc.php.diff (4.1 kB) - added by josephscott on 06/26/08 22:05:33.
wp-app.php.diff (0.7 kB) - added by josephscott on 06/26/08 22:05:44.
wp-admin--includes--schema.php.diff (1.5 kB) - added by josephscott on 06/26/08 22:06:22.
wp-admin--includes--upgrade.php.diff (0.8 kB) - added by josephscott on 06/26/08 22:06:33.
wp-admin--install.php.diff (1.3 kB) - added by josephscott on 06/26/08 22:06:44.
wp-app.php.2.diff (0.6 kB) - added by josephscott on 06/27/08 16:32:44.
wp-app.php.3.diff (0.6 kB) - added by AlanJCastonguay on 07/05/08 18:17:23.
not_allowed() requires an array
wp-app.php.4.diff (0.9 kB) - added by AlanJCastonguay on 07/05/08 18:18:51.
Should use 403 Forbidden instead.

Change History

06/20/08 09:23:33 changed by westi

  • attachment disable_remote_publishing_by_default.diff added.

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

06/20/08 09:24:17 changed by westi

  • cc set to josephscott.

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

06/20/08 15:39:41 changed by ryan

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

06/20/08 15:40:10 changed by ryan

Leaving open for APP lockdown.

06/20/08 16:16:59 changed by redsweater

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

06/20/08 16:19:38 changed by westi

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?

06/20/08 16:20:17 changed by redsweater

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

06/20/08 16:35:46 changed by westi

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

06/21/08 08:59:03 changed by UseShots

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.

06/21/08 14:43:31 changed by redsweater

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?

(follow-up: ↓ 15 ) 06/23/08 02:25:17 changed by matt

"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.

06/23/08 14:03:12 changed by Otto42

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.

06/24/08 00:46:37 changed by ryan

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

06/24/08 00:59:08 changed by ryan

  • attachment xmlrpc_disabled.diff added.

06/24/08 01:00:22 changed by ryan

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

06/24/08 18:34:30 changed by redsweater

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

(in reply to: ↑ 10 ) 06/25/08 23:50:21 changed by lloydbudd

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.

06/25/08 23:59:29 changed by redsweater

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

06/26/08 17:25:15 changed by ryan

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

06/26/08 22:05:33 changed by josephscott

  • attachment xmlrpc.php.diff added.

06/26/08 22:05:44 changed by josephscott

  • attachment wp-app.php.diff added.

06/26/08 22:06:22 changed by josephscott

  • attachment wp-admin--includes--schema.php.diff added.

06/26/08 22:06:33 changed by josephscott

  • attachment wp-admin--includes--upgrade.php.diff added.

06/26/08 22:06:44 changed by josephscott

  • attachment wp-admin--install.php.diff added.

06/26/08 22:18:28 changed by josephscott

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.

06/26/08 22:40:01 changed by ryan

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

06/27/08 00:25:24 changed by DD32

(In [8202])

The atomPub errror message is not translated.

06/27/08 16:32:44 changed by josephscott

  • attachment wp-app.php.2.diff added.

06/27/08 16:38:01 changed by josephscott

Pass the AtomPub? error message through the translation.

07/02/08 01:58:38 changed by markjaquith

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

07/05/08 18:16:48 changed by AlanJCastonguay

$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.

07/05/08 18:17:23 changed by AlanJCastonguay

  • attachment wp-app.php.3.diff added.

not_allowed() requires an array

07/05/08 18:18:51 changed by AlanJCastonguay

  • attachment wp-app.php.4.diff added.

Should use 403 Forbidden instead.

07/07/08 19:19:48 changed by ryan

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