From d26f0eb3bcdd622cc8adae98974a27d7487fc6cb Mon Sep 17 00:00:00 2001 From: iphydf Date: Fri, 16 Dec 2016 03:00:55 +0000 Subject: [PATCH] Change toxencryptsave API to never overwrite pass keys. --- auto_tests/encryptsave_test.c | 17 +++++------ toxencryptsave/toxencryptsave.api.h | 20 +++--------- toxencryptsave/toxencryptsave.c | 47 +++++++++++++++++------------ toxencryptsave/toxencryptsave.h | 24 +++++---------- 4 files changed, 46 insertions(+), 62 deletions(-) diff --git a/auto_tests/encryptsave_test.c b/auto_tests/encryptsave_test.c index dfe85710..7442d4fc 100644 --- a/auto_tests/encryptsave_test.c +++ b/auto_tests/encryptsave_test.c @@ -103,7 +103,8 @@ START_TEST(test_save_friend) size = tox_get_savedata_size(tox3); VLA(uint8_t, data2, size); tox_get_savedata(tox3, data2); - Tox_Pass_Key *key = tox_pass_key_new(); + TOX_ERR_KEY_DERIVATION keyerr; + Tox_Pass_Key *key = tox_pass_key_derive((const uint8_t *)"123qweasdzxc", 12, &keyerr); ck_assert_msg(key != NULL, "pass key allocation failure"); memcpy((uint8_t *)key, test_salt, TOX_PASS_SALT_LENGTH); memcpy((uint8_t *)key + TOX_PASS_SALT_LENGTH, known_key2, TOX_PASS_KEY_LENGTH); @@ -146,14 +147,12 @@ START_TEST(test_keys) TOX_ERR_ENCRYPTION encerr; TOX_ERR_DECRYPTION decerr; TOX_ERR_KEY_DERIVATION keyerr; - Tox_Pass_Key *key = tox_pass_key_new(); - ck_assert_msg(key != NULL, "pass key allocation failure"); - bool ret = tox_pass_key_derive(key, (const uint8_t *)"123qweasdzxc", 12, &keyerr); - ck_assert_msg(ret, "generic failure 1: %u", keyerr); + Tox_Pass_Key *key = tox_pass_key_derive((const uint8_t *)"123qweasdzxc", 12, &keyerr); + ck_assert_msg(key != NULL, "generic failure 1: %u", keyerr); const uint8_t *string = (const uint8_t *)"No Patrick, mayonnaise is not an instrument."; // 44 uint8_t encrypted[44 + TOX_PASS_ENCRYPTION_EXTRA_LENGTH]; - ret = tox_pass_key_encrypt(key, string, 44, encrypted, &encerr); + bool ret = tox_pass_key_encrypt(key, string, 44, encrypted, &encerr); ck_assert_msg(ret, "generic failure 2: %u", encerr); uint8_t encrypted2[44 + TOX_PASS_ENCRYPTION_EXTRA_LENGTH]; @@ -186,10 +185,8 @@ START_TEST(test_keys) TOX_ERR_GET_SALT salt_err; ck_assert_msg(tox_get_salt(encrypted, salt, &salt_err), "couldn't get salt"); ck_assert_msg(salt_err == TOX_ERR_GET_SALT_OK, "get_salt returned an error"); - Tox_Pass_Key *key2 = tox_pass_key_new(); - ck_assert_msg(key != NULL, "pass key allocation failure"); - ret = tox_pass_key_derive_with_salt(key2, (const uint8_t *)"123qweasdzxc", 12, salt, &keyerr); - ck_assert_msg(ret, "generic failure 7: %u", keyerr); + Tox_Pass_Key *key2 = tox_pass_key_derive_with_salt((const uint8_t *)"123qweasdzxc", 12, salt, &keyerr); + ck_assert_msg(key2 != NULL, "generic failure 7: %u", keyerr); ck_assert_msg(0 == memcmp(key, key2, TOX_PASS_KEY_LENGTH + TOX_PASS_SALT_LENGTH), "salt comparison failed"); tox_pass_key_free(key2); tox_pass_key_free(key); diff --git a/toxencryptsave/toxencryptsave.api.h b/toxencryptsave/toxencryptsave.api.h index 61f685f8..3adf3a9d 100644 --- a/toxencryptsave/toxencryptsave.api.h +++ b/toxencryptsave/toxencryptsave.api.h @@ -82,8 +82,7 @@ error for key_derivation { NULL, /** * The crypto lib was unable to derive a key from the given passphrase, - * which is usually a lack of memory issue. The functions accepting keys - * do not produce this error. + * which is usually a lack of memory issue. */ FAILED, } @@ -192,20 +191,11 @@ class pass_Key { * for encryption and decryption. It is derived from a salt and the user- * provided password. * - * The $this structure is hidden in the implementation. It can be allocated - * using $new and must be deallocated using $free. + * The $this structure is hidden in the implementation. It can be created + * using $derive or $derive_with_salt and must be deallocated using $free. */ struct this; - /** - * Create a new $this. The initial value of it is indeterminate. To - * initialise it, use one of the derive_* functions below. - * - * In case of failure, this function returns NULL. The only failure mode at - * this time is memory allocation failure, so this function has no error code. - */ - static this new(); - /** * Deallocate a $this. This function behaves like free(), so NULL is an * acceptable argument value. @@ -227,7 +217,7 @@ class pass_Key { * * @return true on success. */ - bool derive(const uint8_t[passphrase_len] passphrase) + static this derive(const uint8_t[passphrase_len] passphrase) with error for key_derivation; /** @@ -239,7 +229,7 @@ class pass_Key { * * @return true on success. */ - bool derive_with_salt(const uint8_t[passphrase_len] passphrase, const uint8_t[PASS_SALT_LENGTH] salt) + static this derive_with_salt(const uint8_t[passphrase_len] passphrase, const uint8_t[PASS_SALT_LENGTH] salt) with error for key_derivation; /** diff --git a/toxencryptsave/toxencryptsave.c b/toxencryptsave/toxencryptsave.c index 5640e82f..b7360b56 100644 --- a/toxencryptsave/toxencryptsave.c +++ b/toxencryptsave/toxencryptsave.c @@ -71,11 +71,6 @@ struct Tox_Pass_Key { uint8_t key[TOX_PASS_KEY_LENGTH]; }; -Tox_Pass_Key *tox_pass_key_new(void) -{ - return (Tox_Pass_Key *)malloc(sizeof(Tox_Pass_Key)); -} - void tox_pass_key_free(Tox_Pass_Key *pass_key) { free(pass_key); @@ -123,23 +118,23 @@ bool tox_get_salt(const uint8_t *data, uint8_t *salt, TOX_ERR_GET_SALT *error) * * returns true on success */ -bool tox_pass_key_derive(Tox_Pass_Key *out_key, const uint8_t *passphrase, size_t pplength, - TOX_ERR_KEY_DERIVATION *error) +Tox_Pass_Key *tox_pass_key_derive(const uint8_t *passphrase, size_t pplength, + TOX_ERR_KEY_DERIVATION *error) { uint8_t salt[crypto_pwhash_scryptsalsa208sha256_SALTBYTES]; randombytes(salt, sizeof salt); - return tox_pass_key_derive_with_salt(out_key, passphrase, pplength, salt, error); + return tox_pass_key_derive_with_salt(passphrase, pplength, salt, error); } /* Same as above, except with use the given salt for deterministic key derivation. * The salt must be TOX_PASS_SALT_LENGTH bytes in length. */ -bool tox_pass_key_derive_with_salt(Tox_Pass_Key *out_key, const uint8_t *passphrase, size_t pplength, - const uint8_t *salt, TOX_ERR_KEY_DERIVATION *error) +Tox_Pass_Key *tox_pass_key_derive_with_salt(const uint8_t *passphrase, size_t pplength, + const uint8_t *salt, TOX_ERR_KEY_DERIVATION *error) { - if (!salt || !out_key || (!passphrase && pplength != 0)) { + if (!salt || (!passphrase && pplength != 0)) { SET_ERROR_PARAMETER(error, TOX_ERR_KEY_DERIVATION_NULL); - return 0; + return NULL; } uint8_t passkey[crypto_hash_sha256_BYTES]; @@ -157,14 +152,22 @@ bool tox_pass_key_derive_with_salt(Tox_Pass_Key *out_key, const uint8_t *passphr crypto_pwhash_scryptsalsa208sha256_MEMLIMIT_INTERACTIVE) != 0) { /* out of memory most likely */ SET_ERROR_PARAMETER(error, TOX_ERR_KEY_DERIVATION_FAILED); - return 0; + return NULL; } sodium_memzero(passkey, crypto_hash_sha256_BYTES); /* wipe plaintext pw */ + + Tox_Pass_Key *out_key = (Tox_Pass_Key *)malloc(sizeof(Tox_Pass_Key)); + + if (!out_key) { + SET_ERROR_PARAMETER(error, TOX_ERR_KEY_DERIVATION_FAILED); + return NULL; + } + memcpy(out_key->salt, salt, crypto_pwhash_scryptsalsa208sha256_SALTBYTES); memcpy(out_key->key, key, CRYPTO_SHARED_KEY_SIZE); SET_ERROR_PARAMETER(error, TOX_ERR_KEY_DERIVATION_OK); - return 1; + return out_key; } /* Encrypt arbitrary with a key produced by tox_derive_key_*. The output @@ -224,10 +227,10 @@ bool tox_pass_key_encrypt(const Tox_Pass_Key *key, const uint8_t *data, size_t d bool tox_pass_encrypt(const uint8_t *data, size_t data_len, const uint8_t *passphrase, size_t pplength, uint8_t *out, TOX_ERR_ENCRYPTION *error) { - Tox_Pass_Key key; TOX_ERR_KEY_DERIVATION _error; + Tox_Pass_Key *key = tox_pass_key_derive(passphrase, pplength, &_error); - if (!tox_pass_key_derive(&key, passphrase, pplength, &_error)) { + if (!key) { if (_error == TOX_ERR_KEY_DERIVATION_NULL) { SET_ERROR_PARAMETER(error, TOX_ERR_ENCRYPTION_NULL); } else if (_error == TOX_ERR_KEY_DERIVATION_FAILED) { @@ -237,7 +240,9 @@ bool tox_pass_encrypt(const uint8_t *data, size_t data_len, const uint8_t *passp return 0; } - return tox_pass_key_encrypt(&key, data, data_len, out, error); + bool result = tox_pass_key_encrypt(key, data, data_len, out, error); + tox_pass_key_free(key); + return result; } /* This is the inverse of tox_pass_key_encrypt, also using only keys produced by @@ -315,15 +320,17 @@ bool tox_pass_decrypt(const uint8_t *data, size_t length, const uint8_t *passphr memcpy(salt, data + TOX_ENC_SAVE_MAGIC_LENGTH, crypto_pwhash_scryptsalsa208sha256_SALTBYTES); /* derive the key */ - Tox_Pass_Key key; + Tox_Pass_Key *key = tox_pass_key_derive_with_salt(passphrase, pplength, salt, NULL); - if (!tox_pass_key_derive_with_salt(&key, passphrase, pplength, salt, NULL)) { + if (!key) { /* out of memory most likely */ SET_ERROR_PARAMETER(error, TOX_ERR_DECRYPTION_KEY_DERIVATION_FAILED); return 0; } - return tox_pass_key_decrypt(&key, data, length, out, error); + bool result = tox_pass_key_decrypt(key, data, length, out, error); + tox_pass_key_free(key); + return result; } /* Determines whether or not the given data is encrypted (by checking the magic number) diff --git a/toxencryptsave/toxencryptsave.h b/toxencryptsave/toxencryptsave.h index ef1ab152..c5a1dff9 100644 --- a/toxencryptsave/toxencryptsave.h +++ b/toxencryptsave/toxencryptsave.h @@ -99,8 +99,7 @@ typedef enum TOX_ERR_KEY_DERIVATION { /** * The crypto lib was unable to derive a key from the given passphrase, - * which is usually a lack of memory issue. The functions accepting keys - * do not produce this error. + * which is usually a lack of memory issue. */ TOX_ERR_KEY_DERIVATION_FAILED, @@ -241,23 +240,14 @@ bool tox_pass_decrypt(const uint8_t *ciphertext, size_t ciphertext_len, const ui * for encryption and decryption. It is derived from a salt and the user- * provided password. * - * The Tox_Pass_Key structure is hidden in the implementation. It can be allocated - * using tox_pass_key_new and must be deallocated using tox_pass_key_free. + * The Tox_Pass_Key structure is hidden in the implementation. It can be created + * using tox_pass_key_derive or tox_pass_key_derive_with_salt and must be deallocated using tox_pass_key_free. */ #ifndef TOX_PASS_KEY_DEFINED #define TOX_PASS_KEY_DEFINED typedef struct Tox_Pass_Key Tox_Pass_Key; #endif /* TOX_PASS_KEY_DEFINED */ -/** - * Create a new Tox_Pass_Key. The initial value of it is indeterminate. To - * initialise it, use one of the derive_* functions below. - * - * In case of failure, this function returns NULL. The only failure mode at - * this time is memory allocation failure, so this function has no error code. - */ -struct Tox_Pass_Key *tox_pass_key_new(void); - /** * Deallocate a Tox_Pass_Key. This function behaves like free(), so NULL is an * acceptable argument value. @@ -279,8 +269,8 @@ void tox_pass_key_free(struct Tox_Pass_Key *_key); * * @return true on success. */ -bool tox_pass_key_derive(struct Tox_Pass_Key *_key, const uint8_t *passphrase, size_t passphrase_len, - TOX_ERR_KEY_DERIVATION *error); +struct Tox_Pass_Key *tox_pass_key_derive(const uint8_t *passphrase, size_t passphrase_len, + TOX_ERR_KEY_DERIVATION *error); /** * Same as above, except use the given salt for deterministic key derivation. @@ -291,8 +281,8 @@ bool tox_pass_key_derive(struct Tox_Pass_Key *_key, const uint8_t *passphrase, s * * @return true on success. */ -bool tox_pass_key_derive_with_salt(struct Tox_Pass_Key *_key, const uint8_t *passphrase, size_t passphrase_len, - const uint8_t *salt, TOX_ERR_KEY_DERIVATION *error); +struct Tox_Pass_Key *tox_pass_key_derive_with_salt(const uint8_t *passphrase, size_t passphrase_len, + const uint8_t *salt, TOX_ERR_KEY_DERIVATION *error); /** * Encrypt a plain text with a key produced by tox_pass_key_derive or tox_pass_key_derive_with_salt.