由苹果的低级Bug想到的

由苹果的低级Bug

想到的

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/TSL相关的bug 或安全问题，不管是做为用户，还是做为程序员的你，你一定要高度重视起来。因为这个网络通信的加密协议被广泛的应用在很多很多最最需要安全的地方，如果SSL/TSL有问题的话，意味着这个世界的计算机安全体系的崩溃。

Bug 的代码原因

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

sslKeyExchange.c 中找到下面的代码）

1. static OSStatus

2. SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuff

er signedParams,

3. uint8_t *signature, UInt16 signature

Len)

4. {

5. OSStatus err;

6. ...

7. 8. if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)

9. goto fail;

10. if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)

11. goto fail;

12. goto fail;

13. if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)

14. goto fail;

15. err = sslRawVerify(ctx,

16. ctx->peerPubKey,

17. dataToSign, /* plaintext */

18. dataToSignLen, /* plaintext length *

/

19. signature,

20. signatureLen);

21. if (err) {

22. sslErrorLog("SSLDecodeSignedServerKeyExchange: sslRawVerify "

23. "returned %d\n", (int )err);

24. goto fail;

25. }

26.

27. fail:

28. SSLFreeBuffer(&signedHashes);

29. SSLFreeBuffer(&hashCtx);

30. return err;

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

你想一下，这段程序在SSLHashSHA1.update() 返回成功，也就是返回0 的时候会发生什么样的事？是的，真正干活的 sslRawVerify()被bypass 了。而且这个函数

SSLVerifySignedServerKeyExchange() 还返回了0，也就是成功了！尼玛！你可能想到酷壳网上之前《一个空格引发的惨剧》的文章。都是低级bug 。

网址：edu.51CTO.com

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

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

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

如果你想测试一下你的浏览器是否会有问题

一些思考

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

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 网址：edu.51CTO.com

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

http://opensource.apple.com/source/Security/Security-55179.13/libsecurity_ssl/lib/sslKeyExchange.c\?txt) \

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 ，你会注意到这个错误吗？

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

2）关于测试

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

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

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

ServerKeyExchange RFC 文档。

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

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

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

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

3）关于编码风格

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

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

1. if (check_something) do_something();

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

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

1. if ( ...... )

2. return x;

3. if ( ...... )

4. return y; 5. if ( ...... )

6. foo()

7. return z;

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

1. if ( ...... ){

2. return x;

3. }

4. if ( ...... ){ 5. return y;

6. }

7. if ( ...... ){

8. return z;

9. }

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

∙

∙

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

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

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

1. if ( (err = ReadyHash(&SSLHashSHA1, &hashCtx)) != 0 )

2. || (err = SSLHashSHA1.update(&hashCtx, &clientRandom)) != 0)

3. || (err = SSLHashSHA1.update(&hashCtx, &serverRandom) != 0)

4. || (err = SSLHashSHA1.update(&hashCtx, &signedParams) != 0)

5. || (err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)) { 6.

7. goto fail;

8. } 其实，还可以做一些代码上的优化，比如，把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 的开发文档也不好，所以就得读一些别人的源代码。当我看了一个某同学开发的nginxredis 的模块里的这段代码 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 !!!