みなさんこんにちは！
Toxが好きです。このプロジェクトの参加者と彼らの仕事を尊重します。 コミュニティを支援するために、私はコードを見て、
最近、そのようなシステムのセキュリティをP2Pであるという理由だけで過大評価するという不健康な傾向がありました。 客観的な事実を述べ、大げさなフレーズで空間に突入しないようにコメントで補足します。 私は自分で結論を出すことを提案します。
事前に質問に回答します。 プルリクエストが受け入れられました。
事実1 テストでクラッシュするコードはmasterブランチに到達します
それはすべて、ノードのインストールに関するハブの記事から始まりました。
コメントでは、CentOSでのアセンブルとインストールの複雑さについて人々から不満が寄せられたため、CMakeでアセンブリをフラッシュすることにしました。 数日間の仕事の後、FreenodeのToxコミュニティに自分のシェアを提示する準備ができましたが、誤解に会いました。
誰かが最初にcmakeを提供しましたが、他の開発者はそれを使用する方法を知らず、コードをビルドできなかったため、autotools（sic！）に切り替えました。
同時に、Travis CIでテストに合格しないコードがmasterブランチに到達することに注意しましたが、彼らは私に答えました：「テストで何かをする必要があることはわかっていますが、今のところはそうさせてください。」
その手助けをするために、私はこの落下する希望のメッセンジャーのコードを決定し、開きました。
事実2 freeを呼び出す前のmemset（ptr、0、サイズ）
目が離せない
memset(c, 0, sizeof(Net_Crypto)); free(c);
PVS-Studioの記事シリーズから思い出すと、将来メモリ領域が使用されない場合、コンパイラオプティマイザによってmemsetが切り捨てられる可能性があります。 コンパイラのロジックは単純です：「自由な生活の後、メモリアクセスはありません。この意味のないmemsetを削除します。」
勤勉な学生として、私は同様の場所でmemsetの呼び出しをsodium_memzeroに置き換えましたが、TESTSは落ちました。
事実3。 公開鍵の比較は時間攻撃に対して脆弱です
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は暗号ライブラリNaCL / Sodiumの特別な関数です。これにより、タイムアタックを回避できます。 一定時間動作しますが、最初のバイトの不一致で壊れません。 プライベートデータ（キー、HMACなど）を比較するときに使用する必要があります。
toxcoreプロジェクトのコードベースは非常に広範囲であるため、このような脆弱性を持つフリークが生まれました。
bool id_equal(const uint8_t *dest, const uint8_t *src) { return memcmp(dest, src, crypto_box_PUBLICKEYBYTES) == 0; }
しかし、それだけではありません。 id_equal 、 public_key_cmp、またはcrypto_verify_32関数を手元に置いても、開発者は独自の方法でキーを比較することを好みます。
DHTコード、オニオンルーティング、およびその他の重要なサブシステムからの簡単な絞り込みを次に示します。
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関数はタイムアタックに対して脆弱です
/* 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! } }
関数は秘密データに依存するべきではありません（だれがより深く突破したいのか、ここにリンクがあります ）。
サーバーが新しいnonceを生成するためによく使用されるのは、 increment_nonceです 。 ノンスインクリメントの場合、ナトリウムには特別な機能があります。
| ドキュメント | sodium_increment（）を使用して、一定時間でノンスをインクリメントできます。 |
| コード | |
今私を止めないでください！ ブレークはありません。関数は既にバイト数を増やして残りを転送していても、一定の時間でバイトをスラッシングする必要があります！
皮肉なことに、 increment_nonce関数は次の単語で始まるファイルにあります。
このコードは完璧でなければなりません。 暗号化をいじることはありません。
この理想的なファイルを詳しく見てみましょう。
事実5。 スタック上でキーとプライベートデータを見つけることができます
疑わしい場所：
/* 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 function return encrypt_data_symmetric(k, nonce, plain, length, encrypted); // toxcore function }
encrypt_data_symmetricは、Nacl / Sodiumからcrypto_box_detached_afternmを呼び出します。コードは提供しません。ここに、私の言葉を確認したい人のためのリンクがあります。
4行のコードでnakosyachitできる場所のように思えますか？
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; }
すべてのチェックを削減すると、次のようになります。
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;
何にも似ていませんか？ はい、これはtoxcoreからのわずかに変更されたencrypt_data関数コードです。唯一の人は、 sodium_memzeroでスタック上のキーを消去するのを忘れていました ...これらの場所はいっぱいです：
事実6。 愚か者にはコンパイラ警告が出されます！
toxcoreプロジェクトは、すべてのコンパイラ警告を含める必要性を断固として拒否します。 まあ、または彼らは彼らについて知らない。 これらの2つの仮定のうち、どちらが悪いのかという質問に答えるつもりはありません。
未使用の関数（特に、このような警告はテストでトリガーされます）：
../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__,\ ^
そして、未使用の変数、符号付きと符号なしの比較などに関する数十の異なる警告。
私の調査結果
リポジトリから引用：
Toxは、できるだけ安全に保ちながら、できるだけシンプルにしたいと考えています。
暗号化に無知な私が1日でそのような恐怖を見つけることができた場合、専門家は1か月間意図的に掘る人をどれだけ見つけることができますか？
このプロジェクトは、Toxセキュリティに依存するユーザーに大きな危険をもたらします。 いつか、見ていない開発者はあなたのプライバシーをゼロにするコードを受け入れるでしょう。
になる方法
2toxプロジェクトに参加できます。これは、 Haltを使用してtoxcoreを置き換えるために徐々に削減しています。
===
UPD 03/28/2019：
2toxは死産だった。 Rust： tox-rsで書き直すことにしました。 すでに作業ノードがあり、クライアントカーネルのプロトタイプを作成しています。