Apple SSL Vulnerability Hide-and-Seek

If you haven't been living under a rock, you have probably heard of the recent Apple software snafu that fails to validate the authenticity of an SSL/TLS connection in iOS; see here. It was later found that this failure is the result of a bug in Apple's SSL implementation, and thus the exclusivity for iOS is lost as OS X joins the list of affected.

The common Apple user might question the ramifications of this finding. Matthew Green from JHUISI explains it best, "[A] hacker could impersonate a protected site and sit in the middle as email or financial data goes between the user and the real site," [cite].  If you're interested, I recommend taking a look at the CVE-2014-1266 (common vulnerabilities and exposures) on the National Vulnerability Database for more insight.

However, if you don't feel like reading or clicking, I'll give you the gist: there exists a function (SSLVerifySignedServerKeyExchange) in Apple's SSL library that does not properly check the signature in a TLS server key exchange message due to unintended block (we're talking scope in C now) parsing. Thanks to Apple's opensource website, we can play vulnerability hunter and see this for ourselves!

Visiting the source for sslKeyExchange.c, we can look for the function in question and start our analysis. The source:

static OSStatus
SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams,
                                 uint8_t *signature, UInt16 signatureLen)
{
    OSStatus        err;
    SSLBuffer       hashOut, hashCtx, clientRandom, serverRandom;
    uint8_t         hashes[SSL_SHA1_DIGEST_LEN + SSL_MD5_DIGEST_LEN];
    SSLBuffer       signedHashes;
    uint8_t   *dataToSign;
 size_t   dataToSignLen;

 signedHashes.data = 0;
    hashCtx.data = 0;

    clientRandom.data = ctx->clientRandom;
    clientRandom.length = SSL_CLIENT_SRVR_RAND_SIZE;
    serverRandom.data = ctx->serverRandom;
    serverRandom.length = SSL_CLIENT_SRVR_RAND_SIZE;


 if(isRsa) {
  /* skip this if signing with DSA */
  dataToSign = hashes;
  dataToSignLen = SSL_SHA1_DIGEST_LEN + SSL_MD5_DIGEST_LEN;
  hashOut.data = hashes;
  hashOut.length = SSL_MD5_DIGEST_LEN;
  
  if ((err = ReadyHash(&SSLHashMD5, &hashCtx)) != 0)
   goto fail;
  if ((err = SSLHashMD5.update(&hashCtx, &clientRandom)) != 0)
   goto fail;
  if ((err = SSLHashMD5.update(&hashCtx, &serverRandom)) != 0)
   goto fail;
  if ((err = SSLHashMD5.update(&hashCtx, &signedParams)) != 0)
   goto fail;
  if ((err = SSLHashMD5.final(&hashCtx, &hashOut)) != 0)
   goto fail;
 }
 else {
  /* DSA, ECDSA - just use the SHA1 hash */
  dataToSign = &hashes[SSL_MD5_DIGEST_LEN];
  dataToSignLen = SSL_SHA1_DIGEST_LEN;
 }

 hashOut.data = hashes + SSL_MD5_DIGEST_LEN;
    hashOut.length = SSL_SHA1_DIGEST_LEN;
    if ((err = SSLFreeBuffer(&hashCtx)) != 0)
        goto fail;

    if ((err = ReadyHash(&SSLHashSHA1, &hashCtx)) != 0)
        goto fail;
    if ((err = SSLHashSHA1.update(&hashCtx, &clientRandom)) != 0)
        goto fail;
    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;

 err = sslRawVerify(ctx,
                       ctx->peerPubKey,
                       dataToSign,    /* plaintext */
                       dataToSignLen,   /* plaintext length */
                       signature,
                       signatureLen);
 if(err) {
  sslErrorLog("SSLDecodeSignedServerKeyExchange: sslRawVerify "
                    "returned %d\n", (int)err);
  goto fail;
 }

fail:
    SSLFreeBuffer(&signedHashes);
    SSLFreeBuffer(&hashCtx);
    return err;

}

The contentious code:

    if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
        goto fail;
        goto fail;

The root of the problem is as follows: the C parser will parse the conditional if expression first, the statement after the condition, and then search for an else statement to repeat this for some k steps. Using curly braces would result in parsing the condition expression first, the statements between the curly braces, and repeat as above. Note that each control flow is a block that has its own scope; so that single statement after the condition is within the scope of the if and all statements in curly braces after the condition are within the scope of the if.

Unfortunately, it's not really agreed upon whether the practice of omitting curly braces is nothing more than a syntactical choice. If curly braces were used in this implementation, however, I would argue that the offending goto statement would have been within the scope of the if block and thus would have been ignored/mitigated/still looked ugly. Problem solved and no MITM attacks. As it is now, all statements fail because the second goto fail system is within the scope of the function SSLVerifySignedServerKeyExchange and not the if expression.

Thanks to my good friend Shivakumar Melmangalam for keeping me in the loop with respect to fun things. Also, if you're up for a laugh, see this and then this. Disagreements about the C parser or vulnerability? Light up the comments! You learn best when you get things wrong the first time.