由苹果的低级Bug想到的

jopen 11年前

　　2014 年 2 月 22 日，在这个“这么二”的日子里，苹果公司推送了 iOS 7.0.6（版本号 11B651）修复了 SSL 连接验证的一个 bug。官方网页在这里：http://support.apple.com/kb/HT6147，网页中如下描述：

Impact: An attacker with a privileged network position may capture or modify data in sessions protected by SSL/TLS

Description: Secure Transport failed to validate the authenticity of the connection. This issue was addressed by restoring missing validation steps.

　　也就是说，这个 bug 会引起中间人攻击，bug 的描述中说，这个问题是因为 miss 了对连接认证的合法性检查的步骤。

　　这里多说一句，一旦网上发生任何的和 SSL/TL 相关的 bug 或安全问题，不管是做为用户，还是做为程序员的你，你一定要高度重视起来。因为这个网络通信的加密协议被广泛的应用在很多很多最最需要安全的地方，如果 SSL/TLS 有问题的话，意味着这个世界的计算机安全体系的崩溃。

　　Bug 的代码原因

　　Adam Langley 的《Apple’s SSL/TLS bug 》的博文暴出了这个 bug 的细节。（在苹果的开源网站上，通过查看苹果的和 SSL/TLS 有关的代码变更，我们可以在文件 sslKeyExchange.c 中找到下面的代码）

 static OSStatus  SSLVerifySignedServerKeyExchange (SSLContext *ctx, bool isRsa, SSLBuffer signedParams,                                   uint8_t *signature, UInt16 signatureLen)  {      OSStatus        err;      ...         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;  }

　　注意，我高亮的地方，也就是那里有两个 goto fail; 因为 if 语句没有加大括号，所以，只有第一个 goto 是属于 if 的，而第二个 goto 则是永远都会被执行到的（注：这里不是 Python 是C语言，缩进不代表这个语句属于同一个语句块）。也就是说，就算是前面的 if 检查都失败了（err  == 0），也会 goto fail。我们可以看到 fail 标签中释放完内存后就会 return err;

　　你想一下，这段程序在 SSLHashSHA1.update ()  返回成功，也就是返回 0 的时候会发生什么样的事？是的，真正干活的 sslRawVerify ()被 bypass 了。而且这个函数 SSLVerifySignedServerKeyExchange () 还返回了0，也就是成功了！尼玛！你可能想到酷壳网上之前《一个空格引发的惨剧》的文章。都是低级 bug。

　　这个低级 bug 在这个周末在网上被炒翻了天，你可以上 Twiter 上看看#gotofail 的标签的盛况。Goto Fail 必然会成为历史上的一个经典事件。

　　如果你喜欢 XKCD，你一定会想到这个漫画：

f1.png

　　注意：这个 bug 不会影响 TLS 1.2 版本，因为 1.2 版本不会用这个函数，走的是另一套机制。但是别忘了 client 端是可以选择版本的。

　　如果你想测试一下你的浏览器是否会有问题，你可以上一下当天就上线的 https://gotofail.com 网站

　　一些思考

　　下面是我对这个问题的一些思考。

　　0）关于编译报警

　　有人在说苹果的这个代码中的 goto 语句会产生死代码——dead code，也就是永远都不会执行到的代码，C/C++的编程器是会报警的。但，实际上，dead code 在默认上的不会报警的。即使你加上-Wall，GCC 4.8.2 或 Clang 3.3 都不会报警，包括 Visual Studio 2012 在默认的报警级别也不会（默认是/W3 级，需要上升到/W4 级以上，但是升级到/W4 上，你的工程可能会有N多的 Warning，你不一定能看得过来）。gcc 和 Clang 有一个参数叫：-Wunreachable-code，是可以对这种情况报警的，但即没有被包括在-Wall 里。原因是，这个参数有很多的问题，因为编译器的优化代码的行为，这个参数并不能对每种情况都准确地报告。另请注意，GCC 的新版本中剔除了这个参数。当然，其它一些静态的代码检查工具也可以检查这个低级的问题。

　　另外，是不是用 IDE 的代码自动化格式工具也可以帮上一点忙呢？至少可以把那个缩进变成让人一看就觉得有问题。

　　1）关于 Code Merge 和 Code Review

　　你可以通过这里的代码比较看到这个 bug 的 diff，也可以到这里看看（631 行）。

diff -urN <(curl -s http://opensource.apple.com/source/Security/Security-55179.13/libsecurity_ssl/lib/sslKeyExchange.c\?txt) \ <(curl -s http://opensource.apple.com/source/Security/Security-55471/libsecurity_ssl/lib/sslKeyExchange.c\?txt) \

　　通过 code diff 你可以看到，苹果公司是在重构代码——为很多函数去掉了 ctx 的参数。

　　所以，我们可以猜测，两个 goto fail 语句，可能是因为对 code 在不同 branch 上做 merge 发生的。版本工具 merge 代码的时候，经常性的会出现这样的问题。如果代码的 diff 很多，这个问题会很容易就没有注意到。就算有 code review，这个有问题的代码也很难被找出来的。如果你来 review 下面的 diff，你会注意到这个错误吗？

f2.jpg

　　也就是说，在重构分支上的代码是对的，但是在分支 merge 的时候，被 merge 工具搞乱了。所以说，我们在做 code merge 的时候，一定要小心小心再小心，不能完全相信 merge 工具。

　　2）关于测试

　　很明显，这个 bug 很难被 code review 发现。对于重构代码和代码 merge 里众多的 diff，是很难被 review 的。

　　当然，“事后诸葛亮”的人们总是很容易地说这个问题可以被测试发现，但是实际情况是这样的吗？

　　这个问题也很难被功能测试发现，因为这个函数在是在网络握手里很深的地方，功能测试不一定能覆盖得那么深，你要写这样的 case，必需对 TLS 的协议栈非常熟悉，熟悉到对他所有的参数都很熟悉，并能写出针对每一个参数以及这些参数的组合做一堆 test case，这个事情也是一件很复杂的事。要写出所有的 case 本身就是一件很难很难的事情。关于这个叫 SSLVerifySignedServerKeyExchange ()函数的细节，你可以看看相关的 ServerKeyExchange RFC 文档。

　　如果只看这个问题的话，你会说对这个函数做的 Unit Test 可以发现这个问题，是的。但是，别忘了 SSL/TLS 这么多年了，这些基础函数都应该是很稳定的了， 在事前，我们可能不会想到要去为这些稳定了多少年的函数写几个 Unit Test。

　　只要有足够多的时间，我们是可以对所有的功能点，所有的函数都做 UT，也可以去追求做代码覆盖和分支覆盖一样。但有一点我们却永远无法做到，那就是——穷举所有的负面案例。所以，对于测试来说，我们不能走极端，需要更聪明的测试。就像我在《我们需要专职的 QA》文章里的说过的——测试比 coding 难度大多了，测试这个工作只有高级的开发人员才做得好。我从来不相信不写代码的人能做好测试。

　　这里，我并不是说通过测试来发现这个问题的可能性不大，我想说的是，测试很重要，单测更重要。但是，我们无法面面俱到。在我们没有关注到的地方，总会发生愚蠢的错误。

　　P.S.，在各大网站对这个事的讨论中，我们可以看到 OS X 下的 curl 命令居然可以接受一个没有验证过的 IP 地址的 https 的请求，虽然现在还没有人知道这事的原因，但是，这可能是没有在测试中查到的一个原因。

　　3）关于编码风格

　　对于程序员来说，在C语言中，省掉语句大括号是一件非常不明智的事情。如我们强制使用语句块括号，那么，这两个 goto fail 都会在一个 if 的语句块里，而且也容易维护并且易读。（另外，通过这个 bug，我们可以感受到，像 Python 那样，用缩进来表示语句块，的确是挺好的一件事）

　　也有人说，如果你硬要用只有单条语句，且不用语句块括号，那么，这就是一条语句，应该放在同一行上。如下所示：

if (check_something) do_something ();

　　但是这样一来，你在单步调试代码的时候，就有点不爽了，当你 step over 的时候，你完全不知道 if 的条件是真还是假。所以，还是分多行，加上大括号会好一些。

　　相似的问题，我很十多年前也犯过，而且那次我出的问题也比较大，导致了用户的数据出错。那次就是维护别人的代码，别人的代码就是没有 if 的语句块括号，就像苹果的代码那样。我想在 return z 之前调用一个函数，结果就杯具了：

if ( ...... )      return x;if ( ...... )      return y;if ( ...... )      foo ();      return z;

　　这个错误一不小心就犯了，因为人的大脑会相当然地认为缩进的都是一个语句块里的。但是如果原来的代码都加上了大括号，然后把缩进做正常，那么对后面维护的人会是一个非常好的事情。就不会犯我这个低级错误了。就像下面的代码一样，虽然写起来有点罗嗦，但利人利己。

if ( ...... ){      return x;  }if ( ...... ){      return y;  }if ( ...... ){      return z;  }

　　与此类似的代码风格还有如下，你觉得哪个更容易阅读呢？

  • if (!p)    和  if (p == NULL)
  • if (p)    和  if (p != NULL)
  • if (!bflag)  和 if  (bflag == false)
  • if ( CheckSomthing () )  和 if ( CheckSomething () == true )

　　所以说，代码不是炫酷的地方是给别人读的。

　　另外，我在想，为什么苹果的这段代码不写成下面这样的形式？你看，下面这种情况不也很干净吗？

if ( (err = ReadyHash (&SSLHashSHA1, &hashCtx)) != 0 )         || (err = SSLHashSHA1.update (&hashCtx, &clientRandom)) != 0)         || (err = SSLHashSHA1.update (&hashCtx, &serverRandom) != 0)         || (err = SSLHashSHA1.update (&hashCtx, &signedParams) != 0)         || (err = SSLHashSHA1.final (&hashCtx, &hashOut)) != 0)) {          goto fail;  }

　　其实，还可以做一些代码上的优化，比如，把 fail 标签里的那些东西写成一个宏，这样就可以去掉 goto 语句了。

　　4）关于 goto 语句

　　关于 goto 语句，1968 年，Edsger Dijkstra 投了一篇文章到 Communications of the ACM。原本的标题是《A Case Against the Goto Statement》。CACM 编辑 Niklaus Wirth 灵感来了，把标题改为我们熟知的 《Go To Statement Considered Harmful》Dijkstra 写的内容也是其一贯的犀利语气，文中说：“几年前我就观察到，一个程序员的品质是其程序中 goto 语句的密度成反比的”，他还说，“后来我发现了为什么 goto 语句的使用有这么严重的后果，并相信所有高级语言都应该把 goto 废除掉。”  （花絮：因为，这篇文章的出现，计算学界开始用’ X considered harmful ’当文章标题的风潮，直到有人终于受不了为止）

　　为什么 goto 语句不好呢？Dijkstra 说，一个变量代表什么意义要看其上下文。一个程序用N记录房间里的人数，在大部分时候，N代表的是“目前房间里的人”。但在观察到又有一个人进房间后、把N递增的指令前的这段程序区块中，N的值代表的是“目前房间里的人数加一”。因此，要正确诠释程序的状态，必须知道程序执行的历史，或着说，知道现在“算到哪”了。

　　怎么谈“算到哪了”？如果是一直线执行下来的程序，我们只要指到那条语句，说“就是这里”，就可以了。如果是有循环程序，我们可能得说：“现在在循环的这个地方，循环已经执行了第i次”。如果是在函数中，我们可能得说：“现在执行到函数p的这一点；p刚刚被q调用，调用点在一个循环中，这个循环已经执行了i次”。

　　如果有 goto语句了呢？那就麻烦了。因为电脑在执行某个指令前，可能是从程序中许许多多 goto其中之一跳过来的。要谈某变量的性质也几乎变得不可能了。这就是为什么 goto 语句问题。

　　Dijkstra 的这篇文章对后面很多程序员有非常深的影响，包括我在内，都觉得 Goto 语句能不用就不用，虽然，我在十年前的《编程修养》（这篇文章已经严重过时，某些条目已经漏洞百出）中的第 23 条也说过，我只认为在 goto 语句只有一种情况可以使用，就是苹果这个 bug 里的用法。但是我也同意 Dijkstra，goto 语句能不用就不用了。在更为高级的 C++ 中，使用 RAII 技术，这样的 goto 语句已经没有什么存在的意义了。

　　Dijkstra 这篇文章后来成为结构化程式论战最有名的文章之一。长达 19 年之后，Frank Rubin 投了一篇文章到 CACM,标题为《‘ Go To Considered Harmful’ Considered Harmful 》Rubin 说，「虽然 Dijkstra 的说法既太学术又缺乏说服力」，却似乎烙到每个程序员的心里了。这样，当有人说“用 goto 语句来解这题可能会比较好”会被严重鄙视。于是 Rubin 出了一道这样的题：令X为N * N的整数阵列。如果X的第i行全都是零，请输出i。如果不只一行，输出最小的i .

　　Rubin 找了一些惯用 goto 和不用 goto 的程序员来解题，发现用 goto 的程序又快又清楚。而不用 goto 通常花了更多的时间，写出很复杂的解答。你觉得呢？ 另外，你会怎么写这题的程序呢？

　　（花絮：以后几个月的 CACM 热闹死了。编辑收到许多回应，两个月后刊出了其中五篇。文章也包括了《“‘GOTO Considered Harmful’ Considered Harmful” Considered Harmful? 》）

　　对于我而言，goto 语句的弊远远大于利，在 99% 的情况下，我是站在反 goto 这边的。

　　（花絮：这段时间，我在开发 Nginx 的模块，因为以前没有做过，而且 Nginx 的开发文档也不好，所以就得读一些别人的源代码。当我看了一个某同学开发的 nginx redis 的模块里的这段代码 ngx_http_redis2_reply.c 看 到里面飞沙走石的 goto 语句，我崩溃了！虽然，有网友指出这是代码自动生成工具生成出来的（这样的代码放在源码库里跟放个可执行文件有什么差别？），但是不能不说我看到这样的代 码的时候，就像我在某个餐馆看到了他那肮脏的厨房，无论你做菜的技艺有多高超，做的菜做得有多好看多好吃，我都恶心得一点也不想吃了）

　　总结

　　你看，我们不能完全消灭问题，但是，我们可以用下面几个手段来减少问题：

　　1）尽量在编译上发生错误，而不是在运行时。

　　2）代码是让人读的，顺便让机器运行。不要怕麻烦，好的代码风格，易读的代码会减少很多问题。

　　3）Code Review 是一件很严肃的事情，但 Code Reivew 的前提条件是代码的可读性一定要很好。

　　4）测试是一件很重要也是很难的事情，尤其是开发人员要非常重视。

　　5）不要走飞线，用飞线来解决问题是可耻的！所以，用 goto 语句来组织代码的时代过去了，你可以有很多种方式不用 goto 也可以把代码组织得很好。

　　最后，我在淘宝过去的一年里，经历过一些 P1/P2 故障，尤其是去年的8-9 月份故障频发的月份，我发现其中有 70% 的 P1/P2 故障，就是因为没有 code review，没有做好测试，大量地用飞线来解决问题，归根结底就是只重业务结果，对技术没有应有的严谨的态度和敬畏之心。

　　正如苹果的这个“goto fail”事件所暗喻的，如果你对技术没有应有的严谨和敬畏之心，你一定会——

　　Go To Fail !!!

　　在这里唠叨这么多，与大家共勉！

来自: coolshell.cn