Seriously Apple? Seriously? GOTOs? In your Secure-Socket Layer implementation? What were they thinking?
Apple, Apple! When Ed Post wrote in 1983 that Real Programmers aren’t afraid to use GOTOs he was kidding!
No one should ever use go-to statements in any program. As programming genius Edsger W. Dijkstra put it, all the way back in 1968, “goto statement should be abolished from all ‘higher level’ programming languages.” And by higher-level, he meant anything from Assembly language on up!
Why did he say this? Because the goto statement as it stands is just too primitive: “It is too much an invitation to make a mess of one’s program,” wrote Dijkstra. Which, of course, is exactly what Apple did.
In case you don’t follow security or the vagaries of Apple operating system programming, here’s what happened.
On February 21, Apple released a new security update for late model iPhones, iPod touch devices, and iPads. This was to fix a situation where “An attacker with a privileged network position may capture or modify data in sessions protected by SSL/TLS Secure Sockets Layer/Transport Layer Security.”
Anytime someone can break SSL/TLS, the underpinnings of almost all Web-based security, it’s bad news. It turns out that the breech enabled anyone with a certificate signed by a “trusted CA” to do a MITM attack (MITM). With an SSL/TLS MITM, the attacker controls the horizontal and the vertical, a la the Outer Limits. Heck, they control everything including your credit-card numbers.
Worse news was to come: The same security hole was in Mac OS X. Aldo Cortesi, CEO of the security company Nullcube, revealed that he had modified the MITM proxy program mitmproxy so that he was able to confirm “full transparent interception of HTTPS traffic on both IOS (prior to 7.0.6) and OSX Mavericks.”
What was the root problem of this fatal TLS/SSL flaw? Adam Langley, Google Senior Staff Software Engineer and HTTPS specialist, took what little Apple had to say about the problem and it didn’t take him long to find it. See how long it takes:
static OSStatus SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams, uint8_t *signature, UInt16 signatureLen) { OSStatus err; … 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; … fail: SSLFreeBuffer(&signedHashes); SSLFreeBuffer(&hashCtx); return err; }Yeah, that doubled “goto fail” kind of jumps out at you doesn’t it?
As Langley explains, “Note the two goto fail lines in a row. The first one is correctly bound to the if statement but the second, despite the indentation, isn’t conditional at all. The code will always jump to the end from that second goto, err will contain a successful value because the SHA1 update operation was successful and so the signature verification will never fail.”
Or, in other words, if intercepted these “secure” connections can always be broken. That single line of code, since it isn’t a conditional, causes the entire procedure to terminate. It wrecks the security routine’s entire purpose.
Langley kindly says, “This sort of subtle bug deep in the code is a nightmare. I believe that it’s just a mistake and I feel very bad for whoever might have slipped in an editor and created it.”
I’m not so forgiving. This code is at the heart of Mac OS X and iOS Web security. And, what may I ask, are those goto statements doing there in the first place!?
Where was the code review? Apple doesn’t appear to practice even basic code review not just for one of its operating systems but both of them. How can programmers for anything but the most trivial work not do this?!
Yeah, I get it. Code review can be time-consuming. So what? This is the most mission-critical code of mission-critical code for an Internet-connected device.
Besides, code review doesn’t have to eat up man-hours. Tool-based code reviews are much faster than having someone looking over your shoulder. It’s also a heck of a lot more through about spotting the bad lines than even the most expert programmer.
Even if that unconscionable coding error wasn’t caught in code review, how in the world did it ever get past quality assurance? How hard could it be to notice that all of Apple’s family with the newest operating system never met a SSL/TLS site it wouldn’t shake hands with!?
Indeed this is such a fundamentally stupid mistake that the security pro’s pro, Bruce Schneier, thinks it’s possible that the security hole was deliberately introduced. Schneier says, “Was this done on purpose? I have no idea. But if I wanted to do something like this on purpose, this is exactly how I would do it.” He adds, “It would have been trivially easy for one person to add the vulnerability.”
If it was anyone except Schneier I’d think they’re ready to join the tin-foil hat club, but I have to take him seriously.
That said, good code review and even basic quality assurance should have caught this problem. Some people have suggested that Apple’s goto failure shows a programming staff that needs a massive culture change to fix. I must agree.
The lesson here is clear. Code review is vital. Bad coding practices—and they don’t get much worse than using gotos—have to be found and eradicated. Even if the code itself works and isn’t buggy, bad code practicing is just asking for trouble down the road.
Apple needs to get its programming, code review, and quality assurance acts together. If you have even the faintest fear that your company’s developer team may have similar problem you must get on top of these issues now. Apple’s reputation is strong enough that it can handle even such complete cluster-foul ups as this one and stay in business. Your company may not be so lucky.
GOTO (Still) Considered Harmful was first published in Smart Bear.