-
-
Save JensAyton/9161071 to your computer and use it in GitHub Desktop.
One example of cleanup without using goto. (This isn't an opinion on what Apple “should have” done, it’s just an answer to a specific question on Twitter. It would have done nothing to avoid the actual bug.) I also removed the unused variable “signedHashes”.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| static OSStatus | |
| SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams, | |
| uint8_t *signature, UInt16 signatureLen) | |
| { | |
| SSLBuffer hashCtx; | |
| hashCtx.date = 0; | |
| OSStatus err = SSLVerifySignedServerKeyExchangeInner(ctx, isRsa, signedParams, signature, signatureLen, &hashCtx); | |
| SSLFreeBuffer(hashCtx); | |
| return err; | |
| } | |
| static OSStatus | |
| SSLVerifySignedServerKeyExchangeInner(SSLContext *ctx, bool isRsa, SSLBuffer signedParams, | |
| uint8_t *signature, UInt16 signatureLen, SSLBuffer *hashCtx) | |
| { | |
| OSStatus err; | |
| SSLBuffer hashOut, clientRandom, serverRandom; | |
| uint8_t hashes[SSL_SHA1_DIGEST_LEN + SSL_MD5_DIGEST_LEN]; | |
| uint8_t *dataToSign; | |
| size_t dataToSignLen; | |
| 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) | |
| return err; | |
| if ((err = SSLHashMD5.update(hashCtx, &clientRandom)) != 0) | |
| return err; | |
| if ((err = SSLHashMD5.update(hashCtx, &serverRandom)) != 0) | |
| return err; | |
| if ((err = SSLHashMD5.update(hashCtx, &signedParams)) != 0) | |
| return err; | |
| if ((err = SSLHashMD5.final(hashCtx, &hashOut)) != 0) | |
| return err; | |
| } | |
| 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) | |
| return err; | |
| if ((err = ReadyHash(&SSLHashSHA1, hashCtx)) != 0) | |
| return err; | |
| if ((err = SSLHashSHA1.update(hashCtx, &clientRandom)) != 0) | |
| return err; | |
| if ((err = SSLHashSHA1.update(hashCtx, &serverRandom)) != 0) | |
| return err; | |
| if ((err = SSLHashSHA1.update(hashCtx, &signedParams)) != 0) | |
| return err; | |
| if ((err = SSLHashSHA1.final(hashCtx, &hashOut)) != 0) | |
| return err; | |
| err = sslRawVerify(ctx, | |
| ctx->peerPubKey, | |
| dataToSign, /* plaintext */ | |
| dataToSignLen, /* plaintext length */ | |
| signature, | |
| signatureLen); | |
| if(err) { | |
| sslErrorLog("SSLDecodeSignedServerKeyExchange: sslRawVerify " | |
| "returned %d\n", (int)err); | |
| } | |
| return err; | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It’s worth noting that in this particular case, this approach adds no value except removing a keyword some people don’t like (which was the immediate goal). The code flow is no more or less clear.