From 78edc1d4255ed1ea6b62ff545a59012c0f2d8b98 Mon Sep 17 00:00:00 2001 From: iphydf Date: Tue, 7 Dec 2021 12:14:12 +0000 Subject: [PATCH] cleanup: Avoid endian-specific code in `crypto_core`. --- toxcore/crypto_core.api.h | 2 +- toxcore/crypto_core.c | 24 +++++----------------- toxcore/crypto_core.h | 2 +- toxcore/crypto_core_test.cc | 41 ++++++++++++++++++++++++++++++++++--- 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/toxcore/crypto_core.api.h b/toxcore/crypto_core.api.h index 0db3a420014..84e7eff2d23 100644 --- a/toxcore/crypto_core.api.h +++ b/toxcore/crypto_core.api.h @@ -237,7 +237,7 @@ static void increment_nonce(uint8_t[CRYPTO_NONCE_SIZE] nonce); * Increment the given nonce by a given number. The number should be in host * byte order. */ -static void increment_nonce_number(uint8_t[CRYPTO_NONCE_SIZE] nonce, uint32_t host_order_num); +static void increment_nonce_number(uint8_t[CRYPTO_NONCE_SIZE] nonce, uint32_t increment); /** * Fill a key CRYPTO_SYMMETRIC_KEY_SIZE big with random bytes. diff --git a/toxcore/crypto_core.c b/toxcore/crypto_core.c index 35e3a60307f..3cc42174eff 100644 --- a/toxcore/crypto_core.c +++ b/toxcore/crypto_core.c @@ -246,33 +246,19 @@ void increment_nonce(uint8_t *nonce) } } -static uint32_t host_to_network(uint32_t x) -{ -#if !defined(BYTE_ORDER) || BYTE_ORDER == LITTLE_ENDIAN - return ((x >> 24) & 0x000000FF) | // move byte 3 to byte 0 - ((x >> 8) & 0x0000FF00) | // move byte 2 to byte 1 - ((x << 8) & 0x00FF0000) | // move byte 1 to byte 2 - ((x << 24) & 0xFF000000); // move byte 0 to byte 3 -#else - return x; -#endif -} - /* increment the given nonce by num */ -void increment_nonce_number(uint8_t *nonce, uint32_t host_order_num) +void increment_nonce_number(uint8_t *nonce, uint32_t increment) { /* NOTE don't use breaks inside this loop * In particular, make sure, as far as possible, * that loop bounds and their potential underflow or overflow * are independent of user-controlled input (you may have heard of the Heartbleed bug). */ - const uint32_t big_endian_num = host_to_network(host_order_num); - const uint8_t *const num_vec = (const uint8_t *)&big_endian_num; uint8_t num_as_nonce[crypto_box_NONCEBYTES] = {0}; - num_as_nonce[crypto_box_NONCEBYTES - 4] = num_vec[0]; - num_as_nonce[crypto_box_NONCEBYTES - 3] = num_vec[1]; - num_as_nonce[crypto_box_NONCEBYTES - 2] = num_vec[2]; - num_as_nonce[crypto_box_NONCEBYTES - 1] = num_vec[3]; + num_as_nonce[crypto_box_NONCEBYTES - 4] = increment >> 24; + num_as_nonce[crypto_box_NONCEBYTES - 3] = increment >> 16; + num_as_nonce[crypto_box_NONCEBYTES - 2] = increment >> 8; + num_as_nonce[crypto_box_NONCEBYTES - 1] = increment; uint32_t i = crypto_box_NONCEBYTES; uint_fast16_t carry = 0U; diff --git a/toxcore/crypto_core.h b/toxcore/crypto_core.h index 425c3244f03..2c828a8c1bb 100644 --- a/toxcore/crypto_core.h +++ b/toxcore/crypto_core.h @@ -223,7 +223,7 @@ void increment_nonce(uint8_t *nonce); * Increment the given nonce by a given number. The number should be in host * byte order. */ -void increment_nonce_number(uint8_t *nonce, uint32_t host_order_num); +void increment_nonce_number(uint8_t *nonce, uint32_t increment); /** * Fill a key CRYPTO_SYMMETRIC_KEY_SIZE big with random bytes. diff --git a/toxcore/crypto_core_test.cc b/toxcore/crypto_core_test.cc index 9f60fb837d7..15b7d763716 100644 --- a/toxcore/crypto_core_test.cc +++ b/toxcore/crypto_core_test.cc @@ -65,14 +65,14 @@ std::pair memcmp_median(uint8_t const *src, uint8_t const *sam */ TEST(CryptoCore, MemcmpTimingIsDataIndependent) { // A random piece of memory. - std::vector src(CRYPTO_TEST_MEMCMP_SIZE); + std::array src; random_bytes(src.data(), CRYPTO_TEST_MEMCMP_SIZE); // A separate piece of memory containing the same data. - std::vector same = src; + std::array same = src; // Another piece of memory containing different data. - std::vector not_same(CRYPTO_TEST_MEMCMP_SIZE); + std::array not_same; random_bytes(not_same.data(), CRYPTO_TEST_MEMCMP_SIZE); // Once we have C++17: @@ -89,4 +89,39 @@ TEST(CryptoCore, MemcmpTimingIsDataIndependent) { << "Time of the different data comparison: " << result.second << " clocks"; } +TEST(CryptoCore, IncrementNonce) { + using Nonce = std::array; + Nonce nonce{}; + increment_nonce(nonce.data()); + EXPECT_EQ(nonce, + (Nonce{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}})); + + for (int i = 0; i < 0x1F4; ++i) { + increment_nonce(nonce.data()); + } + + EXPECT_EQ( + nonce, + (Nonce{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01, 0xF5}})); +} + +TEST(CryptoCore, IncrementNonceNumber) { + using Nonce = std::array; + Nonce nonce{}; + + increment_nonce_number(nonce.data(), 0x1F5); + EXPECT_EQ( + nonce, + (Nonce{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01, 0xF5}})); + + increment_nonce_number(nonce.data(), 0x1F5); + EXPECT_EQ( + nonce, + (Nonce{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x03, 0xEA}})); + + increment_nonce_number(nonce.data(), 0x12345678); + EXPECT_EQ(nonce, (Nonce{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0x12, 0x34, 0x5A, 0x62}})); +} + } // namespace