Skip to content

Commit

Permalink
cleanup: Avoid endian-specific code in crypto_core.
Browse files Browse the repository at this point in the history
  • Loading branch information
iphydf committed Dec 7, 2021
1 parent 2db6599 commit 78edc1d
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 24 deletions.
2 changes: 1 addition & 1 deletion toxcore/crypto_core.api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 5 additions & 19 deletions toxcore/crypto_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion toxcore/crypto_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
41 changes: 38 additions & 3 deletions toxcore/crypto_core_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ std::pair<clock_t, clock_t> memcmp_median(uint8_t const *src, uint8_t const *sam
*/
TEST(CryptoCore, MemcmpTimingIsDataIndependent) {
// A random piece of memory.
std::vector<uint8_t> src(CRYPTO_TEST_MEMCMP_SIZE);
std::array<uint8_t, CRYPTO_TEST_MEMCMP_SIZE> src;
random_bytes(src.data(), CRYPTO_TEST_MEMCMP_SIZE);

// A separate piece of memory containing the same data.
std::vector<uint8_t> same = src;
std::array<uint8_t, CRYPTO_TEST_MEMCMP_SIZE> same = src;

// Another piece of memory containing different data.
std::vector<uint8_t> not_same(CRYPTO_TEST_MEMCMP_SIZE);
std::array<uint8_t, CRYPTO_TEST_MEMCMP_SIZE> not_same;
random_bytes(not_same.data(), CRYPTO_TEST_MEMCMP_SIZE);

// Once we have C++17:
Expand All @@ -89,4 +89,39 @@ TEST(CryptoCore, MemcmpTimingIsDataIndependent) {
<< "Time of the different data comparison: " << result.second << " clocks";
}

TEST(CryptoCore, IncrementNonce) {
using Nonce = std::array<uint8_t, CRYPTO_NONCE_SIZE>;
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<uint8_t, CRYPTO_NONCE_SIZE>;
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

0 comments on commit 78edc1d

Please sign in to comment.