wire tox

Hi everyone!

嗨，大家好！

I like Tox and respect the participants of this project and their work. In an effort to help Tox developers and users, I looked into the code and noticed potential problems that could lead to a false sense of security. Since I originally published this article in 2016 (in Russian), many improvements have been made to Tox, and I lead a team that re-wrote secure Tox software from scratch using the Rust programming language (check out Tox-rs). I DO recommend using tox in 2019. Let's take a look what actually made us rewrite Tox in Rust.

我喜欢Tox，并尊重这个项目及其工作的参与者。 为了帮助Tox开发人员和用户，我仔细研究了代码并注意到可能导致错误的安全感的潜在问题。 自从我最初于2016年以俄语发布此文章以来，Tox进行了许多改进，我领导的团队使用Rust编程语言从头开始重新编写了安全的Tox软件(请参阅Tox-rs )。 我确实建议在2019年使用Tox。让我们看一下实际上使我们在Rust中重写Tox的原因。

2016年度的原创文章 (Original article of 2016)

There is an unhealthy tendency to overestimate the security of E2E systems only on the basis that they are E2E. I will present objective facts supplemented with my own comments for you to draw your own conclusions.

有一种不健康的趋势，就是仅基于E2E系统来高估E2E系统的安全性。 我将提出客观事实，并附上我自己的评论，以便您得出自己的结论。

Spoiler: The Tox developers agree with my points and my source code pull request was accepted.

剧透：Tox开发人员同意我的观点，并且接受了我的源代码提取请求 。

事实№1。 主分支测试失败 (Fact №1. master branch fails tests)

It all started with articles on Habr about installing the node(in Russian). In the comments, people complained about the complexity of building and installing the node on CentOS, so I decided to write a build system on CMake. After a few days, I was ready to present my PR to the Tox community on Freenode, but I was met with a lack of understanding:

一切始于有关Habr的文章，有关安装节点( 俄语 )。 在评论中，人们抱怨在CentOS上构建和安装节点的复杂性，因此我决定在CMake上编写一个构建系统。 几天后，我准备向Freenode上的Tox社区介绍我的PR，但是我感到缺乏理解：

someone has contributed cmake initially, but other developers didn't know how to use it and couldn't make it build their code, so they switched to autotools (sic!), which they become to know better now.

最初有人提供了cmake，但是其他开发人员不知道如何使用它，也无法使其构建代码，因此他们改用了autotools(原文如此！)，现在他们变得更加了解。

I noted that code that fails the tests in Travis CI still gets accepted into the master branch, but they answered: "we understand we need to do something with the tests, but let it be for now."

我注意到，在Travis CI中未能通过测试的代码仍被master分支接受，但他们回答：“我们知道我们需要对测试做点事，但现在就解决。”

Next, I dived into looking deeper at the code of this attractive messenger.

接下来，我深入研究了这个诱人的Messenger的代码。

事实№2。 memset(ptr，0，size)在免费呼叫之前 (Fact №2. memset(ptr, 0, size) before calling free)

My eye caught

我的眼睛被抓住了

memset(c, 0, sizeof(Net_Crypto));
free(c);

If you still are not familiar with PVS-Studio and its article about memset function PVS-Studio: the compiler can delete the 'memset' function call if that memory region is not used afterwards. Compiler's logic is straightforward: "You are not going to use this variable after calling 'free', memset will not affect the observed behavior, let me delete this useless call to 'memset'".

如果您仍然不熟悉PVS-Studio及其有关memset函数 PVS-Studio的文章：如果此后不使用该内存区域，则编译器可以删除“ memset”函数调用。 编译器的逻辑很简单：“调用'free'之后，您将不使用此变量，memset不会影响观察到的行为，让我删除对'memset'的无用调用”。

As a diligent student I replaced each occurrence of memset($DST, 0, $SIZE) with sodium_memzero and the TESTS CRASHED.

作为一个勤奋的学生，我将每次出现的memset($DST, 0, $SIZE)替换为sodium_memzero和TESTS CRASHED。

事实№3。 比较公钥容易受到定时攻击 (Fact №3. Comparison of public keys is vulnerable to timing attacks)

There is a really great special function to compare public keys in toxcore:

有一个很棒的特殊功能可以比较toxcore中的toxcore ：

/* compare 2 public keys of length crypto_box_PUBLICKEYBYTES, not vulnerable to timing attacks.returns 0 if both mem locations of length are equal,return -1 if they are not. */
int public_key_cmp(const uint8_t *pk1, const uint8_t *pk2)
{return crypto_verify_32(pk1, pk2);
}

crypto_verify_32 — is a function from the NaCL/Sodium crypto library, that can help you to avoid timing attacks, because it works in constant time, while memcmp can break on the first unequal byte. You should use crypto_verify_32 to compare sensitive data like keys.

crypto_verify_32 —是NaCL / 钠密码库中的一个函数，可以帮助您避免定时攻击，因为它在恒定的时间内工作，而memcmp可以在第一个不相等的字节上中断。 您应该使用crypto_verify_32比较敏感数据，例如密钥。

String comparisons performed byte-per-byte are vulnerable to exploitation by timing attacks, for example in order to forge MACs (see this vulnerability in Google's Keyczar crypto library).

逐字节执行的字符串比较容易受到定时攻击的攻击，例如为了伪造MAC(请参阅Google的Keyczar密码库中的此 漏洞 )。

The code base of the toxcore project is quite extensive, which is why Tox was born with a timing vulnerability bug:

toxcore项目的代码库非常广泛，这就是Tox诞生时带有计时漏洞的原因：

bool id_equal(const uint8_t *dest, const uint8_t *src)
{return memcmp(dest, src, crypto_box_PUBLICKEYBYTES) == 0;
}

But that's not all. The developers still prefer to compare keys their own way using three different functions: id_equal or public_key_cmp and crypto_verify_32. Here is a short grep output from DHT, onion routing and other critical subsystems:

但这还不是全部。 开发人员仍然更喜欢使用三种不同的功能以自己的方式比较密钥： id_equal或public_key_cmp和crypto_verify_32 。 这是DHT，洋葱路由和其他关键子系统的简短grep输出：

if (memcmp(ping->to_ping[i].public_key, public_key, crypto_box_PUBLICKEYBYTES) == 0) {
if (memcmp(public_key, onion_c->friends_list[i].real_public_key, crypto_box_PUBLICKEYBYTES) == 0)
if (memcmp(public_key, onion_c->path_nodes_bs[i].public_key, crypto_box_PUBLICKEYBYTES) == 0)
if (memcmp(dht_public_key, dht_public_key_temp, crypto_box_PUBLICKEYBYTES) != 0)
if (Local_ip(ip_port.ip) && memcmp(friend_con->dht_temp_pk, public_key, crypto_box_PUBLICKEYBYTES) == 0)

事实№4。 非恒定时间内的increment_nonce (Fact №4. increment_nonce in a non constant time)

/* Increment the given nonce by 1. */
void increment_nonce(uint8_t *nonce)
{uint32_t i;for (i = crypto_box_NONCEBYTES; i != 0; --i) {++nonce[i - 1];if (nonce[i - 1] != 0)break;               // <=== sic!}
}

If such operations involve secret parameters, these timing variations can leak some information. With enough knowledge of the implementation is at hand, a careful statistical analysis could even lead to the total recovery of secret parameters.

如果此类操作涉及秘密参数，则这些时序变化可能会泄漏某些信息。 有了足够的执行知识，仔细的统计分析甚至可能导致秘密参数的完全恢复。

There is a special function in Sodium to increment nonces:

钠中有一个特殊功能可以增加随机数：

void
sodium_increment(unsigned char *n, const size_t nlen)
{size_t        i = 0U;uint_fast16_t c = 1U;for (; i < nlen; i++) {c += (uint_fast16_t) n[i];n[i] = (unsigned char) c;c >>= 8;}
}

An accidentally ironic easter egg is that the

一个不具讽刺意味的复活节彩蛋是

increment_nonce (increment_nonce)

function is located in a file that starts with the words:

函数位于以以下单词开头的文件中：

This code has to be perfect. We don't mess around with encryption.

此代码必须是完美的。 我们不会搞乱加密。

Let's take a closer look at this perfect file.

让我们仔细看看这个完美的文件。

事实№5。 您可以在堆栈中找到密钥和私有数据 (Fact №5. You can find keys and private data in the stack)

Troublesome code:

麻烦的代码：

/* Precomputes the shared key from their public_key and our secret_key.* This way we can avoid an expensive elliptic curve scalar multiply for each* encrypt/decrypt operation.* enc_key has to be crypto_box_BEFORENMBYTES bytes long.*/
void encrypt_precompute(const uint8_t *public_key, const uint8_t *secret_key, uint8_t *enc_key)
{crypto_box_beforenm(enc_key, public_key, secret_key); // Nacl/Sodium function
}/* Encrypts plain of length length to encrypted of length + 16 using the* public key(32 bytes) of the receiver and the secret key of the sender and a 24 byte nonce.**  return -1 if there was a problem.*  return length of encrypted data if everything was fine.*/
int encrypt_data(const uint8_t *public_key, const uint8_t *secret_key, const uint8_t *nonce,const uint8_t *plain, uint32_t length, uint8_t *encrypted)
{uint8_t k[crypto_box_BEFORENMBYTES];encrypt_precompute(public_key, secret_key, k); // toxcore functionreturn encrypt_data_symmetric(k, nonce, plain, length, encrypted); // toxcore function
}

encrypt_data_symmetric calls crypto_box_detached_afternm from Nacl/Sodium, I will not put the whole code, here is a link to check for yourself.

crypto_data_symmetric从Nacl / Sodium调用crypto_box_detached_afternm ，我不会放置整个代码，这是一个检查自己的链接 。

It seems difficult to make a mistake in four lines of code, doesn't it?

在四行代码中似乎很难犯错，不是吗？

Let's dig into Sodium:

让我们深入研究钠：

int
crypto_box_detached(unsigned char *c, unsigned char *mac,const unsigned char *m, unsigned long long mlen,const unsigned char *n, const unsigned char *pk,const unsigned char *sk)
{unsigned char k[crypto_box_BEFORENMBYTES];int           ret;(void) sizeof(int[crypto_box_BEFORENMBYTES >=crypto_secretbox_KEYBYTES ? 1 : -1]);if (crypto_box_beforenm(k, pk, sk) != 0) {return -1;}ret = crypto_box_detached_afternm(c, mac, m, mlen, n, k);sodium_memzero(k, sizeof k);return ret;
}

Erasing all checks we get:

消除所有检查，我们得到：

unsigned char k[crypto_box_BEFORENMBYTES];int           ret;crypto_box_beforenm(k, pk, sk);ret = crypto_box_detached_afternm(c, mac, m, mlen, n, k);sodium_memzero(k, sizeof k);return ret;

Does it look familiar? Yes! It's a slightly modified code of function

看起来熟悉吗？ 是! 这是稍微修改的功能代码

crypto_data (encrypt_data)

from toxcore, the only difference is that they forgot to clean the key on the stack with

从toxcore，唯一的区别是他们忘了用

sodium_memzero (sodium_memzero)

… And there are also mistakes in:

……还有以下错误：

handle_TCP_handshake (handle_TCP_handshake)

,

，

handle_handshake (handle_handshake)

, and maybe somewhere else too.

，也许还有其他地方。

事实№6。 编译器警告适用于dummiez！ (Fact №6. Compiler warnings are for dummiez!)

The devs from the toxcore project flatly deny the necessity of turning on all compiler warnings, or they do not know about them.

来自toxcore项目的开发人员断然拒绝打开所有编译器警告的必要性，或者他们不知道它们。

Unused functions (I am particularly pleased with the warnings in tests):

未使用的功能(我对测试中的警告特别满意)：

../auto_tests/dht_test.c:351:12: warning: unused function 'test_addto_lists_ipv4' [-Wunused-function]
START_TEST(test_addto_lists_ipv4)^
../auto_tests/dht_test.c:360:12: warning: unused function 'test_addto_lists_ipv6' [-Wunused-function]
START_TEST(test_addto_lists_ipv6)^
../toxcore/TCP_server.c:1026:13: warning: unused function 'do_TCP_accept_new' [-Wunused-function]
static void do_TCP_accept_new(TCP_Server *TCP_server)^
../toxcore/TCP_server.c:1110:13: warning: unused function 'do_TCP_incomming' [-Wunused-function]
static void do_TCP_incomming(TCP_Server *TCP_server)^
../toxcore/TCP_server.c:1119:13: warning: unused function 'do_TCP_unconfirmed' [-Wunused-function]
static void do_TCP_unconfirmed(TCP_Server *TCP_server)^

../toxcore/Messenger.c:2040:28: warning: comparison of constant 256 with expression of type 'uint8_t' (aka 'unsigned char') is always false[-Wtautological-constant-out-of-range-compare]if (filenumber >= MAX_CONCURRENT_FILE_PIPES)~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
../toxcore/Messenger.c:2095:28: warning: comparison of constant 256 with expression of type 'uint8_t' (aka 'unsigned char') is always false[-Wtautological-constant-out-of-range-compare]if (filenumber >= MAX_CONCURRENT_FILE_PIPES)~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~
../toxcore/Messenger.c:2110:28: warning: comparison of constant 256 with expression of type 'uint8_t' (aka 'unsigned char') is always false[-Wtautological-constant-out-of-range-compare]if (filenumber >= MAX_CONCURRENT_FILE_PIPES)~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~

../auto_tests/TCP_test.c:205:24: warning: unsequenced modification and access to 'len' [-Wunsequenced]ck_assert_msg((len = recv(con->sock, data, length, 0)) == length, "wrong len %i\n", len);^                                                                ~~~
/usr/include/check.h:273:18: note: expanded from macro 'ck_assert_msg'_ck_assert_msg(expr, __FILE__, __LINE__,\^

And a few dozen warnings about unused variables, the comparison of signed and unsigned, and more.

还有几十条关于未使用变量，有符号和无符号比较的警告，等等。

我的结论 (My conclusion)

Quote from the repository:

来自资源库的报价：

We want Tox to be as simple as possible while remaining as secure as possible.

我们希望Tox尽可能简单，同时又要保持尽可能的安全。

If I, a non-cryptographer, could find such horrible bugs in a day, imagine how many things can a cryptography specialist might find after purposely dig for them for a month?.

如果我(不是密码学家)一天可以找到如此可怕的错误，想象一下密码专家在有意地挖掘一个月后能找到多少东西？

The early versions of the Tox posed a great danger to users who rely on Tox security. Proprietary solutions are not trust-worthy and even open source solutions are not as safe as you want them to be. Take a look at the recent security breach in Matrix. Nowadays a lot of bugs are fixed and Tox is the best option available for security and privacy for users.

Tox的早期版本给依赖Tox安全性的用户带来了极大的危险。 专有解决方案不值得信赖，甚至开源解决方案也不如您希望的那样安全。 看一下Matrix中最近的安全漏洞。 如今，许多错误已得到修复，Tox是为用户提供安全性和隐私性的最佳选择。

Next time I will tell you more about the current state of tox-rs. What we have implemented in Rust and why you should try it.

下次我将告诉您有关tox-rs当前状态的更多信息。 我们在Rust中实现了什么以及为什么要尝试。

Reddit: comments

Reddit： 评论

翻译自: https://habr.com/en/post/447994/

wire tox