Code audits are often ugly tasks and can sometimes find ugly things. Case in point: the GnuTLS goto bug.
Chief architect and Red Hat engineer Nikos Mavrogiannopoulos initiated a code audit of the open source crypto library that eventually turned up last week’s critical bug. The bad code has been present since 2005, meaning that for nearly a decade GnuTLS has not been properly verifying x.509 certificates by incorrectly handling certain errors and consequently incorrectly reporting some verifications as successful.
The upshot is that attackers with man-in-the-middle positioning could present a specially crafted certificate that would be accepted by GnuTLS giving them access to supposedly secure communication between parties.
“I was adding new features to the certificate validation procedure of GnuTLS. Then I noticed some issues and that was the point the full audit started,” Mavrogiannopoulos told Threatpost via email. “[The bug is] as serious as it could get.”
Veracode security researcher Melissa Elliott said the faulty code snippet in question is supposed to return either a true or false variable depending on whether the certificate is valid; this paradigm is called Boolean return code. The GnuTLS bug, however, returns specific error codes including some identified by negative numbers, each signifying something different, she said.
“The mistake was that when one of these functions returned an error, it would be treated as though it were Boolean without changing the actual number. Under Boolean rules, anything that is not a zero is ‘true,'” Elliott said. “Hence, an error meant to indicate failure would be passed up the chain as ‘true’ (no error) instead of ‘false’ (error).”
Mavrogiannopoulos said the error was his and has been present since version 1.0.0.
The wonky code has been fixed and patches made available. Reportedly there are anywhere between 200 and 350 open source software packages, including a number of flavors of Red Hat Enterprise Linux, Debian and Ubuntu that make use of GnuTLS as a crypto library. It’s no OpenSSL in terms of deployment, but regardless, it’s still worrisome to many that such a problem existed for so long.
“It is distressingly easy to accidentally write a bug like this. It does not cause anything to crash. Full-featured C compilers can warn you about this bug, but the false positive rate (that is, instances where it can’t possibly do any harm) is high enough that most programmers are inclined to ignore them,” said Elliott. “Unfortunately, this is security-sensitive code, so the consequences of missing the one important warning in a list of benign ones can be catastrophic.”
GnuTLS is a volunteer-driven project, Mavrogiannopoulos pointed out, meaning that code gets reviewed and patched and updated as manpower is available.
“However, due to this incident I received mail from people that were interested in doing code review,
so I’ve provided information to assist them (as an audit competition),” he said, providing additional details on the GnuTLS mailing list.
Experts such as cryptographer and Johns Hopkins professor Matthew Green said that there are an insufficient number of quality TLS code scanners available that could have helped catch this while in development.
“Clearly people need to run their TLS implementations through test harnesses and tools that may not exist yet,” Green said.
Veracode’s Elliott concurred.
“I think learning the lesson about code auditing is more important than fretting about the past exposure risk of this specific bug in this specific product,” Elliott said. “Any one library may have relatively few users but there are more bugs in more libraries, and cumulatively, all of us are exposed somewhere. Systematically flushing out the bugs will help all of us.”
Mavrogiannopoulos wrote in a separate post to the GnuTLS mailing list that the bug went unnoticed for so long because it cannot be detected by any certificate validation tests, including a certificate validation path suite developed for the Department of Defense and another one developed in-house, he said.
“That didn’t help with the issue either, because it requires a specially crafted certificate (and I’m not revealing more details on that yet),” Mavrogiannopoulos said, adding that the code audit was the only means available to catch something like this.
“As this code was on a critical part of the library it was touched and thus read, very rarely. Moreover, the code in question followed the usual form of error checking in the library ‘if(err<0) return err’, making it look correct, unless one would notice that the function returned a boolean value (and we have very few such functions in the library),” Mavrogiannopoulos said.
