10 files changed, 258 insertions, 142 deletions

diff --git a/auto_tests/dht_test.c b/auto_tests/dht_test.c
index 7734e64a..9cec1643 100644
--- a/auto_tests/dht_test.c
+++ b/auto_tests/dht_test.c
@@ -2,13 +2,13 @@
 #include "config.h"
 #endif
 
-#include <sys/param.h>
-#include <time.h>
+#include "helpers.h"
 
 #include "../toxcore/DHT.c"
 #include "../toxcore/tox.h"
 
-#include "helpers.h"
+#include <sys/param.h>
+#include <time.h>
 
 
 // These tests currently fail.
@@ -299,6 +299,10 @@ static void test_addto_lists_good(DHT            *dht,
     ck_assert_msg(client_in_list(list, length, public_key) == -1, "Good client id is in the list");
 }
 
+#ifndef MAX
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+#endif
+
 static void test_addto_lists(IP ip)
 {
     Networking_Core *net = new_networking(NULL, ip, TOX_PORT_DEFAULT);
diff --git a/auto_tests/messenger_test.c b/auto_tests/messenger_test.c
index eed52a31..03697ce6 100644
--- a/auto_tests/messenger_test.c
+++ b/auto_tests/messenger_test.c
@@ -14,18 +14,27 @@
 #include "config.h"
 #endif
 
+#include "helpers.h"
+
 #include "../testing/misc_tools.c" // hex_string_to_bin
 #include "../toxcore/Messenger.h"
+
 #include <check.h>
 #include <stdint.h>
 #include <string.h>
 #include <sys/types.h>
 
-#include "helpers.h"
+#if VANILLA_NACL
+#include <crypto_box.h> // crypto_box_PUBLICKEYBYTES and other defines.
+#else
+#include <sodium.h>
+#endif
 
 #define REALLY_BIG_NUMBER ((1) << (sizeof(uint16_t) * 7))
 #define STRINGS_EQUAL(X, Y) (strcmp(X, Y) == 0)
 
+static bool enable_broken_tests = false;
+
 static const char *friend_id_str = "e4b3d5030bc99494605aecc33ceec8875640c1d74aa32790e821b17e98771c4a00000000f1db";
 
 /* in case we need more than one ID for a test */
@@ -121,11 +130,10 @@ START_TEST(test_m_delfriend)
 }
 END_TEST
 
-#if 0
 START_TEST(test_m_addfriend)
 {
-    char *good_data = "test";
-    char *bad_data = "";
+    const char *good_data = "test";
+    const char *bad_data = "";
 
     int good_len = strlen(good_data);
     int bad_len = strlen(bad_data);
@@ -134,30 +142,29 @@ START_TEST(test_m_addfriend)
                           + crypto_box_ZEROBYTES + 100);
 
     /* TODO(irungentoo): Update this properly to latest master */
-    if (m_addfriend(m, (uint8_t *)friend_id, (uint8_t *)good_data, really_bad_len) != FAERR_TOOLONG) {
+    if (m_addfriend(m, (const uint8_t *)friend_id, (const uint8_t *)good_data, really_bad_len) != FAERR_TOOLONG) {
         ck_abort_msg("m_addfriend did NOT catch the following length: %d\n", really_bad_len);
     }
 
     /* this will return an error if the original m_addfriend_norequest() failed */
-    if (m_addfriend(m, (uint8_t *)friend_id, (uint8_t *)good_data, good_len) != FAERR_ALREADYSENT) {
+    if (m_addfriend(m, (const uint8_t *)friend_id, (const uint8_t *)good_data, good_len) != FAERR_ALREADYSENT) {
         ck_abort_msg("m_addfriend did NOT catch adding a friend we already have.\n"
                      "(this can be caused by the error of m_addfriend_norequest in"
                      " the beginning of the suite)\n");
     }
 
-    if (m_addfriend(m, (uint8_t *)good_id_b, (uint8_t *)bad_data, bad_len) != FAERR_NOMESSAGE) {
+    if (m_addfriend(m, (const uint8_t *)good_id_b, (const uint8_t *)bad_data, bad_len) != FAERR_NOMESSAGE) {
         ck_abort_msg("m_addfriend did NOT catch the following length: %d\n", bad_len);
     }
 
     /* this should REALLY return an error */
     /* TODO(irungentoo): validate client_id in m_addfriend? */
-    if (m_addfriend((uint8_t *)bad_id, (uint8_t *)good_data, good_len) >= 0) {
+    if (m_addfriend(m, (const uint8_t *)bad_id, (const uint8_t *)good_data, good_len) >= 0) {
         ck_abort_msg("The following ID passed through "
                      "m_addfriend without an error:\n'%s'\n", bad_id_str);
     }
 }
 END_TEST
-#endif
 
 START_TEST(test_setname)
 {
@@ -193,7 +200,7 @@ END_TEST
  *  ideas:
  *      if we have access to the friends list, we could
  *      just add a status manually ourselves. */
-/*
+#if 0
 START_TEST(test_m_copy_userstatus)
 {
     assert(m_copy_userstatus(-1, buf, MAX_USERSTATUS_LENGTH) == -1);
@@ -203,7 +210,7 @@ START_TEST(test_m_copy_userstatus)
     assert(STRINGS_EQUAL(name_buf, friend_id_status));
 }
 END_TEST
-*/
+#endif
 
 START_TEST(test_getname)
 {
@@ -316,7 +323,10 @@ static Suite *messenger_suite(void)
     DEFTESTCASE(m_get_userstatus_size);
     DEFTESTCASE(m_set_userstatus);
 
-    /* DEFTESTCASE(m_addfriend); */
+    if (enable_broken_tests) {
+        DEFTESTCASE(m_addfriend);
+    }
+
     DEFTESTCASE(m_friend_exists);
     DEFTESTCASE(m_get_friend_connectionstatus);
     DEFTESTCASE(m_delfriend);
diff --git a/auto_tests/toxav_basic_test.c b/auto_tests/toxav_basic_test.c
index b5028da4..767fca87 100644
--- a/auto_tests/toxav_basic_test.c
+++ b/auto_tests/toxav_basic_test.c
@@ -20,7 +20,8 @@
 #include "../toxcore/util.h"
 
 
-#if defined(_WIN32) || defined(__WIN32__) || defined (WIN32)
+#if defined(_WIN32) || defined(__WIN32__) || defined(WIN32)
+#include <windows.h>
 #define c_sleep(x) Sleep(1*x)
 #else
 #include <unistd.h>
diff --git a/auto_tests/toxav_many_test.c b/auto_tests/toxav_many_test.c
index 59e4f94b..b33b93c8 100644
--- a/auto_tests/toxav_many_test.c
+++ b/auto_tests/toxav_many_test.c
@@ -19,7 +19,8 @@
 #include "../toxcore/tox.h"
 #include "../toxcore/util.h"
 
-#if defined(_WIN32) || defined(__WIN32__) || defined (WIN32)
+#if defined(_WIN32) || defined(__WIN32__) || defined(WIN32)
+#include <windows.h>
 #define c_sleep(x) Sleep(1*x)
 #else
 #include <pthread.h>
diff --git a/toxcore/DHT.c b/toxcore/DHT.c
index e31405fd..23faf57b 100644
--- a/toxcore/DHT.c
+++ b/toxcore/DHT.c
@@ -197,7 +197,7 @@ int create_request(const uint8_t *send_public_key, const uint8_t *send_secret_ke
 
     uint8_t *nonce = packet + 1 + CRYPTO_PUBLIC_KEY_SIZE * 2;
     random_nonce(nonce);
-    uint8_t temp[MAX_CRYPTO_REQUEST_SIZE]; // TODO(irungentoo): sodium_memzero before exit function
+    uint8_t temp[MAX_CRYPTO_REQUEST_SIZE]; // TODO(irungentoo): crypto_memzero before exit function
     memcpy(temp + 1, data, length);
     temp[0] = request_id;
     int len = encrypt_data(recv_public_key, send_secret_key, nonce, temp, length + 1,
@@ -238,7 +238,7 @@ int handle_request(const uint8_t *self_public_key, const uint8_t *self_secret_ke
 
     memcpy(public_key, packet + 1 + CRYPTO_PUBLIC_KEY_SIZE, CRYPTO_PUBLIC_KEY_SIZE);
     const uint8_t *nonce = packet + 1 + CRYPTO_PUBLIC_KEY_SIZE * 2;
-    uint8_t temp[MAX_CRYPTO_REQUEST_SIZE]; // TODO(irungentoo): sodium_memzero before exit function
+    uint8_t temp[MAX_CRYPTO_REQUEST_SIZE]; // TODO(irungentoo): crypto_memzero before exit function
     int len1 = decrypt_data(public_key, self_secret_key, nonce,
                             packet + 1 + CRYPTO_PUBLIC_KEY_SIZE * 2 + CRYPTO_NONCE_SIZE,
                             length - (CRYPTO_PUBLIC_KEY_SIZE * 2 + CRYPTO_NONCE_SIZE + 1), temp);
diff --git a/toxcore/crypto_core.api.h b/toxcore/crypto_core.api.h
index 78a17280..6f0b1b0b 100644
--- a/toxcore/crypto_core.api.h
+++ b/toxcore/crypto_core.api.h
@@ -21,10 +21,12 @@
  *  along with Tox.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
-#ifndef CORE_CRYPTO_H
-#define CORE_CRYPTO_H
+#ifndef CRYPTO_CORE_H
+#define CRYPTO_CORE_H
 
-#include "network.h"
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
 %}
 
 /**
@@ -38,12 +40,12 @@ const CRYPTO_PUBLIC_KEY_SIZE = 32;
 const CRYPTO_SECRET_KEY_SIZE = 32;
 
 /**
- * The number of bytes in a shared key computed from public and secret key.
+ * The number of bytes in a shared key computed from public and secret keys.
  */
 const CRYPTO_SHARED_KEY_SIZE = 32;
 
 /**
- * The number of bytes in a random symmetric key.
+ * The number of bytes in a symmetric key.
  */
 const CRYPTO_SYMMETRIC_KEY_SIZE = CRYPTO_SHARED_KEY_SIZE;
 
@@ -68,20 +70,44 @@ const CRYPTO_SHA256_SIZE = 32;
  */
 const CRYPTO_SHA512_SIZE = 64;
 
+/**
+ * A `memcmp`-like function whose running time does not depend on the input
+ * bytes, only on the input length. Useful to compare sensitive data where
+ * timing attacks could reveal that data.
+ *
+ * This means for instance that comparing "aaaa" and "aaaa" takes 4 time, and
+ * "aaaa" and "baaa" also takes 4 time. With a regular `memcmp`, the latter may
+ * take 1 time, because it immediately knows that the two strings are not equal.
+ */
 static int32_t crypto_memcmp(const void *p1, const void *p2, size_t length);
+
+/**
+ * A `bzero`-like function which won't be optimised away by the compiler. Some
+ * compilers will inline `bzero` or `memset` if they can prove that there will
+ * be no reads to the written data. Use this function if you want to be sure the
+ * memory is indeed zeroed.
+ */
 static void crypto_memzero(void *data, size_t length);
 
-static void crypto_sha256(uint8_t *hash, const uint8_t[length] data);
-static void crypto_sha512(uint8_t *hash, const uint8_t[length] data);
+/**
+ * Compute a SHA256 hash (32 bytes).
+ */
+static void crypto_sha256(uint8_t[CRYPTO_SHA256_SIZE] hash, const uint8_t[length] data);
 
-static void crypto_derive_public_key(uint8_t *public_key, uint8_t *secret_key);
+/**
+ * Compute a SHA512 hash (64 bytes).
+ */
+static void crypto_sha512(uint8_t[CRYPTO_SHA512_SIZE] hash, const uint8_t[length] data);
 
 /**
- * compare 2 public keys of length CRYPTO_PUBLIC_KEY_SIZE, not vulnerable to timing attacks.
- * returns 0 if both mem locations of length are equal,
- * return -1 if they are not.
+ * Compare 2 public keys of length CRYPTO_PUBLIC_KEY_SIZE, not vulnerable to
+ * timing attacks.
+ *
+ * @return 0 if both mem locations of length are equal, -1 if they are not.
  */
-static int32_t public_key_cmp(const uint8_t *pk1, const uint8_t *pk2);
+static int32_t public_key_cmp(
+    const uint8_t[CRYPTO_PUBLIC_KEY_SIZE] pk1,
+    const uint8_t[CRYPTO_PUBLIC_KEY_SIZE] pk2);
 
 /**
  * Return a random 32 bit integer.
@@ -94,88 +120,120 @@ static uint32_t random_int();
 static uint64_t random_64b();
 
 /**
- * Check if a Tox public key CRYPTO_PUBLIC_KEY_SIZE is valid or not.
- * This should only be used for input validation.
+ * Check if a Tox public key CRYPTO_PUBLIC_KEY_SIZE is valid or not. This
+ * should only be used for input validation.
  *
- * return 0 if it isn't.
- * return 1 if it is.
+ * @return false if it isn't, true if it is.
+ */
+static bool public_key_valid(const uint8_t[CRYPTO_PUBLIC_KEY_SIZE] public_key);
+
+/**
+ * Generate a new random keypair. Every call to this function is likely to
+ * generate a different keypair.
  */
-static int32_t public_key_valid(const uint8_t *public_key);
+static int32_t crypto_new_keypair(
+    uint8_t[CRYPTO_PUBLIC_KEY_SIZE] public_key,
+    uint8_t[CRYPTO_SECRET_KEY_SIZE] secret_key);
 
-static int32_t crypto_new_keypair(uint8_t *public_key, uint8_t *secret_key);
+/**
+ * Derive the public key from a given secret key.
+ */
+static void crypto_derive_public_key(
+    uint8_t[CRYPTO_PUBLIC_KEY_SIZE] public_key,
+    const uint8_t[CRYPTO_SECRET_KEY_SIZE] secret_key);
 
 /**
- * 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.
+ * Encrypt plain text of the given length to encrypted of length +
+ * $CRYPTO_MAC_SIZE using the public key ($CRYPTO_PUBLIC_KEY_SIZE bytes) of the
+ * receiver and the secret key of the sender and a $CRYPTO_NONCE_SIZE byte
+ * nonce.
  *
- *  return -1 if there was a problem.
- *  return length of encrypted data if everything was fine.
+ * @return -1 if there was a problem, length of encrypted data if everything
+ * was fine.
  */
 static int32_t 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);
+    const uint8_t[CRYPTO_PUBLIC_KEY_SIZE] public_key,
+    const uint8_t[CRYPTO_SECRET_KEY_SIZE] secret_key,
+    const uint8_t[CRYPTO_NONCE_SIZE] nonce,
+    const uint8_t[length] plain,
+    uint8_t *encrypted);
 
 
 /**
- * Decrypts encrypted of length length to plain of length length - 16 using the
- * public key(32 bytes) of the sender, the secret key of the receiver and a 24 byte nonce.
+ * Decrypt encrypted text of the given length to plain text of the given length
+ * - $CRYPTO_MAC_SIZE using the public key ($CRYPTO_PUBLIC_KEY_SIZE bytes) of
+ * the sender, the secret key of the receiver and a $CRYPTO_NONCE_SIZE byte
+ * nonce.
  *
- *  return -1 if there was a problem (decryption failed).
- *  return length of plain data if everything was fine.
+ * @return -1 if there was a problem (decryption failed), length of plain text
+ * data if everything was fine.
  */
 static int32_t decrypt_data(
-    const uint8_t *public_key, const uint8_t *secret_key, const uint8_t *nonce,
-    const uint8_t *encrypted, uint32_t length, uint8_t *plain);
+    const uint8_t[CRYPTO_PUBLIC_KEY_SIZE] public_key,
+    const uint8_t[CRYPTO_SECRET_KEY_SIZE] secret_key,
+    const uint8_t[CRYPTO_NONCE_SIZE] nonce,
+    const uint8_t[length] encrypted,
+    uint8_t *plain);
 
 /**
  * Fast encrypt/decrypt operations. Use if this is not a one-time communication.
- * encrypt_precompute does the shared-key generation once so it does not have
+ * $encrypt_precompute does the shared-key generation once so it does not have
  * to be preformed on every encrypt/decrypt.
  */
 static int32_t encrypt_precompute(
-    const uint8_t *public_key, const uint8_t *secret_key, uint8_t *enc_key);
+    const uint8_t[CRYPTO_PUBLIC_KEY_SIZE] public_key,
+    const uint8_t[CRYPTO_SECRET_KEY_SIZE] secret_key,
+    uint8_t[CRYPTO_SHARED_KEY_SIZE] shared_key);
 
 /**
- * Encrypts plain of length length to encrypted of length + 16 using a
- * secret key CRYPTO_SYMMETRIC_KEY_SIZE big and a 24 byte nonce.
+ * Encrypts plain of length length to encrypted of length + $CRYPTO_MAC_SIZE
+ * using a shared key $CRYPTO_SYMMETRIC_KEY_SIZE big and a $CRYPTO_NONCE_SIZE
+ * byte nonce.
  *
- *  return -1 if there was a problem.
- *  return length of encrypted data if everything was fine.
+ * @return -1 if there was a problem, length of encrypted data if everything
+ * was fine.
  */
 static int32_t encrypt_data_symmetric(
-    const uint8_t *secret_key, const uint8_t *nonce, const uint8_t *plain,
-    uint32_t length, uint8_t *encrypted);
+    const uint8_t[CRYPTO_SHARED_KEY_SIZE] shared_key,
+    const uint8_t[CRYPTO_NONCE_SIZE] nonce,
+    const uint8_t[length] plain,
+    uint8_t *encrypted);
 
 /**
- * Decrypts encrypted of length length to plain of length length - 16 using a
- * secret key CRYPTO_SYMMETRIC_KEY_SIZE big and a 24 byte nonce.
+ * Decrypts encrypted of length length to plain of length length -
+ * $CRYPTO_MAC_SIZE using a shared key CRYPTO_SHARED_KEY_SIZE big and a
+ * $CRYPTO_NONCE_SIZE byte nonce.
  *
- *  return -1 if there was a problem (decryption failed).
- *  return length of plain data if everything was fine.
+ * @return -1 if there was a problem (decryption failed), length of plain data
+ * if everything was fine.
  */
 static int32_t decrypt_data_symmetric(
-    const uint8_t *secret_key, const uint8_t *nonce, const uint8_t *encrypted,
-    uint32_t length, uint8_t *plain);
+    const uint8_t[CRYPTO_SHARED_KEY_SIZE] shared_key,
+    const uint8_t[CRYPTO_NONCE_SIZE] nonce,
+    const uint8_t[length] encrypted,
+    uint8_t *plain);
 
 /**
- * Increment the given nonce by 1.
+ * Increment the given nonce by 1 in big endian (rightmost byte incremented
+ * first).
  */
-static void increment_nonce(uint8_t *nonce);
+static void increment_nonce(uint8_t[CRYPTO_NONCE_SIZE] nonce);
 
 /**
- * Increment the given nonce by num.
+ * Increment the given nonce by a given number. The number should be in host
+ * byte order.
  */
-static void increment_nonce_number(uint8_t *nonce, uint32_t host_order_num);
+static void increment_nonce_number(uint8_t[CRYPTO_NONCE_SIZE] nonce, uint32_t host_order_num);
 
 /**
  * Fill the given nonce with random bytes.
  */
-static void random_nonce(uint8_t *nonce);
+static void random_nonce(uint8_t[CRYPTO_NONCE_SIZE] nonce);
 
 /**
  * Fill a key CRYPTO_SYMMETRIC_KEY_SIZE big with random bytes.
  */
-static void new_symmetric_key(uint8_t *key);
+static void new_symmetric_key(uint8_t[CRYPTO_SYMMETRIC_KEY_SIZE] key);
 
 /**
  * Fill an array of bytes with random values.
@@ -183,5 +241,5 @@ static void new_symmetric_key(uint8_t *key);
 static void random_bytes(uint8_t[length] bytes);
 
 %{
-#endif
+#endif /* CRYPTO_CORE_H */
 %}
diff --git a/toxcore/crypto_core.c b/toxcore/crypto_core.c
index d4f7c562..d3a3e1fc 100644
--- a/toxcore/crypto_core.c
+++ b/toxcore/crypto_core.c
@@ -29,6 +29,10 @@
 
 #include "crypto_core.h"
 
+#include "network.h"
+
+#include <string.h>
+
 #ifndef VANILLA_NACL
 /* We use libsodium by default. */
 #include <sodium.h>
@@ -41,9 +45,6 @@
 #include <crypto_verify_32.h>
 #include <randombytes.h>
 #define crypto_box_MACBYTES (crypto_box_ZEROBYTES - crypto_box_BOXZEROBYTES)
-/* I know */
-#define sodium_memcmp(a, b, c) memcmp(a, b, c)
-#define sodium_memzero(a, c) memset(a, 0, c)
 #endif
 
 #if CRYPTO_PUBLIC_KEY_SIZE != crypto_box_PUBLICKEYBYTES
@@ -78,19 +79,14 @@
 #error CRYPTO_SHA512_SIZE should be equal to crypto_hash_sha512_BYTES
 #endif
 
-/* 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)
+int32_t public_key_cmp(const uint8_t *pk1, const uint8_t *pk2)
 {
-#if crypto_box_PUBLICKEYBYTES != 32
-#error crypto_box_PUBLICKEYBYTES is required to be 32 bytes for public_key_cmp to work,
+#if CRYPTO_PUBLIC_KEY_SIZE != 32
+#error CRYPTO_PUBLIC_KEY_SIZE is required to be 32 bytes for public_key_cmp to work,
 #endif
     return crypto_verify_32(pk1, pk2);
 }
 
-/*  return a random number.
- */
 uint32_t random_int(void)
 {
     uint32_t randnum;
@@ -105,13 +101,7 @@ uint64_t random_64b(void)
     return randnum;
 }
 
-/* Check if a Tox public key crypto_box_PUBLICKEYBYTES is valid or not.
- * This should only be used for input validation.
- *
- * return 0 if it isn't.
- * return 1 if it is.
- */
-int public_key_valid(const uint8_t *public_key)
+bool public_key_valid(const uint8_t *public_key)
 {
     if (public_key[31] >= 128) { /* Last bit of key is always zero. */
         return 0;
@@ -123,15 +113,15 @@ int public_key_valid(const uint8_t *public_key)
 /* 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.
+ * shared_key has to be crypto_box_BEFORENMBYTES bytes long.
  */
-int encrypt_precompute(const uint8_t *public_key, const uint8_t *secret_key, uint8_t *enc_key)
+int32_t encrypt_precompute(const uint8_t *public_key, const uint8_t *secret_key, uint8_t *shared_key)
 {
-    return crypto_box_beforenm(enc_key, public_key, secret_key);
+    return crypto_box_beforenm(shared_key, public_key, secret_key);
 }
 
-int encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce, const uint8_t *plain, uint32_t length,
-                           uint8_t *encrypted)
+int32_t encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce, const uint8_t *plain, size_t length,
+                               uint8_t *encrypted)
 {
     if (length == 0 || !secret_key || !nonce || !plain || !encrypted) {
         return -1;
@@ -152,8 +142,8 @@ int encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce, cons
     return length + crypto_box_MACBYTES;
 }
 
-int decrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce, const uint8_t *encrypted, uint32_t length,
-                           uint8_t *plain)
+int32_t decrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce, const uint8_t *encrypted, size_t length,
+                               uint8_t *plain)
 {
     if (length <= crypto_box_BOXZEROBYTES || !secret_key || !nonce || !encrypted || !plain) {
         return -1;
@@ -173,8 +163,8 @@ int decrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce, cons
     return length - crypto_box_MACBYTES;
 }
 
-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)
+int32_t encrypt_data(const uint8_t *public_key, const uint8_t *secret_key, const uint8_t *nonce,
+                     const uint8_t *plain, size_t length, uint8_t *encrypted)
 {
     if (!public_key || !secret_key) {
         return -1;
@@ -187,8 +177,8 @@ int encrypt_data(const uint8_t *public_key, const uint8_t *secret_key, const uin
     return ret;
 }
 
-int decrypt_data(const uint8_t *public_key, const uint8_t *secret_key, const uint8_t *nonce,
-                 const uint8_t *encrypted, uint32_t length, uint8_t *plain)
+int32_t decrypt_data(const uint8_t *public_key, const uint8_t *secret_key, const uint8_t *nonce,
+                     const uint8_t *encrypted, size_t length, uint8_t *plain)
 {
     if (!public_key || !secret_key) {
         return -1;
@@ -263,7 +253,7 @@ int32_t crypto_new_keypair(uint8_t *public_key, uint8_t *secret_key)
     return crypto_box_keypair(public_key, secret_key);
 }
 
-void crypto_derive_public_key(uint8_t *public_key, uint8_t *secret_key)
+void crypto_derive_public_key(uint8_t *public_key, const uint8_t *secret_key)
 {
     crypto_scalarmult_curve25519_base(public_key, secret_key);
 }
@@ -280,12 +270,23 @@ void crypto_sha512(uint8_t *hash, const uint8_t *data, size_t length)
 
 void crypto_memzero(void *data, size_t length)
 {
+#ifdef VANILLA_NACL
+    /* TODO(c-toxcore#347): this is insecure. We need to provide our own
+     * secure memzero/memcmp for NaCL. */
+    memset(data, 0, length);
+#else
     sodium_memzero(data, length);
+#endif
 }
 
 int32_t crypto_memcmp(const void *p1, const void *p2, size_t length)
 {
+#ifdef VANILLA_NACL
+    /* TODO(c-toxcore#347): Implement secure memcmp. */
+    return memcmp(p1, p2, length);
+#else
     return sodium_memcmp(p1, p2, length);
+#endif
 }
 
 void random_bytes(uint8_t *data, size_t length)
diff --git a/toxcore/crypto_core.h b/toxcore/crypto_core.h
index a5cea019..56476e38 100644
--- a/toxcore/crypto_core.h
+++ b/toxcore/crypto_core.h
@@ -20,10 +20,12 @@
  *  along with Tox.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
-#ifndef CORE_CRYPTO_H
-#define CORE_CRYPTO_H
+#ifndef CRYPTO_CORE_H
+#define CRYPTO_CORE_H
 
-#include "network.h"
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdlib.h>
 
 /**
  * The number of bytes in a Tox public key.
@@ -40,14 +42,14 @@ uint32_t crypto_public_key_size(void);
 uint32_t crypto_secret_key_size(void);
 
 /**
- * The number of bytes in a shared key computed from public and secret key.
+ * The number of bytes in a shared key computed from public and secret keys.
  */
 #define CRYPTO_SHARED_KEY_SIZE         32
 
 uint32_t crypto_shared_key_size(void);
 
 /**
- * The number of bytes in a random symmetric key.
+ * The number of bytes in a symmetric key.
  */
 #define CRYPTO_SYMMETRIC_KEY_SIZE      CRYPTO_SHARED_KEY_SIZE
 
@@ -82,20 +84,40 @@ uint32_t crypto_sha256_size(void);
 
 uint32_t crypto_sha512_size(void);
 
+/**
+ * A `memcmp`-like function whose running time does not depend on the input
+ * bytes, only on the input length. Useful to compare sensitive data where
+ * timing attacks could reveal that data.
+ *
+ * This means for instance that comparing "aaaa" and "aaaa" takes 4 time, and
+ * "aaaa" and "baaa" also takes 4 time. With a regular `memcmp`, the latter may
+ * take 1 time, because it immediately knows that the two strings are not equal.
+ */
 int32_t crypto_memcmp(const void *p1, const void *p2, size_t length);
 
+/**
+ * A `bzero`-like function which won't be optimised away by the compiler. Some
+ * compilers will inline `bzero` or `memset` if they can prove that there will
+ * be no reads to the written data. Use this function if you want to be sure the
+ * memory is indeed zeroed.
+ */
 void crypto_memzero(void *data, size_t length);
 
+/**
+ * Compute a SHA256 hash (32 bytes).
+ */
 void crypto_sha256(uint8_t *hash, const uint8_t *data, size_t length);
 
+/**
+ * Compute a SHA512 hash (64 bytes).
+ */
 void crypto_sha512(uint8_t *hash, const uint8_t *data, size_t length);
 
-void crypto_derive_public_key(uint8_t *public_key, uint8_t *secret_key);
-
 /**
- * compare 2 public keys of length CRYPTO_PUBLIC_KEY_SIZE, not vulnerable to timing attacks.
- * returns 0 if both mem locations of length are equal,
- * return -1 if they are not.
+ * Compare 2 public keys of length CRYPTO_PUBLIC_KEY_SIZE, not vulnerable to
+ * timing attacks.
+ *
+ * @return 0 if both mem locations of length are equal, -1 if they are not.
  */
 int32_t public_key_cmp(const uint8_t *pk1, const uint8_t *pk2);
 
@@ -110,70 +132,86 @@ uint32_t random_int(void);
 uint64_t random_64b(void);
 
 /**
- * Check if a Tox public key CRYPTO_PUBLIC_KEY_SIZE is valid or not.
- * This should only be used for input validation.
+ * Check if a Tox public key CRYPTO_PUBLIC_KEY_SIZE is valid or not. This
+ * should only be used for input validation.
  *
- * return 0 if it isn't.
- * return 1 if it is.
+ * @return false if it isn't, true if it is.
  */
-int32_t public_key_valid(const uint8_t *public_key);
+bool public_key_valid(const uint8_t *public_key);
 
+/**
+ * Generate a new random keypair. Every call to this function is likely to
+ * generate a different keypair.
+ */
 int32_t crypto_new_keypair(uint8_t *public_key, uint8_t *secret_key);
 
 /**
- * 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.
+ * Derive the public key from a given secret key.
+ */
+void crypto_derive_public_key(uint8_t *public_key, const uint8_t *secret_key);
+
+/**
+ * Encrypt plain text of the given length to encrypted of length +
+ * CRYPTO_MAC_SIZE using the public key (CRYPTO_PUBLIC_KEY_SIZE bytes) of the
+ * receiver and the secret key of the sender and a CRYPTO_NONCE_SIZE byte
+ * nonce.
  *
- *  return -1 if there was a problem.
- *  return length of encrypted data if everything was fine.
+ * @return -1 if there was a problem, length of encrypted data if everything
+ * was fine.
  */
 int32_t 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);
+                     size_t length, uint8_t *encrypted);
 
 /**
- * Decrypts encrypted of length length to plain of length length - 16 using the
- * public key(32 bytes) of the sender, the secret key of the receiver and a 24 byte nonce.
+ * Decrypt encrypted text of the given length to plain text of the given length
+ * - CRYPTO_MAC_SIZE using the public key (CRYPTO_PUBLIC_KEY_SIZE bytes) of
+ * the sender, the secret key of the receiver and a CRYPTO_NONCE_SIZE byte
+ * nonce.
  *
- *  return -1 if there was a problem (decryption failed).
- *  return length of plain data if everything was fine.
+ * @return -1 if there was a problem (decryption failed), length of plain text
+ * data if everything was fine.
  */
 int32_t decrypt_data(const uint8_t *public_key, const uint8_t *secret_key, const uint8_t *nonce,
-                     const uint8_t *encrypted, uint32_t length, uint8_t *plain);
+                     const uint8_t *encrypted, size_t length, uint8_t *plain);
 
 /**
  * Fast encrypt/decrypt operations. Use if this is not a one-time communication.
  * encrypt_precompute does the shared-key generation once so it does not have
  * to be preformed on every encrypt/decrypt.
  */
-int32_t encrypt_precompute(const uint8_t *public_key, const uint8_t *secret_key, uint8_t *enc_key);
+int32_t encrypt_precompute(const uint8_t *public_key, const uint8_t *secret_key, uint8_t *shared_key);
 
 /**
- * Encrypts plain of length length to encrypted of length + 16 using a
- * secret key CRYPTO_SYMMETRIC_KEY_SIZE big and a 24 byte nonce.
+ * Encrypts plain of length length to encrypted of length + CRYPTO_MAC_SIZE
+ * using a shared key CRYPTO_SYMMETRIC_KEY_SIZE big and a CRYPTO_NONCE_SIZE
+ * byte nonce.
  *
- *  return -1 if there was a problem.
- *  return length of encrypted data if everything was fine.
+ * @return -1 if there was a problem, length of encrypted data if everything
+ * was fine.
  */
-int32_t encrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce, const uint8_t *plain, uint32_t length,
+int32_t encrypt_data_symmetric(const uint8_t *shared_key, const uint8_t *nonce, const uint8_t *plain, size_t length,
                                uint8_t *encrypted);
 
 /**
- * Decrypts encrypted of length length to plain of length length - 16 using a
- * secret key CRYPTO_SYMMETRIC_KEY_SIZE big and a 24 byte nonce.
+ * Decrypts encrypted of length length to plain of length length -
+ * CRYPTO_MAC_SIZE using a shared key CRYPTO_SHARED_KEY_SIZE big and a
+ * CRYPTO_NONCE_SIZE byte nonce.
  *
- *  return -1 if there was a problem (decryption failed).
- *  return length of plain data if everything was fine.
+ * @return -1 if there was a problem (decryption failed), length of plain data
+ * if everything was fine.
  */
-int32_t decrypt_data_symmetric(const uint8_t *secret_key, const uint8_t *nonce, const uint8_t *encrypted,
-                               uint32_t length, uint8_t *plain);
+int32_t decrypt_data_symmetric(const uint8_t *shared_key, const uint8_t *nonce, const uint8_t *encrypted, size_t length,
+                               uint8_t *plain);
 
 /**
- * Increment the given nonce by 1.
+ * Increment the given nonce by 1 in big endian (rightmost byte incremented
+ * first).
  */
 void increment_nonce(uint8_t *nonce);
 
 /**
- * Increment the given nonce by num.
+ * 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);
 
@@ -192,5 +230,4 @@ void new_symmetric_key(uint8_t *key);
  */
 void random_bytes(uint8_t *bytes, size_t length);
 
-#endif
-
+#endif /* CRYPTO_CORE_H */
diff --git a/toxcore/util.c b/toxcore/util.c
index 2dfa360e..7a390ea9 100644
--- a/toxcore/util.c
+++ b/toxcore/util.c
@@ -29,6 +29,7 @@
 #include "util.h"
 
 #include "crypto_core.h" /* for CRYPTO_PUBLIC_KEY_SIZE */
+#include "network.h" /* for current_time_monotonic */
 
 #include <time.h>
 
diff --git a/toxencryptsave/toxencryptsave.c b/toxencryptsave/toxencryptsave.c
index 360952ad..bd56e780 100644
--- a/toxencryptsave/toxencryptsave.c
+++ b/toxencryptsave/toxencryptsave.c
@@ -33,8 +33,11 @@
 #ifdef VANILLA_NACL
 #include <crypto_hash_sha256.h>
 #include "crypto_pwhash_scryptsalsa208sha256/crypto_pwhash_scryptsalsa208sha256.h"
-#endif
+#else
 #include <sodium.h>
+#endif
+
+#include <string.h>
 
 #if TOX_PASS_SALT_LENGTH != crypto_pwhash_scryptsalsa208sha256_SALTBYTES
 #error TOX_PASS_SALT_LENGTH is assumed to be equal to crypto_pwhash_scryptsalsa208sha256_SALTBYTES