coreinfrastructure / best-practices-badge

🏆Open Source Security Foundation (OpenSSF) Best Practices Badge (formerly Core Infrastructure Initiative (CII) Best Practices Badge)
https://www.bestpractices.dev
MIT License
1.21k stars 203 forks source link

Add negative testing to the criteria (probably silver) #1127

Open david-a-wheeler opened 6 years ago

david-a-wheeler commented 6 years ago

Negative testing is the use of tests to ensure that requests are rejected if they should be rejected. It's important for security, e.g., it would have immediately revealed the Apple goto fail vulnerability and if done thoroughly it would have revealed Heatbleed. Negative testing is also listed in the Software SOAR that Amy Henninger and I developed. Unsurprisingly, if you don't test for it, you're likely to not get it.

So I propose adding something like this as a criterion:

CRITERION:

If the software produced by the project includes a login function, certificate validation, access requiring authentication, access requiring special authorization such as "admin" or "root", or a similar security check, then the automated test suite MUST include tests in each of those cases to check that requests are rejected when they should be rejected. This criterion applies EVEN IF the software reuses third-party software to implement this functionality (to ensure that third-party software is installed and configured correctly). [negative_testing]

DETAILS: For example, automated tests should ensure that the following are rejected (where applicable): blank/invalid password, certificates with wrong hostname or lacking an acceptable certificate chain, requests that require login without logging in, and requests for an admin action by a non-admin user. Many test suites check that requests that should work actually work, but fail to include tests that requests that should fail actually fail. This is especially important for security, because in security, it's often the rejection that is more important. Testing to ensure that what should be rejected is actually rejected is sometimes called "negative testing". If negative testing is not part of the automated test suite, there is a significant risk that changes over time will accidentally disable important security measures. One example is the Apple goto fail vulnerability. "The most dangerous code in the world: validating SSL certificates in non-browser software" by M. Georgiev et al found that "SSL certificate validation is completely broken in many security-critical applications and libraries".

Unfortunately, approaches like Test-Driven Development (TDD), as practiced, often lead people to fail to test for rejections. Most material on TDD never mentions negative testing, and naively executing "add a test that should fail, then modify to succeed" doesn't work because the initial test would often pass for the wrong reason. I think that's an especially good reason to add a negative testing criterion - it's not that TDD is bad, it's that this is often a blind spot in TDD as it is actually practiced.

Note that we merely require some tests in the test suite; systems may be badly configured, and there's always another test that could be added. Like anything else, passing tests only show that those tests pass in the test environment; other environments may produce a different result. But if there are some tests, many mistakes that can happen in later maintenance will be automatically detected before they get released to harm others. I think doing thorough negative testing, on every request, would be even better, but that may be a bridge too far. We could add a higher-level criterion, e.g., "thorough_negative_testing", to require that negative tests be created for multiple kinds of certificate failures and for every type of request.

Comments welcome.

david-a-wheeler commented 6 years ago

@jdossett - this is the proposed criterion I mentioned to you.