Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Apple's SSL/TLS bug

imperialviolet.org

Post reply on HN
Print view View original

151–160 of 295 posts

Re: Apple's SSL/TLS bug

#151

Post by thrownaway2424 »

The code is crap and while I can understand why a single person might have written it this way, I think any organization of full-time professional software developers should be collectively embarrassed to have accepted it. The only reason to use the "goto fail;" idiom is to free two buffers before returning. But the buffers in question are just sslBuffer structs that live on the local stack. Their destructors will be called, in reverse order, regardless of how the function is exited. If this code simply had a fucking destructor for sslBuffer to delete the data pointer, none of the rest of these call sites for SSLFreeBuffer would need to exist at all! And there's 24 such call sites in this file alone. Anyone with any sense would chose the 2-line destructor over dozens of calls to free buffers.

There's even this bullshit:

    if ((err = SSLFreeBuffer(&hashCtx)) != 0)
        goto fail;
Which is hilarious, because the only way for SSLFreeBuffer to fail is if the buffer doesn't exist, but hashCtx is a local temporary object and it MUST exist. Anyway the only thing we do by jumping to fail is to free the non-existent object once again.

In short: wow.

Re: Apple's SSL/TLS bug

#152

Post by gsnedders »

lisper wrote: post #142
rdl wrote: post #54

Unfortunately OSX does not appear patched even in the latest developer center 10.9.2 build (13C62). Tested in both Safari and OS-distributed curl. Chrome/Firefox of course is still fine since it uses the NSS stuff, but plenty of OS services use the OS crypto. (I'm violating NDA by commenting on pre-release Apple stuff, of course.) Windows or ubuntu bootcamp until they fix this, I think.

Since the source code is available, might it be possible to produce a hot patch to the binary so that those of us running Mavericks won't have to go through the next few days or weeks with our pants down? It would be a simple matter of finding the JMP instruction generated by the second GOTO and replacing it with no-ops. How hard could it be,at least for someone who actually knows their way around OS X binary file fo…

Likely the compiler will have optimized away the rest of the function as dead-code, so it's not as simple as just getting rid of the unconditional jump.

Re: Apple's SSL/TLS bug

#153

Post by Jayschwa »

jcurbo wrote: post #76
badman_ting wrote: post #3

If blocks without curly braces ಠ_ಠ

Funny thing is, I was finally looking at Rust the other day and thought, hey, that's nice that if statements are forced to have braces, I never liked one-liner if statements in C. And here we have a perfect example of what can happen with the brace-less if statements.

Yeah, it seems like this is something many newer languages are requiring, which is good. Go requires braces as well.

Re: Apple's SSL/TLS bug

#154

Post by lisper »

gsnedders wrote: post #152
lisper wrote: post #142

Earlier quoted context omitted.

Since the source code is available, might it be possible to produce a hot patch to the binary so that those of us running Mavericks won't have to go through the next few days or weeks with our pants down? It would be a simple matter of finding the JMP instruction generated by the second GOTO and replacing it with no-ops. How hard could it be,at least for someone who actually knows their way around OS X binary file fo…

Likely the compiler will have optimized away the rest of the function as dead-code, so it's not as simple as just getting rid of the unconditional jump.

Ah, right. Good point.

Re: Apple's SSL/TLS bug

#155

Post by _pmf_ »

thrownaway2424 wrote: post #151

The code is crap and while I can understand why a single person might have written it this way, I think any organization of full-time professional software developers should be collectively embarrassed to have accepted it. The only reason to use the "goto fail;" idiom is to free two buffers before returning. But the buffers in question are just sslBuffer structs that live on the local stack. Their destructors will be…

> Their destructors will be called, in reverse order, regardless of how the function is exited.

Not in C.

Re: Apple's SSL/TLS bug

#156

Post by sosborn »

rdl wrote: post #39
higherpurpose wrote: post #34

Earlier quoted context omitted.

Whether it was intentional backdoor or not, NSA must've had a field day with it by now.

In general, NSA is not my most serious threat (in terms of actual harm; they are the most powerful by far though) -- I agree they overreach, and for some people are a serious threat, but for me my primary concerns are Chinese/other foreign intel (who are documented as going after industrial/economic material much more than NSA) and independent/criminal/etc. types. The scary thing is the bar is so low; even I could tu…

> NSA is not my most serious threat

I wish more people understood this. Not to let NSA off the hook (what they are doing is awful), but the threat posed by the NSA is a higher-level down the road/slippery slope threat.

There are other more immediate and real dangers out there that are actively trying to steal whatever they can find.

Re: Apple's SSL/TLS bug

#157

Post by fhd2 »

fidotron wrote: post #9
badman_ting wrote: post #3

If blocks without curly braces ಠ_ಠ

Instant code review fail in my book too. Outside of python, for obvious reasons, it should never be allowed. The slightly nuts conditionals in Erlang are justifiable for avoiding exactly this kind of problem.

Agreed. Mandatory curlies might have helped avoid disaster here, but the code in question is still utter rubbish. If blatant nonsense makes it past review (or if there is no review), no coding style rule in the world is going to help.

Re: Apple's SSL/TLS bug

#158

Post by Sanddancer »

atmosx wrote: post #96
modeless wrote: post #87

Earlier quoted context omitted.

MITM of any SSL connection in Safari and other system apps, and they couldn't even bother to have an OS X patch ready at the same time as the disclosure for iOS? I think anyone relying on the security of OS X is going to have to seriously rethink their OS choice after this.

Indeed the only secure OS nowadays is Linux. Everything else should be considered compromised by default a priori.

I think the various BSD folks, especially those from the OpenBSD team, would like to have a word with you. There are many other unixes out there that put a strong, probably even stronger than linux, emphasis on security.

Re: Apple's SSL/TLS bug

#159

Post by _pmf_ »

apaprocki wrote: post #23

Worth noting that static analysis finds bugs like this immediately. TLS code seems like the perfect candidate to run through static analysis on every checkin. There are products such as Coverity and PVS-Studio that would have immediately flagged this and probably some open-source ones built around LLVM as well (unsure about this one, though). I personally use Coverity and have it hooked up in the same way everyone co…

> Worth noting that static analysis finds bugs like this immediately.

I'd like to point out the case where Debian maintainers "fixed" a "bug" discovered by static analysis; https://blog.isotoma.com/2008/05/debians-openssl-disaster/.

Re: Apple's SSL/TLS bug

#160

Post by thrownaway2424 »

_pmf_ wrote: post #155
thrownaway2424 wrote: post #151

The code is crap and while I can understand why a single person might have written it this way, I think any organization of full-time professional software developers should be collectively embarrassed to have accepted it. The only reason to use the "goto fail;" idiom is to free two buffers before returning. But the buffers in question are just sslBuffer structs that live on the local stack. Their destructors will be…

> Their destructors will be called, in reverse order, regardless of how the function is exited. Not in C.

Yes, one makes a conscious choice of implementation language. They aren't dictated by the laws of physics. Choosing your implementation language is a rather important step.
Post reply on HN