رحلة طويلة إلى Tox-rs. الجزء 1

مرحبا بالجميع!

أنا أحب Tox وأحترم المشاركين في هذا المشروع وعملهم. في محاولة لمساعدة مطوري ومستخدمي Tox ، نظرت إلى الكود ولاحظت وجود مشاكل محتملة قد تؤدي إلى شعور زائف بالأمان. منذ أن نشرت هذا المقال في الأصل عام 2016 ( باللغة الروسية ) ، تم إجراء العديد من التحسينات على Tox ، وأقود فريقًا أعاد كتابة برنامج Tox آمن من البداية باستخدام لغة برمجة Rust (راجع Tox-rs ). أوصي باستخدام توكس في عام 2019. دعنا نلقي نظرة على ما جعلنا نعيد كتابة توكس في الصدأ.

المقالة الأصلية لعام 2016

هناك ميل غير صحي إلى المبالغة في تقدير أمان أنظمة E2E فقط على أساس أنها E2E. سأقدم حقائق موضوعية مستكملة بتعليقاتي الخاصة لك لاستخلاص استنتاجاتك الخاصة.

Spoiler: يتفق مطورو Tox مع نقاطي وتم قبول طلب سحب شفرة المصدر.

الحقيقة رقم 1. فشل فرع سيد الاختبارات

بدأ كل شيء بمقالات عن Habr حول تثبيت العقدة ( باللغة الروسية ).
في التعليقات ، اشتكى الناس من تعقيد بناء وتثبيت العقدة على CentOS ، لذلك قررت أن أكتب نظام بناء على CMake. بعد بضعة أيام ، كنت مستعدًا لتقديم مستقبلي إلى مجتمع Tox على Freenode ، لكني قوبلت بنقص في الفهم:

لقد ساهم أحدهم cmake في البداية ، لكن المطورين الآخرين لم يكونوا يعرفون كيفية استخدامه ولم يتمكنوا من بناء التعليمات البرمجية الخاصة بهم ، لذلك تحولوا إلى autotools (كذا!) ، والتي أصبحوا يعرفونها بشكل أفضل الآن.

لاحظت أنه لا يزال يتم قبول الشفرة التي تفشل الاختبارات في Travis CI في الفرع الرئيسي ، لكنهم أجابوا: "نحن نفهم أننا بحاجة إلى القيام بشيء ما مع الاختبارات ، لكن يجب أن يكون الأمر حاليًا."

بعد ذلك ، غطست في النظر أعمق في رمز هذا الرسول الجذاب.

الحقيقة رقم 2. memset (ptr ، 0 ، الحجم) قبل الاتصال مجانًا

اشتعلت عيني

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

إذا كنت لا تزال غير معتاد على PVS-Studio ومقالتها حول وظيفة memset PVS-Studio: يمكن للمترجم حذف استدعاء وظيفة "memset" إذا لم يتم استخدام منطقة الذاكرة بعد ذلك. منطق المترجم واضح: "لن تستخدم هذا المتغير بعد استدعاء" مجاني "، لن تؤثر memset على السلوك المرصود ، واسمحوا لي أن أحذف هذه المكالمة غير المجدية إلى" memset ".

كطالب مجتهد ، استبدلت كل تواجد لـ memset($DST, 0, $SIZE) بـ sodium_memzero و TESTS CRASHED.

الحقيقة رقم 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 ، والتي يمكن أن تساعدك على تجنب هجمات التوقيت ، لأنها تعمل في وقت ثابت ، في حين أن memcmp يمكنه كسر البايت غير المتكافئ الأول. يجب عليك استخدام crypto_verify_32 لمقارنة البيانات الحساسة مثل المفاتيح.

تكون مقارنات السلسلة التي يتم تنفيذها بواسطة بايت لكل بايت عرضة للاستغلال من خلال توقيت الهجمات ، على سبيل المثال لتزوير أجهزة MAC (انظر هذه الثغرة الأمنية في مكتبة تشفير Google Keyczar ).

قاعدة الكود الخاصة بمشروع toxcore شاملة للغاية ، ولهذا السبب وُلد Tox مع خلل في ثغرة التوقيت:

 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 .
فيما يلي إخراج grep قصير من 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! } } 

إذا كانت مثل هذه العمليات تتضمن معلمات سرية ، فيمكن لهذه الاختلافات في التوقيت أن تسرب بعض المعلومات. مع وجود معرفة كافية بالتنفيذ ، يمكن أن يؤدي التحليل الإحصائي الدقيق إلى التعافي الكامل للمعلمات السرية.

هناك وظيفة خاصة في الصوديوم لزيادة عدم التزايد:

 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; } } 

بيضة عيد الفصح المفارقة هي أن الدالة 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 calls crypto_box_detached_afternm من Nacl / 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; 

هل تبدو مألوفة؟ نعم! إنه رمز وظيفة مُعدل قليلاً من encrypt_data من toxcore ، والفرق الوحيد هو أنهم نسوا تنظيف المفتاح على المكدس باستخدام sodium_memzero ... وهناك أيضًا أخطاء في: handle_TCP_handshake ، handle_handshake ، وربما في مكان آخر أيضًا.

الحقيقة رقم 6. تحذيرات المترجم هي ل dummiez!

تنكر المطوِّرون (devs) من مشروع toxcore بشكل قاطع ضرورة تشغيل جميع تحذيرات برنامج التحويل البرمجي ، أو أنهم لا يعرفون عنها.

الوظائف غير المستخدمة (يسرني بشكل خاص التحذيرات في الاختبارات):

 ../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) ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ -range-مقارنة] ../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) ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ -range-مقارنة] ../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) ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ -range-مقارنة] ../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 بسيطًا قدر الإمكان مع الحفاظ على أمانه قدر الإمكان.

إذا تمكنت من العثور على هذه الأخطاء الفظيعة في يوم واحد ، وأنا أتصور عدد الأشياء التي يمكن أن يجدها أخصائي التشفير بعد البحث عن قصد لمدة شهر؟

تشكل الإصدارات المبكرة من Tox خطراً كبيراً على المستخدمين الذين يعتمدون على أمان Tox. الحلول الاحتكارية ليست جديرة بالثقة ، وحتى الحلول مفتوحة المصدر ليست آمنة كما تريدها. ألق نظرة على الاختراق الأمني ​​الأخير في Matrix . في الوقت الحاضر ، تم إصلاح الكثير من الأخطاء وتوكس هو أفضل خيار متاح للأمن والخصوصية للمستخدمين.

في المرة القادمة ، سوف أخبركم أكثر عن الحالة الحالية للتكسينات. ما قمنا بتطبيقه في رست ولماذا يجب أن تجربه.

رديت: تعليقات