[Cryptography] GnuTLS -- time to look at the diff.

[Nico Williams]

On Fri, Mar 7, 2014 at 12:55 AM, Watson Ladd <watsonbladd at gmail.com> wrote:
>> How would that have helped with either the Apple SSL or the GNUtls bugs?
>
> There is no reason why either function needed manual resource control.
> The absence of memory safety and nonexistence of a genuine bool type
> made both functions impossible to understand. Staying in C, applying
> the Power of Ten would have worked fine. But because C forces manual
> memory management cleanup gotos have become accepted style.

Memory management is not the only problem.  One could (and with
CoreFoundation, Apple engineers often do) resort to abstractions that
make memory management easy (ref-counted).  There is often more than
just memory management to take care of: you might need to record
detailed error information (more than just returning an error code)
somewhere, you might need to map error codes.  Whenever you have
repeated error-handling you want to apply the DRY principle, and
that's what goto failure helps with.

Consider some alternatives:

    ret = func(...);
    if (ret != success) {
        /* cleanup */
        ...
    }

    ret = ...
    if (ret != success) {
        /* cleanup */
        /* oops!  I'd better not forget to check that this does
everything that needed to be done, plus any additional cleanup work
needed since the previous cleanup section! */
        ...
    }

Yuck.   Really, that's what you want?  You don't think that's error-prone?  Or:

    if ((ret = ...) != success ||
        (ret = ...) != success ||
        ...) {
        /* single cleanup section; look ma'!  no gotos in sight!  no
needless repetition either */
        ...
    }

But now you have assignments in if conditions (which many people will
complain about; can't win eh).  And source-level debuggers don't make
stepping through this sort of code easy (but then, real programmers
don't use source-level debuggers, so maybe this is a non-issue).

> Correctness simply wasn't a priority.

On the contrary, clearly the concern about memory deallocation shows a
concern about correctness.  That they screwed up is not evidence that
they didn't care about correctness.

Goto isn't the problem.  Cargo cult can be a problem; never using
gotos is cargo cult (as opposed to not using them to write spaghetti
code, which is a different matter altogether).  Refexive goto-hate is
inconsistent unless you also apply it to callback-hell, or, really,
any LISP or Scheme or similar, because, really, lambdas are the
ultimate goto, and unsurprisingly you can write spaghetti code in just
about any Turing-complete language.  I put a premium on code
readability: if it's hard to read your code, it's that much easier for
me to miss a bug like this when reviewing it -- this is universally a
good rule of thumb, cutting across all languages.

Manual memory management also isn't the problem.  Sometimes you really
want manual memory management in security-critical components: because
you need to be able to characterize performance under load (think
DDoS), or because you want secrets wiped from memory as soon as
possible.  And sometimes you need it because you're working in a very
constrained environment (such as embedded devices).  Good engineering
can handle manual memory management.

The problem here was a lack of testing and possibly incomplete or
incompletely-followed software engineering processes (maybe also
deficient static analysis tools) (but I repeat myself).

Nico
--