Hacker News new | ask | show | jobs
by tialaramex 1524 days ago
This is the sort of dumb mistake that ought to get caught by unit testing. A junior, assigned the task of testing this feature, ought to see that in the cryptographic signature design these values are checked as not zero, try setting them to zero, and... watch it burn to the ground.

Except that, of course, people don't actually do unit testing, they're too busy.

Somebody is probably going to mention fuzz testing. But, if you're "too busy" to even write the unit tests for the software you're about to replace, you aren't going to fuzz test it are you?

3 comments

The issue is the assumption juniors should be writing the unit tests, sounds like you might be part of the problem.
I think I probably technically count as a junior in my current role, which is very amusing and "I don't write enough unit tests" was one of the things I wrote in the self-assessed annual review.

So, sure.

It’s more unit testing is everybodies job, especially complex cryptographic functions, which should really have at least two sets of eyes, or even two test case sets where each developer doesn’t see the other developers test cases to reduce the likelihood that positive bias may overlook missed tests.

But i say that as someone who regularly audits code with almost certainly no unit tests based on the quality of the applications, just one set would do me fine.

The point of fuzz testing is not having to think of test cases in the first place.
[Somebody had down-voted you when I saw this, but it wasn't me]

These aren't alternatives, they're complementary. I appreciate that fuzz testing makes sense over writing unit tests for weird edge cases, but "these parameters can't be zero" isn't an edge case, it's part of the basic design. Here's an example of what X9.62 says:

> If r’ is not an integer in the interval [1, n-1], then reject the signature.

Let's write a unit test to check say, zero here. Can we also use fuzz testing? Sure, why not. But lines like this ought to scream out for a unit test.

Right, I'm just saying: there's a logic that says fuzz tests are easier than specific test-cases: the people that run the fuzz tests barely need to understand the code at all, just the basic interface for verifying a signature.
You still need your tests to cover all possible errors (or at least all plausible errors). If you try random numbers and your prime happens to be close to a power of two, evenly distributed random numbers won't end up outside the [0,n-1] range you are supposed to validate. Even if your prime is far enough from a power of two, you still won't hit zero by chance (and you need to test zero, because you almost certainly need two separate pieces of code to reject the =0 and >=n cases).

Another example is Poly1305. When you look at the test vectors from RFC 8439, you notice that some are specially crafted to trigger overflows that random tests wouldn't stumble upon.

Thus, I would argue that proper testing requires some domain knowledge. Naive fuzz testing is bloody effective but it's not enough.

That’s all true, but fuzz testing is very effective at checking boundary conditions (near 0, near max/mins) and would have caught this particular problem easily.
Do you mean fuzz testing does not use even distributions? There’s a bias towards extrema, or at least some guarantee to test zero and MAX? I guess that would work.

Also, would you consider the following to be fuzz testing? https://github.com/LoupVaillant/Monocypher/blob/master/tests...

No, most fuzz testing frameworks I know of these days do not use even distributions. Most use even more sophisticated techniques such as instrumenting the code to detect when state transitions are triggered to try to maximize hitting all code paths in a program instead of repeatedly fuzzing the same path.
The usual trick is coverage-guided fuzzing. https://google.github.io/clusterfuzz/reference/coverage-guid...
If we write an automated test case for known acceptance criteria, and then write necessary and sufficient code to get those tests to pass, we would know what known acceptance criteria are being fulfilled. When someone else adds to the code and causes a test to fail, the test case and the specific acceptance criteria would thus help the developer understand intended behaviour (verify behaviour, review implementation). Thus, the test suite would become a catalogue of programmatically verifiable acceptance criteria.

Certainly, fuzz tests would help us test boundary conditions and more, but they are not a catalogue of known acceptance criteria.

While fuzz testing is good and all, when it comes to cryptography, the input spaces is so large that chances of finding something are even worse than finding a needle in a hay stack.

For instance here the keys are going to be around 256 bits in a size, so if your fuzzer is just picking keys at random, your basically never likely to pick zero at random.

With cryptographic primitives you really should be testing all known invalid input parameters for the particular algorithm. A a random fuzzer is not going to know that. Additionally, you should be testing inputs that can cause overflows and are handled correctly ect...

Yes, but here we're just looking for (0,0).
This is true in principle but in practice most fuzz testing frameworks demand a fair bit of setup. It’s worth it!

But if you are in a time constrained environment where basic unit tests are skipped fuzz testing will be as well.

Imaging being so senior you no longer need to write unit tests yourself, but just delegate them.

Sounds exactly like the kind of disconnected environment that would lead to such bugs.