Skip to content

Commit

Permalink
Change toxencryptsave API to never overwrite pass keys.
Browse files Browse the repository at this point in the history
  • Loading branch information
@iphydf
iphydf committed Jan 12, 2017
1 parent 6480765 commit 16ee763
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 62 deletions.
17 changes: 7 additions & 10 deletions auto_tests/encryptsave_test.c
View file
Expand Up @@ -101,7 +101,8 @@ START_TEST(test_save_friend)
size = tox_get_savedata_size(tox3);
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);
Expand Down Expand Up @@ -143,14 +144,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];
Expand Down Expand Up @@ -183,10 +182,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);
Expand Down
20 changes: 5 additions & 15 deletions toxencryptsave/toxencryptsave.api.h
View file
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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.
Expand All @@ -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;

/**
Expand All @@ -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;

/**
Expand Down
47 changes: 27 additions & 20 deletions toxencryptsave/toxencryptsave.c
View file
Expand Up @@ -58,11 +58,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);
Expand Down Expand Up @@ -110,23 +105,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];
Expand All @@ -144,14 +139,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
Expand Down Expand Up @@ -211,10 +214,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) {
Expand All @@ -224,7 +227,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
Expand Down Expand Up @@ -302,15 +307,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)
Expand Down
24 changes: 7 additions & 17 deletions toxencryptsave/toxencryptsave.h
View file
Expand Up @@ -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,

Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down

0 comments on commit 16ee763

Please sign in to comment.