저희 팀장이 Reddit에 올린글을 번역해서 올립니다.
안녕하세요 ?
저희 팀장이 Reddit에 올린글을 번역해서 옮깁니다.
제가 번역이 서툴러 매끄럽지 못한점 양해를 구합니다.
-------------------------------------------------
Tox-rs 로의 긴 여정, 1부
안녕하세요? 저는 Tox를 좋아하고, 이 프로젝트의 참여자와 그들이 이룩한 것을 존경합니다. Tox개발자들과 사용자들을 도우려는 일련의 노력중에, 저는 코드를 들여다보고, 보안적 측면에서 부정적인 결과를 가져올 수 있는 몇가지 잠재적 문제점을 알리고자 합니다. 이 글은 원래 러시아 어로 2016년에 작성된 것이어서, 현재는 많은 개선이 이루어 졌으며, 저는 현재 Tox 소프트웨어를 Rust 언어로 처음부터 다시 작성하는 팀의 리더 입니다. 저는 진실로 2019년에는 Tox를 여러분의 메신저로 사용하시기를 권하며, 우리가 실제로 Tox를 Rust로 재작성 하는 일을 실제적으로 살펴보도록 하겠습니다.
2016년도의 원본 글
E2E라는 것 자체만으로 시스템의 보안성을 과대평가하는 건강하지 못한 경향이 있어 왔습니다. 저는 E2E라 하더라도 보안상 결함이 있을 수 있다는 것을 다믕의 사실로서 예를 들어 드려서 여러분 자신이 스스로의 결론을 내리도록 도우려 합니다.
험담꾼들에게: Tox개발자들은 저의 의견에 동의했으며, 저의 코드는 받아들여 졌습니다.
- Fact no 1. master branch fails tests
러시아 어로 작성된 저의 포스트에서 저는 먼저 노드를 설치하는 것에 대해 다루었습니다. 댓글들 중에 사람들은 CentOS에서 빌드하고 인스톨 하는 과정의 어려움에 대해 불평했습니다. 그래서 저는 CMake상에서 빌드하는 시스템을 작성하기로 결정했습니다. 몇일이 지난 후, 저는 저의 Pull Request를 제출할 준비가 되었습니다. 그러나 저는 이해의 부족에 직면했습니다.
누군가가 cmake를 맨처음 컨트리뷰션을 하였으나, 다른 개발자가 그것을 어떻게 사용해서 그들의 코드를 빌드하는지 몰랐습니다. 그래서 그들은 autotools로 전환했습니다.
저는 Travis CI상에서 태스트가 실패하는 코드에 대해서도 여전히 마스터 브랜치에 머지되는 사실을 알려주었고, 그들은 다음과 같이 대답했습니다.: " 우리는 테스트에 대해서 무언가를 해야할 필요가 있다는 걸 이해하고 있다. 그러나 당장은 그대로 두고 있다."
역자주) 테스트가 실패한다는 것은 잠재적 보안 취약점이 테스트 되지 못하고 마스터 브랜치에 머지될 위험에 노출됩니다.
- Fact no 2. memset(ptr,0,size) before calling free
제 눈은 다음을 잡아냈습니다.
memset(c, 0, sizeof(Net_Crypto));
free(c);
만약 당신이 PVS-Studio와 memset함수에 대해 익숙하지 않다면 : 컴파일러는 만약 메모리 영역이 향후 사용되지 않으면 memset 함수 호출 자체를 최적화 과정에서 제거할 수 있습니다.: "당신은 free를 호출 함으로써 메모리 영역을 사용하지 않을 거라고 명시적으로 나타냈습니다. 따라서 memset은 아무런 의미가 없으니 삭제하도록 하겠습니다."
저는 모든 memset을 sodium_memzero 함수로 대체하는 작업을 했습니다. 그 결과 Trvis CI의 test가 crash됐습니다.
- Fact no 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가 서로 다른 바이트를 만나면 바로 중지하는 것과 달리, 일정한 시간동안 실행됩니다. 여러분은 퍼블릭 키와 같은 민감한 데이터를 비교할 때에는 이 함수를 사용해야 합니다.
문자열 비교를 바이드 단위로 하는 것은 보안 취약점을 노출합니다. timing attack의 대상이 됩니다.
다음의 toxcore 코드는 보안 취약점을 가진 채로 광버위하게 사용되고 있었습니다.
bool id_equal(const uint8_t *dest, const uint8_t *src)
{
return memcmp(dest, src, crypto_box_PUBLICKEYBYTES) == 0;
}
이게 다가 아니었습니다. 개발자들은 여전히 그들 나름의 세가지 방식으로 키를 비교하고 있었습니다.: id_equal or public_key_cmp and crypto_verify_32. 다음은 짧은 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)
- Fact no 4. increment_nonce in a non constrant 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!
}
}
위의 연산은 시간적으로 변이가 발생하고, 중요한 정보의 유출을 가져옵니다. toxcore 는 오픈소스이고 충분한 소스코드의 검토와 조심스런 통계학적 처리로 민감한 정보를 알아낼 소지가 충분히 있습니다.
Sodium에는 nonce를 증가시키는 특별한 함수가 있습니다.
sodium_increment() can be used to increment nonces in constant time.
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;
}
}
Fact no 5. stack에서 키와 민감한 데이터를 찾을 수 있습니다.
문제가 되는 코드는 이것입니다.
/* 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 는 Sodium의 crypto_box_detached_afternm 를 호출합니다.
단지 4줄의 코드로 결함을 만들기는 어려워 보입니다. 그렇죠?
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 함수를 살짝 고친 거 맞습니다. 유일한 차이는 스택을 클리어 해주는 sodium_memzero 함수를 빼먹은 것입니다. 스택을 클리어 해주지 않은 곳은 toxcore 의 곳곳에 있습니다.
Fact no 6. Compiler warning 을 무시해서는 안됩니다.
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)
~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
../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는 현존 세계 최고의 보안성를 보유하고 있다고 생각합니다.
다음 글에서 저는 Rust언어를 사용하여 tox를 재개발 하는 프로젝트의 리더로서, 우리의 이야기를 하고저 합니다.
Roman Proskuryakov
Team Lead
