Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change toxencryptsave API to never overwrite pass keys. #334

Merged
merged 1 commit into from Dec 30, 2017
Merged

Change toxencryptsave API to never overwrite pass keys. #334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 7 additions & 10 deletions auto_tests/encryptsave_test.c
View file
Open in desktop
Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 5 additions & 15 deletions toxencryptsave/toxencryptsave.api.h
View file
Open in desktop
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
Open in desktop
Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
24 changes: 7 additions & 17 deletions toxencryptsave/toxencryptsave.h
View file
Open in desktop
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