Turn ms-files.php off by default

[nacin]

Similar to what we did with global terms, I suggest we disable ms-files.php handling for new installs. Reasoning follows.

Spam Control?

The primary intent of ms-files.php, I presumed, was to prevent images from being accessible for spammed, archived, or deleted blogs. Ultimately, this is not indicated in the UI, and if it goes away, most admins are not going to miss it. If a site needs to be deleted, it can just be deleted (for most networks). If you do need this kind of protection, then you're clearly running a more advanced network, and you should enable it manually.

Security?

ms-files.php additionally does mime-type checking. Contrary to what many may assume, though, it does not check against the upload file types option that is on the Network Settings page. Rather, it only uses wp_check_filetype(). There is a filter on wp_check_filetype() which could be used to add/remove mime types, but since plugins are not yet included, it cannot be used out of the box.

Performance?

Reading a file out via PHP is terrible for performance. While some caching headers are applied, it would be far more efficient if these are applied directly by the server. To load a typical blog page with 10-15 images in the posts might just require loading WordPress 11-16 times, depending on what is in browser cache. (Granted, SHORTINIT, but it's still bad to spin up a PHP process for this.)

If you want to leverage ms-files.php, you will need object caching (at the very least). But realistically, it's the only aspect of multisite that truly requires object caching out of the box. Removing it will make multisite easier to use and remove one of the biggest differences between single-site and multisite. Someone like otto42 who is simply running a few blogs that are statically cached has no necessity for ms-files.php.

We Can Be Backwards Compatible

ms-files.php is simply enabled by routing /files/ through the handler via mod_rewrite or web.config. Since we do not touch rewrite rules for network sites after installation, we can turn this off for new installs without any impact.

For implementation, we should just continue to use the /wp-content/uploads/ directory. We can simply create a new directory called /sites/ in /uploads/ — we'll already have permissions for wp_mkdir_p() — and then add blog ID folders in there. No more blogs.dir, no more blogs.php, no more ms-files.php.

For existing sites wanting to transition over, they will need special rules to rewrite /files/ to the new location. Since we will be keying things by blog ID on the FS level, we may need some sort of ms-files.php-like migration script that simply does the lookup then fires a 301 redirect. This of course would only be opt-in if they modify their rewrite rules.

Finally, we would need to modify (for new installs) the upload paths and URLs (fileupload_url, upload_path, etc.). We would probably need to introduce a function similar to global_terms_enabled(), such as ms_files_rewriting_enabled(), to handle such toggles.

Note, these new filesystem paths would expose blog IDs. I don't know of another instance where these are currently exposed, but I don't see a reason to care. It should be noted that blog IDs on WordPress.com are exposed assuming you realize how wp.me shortlinks are generated.

—

Is this going to be a pain? Yes. But in the end, it'll definitely be a worthwhile transition.

[scribu]

Completely agree that the performance penalty and added complexity far outweigh the benefit of ms-files.php in most cases.

[wpmuguru]

One of the original reasons for hiding the blog id probably was that the table prefix was hard coded to wp_ in MU.

I don't see any reason not to do this. By doing this we would also fix the media path/url generation while switched which technically works but is less than ideal.

[andrea_r]

It would be one less step in setup, and also help with user perception. Many people think the blogs actually live in blogs.dir. Also I have seen issues with Apache needing tweaks to handle just the image rewrite. You can imagine how that goes over with shared hosts.

I'd love to see more chime in.

[markjaquith]

Yes, please.

[wpmuguru]

What if the sitemeta key were the path to the ms media root ex /wp-content/uploads/sites and that was used on blog/site creation to set the upload_path, etc. Then if the network owner wanted to split media across multiple folders they could just change that path. It also could be added to the create a network screen.

[nacin]

What if the sitemeta key were the path to the ms media root ex /wp-content/uploads/sites and that was used on blog/site creation to set the upload_path, etc.

That seems like a good idea. Then they can filter it and shard it to their heart's delight.

[ericmann]

Gut instinct: I don't like it. But that's more due to all of the weird handling I've been doing lately to fix an issue with a particular image management plugin.

It was built with single installations in mind, so /wp-content/uploads is hard-coded in many places within the plugin. But the plugin alleges network compatibility and has started using /blogs.dir/X/ in many places as well ... and /files in yet others. I've spent a week now hacking the thing apart to make it more flexible - so it will work for single sites and networks.

So from a forward momentum perspective, I think this is a great proposal. It will save a lot of time in the future and prevent convoluted, poorly-coded systems like this. But a large part of me is wondering how many existing systems this will break? How many plugins and themes have hard-coded the existing schema already?

I'd be all for making this move for the future, so long as we test the crap out of any backwards compatibility that gets baked in.

[nacin]

Replying to ericmann:

Gut instinct: I don't like it. But that's more due to all of the weird handling I've been doing lately to fix an issue with a particular image management plugin.

We'll be backwards compatible in core, that's for sure. But you can't fix stupid.

[ericmann]

Replying to nacin:

But you can't fix stupid.

:-)

[Otto42]

I wouldn't miss it if it was gone, but I was kinda hoping to use it at some future point to do dynamic image resizing. Still, that can be done without ms-files.php.

[adambackstrom]

Just to add to the discussion, allowing direct access to the files under nginx/php-fpm can allow remote code execution if the server is configured poorly:

​http://wiki.nginx.org/Pitfalls#Pass_Non-PHP_Requests_to_PHP.

Under WordPress 3.2.1, I can upload a file "foo.jpg" that contains PHP, and an attacker could craft a URL that causes PHP to evaluate the contents of this file. There are several ways to protect yourself, and nginx/php-fpm is the less common server setup, but ms-blogs.php offers basic protection if you keep blogs.dir out of the document root. Felt like it should be part of the thread.

[nacin]

Replying to Otto42:

I wouldn't miss it if it was gone, but I was kinda hoping to use it at some future point to do dynamic image resizing. Still, that can be done without ms-files.php.

Yeah — It'd actually be easier to implement dynamic image resizing without ms-files.php. Because plugins aren't loaded, nothing can alter ms-files.php. You'd have to create a new file to serve images and alter the rewriting.

The best way to deal with dynamic image resizing is to handle it on 404s within wp-content/uploads/*, which can then be processed to generate the image and place it where it belongs.

[nacin]

Replying to adambackstrom:

Under WordPress 3.2.1, I can upload a file "foo.jpg" that contains PHP, and an attacker could craft a URL that causes PHP to evaluate the contents of this file. There are several ways to protect yourself, and nginx/php-fpm is the less common server setup, but ms-blogs.php offers basic protection if you keep blogs.dir out of the document root. Felt like it should be part of the thread.

The solution for that would be to finally implement proper content sniffing on uploads, which is a separate matter. And making sure that PHP cannot be executed in blogs.dir or wp-content/uploads is best practice for performance, scaling, and security reasons anyway.

[ryanhellyer]

Looks good to me.

[wpmuguru]

Related #12756 - Cover this base while doing the proper sniffing.

[Ipstenu]

From a setup/host perspective, yes, please. It would save us havign to say "No, no, blogs.dir is the name of the folder."

The blog ID was never a huge secret anyway, no more than wp-content/uploads was (of course, it will screw up the people who, against all recomendations, went and changed the uploads location manually - I keep telling people not to mess with it if they don't understand how ms-files works, but you can't fix stupid).

I don't think it would cause any more plugin problems than it does now, though this is something where perhaps we should look and see how many (if any) plugins are using ms-files.php...

I did a quick check and came up with these:

Two exploit scanners (which makes sense given how they work):

• exploit-scanner
• jw-cloud-sites-wp-scanner
• ultimate-security-checker
• ultimate-security-checker

Directions on how to edit your permalinks/htaccess:

• nginx-compatibility
• wcs-custom-permalinks-hotfix
• wordpress-multi-site-enabler-plugin-v10

Role Scoper calls in rewrite-mu_rs.php (interesting):

• role-scoper

The only one that surprises me is Role Scoper, though. The rest are nothing to worry about.

[unsalkorkmaz]

Related: #19030

[djeyewater]

I would definitely appreciate this change as well. I don't think it should strictly be necessary to expose the blog id, you could probably use a rewrite map or otherwise a separate filepath structure and rewrite rule depending on whether multisite was set up to use path, domain, or subdomains.

[juliobox]

About Security, my view :
Test: ​http://hollywoodpq.com/wp-content/blogs.dir/2/files/obm-gallery/widgetCache.php
Now just remove "wp-content/blogs.dir/2/" you got now:
New test: ​http://hollywoodpq.com/files/obm-gallery/widgetCache.php

Php files are downloadables ? Damn . . .
What do you think about that ?

ps: Demo site found with google.
_{Julio - Web Security Consultant - boiteaweb.fr}

[wpmuguru]

Replying to juliobox:

About Security, my view :
Test: ​http://hollywoodpq.com/wp-content/blogs.dir/2/files/obm-gallery/widgetCache.php
Now just remove "wp-content/blogs.dir/2/" you got now:
New test: ​http://hollywoodpq.com/files/obm-gallery/widgetCache.php

Php files are downloadables ? Damn . . .
What do you think about that ?

ps: Demo site found with google.
_{Julio - Web Security Consultant - boiteaweb.fr}

Why are you putting PHP files in your media folders? If you are going to upload PHP files to your media folders don't expect WP security to protect your site.

WP does not allow a user to upload PHP files to the media folder.

[juliobox]

Replying to wpmuguru:

Replying to juliobox:

About Security, my view :
Test: ​http://hollywoodpq.com/wp-content/blogs.dir/2/files/obm-gallery/widgetCache.php
Now just remove "wp-content/blogs.dir/2/" you got now:
New test: ​http://hollywoodpq.com/files/obm-gallery/widgetCache.php

Php files are downloadables ? Damn . . .
What do you think about that ?

ps: Demo site found with google.
_{Julio - Web Security Consultant - boiteaweb.fr}

Why are you putting PHP files in your media folders? If you are going to upload PHP files to your media folders don't expect WP security to protect your site.

WP does not allow a user to upload PHP files to the media folder.

I do not put PHP files, but people and plugins are doing it.
I found some plugins which copy some php files also, just google some dorks.

[SergeyBiryukov]

Version field indicates the earliest affected version. ms-files.php was copied from MU in 3.0.

[wpmuguru]

Replying to juliobox:

I found some plugins which copy some php files also, just google some dorks.

If you want to report a plugin try ​http://wordpress.org/extend/kvetch/. Core trac is for WP core.

[nacin]

Replying to wpmuguru:

Replying to juliobox:

I found some plugins which copy some php files also, just google some dorks.

If you want to report a plugin try ​http://wordpress.org/extend/kvetch/. Core trac is for WP core.

Kvetch isn't monitored — try the support forums, or plugins@….

[juliobox]

So this i not a WP issue but a plugin issue ?
Ok, thank you all !

[djeyewater]

If anyone's interested I'm using a ​work-around using rewrite rules or a rewrite map for my site. It's not really suitable if you have 100s of blogs on your site though.

[wpmuguru]

I'll have time to contribute to this in the second half of August.

[braydonf]

There are also streaming problems with using ms-files.php for ogg audio files on multi-site, so this will be good, to serve direct from the Apache/Nginx.

[deltafactory]

Re: streaming problems - This would also fix the default behavior for video streaming in iOS. The video player ​requires byte-range requests but mod_rewrite blocks that functionality.

[nacin]

First pass rips out everything without regard for an upgrade path or any kind of back compat. Meant for basic testing/review of the approach.

[nacin]

19235.diff​:

First pass rips out everything without regard for an upgrade path or any kind of back compat. Meant for basic testing/review of the approach.

[wpmuguru]

That looks like it will work.

For non rewrite installs I had been thinking about the possibility of checking for ms_rewriting_enabled & if not enabled just use the upload_path option and bypass all the other checks.

[nacin]

19235.3.diff​ — fully backwards compatible. Handles the upgrade seamlessly. Works great in my testing.

• The URL becomes wp-content/uploads/sites/%d. Not the cleanest, but definitely the best option.

• ms_files_rewriting becomes a network-wide option set in upgrade_network() and populate_network(). Defaults to 0 for new installs, 1 for existing networks, and new networks on an existing install inherit. If upgrade_network() hasn't run yet, all existing networks will get screwed up, so I use a filter to enforce, globally, the default of get_site_option( 'ms_files_rewriting' ) to be true until something is inserted into the DB.

• All logic ends up in wp_upload_dir(). 'UPLOADS' is again a fully functional constant, regardless of is_main_site() or $switched. ($switched is simply there to ensure the old MS constants aren't lying, given that they hard-code a blog ID.)

• upload_path for new sites (on a non-rewriting network) is whatever the main site's upload_path is, which is going to be empty. Note that "sites/%d" is always appended to upload_path (which in this case would translate to "wp-content/uploads"). Some direct DB queries in install_blog() are cleaned up as well.

• wp_upload_dir() is changed to always return the data you wanted, even if it returns an error due to directory creation failing. Huzzah! _wp_relative_upload_path() is changed to always trim off the basedir, even when there is an error, as an enhancement.

• Warnings for custom content directories are now only issued for subdirectory networks (thanks to the wp-(content|admin|includes) rewrite). Subdomain networks are now in the clear.

[wpmuguru]

19235.3.diff looks good.

[SergeyBiryukov]

Replying to braydonf:

There are also streaming problems with using ms-files.php for ogg audio files on multi-site

Related: #15552

[wpmuguru]

refresh of 19235.4.diff to current trunk

[wpmuguru]

That should have been a refresh of 19235.3.diff. That worked fine for me for backward compatibility.

[nacin]

In [21822]:

Always return upload directory information from wp_upload_dir(), even if there is an error. Append the error to the array. see #19235.

[nacin]

In [21823]:

Remove ms-files.php rewriting from WordPress multisite. fixes #19235.

Keep existing networks compatible with a ms_files_rewriting network option.

[nacin]

In [21881]:

Attach the default ms_files_rewriting site option filter in ms-default-constants, as ms-default-filters is not included during SHORTINIT. see #19235.

[nacin]

In [21892]:

Account for the old 'main override' in wp_upload_dir() for the main site in a post-MU network by declining to define the UPLOADS constant.

Fixes uploads on the main site of a post-MU network that uses ms-files rewriting. see #19235.

[nacin]

In [22038]:

If a pre-3.0 (MU era) network disables ms-files manually, they don't need /sites/ in their wp-content/uploads directory. see #19235.

[ryan]

Possible bug: ​http://wordpress.org/support/topic/switch_to_blog-does-not-properly-set-wp_upload_dir-vars

[nacin]

In [22106]:

Pass the current blog id to is_main_site() in wp_upload_dir(), because is_main_site() without arguments does not respond correctly on switch. see #19235.

[nacin]

For [22106], originally posted as comment:44:ticket:21459:

Before #19235, switching never worked because it relied on constants. Now, it calls is_main_site(). Unfortunately, is_main_site() uses $current_blog->blog_id rather than $blog_id or $wpdb->blogid, and therefore does not respond when the site is switched.

That means that when switched, is_main_site() returns a different value than is_main_site( get_current_blog_id() ).

That's enough to make me want to start switching $current_blog and $current_site, see #14190. Unfortunately, I have no idea what it'd break.

Switching to the cryptic, seemingly nonsensical, and unnecessary-but-necessary is_main_site( get_current_blog_id() ) should fix this.

[ryan]

Opened #22090.

[nacin]

On networks that haven't yet run the upgrade routine, this has the potential to cause a boatload of queries. This is because sitemeta options don't have a notoptions cache like regular options do, and wp_load_core_site_options() does not try to cache any kind of default values for those that don't exist. We may want to consider making improvements to either/both of those in the future.

[ryan]

19235-ut.diff passes with or without 19235.5.diff. It's hard to get full test coverage due to the constants.

[nacin]

In [22222]:

Have wp_upload_dir() account for blog switching, ms-files rewriting, and the UPLOADS constant properly. This type of logic needs a lot of code comments.

Prevents wp_upload_dir() from obeying the UPLOADS constant when ms-files rewriting is enabled and a blog is switched.

Reverts [22106] thanks to [22108].

see #19235.

[SergeyBiryukov]

#12496 was marked as a duplicate.

[DrewAPicture]

@nacin Welp, here's another one. ms_deprecated_blogs_file() was effectively soft-deprecated in [21823] for this ticket sans any sort of DocBlock or anything.

Is it possible that it wasn't "hard-deprecated" simply because it slipped through the cracks, or was there some other reasoning in play?