Apple’s SSL/TLS Bug
The SSLVerifySignedServerKeyExchange function in libsecurity_ssl/lib/sslKeyExchange.c in the Secure Transport feature in the Data Security component in Apple iOS 6.x before 6.1.6 and 7.x before 7.0.6, Apple TV 6.x before 6.0.2, and Apple OS X 10.9.x before 10.9.2 does not check the signature in a TLS Server Key Exchange message, which allows man-in-the-middle attackers to spoof SSL servers by (1) using an arbitrary private key for the signing step or (2) omitting the signing step.
This signature verification is checking the signature in a ServerKeyExchange message. This is used in DHE and ECDHE ciphersuites to communicate the ephemeral key for the connection. The server is saying “here's the ephemeral key and here's a signature, from my certificate, so you know that it's from me”. Now, if the link between the ephemeral key and the certificate chain is broken, then everything falls apart. It's possible to send a correct certificate chain to the client, but sign the handshake with the wrong private key, or not sign it at all! There's no proof that the server possesses the private key matching the public key in its certificate.
[…]
If I compile with -Wall (enable all warnings), neither GCC 4.8.2 or Clang 3.3 from Xcode make a peep about the dead code. That's surprising to me. A better warning could have stopped this but perhaps the false positive rate is too high over real codebases? (Thanks to Peter Nelson for pointing out the Clang does have -Wunreachable-code to warn about this, but it's not in -Wall.)
John Gruber on the NSA angle:
These three facts prove nothing; it’s purely circumstantial. But the shoe fits.
You can test whether your device is affected at gotofail.com or imperialviolet.org:1266. At this writing, Mac OS X 10.9, including current seeds, is still vulnerable. iOS 5 and Mac OS X 10.8 never had the bug. It’s fixed in iOS 6.1.6 and iOS 7.0.6:
Secure Transport failed to validate the authenticity of the connection. This issue was addressed by restoring missing validation steps.
The offending line of code is a single extra goto in SSLVerifySignedServerKeyExchange(). In my view, this is not an improper use of goto. The code follows a standard C error handling style. I’m also unpersuaded by the argument that the bug should be blamed on brace format preferences.
Any of us could have written a bug like this, especially when merging changes from different sources. But a flaw in process is what let the bug ship. If ever there were code that should be unit tested, it’s Secure Transport. Landon Fuller shows that it would have been easy to write a test to detect this regression.
Update (2014-02-24): Lloyd Chambers:
This one is unforgiveable. It could have compromised interactions with tens of millions of devices, had hackers exploited it (have they?), and that fact remains true for some time to come because plenty of people won’t update their devices and OS X doesn’t even have a fix as this was written.
You just don’t break a core security protocol like this. Who is in charge over there? Test suites should validate such stuff; it’s not exactly a new protocol. Heads ought to roll on this one and right up to high levels perhaps.
He and Chris Breen suggest that using Firefox or Chrome may be safer than Safari.
Update (2014-02-25): Macworld:
In addition, you may be able to potentially save your traffic from prying eyes with a VPN (Virtual Private Network). Although the VPN hooks into the security framework where the SSL/TLS bug exists, the VPN protocols supported by OS X don’t directly use SSL. You’ll need to check with your network administrator to make sure all your traffic runs through the VPN, however, and it’s not just site-specific (as some work-related VPNs can be).
The bug is fixed in Mac OS X 10.9.2.
Update (2014-05-12): Martin Fowler:
Also, the Security-55471 version of ssl_regressions.h, which appears to list a number of SSL regression tests for this library, remains unchanged in the Security-55471.14 version of ssl_regressions.h. The only substantial difference between the two versions of the library is the deletion of the
goto failstatement itself, with no added tests or eliminated duplication.[…]
The presence of six separate copies of the same algorithm clearly indicates that this bug was not due to a one-time programmer error: This was a pattern. This is evidence of a development culture that tolerates duplicated, untested code.
4 Comments RSS · Twitter
I agree that this should have been caught by a unit test. I disagree that this wouldn't have been simpler to spot if proper brace style was in use in this code.
Also, if it's not a simple merge error, it could easily be someone who programs in both Python and C context-switching between the languages and slipping up. Another reason to avoid Python like the plague.
