Re: Is crypto developed the way it should?

[Michael Kjörling <michael@kjorling.se>]

On 3 Aug 2014 17:16 +0200, from lazyvirus@gmx.com (Bzzzz):
> * Is C a good candidate to write crypto?
>    NOT AT ALL, a _very strict_ language should be used instead,
>    such as ADA (think contracts, and do not think it is slow).
>    Programs have bugs, we all know that, but crypto bugs are
>    the most terrible (for millions of people and for freedom)
>    (when they are really bugs…)

Bugs can happen in any language. Certain languages make certain
classes of bugs less likely. Techniques like static checking, strict
style guidelines and unit testing can further reduce the number of
bugs that slips through, and are available for many mainstream
languages. Many techniques to reduce bugs can be employed with almost
any language; some languages require certain such techniques by
design.

Even something as simple as requiring _all_ possible-block-statements
to be surrounded by the language's block opening and closing markers,
could easily have prevented a bug like the Apple "goto fail;" error at
least from going unspotted (that's CVE-2014-1266), because it would
have stood out like a sore thumb. That's not even a technical problem,
it's simply a code style issue, and as such can be addressed by mere
decree from a person within the company with the power to enforce such
things. Compare these two, borrowing from [1]:

     . . .
     hashOut.data = hashes + SSL_MD5_DIGEST_LEN;
     hashOut.length = SSL_SHA1_DIGEST_LEN;
     if ((err = SSLFreeBuffer(&hashCtx)) != 0)
         goto fail;
     if ((err = ReadyHash(&SSLHashSHA1, &hashCtx)) != 0)
         goto fail;
     if ((err = SSLHashSHA1.update(&hashCtx, &clientRandom)) != 0)
         goto fail;
     if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
         goto fail;
     if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
         goto fail;
         goto fail;
     if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
         goto fail;

     err = sslRawVerify(...);
     . . .

with the bug, and this:

     . . .
     hashOut.data = hashes + SSL_MD5_DIGEST_LEN;
     hashOut.length = SSL_SHA1_DIGEST_LEN;
     if ((err = SSLFreeBuffer(&hashCtx)) != 0)
     {
         goto fail;
     }
     if ((err = ReadyHash(&SSLHashSHA1, &hashCtx)) != 0)
     {
         goto fail;
     }
     if ((err = SSLHashSHA1.update(&hashCtx, &clientRandom)) != 0)
     {
         goto fail;
     }
     if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
     {
         goto fail;
     }
     if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
     {
         goto fail;
     }
         goto fail;
     if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
     {
         goto fail;
     }

     err = sslRawVerify(...);
     . . .

which still has the bug. See how the errant "goto" stands out a lot
more in the second snippet? Putting the second "goto fail;" inside the
braces would have led to an absolute non-issue. Even if you indent it
properly (in line with the "if" statements), it still stands out
because it breaks the pattern.

Compiling with "treat warnings as errors" or the specific compiler's
counterpart can also often uncover mistakes like these. A compiler
that can't generate an "unreachable code" warning for the above (the
"if(...final())" call) should probably be patched such that it can;
detecting such an error certainly shouldn't be rocket science.

And of course, even a perfect programming language with all sorts of
checks and balances does you no good if the code as written perfectly
implements a flawed or potentially flawed _algorithm_. Dual_EC_DRBG,
anyone?

[1] http://nakedsecurity.sophos.com/2014/02/24/anatomy-of-a-goto-fail-apples-ssl-bug-explained-plus-an-unofficial-patch/

-- 
Michael Kjörling • https://michael.kjorling.se • michael@kjorling.se
OpenPGP B501AC6429EF4514 https://michael.kjorling.se/public-keys/pgp
                 “People who think they know everything really annoy
                 those of us who know we don’t.” (Bjarne Stroustrup)