10 ms·

As it happens, Secure Transport (edit: on OS X) is open source. I just spent the last hour rummaging through source code ... It was later renamed to "libsecurit

As it happens, Secure Transport (edit: on OS X) is open source. I just spent the last hour rummaging through source code ... It was later renamed to "libsecurity_ssl" when it landed on iOS. It's been around since OS X launched. They also have a folder with about four dozen regression tests and a test app, not that I've had the chance to inspect either that closely. The tarballs are spread across two folders in the tarballs index: Security (look for multi-MB file sizes) and libsecurity_ssl. The 5-digit numbers appear to be sequential revision IDs, any single-digit numbers seem historical. (E.g. libsecurity_ssl-6 is before libsecurity_ssl-10 rather than after libsecurity_ssl-55471.) It appears they haven't posted newer source than this. The most recent timestamp I could find was Oct 11, 2013 in 55471, which corresponds to 7.0 and my 10.9 system has the same version number for Security.framework -- same bundle version of 55471 for 10.9.1 aka 13A581. Previous version numbers don't appear to be as well-maintained. I don't expect a newer release to be posted until the next OS X release, as the source was only published under 10.9, not iOS. Additionally, there's no mention of iOS 7 on http://www.opensource.apple.com/ http://www.opensource.apple.com/ I couldn't easily find the bug without more to go on, because the code is spread across a few components and really, I'm not an expert in TLS. It appears to have been largely unchanged from 2000-2006 or so. TLS 1.2 brought quite a few changes, but it was neat to browse through the lines of "FIXME" and "TODO" comments, as well as various diffs between releases. And neat to see how much code today still goes back to 1999-2001, sometimes all they did was add a 'k' in front of a few variable names or delete the line in the first README saying the server code wasn't tested against Windows ;-) It sounds like when 10.9.2 is released, or at worst when 10.10 comes along, you'll see a new push of code to the opensource site. We can all diff 55471 against what comes next to see the changes. (If someone's already running 10.9.2 and its unaffected by the bug exhibited via curl, open /System/Library/Frameworks/Security.framework/Versions/Current/Resources/Info.plist and post the Bundle version.)

pencilo 13y ago
Take a look at http://opensource.apple.com/source/Security/Security-55471/libsecurity_ssl/lib/sslKeyExchange.c?txt http://opensource.apple.com/source/Security/Security-55471/l... specifically check the function SSLVerifySignedServerKeyExchange I leave the joy of spotting it to you. It is obvious and if you know c you'll see it(You don't need any knowledge of crypto).
- Someone 13y ago
  For the likely error I spot, you don't even need To know C. Anything remotely algol-like Will do. I wonder whether the software from the guys at viva64.com would spot that.
  - pencilo 13y ago
    You don't need fancy software, compilers warn about this kind of thing.
    - d0 13y ago
      Just eyeballs and a clue stick! However I can't see a valid case for this pattern so static analyser rule to knock it on the head would be a good idea.
      - Someone 13y ago
        Oops. I was doing this from an iPad, and iOS Safari is extremely bad at rendering source code (it's plain text, so it wraps it. It also chooses an incredibly large font size, so lines wrap at 50 characters or so in portrait mode. That made me bail out early, after reading this and not spotting the SRVR = SERVER abbreviation: clientRandom.data = ctx->clientRandom; clientRandom.length = SSL_CLIENT_SRVR_RAND_SIZE; serverRandom.data = ctx->serverRandom; serverRandom.length = SSL_CLIENT_SRVR_RAND_SIZE; That is something that I think static analysis tools could signal. It would be a red herring, though.
- mau 13y ago
  Don't C compilers warn about code that is never executed? I'm actually pretty impressed how this bug was not caught before. The compiler warning is just one thing that came to my mind but test coverage should have been the strongest hint. A linter could have also make the thing easier for a code review. Probably these parts of the code should have stricter coding guideline.
  - 13y ago

pencilo 13y ago

Take a look at http://opensource.apple.com/source/Security/Security-55471/libsecurity_ssl/lib/sslKeyExchange.c?txt http://opensource.apple.com/source/Security/Security-55471/l... specifically check the function SSLVerifySignedServerKeyExchange I leave the joy of spotting it to you. It is obvious and if you know c you'll see it(You don't need any knowledge of crypto).

Someone 13y ago
For the likely error I spot, you don't even need To know C. Anything remotely algol-like Will do. I wonder whether the software from the guys at viva64.com would spot that.
- pencilo 13y ago
  You don't need fancy software, compilers warn about this kind of thing.
  - d0 13y ago
    Just eyeballs and a clue stick! However I can't see a valid case for this pattern so static analyser rule to knock it on the head would be a good idea.
    - Someone 13y ago
      Oops. I was doing this from an iPad, and iOS Safari is extremely bad at rendering source code (it's plain text, so it wraps it. It also chooses an incredibly large font size, so lines wrap at 50 characters or so in portrait mode. That made me bail out early, after reading this and not spotting the SRVR = SERVER abbreviation: clientRandom.data = ctx->clientRandom; clientRandom.length = SSL_CLIENT_SRVR_RAND_SIZE; serverRandom.data = ctx->serverRandom; serverRandom.length = SSL_CLIENT_SRVR_RAND_SIZE; That is something that I think static analysis tools could signal. It would be a red herring, though.
mau 13y ago
Don't C compilers warn about code that is never executed? I'm actually pretty impressed how this bug was not caught before. The compiler warning is just one thing that came to my mind but test coverage should have been the strongest hint. A linter could have also make the thing easier for a code review. Probably these parts of the code should have stricter coding guideline.
- 13y ago