See #594412: Correctly label all site-owning super-admin permissions for some background discussion, but splitting it off into its own issue since it could use some dedicated discussion in the SimpleTest queue.

We are trying to only put the security warning label on Drupal core permissions which truly should be restricted to trusted users and which, by design, allow a site to be taken over. The "Administer tests" permission currently has the warning on it (which dates back to when the module was first committed to core, as far as I can tell from CVS history). However, it's not clear to me that there's any way in which it is designed to allow someone to take over a site. If I'm right, the warning should be removed.

Is there anything I'm missing that would invalidate that? (Note #799932: Simpletest HTTP authentication credentials should use the 'password' form element is somewhat related, although that's definitely not the reason this permission got that warning originally, plus it isn't really a security risk by itself.)

CommentFileSizeAuthor
#18 interdiff.txt1.05 KBmile23
#18 799936_18.patch1.42 KBmile23
#14 799936_14.patch1.33 KBmile23
#2 799936-simpletest-restrict-access.patch406 bytesbrianV
#1 simpletest-permission-warning-799936-1.patch583 bytesDavid_Rothstein

Comments

David_Rothstein’s picture

David_Rothstein commented
Status: Active » Needs review
StatusFileSize
new simpletest-permission-warning-799936-1.patch583 bytes

Here's the patch for review - the code itself is pretty trivial :)

brianV’s picture

brianV commented
Version: 7.x-dev » 8.x-dev
StatusFileSize
new 799936-simpletest-restrict-access.patch406 bytes

I agree...

Perhaps someone thought that this should be restricted as running a battery of tests on a live site would be detrimental to performance, but I don't that's a security issue...

Rerolled for D8.

devin carlson’s picture

devin carlson commented
Status: Needs review » Reviewed & tested by the community

Everything looks good; having access to run tests shouldn't compromise site security. The patch still applies cleanly.

This should be committed to both D8 and D7.

dries’s picture

dries commented

I agree with this too. Any objections?

catch’s picture

catch
he/him
Primary language English
commented

Seems fine to me. It is more of a DOS issue than a security issue and there are other things people could do for that. As well as it being a bit strange to enable simpletest on a live site at all in the first place.

Bojhan’s picture

Bojhan commented

Yhea, I have no idea why this error is there - lets remove it!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Discussed this a bit in irc with chx and boombatower.

* we should go over the simpletest settings screen to make sure there's no XSS or similar in there. Having this warning means we might have been sloppy on that screen, and it's the difference between an SA or not.

* if we remove this warning, could we add another warning somewhere to make it really, really clear that you should never enable simpletest on a production site.

boombatower’s picture

boombatower commented
Status: Needs work » Reviewed & tested by the community

sub

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Looks like a cross post.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
Status: Needs work » Needs review
Related issues: +#2830880: Warn site admins when composer dev dependencies are installed inside of docroot, +#2866082: [Plan] Roadmap for Simpletest
StatusFileSize
new 799936_14.patch1.33 KB

Rerolled (really re-done).

Added a report to the status page saying "turn off simpletest, please," or words to that effect, per #7. Part of the reason users shouldn't have simpletest enabled is because it has composer dev requirements as a dependency.

We could combine #7 with #2830880: Warn site admins when composer dev dependencies are installed inside of docroot but it's also true that the user could have dev requirements installed but not simpletest.

#7 also says:

* we should go over the simpletest settings screen to make sure there's no XSS or similar in there. Having this warning means we might have been sloppy on that screen, and it's the difference between an SA or not.

Probably a good idea, but that looks a lot like maintenance, and we don't really want to do that because we're deprecating simpletest. #2866082: [Plan] Roadmap for Simpletest

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

I agree that just adding the warning is sufficient. I'm not sure if we should add a test just for this warning - I think it's sufficiently trivial.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

That was quick. Thanks.

And... You know... Good luck testing whether the simpletest module is enabled or not. :-)

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/modules/system/system.install
@@ -74,6 +74,19 @@ function system_requirements($phase) {
+      'description' => t('The Testing/simpletest module should not be enabled on production sites, because it represents a security risk.'),

    Should this just say 'Testing' module? 

  2. +++ b/core/modules/system/system.install
@@ -74,6 +74,19 @@ function system_requirements($phase) {
+    if (array_key_exists('simpletest', $enabled_modules)) {

    This can just be isset(), the values of array are always Extension objects.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs work » Needs review
StatusFileSize
new 799936_18.patch1.42 KB
new interdiff.txt1.05 KB

Done and done. Also, it now links to https://www.drupal.org/docs/8/security/secure-configuration-for-site-bui... which I updated to talk a little bit about simpletest and composer install --no-dev.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Reviewed & tested by the community

Changes requested in #17 are done in #18, back to RTBC.

David_Rothstein’s picture

David_Rothstein commented 
  1. +      'description' => t('The Testing module should not be enabled on production sites, because it represents a <a href=":url">security risk</a>.', [':url' => 'https://www.drupal.org/docs/8/security/secure-configuration-for-site-builders']),

    I think this is misleading, because it implies that turning off the Simpletest module will remove the security risk. But if the actual concern is dev dependencies in the codebase, then turning off the module won't do anything to fix that, right? In other words, it seems like the actual warning that's needed is #2830880: Warn site admins when composer dev dependencies are installed inside of docroot, not this one.

  2. I have similar comments about the changes to https://www.drupal.org/docs/8/security/secure-configuration-for-site-bui... - if the actual security issue is composer dev dependencies, the text should focus on that, with "don't turn simpletest on in production" as a corollary. And for this part:

    Disable, or don't enable the Testing (simpletest) module.. If some users have permission to run tests, they could maliciously run them over and over.

    Although that would certainly be annoying and a performance concern if it were to happen, I don't see how it's a specific security risk.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

quietone commented
Project: Drupal core » SimpleTest
Version: 8.9.x-dev » 8.x-3.x-dev
Component: simpletest.module » Code

Triaging issues in simpletest.module as part of the Bug Smash Initiative to determine if they should be in the Simpletest Project or core.

This looks like it belongs in the Simpletest project.