drupal_valid_test_ua() / drupal_generate_test_ua() in bootstrap.inc causes problems with loadbalancers during simpletest runs.

We can completely remove the filectime()/fileinode(). They are not really necessary since we moved to $drupal_hash_salt.

On the other hand, it's your job (and it's critical) that $drupal_hash_salt be the same on all the servers in your cluster.

Nothing much we can do about your replication problem. The only thing I can suggest would be to configure your cluster properly so that it can handle such a spike of replication data. It's probably a good idea anyway :)

That should already reduce the pain of running tests on a cluster without a shared filesystem for the code.

This is by design - so basically needs documentation. I don't support this change.

The way we are running simpletest basically means a flaw that allows an attacker to set a DB prefix means they can install an added Drupal and then enable PHP module and execute arbitrary code on your server. So, we should be focused on ways to make this more secure, not less secure - note that the inode, etc, values are used here since there is no way to determine these even in the event of arbitrary SQL injection or arbitrary file inclusion (i.e. exposing the contents of settings.php).

If you can imagine an equally secure substitute, then let's work on that.

The actual changes in this patch as well as the controversial POVs on this issue contradict the major priority.

We may have bumped into this at Acquia. The following quote is from an internal Acquia e-mail written by Barry Jaspan (bjaspan):

Based on David's hint, I just found the problem. Drupal uses $_SERVER['HTTP_USER_AGENT'] to decide when a request is coming from simpletest. Because we are running mod_fcgid, PHP does not automatically get that environment variable from Apache. I'm pretty sure we can fix our vhost configs to pass it along explicitly.

The way we are running simpletest basically means a flaw that allows an attacker to set a DB prefix means they can install an added Drupal and then enable PHP module and execute arbitrary code on your server.

I don't understand how this could happen? The installer has code to prevent anyone with a simpletest-like user agent string from ever installing Drupal, doesn't it?

My impression is that the exploit would be limited to being able to make requests against an existing simpletest-installed Drupal instance (for example, due to a test run that's in progress, or a failed test run that was never cleaned up). For the most part, that doesn't seem dangerous, although I guess there would be an issue if the test site happened to be configured in a way that gave anonymous or regular authenticated users high level permissions (enough that they could self-escalate up to a point where they could use the PHP module). In theory this is something that could happen during a test, although probably pretty rare.

Looking at the patch, I don't think it's safe to use $drupal_hash_salt directly though (since it can be an empty string sometimes). The code should use drupal_get_hash_salt() instead.... However, I'm also wondering why we can't just validate against something stored in the database also (for example, the site's private key). The existing code comments say that's not possible because the database isn't initialized yet. But, if we wanted to, we could connect to the database first, and validate afterwards. It would require moving some code around, but overall I don't see why it's impossible (at least for Drupal 8).

At that point, to be able to execute arbitrary code on the server, it seems like an attacker would need to:

• Read the contents of settings.php
• Read from the database (i.e., know the site's private key). Note that if they could write to the database also, it's game over anyway.
• Come across a site that happens to have an existing simpletest-installed Drupal instance.
• That simpletest instance must be currently configured to allow unprivileged users to have high-level admin access.
• The attacker needs to either know (or determine via brute force) the database prefix for that instance (there are around 1,000,000 possibilities).

I think if all or even some of the above is correct, we don't have to worry much about this exploit. So perhaps removing the filectime() and fileinode() checks can still be on the table.

Based on David's hint, I just found the problem. Drupal uses $_SERVER['HTTP_USER_AGENT'] to decide when a request is coming from simpletest. Because we are running mod_fcgid, PHP does not automatically get that environment variable from Apache. I'm pretty sure we can fix our vhost configs to pass it along explicitly.

Dries, I think that's a different issue? (Although maybe a documentation issue that Simpletest requires the server to be configured in a way that $_SERVER['HTTP_USER_AGENT'] is available to PHP.)

The issue here is that even if $_SERVER['HTTP_USER_AGENT'] is correctly set, Drupal may not recognize it as a valid simpletest user agent if the code that makes the request (i.e. the process that is running the simpletests and calling something like $this->drupalGet('some/path')) is on one web node, but the code that serves that request happens to be on a different web node.

Anyone who understands this issue... There was a follow-up core/doc issue filed to clarify the docs in default.settings.php around the salt, which refrenced a comment here.... Could someone knowlegeable please review the suggested change for accuracy? Thanks...
#1485810: Ambiguous wording in documentation of $drupal_hash_salt

For those of you following from home, there's also #1739986: Fix fallback in drupal_get_hash_salt(), move it to bootstrap.inc, use instead of $GLOBALS['drupal_hash_salt'], which somewhat related as it cleans up usage of the hash salt variable.

right, the other fix should precede anything here. However, here's a patch that applies directly to core. It will have to be re-rolled since #1739986: Fix fallback in drupal_get_hash_salt(), move it to bootstrap.inc, use instead of $GLOBALS['drupal_hash_salt'] should go in first.

Here's an updated version of the previous patch which fixes a couple issues. However, I don't see how this is appreciably different than using $drupal_hash_salt if the user follows the recommendation for defining $drupal_hash_salt ("for enhanced security"): http://drupalcode.org/project/drupal.git/blob/refs/heads/7.x:/sites/defa...

@pdrake - the security isn't really different than following the suggestion in settings.php - the point is that the code generating this HMAC cannot determine whether the salt value was set is such a secure way. So, we need some other flag or condition.

Yes, we could e.g. make it a simple boolean that indicates we have this secure generation, but that would seem likely to invite mistakes when people set it and then degrade their security becuase the hash salt is visible e.g. in settings.php on github.

Ok, so this code would force the shared secret to at minimum exist in a separate file, which is good. That said, there's no reason this couldn't be equally insecure (such as using a file that is committed along with the code) or even more-so (such as using a common file like README.txt or a file loadable via apache such as /var/www/html/shared-secret.txt).

Here's a re-roll of the patch in #12 against the latest 7.x.

We don't support running phpunit tests against load balancers - it's best to run tests on an environment that doesn't have one. Closing as 'as designed'.