WordPress.org

Make WordPress Core

Opened 20 months ago

Last modified 20 months ago

#25840 new enhancement

Feature Request: WP_ACCESSIBLE_HOSTS as option

Reported by: xFireFartx Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.7.1
Component: HTTP API Keywords: has-patch needs-testing
Focuses: Cc:

Description

Currently WP_ACCESSIBLE_HOSTS is defined as a constant. It would be great if this is a wordpress option (or something equivalent) so you can change it at runtime. If you have a multisite installation and need to add domains to the whitelist you must reload the whole installation to enable them. Writing a simple plugin for this is also not possible since constants can not be redefined.

My suggestion:
1) Store a site_option for mutlisites. This should contain a general whitelist for all blogs
2) Store a option per blog to contain additional whitelists for this single blog
3) Make it configurable via the admin interface (single textbox to enter the domains)
4) In the block_request function (https://github.com/WordPress/WordPress/blob/master/wp-includes/class-http.php#L507) the 2 options should be merged and handled like the constant

This way you could manage the whitelist at runtime.

What do you think about this?

Chris

Attachments (4)

25840.diff (686 bytes) - added by leewillis77 20 months ago.
Proposed patch
25840.1.diff (909 bytes) - added by JustinSainton 20 months ago.
wp_http_ prefix.
25840.2.diff (1.1 KB) - added by leewillis77 20 months ago.
With the previous patch that the function would still block requests if WP_ACCESSIBLE_HOSTS was undefined - without ever invoking the filter. Revised patch attached - this includes the suggested rename as per Justin's patch.
25840.3.diff (2.3 KB) - added by dd32 20 months ago.

Download all attachments as: .zip

Change History (17)

@leewillis7720 months ago

Proposed patch

comment:1 @leewillis7720 months ago

Would that patch allow you to do what you want to (By filtering the list of accessible sites within block_request()) ?

comment:2 @ocean9020 months ago

  • Type changed from feature request to enhancement

-1 for an UI option.

A filter like 25840.diff looks good for me.

comment:3 @dd3220 months ago

A filter on any/all existing constants sounds like a good idea to me.

comment:4 @dd3220 months ago

  • Milestone changed from Awaiting Review to 3.8

Only note, is that we currently have filters prefixed with 'wp_http_' and 'http_', so I'd prefix that filter with the former.

@JustinSainton20 months ago

wp_http_ prefix.

comment:5 @JustinSainton20 months ago

25840.1.diff prefixes the filter as requested.

@leewillis7720 months ago

With the previous patch that the function would still block requests if WP_ACCESSIBLE_HOSTS was undefined - without ever invoking the filter. Revised patch attached - this includes the suggested rename as per Justin's patch.

comment:6 @xFireFartx20 months ago

Hi,
thanks for the patch! I want to discuss the use of a filter here.
Currently the WP_ACCESSIBLE_HOSTS can only be managed by an admin because it is a constant and can not be overwritten by a plugin. With the use of a filter, every plugin can register it's own domains. I think the purpose of the whitelist is that it is managed by an admin and not via plugins.
Example: A plugin sends usage statistics to xxx.yyy.com. Now an admin can manage the whitelist and deny the access to this site. With the use of a filter the plugin can register xxx.yyy.com for the whitelist and the admin can not deny it.

How do you see this whitelisting feature?

comment:7 follow-up: @leewillis7720 months ago

With the use of a filter, every plugin can register it's own domains.

Correct - but plugins may also remove domains.

A plugin sends usage statistics to xxx.yyy.com. Now an admin can manage the whitelist and deny the access to this site. With the use of a filter the plugin can register xxx.yyy.com for the whitelist and the admin can not deny it.

The admin can deny it by setting the priority of their own filter higher than that of the plugins.

comment:8 @xFireFartx20 months ago

Ok that will do the trick :)

comment:9 in reply to: ↑ 7 ; follow-up: @Christian Buchhas20 months ago

Replying to leewillis77:

With the use of a filter, every plugin can register it's own domains.

Correct - but plugins may also remove domains.

A plugin sends usage statistics to xxx.yyy.com. Now an admin can manage the whitelist and deny the access to this site. With the use of a filter the plugin can register xxx.yyy.com for the whitelist and the admin can not deny it.

The admin can deny it by setting the priority of their own filter higher than that of the plugins.

Hi,

that is an interesting argument, but there is a possibility where you can break the security!
When the plugin uses the maximum prio, then there is no more room for the admin to add a higher prio, and the queue of filters will be processed with the order of their names ... ?

The easiest and most secure solution is to set the constants in a plugin, which name begin with zero, because the plugins will be executed with the order of their names!

The constant hasn't be set in wp-config.php, so there is no problem.

Best regards,
Christian

This filter destroys the security feature of the constants

comment:10 in reply to: ↑ 9 ; follow-up: @rmccue20 months ago

Replying to Christian Buchhas:

that is an interesting argument, but there is a possibility where you can break the security!
When the plugin uses the maximum prio, then there is no more room for the admin to add a higher prio, and the queue of filters will be processed with the order of their names ... ?

The easiest and most secure solution is to set the constants in a plugin, which name begin with zero, because the plugins will be executed with the order of their names!

That would be true, but there's not really a "maximum" priority as such. Priorities aren't guaranteed to be integers, floats, or even numbers, so there's (almost) always something higher that you can set. e.g. if something uses PHP_INT_MAX, you can use PHP_INT_MAX + 1 and it'll transparently become a larger float. (Interesting thought: what *is* the maximum priority (last sorted value) you can get in PHP? I'd guess INF.)

---

Apart from the intellectual exercise, I'm not sure it really matters. If you enable a plugin, it can already run arbitrary code, so it's hardly a security issue.

Last edited 20 months ago by rmccue (previous) (diff)

comment:11 in reply to: ↑ 10 @dd3220 months ago

Replying to rmccue:

Apart from the intellectual exercise, I'm not sure it really matters. If you enable a plugin, it can already run arbitrary code, so it's hardly a security issue.

Yeah, these constants aren't designed as a security measure (you should have a firewall between the web server and the LAN/Web if you need to prevent connections) and are rather more designed to prevent HTTP timeouts to inaccessible hosts.

Looking at 25840.2.diff we'll also need to change the code further down that expects WP_ACCESSIBLE_HOSTS to be defined. It should instead be changed to loop over the filters results, and if * is present, add it to the wildcard list, etc.

@dd3220 months ago

comment:12 @dd3220 months ago

  • Keywords has-patch needs-testing added

25840.3.diff is an untested patch that covers all angles of this constant & filter.

The previous patches filter only once per page load, where as this filters on each request, in addition, the logic has been changed to accomodate that in hopefully a more performance-friendly manner.

comment:13 @dd3220 months ago

  • Milestone changed from 3.8 to Future Release
Note: See TracTickets for help on using tickets.