From 3c30d8871b5ac224f1c8e0163773b8cda2131ee1 Mon Sep 17 00:00:00 2001 From: TLINDEN Date: Tue, 13 Jan 2015 13:07:32 +0100 Subject: [PATCH] using secure memory where applicable using sodium_malloc or sodium_mlock, where not --- include/pcp/key.h | 3 +-- include/pcp/mem.h | 10 ++++++++++ libpcp/key.c | 9 ++++----- libpcp/keyhash.c | 6 +++++- libpcp/mem.c | 29 ++++++++++++++++++++++++++++- libpcp/mgmt.c | 6 +++--- libpcp/scrypt.c | 2 +- libpcp/vault.c | 9 ++++++--- src/encryption.c | 32 ++++++++++++++++---------------- src/keymgmt.c | 22 +++++++++------------- src/keymgmt.h | 1 - src/readpass.c | 5 +++++ src/signature.c | 6 +++--- tests/statictest.c | 2 +- 14 files changed, 92 insertions(+), 50 deletions(-) diff --git a/include/pcp/key.h b/include/pcp/key.h index 550e8a0..04bf54b 100644 --- a/include/pcp/key.h +++ b/include/pcp/key.h @@ -31,10 +31,9 @@ #include "defines.h" #include "platform.h" #include "mem.h" -#include "mac.h" +#include "crypto.h" #include "randomart.h" #include "version.h" -//#include "z85.h" #include "uthash.h" #include "jenhash.h" #include "structs.h" diff --git a/include/pcp/mem.h b/include/pcp/mem.h index fe43ad9..3262079 100644 --- a/include/pcp/mem.h +++ b/include/pcp/mem.h @@ -25,6 +25,7 @@ #include #include +#include #include "defines.h" #include "platform.h" @@ -36,13 +37,22 @@ /* available. */ void *ucmalloc(size_t s); +/* same as ucmalloc, but uses secure allocation */ +void *smalloc(size_t s); + /* the same but it fills the pointer with random values */ void *urmalloc(size_t s); +/* same as urmalloc(), but uses secure allocation using sodium_malloc() */ +void *srmalloc(size_t s); + /* resize a a pointer and fill the added remainder with zeroes */ void *ucrealloc(void *d, size_t oldlen, size_t newlen); /* clear and free */ void ucfree(void *d, size_t len); +/* same, but free a sodium pointer */ +void sfree(void *d); + #endif /* _HAVE_PCP_MEM */ diff --git a/libpcp/key.c b/libpcp/key.c index 2b93c8d..38312c7 100644 --- a/libpcp/key.c +++ b/libpcp/key.c @@ -29,7 +29,7 @@ * result anyway because I need a curve25519 secret. */ byte *pcp_derivekey(PCPCTX *ptx, char *passphrase, byte *nonce) { - byte *key = ucmalloc(crypto_secretbox_KEYBYTES); + byte *key = smalloc(crypto_secretbox_KEYBYTES); size_t plen = strnlen(passphrase, 255); /* create the scrypt hash */ @@ -44,7 +44,7 @@ byte *pcp_derivekey(PCPCTX *ptx, char *passphrase, byte *nonce) { key[31] |= 64; /* done */ - ucfree(scrypted, 64); + sfree(scrypted); return key; } @@ -168,9 +168,8 @@ pcp_key_t *pcpkey_encrypt(PCPCTX *ptx, pcp_key_t *key, char *passphrase) { es = pcp_sodium_mac(&encrypted, buffer_get(both), buffer_size(both), key->nonce, encryptkey); - memset(encryptkey, 0, 32); buffer_free(both); - free(encryptkey); + sfree(encryptkey); if(es == 176) { /* FIXME: calc! */ /* success */ @@ -201,7 +200,7 @@ pcp_key_t *pcpkey_decrypt(PCPCTX *ptx, pcp_key_t *key, char *passphrase) { es = pcp_sodium_verify_mac(&decrypted, key->encrypted, 176, key->nonce, encryptkey); - ucfree(encryptkey, 32); + sfree(encryptkey); if(es == 0) { /* success */ diff --git a/libpcp/keyhash.c b/libpcp/keyhash.c index 512648a..674e03c 100644 --- a/libpcp/keyhash.c +++ b/libpcp/keyhash.c @@ -27,6 +27,7 @@ void pcphash_del(PCPCTX *ptx, void *key, int type) { if(type == PCP_KEY_TYPE_SECRET) { HASH_DEL(ptx->pcpkey_hash, (pcp_key_t *)key); memset(key, 0, sizeof(pcp_key_t)); + sodium_munlock(key, sizeof(pcp_key_t)); } else if(type == PCP_KEYSIG_NATIVE || type == PCP_KEYSIG_PBP) { pcp_keysig_t *keysig = (pcp_keysig_t *)key; @@ -37,6 +38,7 @@ void pcphash_del(PCPCTX *ptx, void *key, int type) { else { HASH_DEL(ptx->pcppubkey_hash, (pcp_pubkey_t *)key); memset(key, 0, sizeof(pcp_pubkey_t)); + sodium_munlock(key, sizeof(pcp_pubkey_t)); } free(key); } @@ -94,6 +96,7 @@ pcp_pubkey_t *pcphash_pubkeyexists(PCPCTX *ptx, char *id) { void pcphash_add(PCPCTX *ptx, void *key, int type) { if(type == PCP_KEY_TYPE_PUBLIC) { pcp_pubkey_t *k = (pcp_pubkey_t *)key; + sodium_mlock(key, sizeof(pcp_pubkey_t)); HASH_ADD_STR( ptx->pcppubkey_hash, id, k ); } else if(type == PCP_KEYSIG_NATIVE || type == PCP_KEYSIG_PBP) { @@ -101,7 +104,8 @@ void pcphash_add(PCPCTX *ptx, void *key, int type) { HASH_ADD_STR( ptx->pcpkeysig_hash, id, keysig); } else { - pcp_key_t *k = (pcp_key_t *)key; + pcp_key_t *k = (pcp_key_t *)key; + sodium_mlock(key, sizeof(pcp_key_t)); HASH_ADD_STR( ptx->pcpkey_hash, id, k); } } diff --git a/libpcp/mem.c b/libpcp/mem.c index b0810f0..9b2363a 100644 --- a/libpcp/mem.c +++ b/libpcp/mem.c @@ -36,13 +36,28 @@ void *ucmalloc(size_t s) { exit(-1); } - memset (value, 0, size); + sodium_memzero(value, size); /* printf("allocated %ld bytes at %p\n", size, value); */ return value; } +void *smalloc(size_t s) { + if (s == 0) + return NULL; + + size_t size = s * sizeof(byte); + void *value = sodium_malloc (size); + + if (value == NULL) { + err(errno, "Cannot allocate %d bytes of memory", (int)s); + exit(-1); + } + + return value; +} + void *urmalloc(size_t s) { void *value = ucmalloc (s); @@ -51,6 +66,14 @@ void *urmalloc(size_t s) { return value; } +void *srmalloc(size_t s) { + void *value = sodium_malloc (s); + + arc4random_buf(value, s); + + return value; +} + void *ucrealloc(void *d, size_t oldlen, size_t newlen) { newlen = newlen * sizeof(byte); @@ -75,3 +98,7 @@ void ucfree(void *d, size_t len) { free(d); } } + +void sfree(void *d) { + sodium_free(d); +} diff --git a/libpcp/mgmt.c b/libpcp/mgmt.c index 5d37f70..7aa0aa2 100644 --- a/libpcp/mgmt.c +++ b/libpcp/mgmt.c @@ -761,7 +761,7 @@ Buffer *pcp_export_secret(PCPCTX *ptx, pcp_key_t *sk, char *passphrase) { buffer_free(raw); ucfree(nonce, crypto_secretbox_NONCEBYTES); - ucfree(symkey, 64); + sfree(symkey); ucfree(cipher, es); return out; @@ -877,7 +877,7 @@ pcp_key_t *pcp_import_secret_native(PCPCTX *ptx, Buffer *cipher, char *passphras ucfree(clear, cipherlen - PCP_CRYPTO_ADD); ucfree(nonce, crypto_secretbox_NONCEBYTES); buffer_free(blob); - ucfree(symkey, 64); + sfree(symkey); free(id); return sk; @@ -890,6 +890,6 @@ pcp_key_t *pcp_import_secret_native(PCPCTX *ptx, Buffer *cipher, char *passphras ucfree(sk, sizeof(pcp_key_t)); buffer_free(blob); if(symkey != NULL) - ucfree(symkey, 64); + sfree(symkey); return NULL; } diff --git a/libpcp/scrypt.c b/libpcp/scrypt.c index ccfcdb4..11c0345 100644 --- a/libpcp/scrypt.c +++ b/libpcp/scrypt.c @@ -22,7 +22,7 @@ #include "scrypt.h" byte* pcp_scrypt(PCPCTX *ptx, char *passwd, size_t passwdlen, byte *nonce, size_t noncelen) { - uint8_t *dk = ucmalloc(64); /* resulting hash */ + uint8_t *dk = smalloc(64); /* resulting hash */ /* constants */ uint64_t N = 1 << 14; diff --git a/libpcp/vault.c b/libpcp/vault.c index 0477072..84c07a6 100644 --- a/libpcp/vault.c +++ b/libpcp/vault.c @@ -442,7 +442,8 @@ int pcpvault_fetchall(PCPCTX *ptx, vault_t *vault) { vault_item_header_t *item = ucmalloc(sizeof(vault_item_header_t)); got = fread(header, 1, sizeof(vault_header_t), vault->fd); if(got < sizeof(vault_header_t)) { - fatal(ptx, "empty or invalid vault header size (got %ld, expected %ld)\n", got, sizeof(vault_header_t)); + fatal(ptx, "empty or invalid vault header size (got %ld, expected %ld)\n", + got, sizeof(vault_header_t)); goto err; } vh2native(header); @@ -494,12 +495,14 @@ int pcpvault_fetchall(PCPCTX *ptx, vault_t *vault) { buffer_free(rawks); } else { - fatal(ptx, "Failed to read vault - invalid key type: %02X! at %d\n", item->type, readpos); + fatal(ptx, "Failed to read vault - invalid key type: %02X! at %d\n", + item->type, readpos); goto err; } } else { - fatal(ptx, "Failed to read vault - that's no pcp key at %d (size %ld)!\n", readpos, bytesleft); + fatal(ptx, "Failed to read vault - that's no pcp key at %d (size %ld)!\n", + readpos, bytesleft); goto err; } } diff --git a/src/encryption.c b/src/encryption.c index 6f5f2f2..a508c83 100644 --- a/src/encryption.c +++ b/src/encryption.c @@ -70,12 +70,12 @@ int pcpdecrypt(char *id, int useid, char *infile, char *outfile, char *passwd, i "Enter passphrase for symetric decryption", NULL, 1); } else { - passphrase = ucmalloc(strlen(passwd)+1); - strncpy(passphrase, passwd, strlen(passwd)); + passphrase = smalloc(strlen(passwd)+1); + memcpy(passphrase, passwd, strlen(passwd) + 1); } symkey = pcp_scrypt(ptx, passphrase, strlen(passphrase), salt, 90); - ucfree(passphrase, strlen(passphrase)); + sfree(passphrase); free(salt); } else if(head == PCP_ASYM_CIPHER || head == PCP_ASYM_CIPHER_SIG || head == PCP_ASYM_CIPHER_ANON) { @@ -103,12 +103,12 @@ int pcpdecrypt(char *id, int useid, char *infile, char *outfile, char *passwd, i "Enter passphrase to decrypt your secret key", NULL, 1); } else { - passphrase = ucmalloc(strlen(passwd)+1); - strncpy(passphrase, passwd, strlen(passwd)+1); + passphrase = smalloc(strlen(passwd)+1); + memcpy(passphrase, passwd, strlen(passwd)+1); } secret = pcpkey_decrypt(ptx, secret, passphrase); - ucfree(passphrase, strlen(passphrase)); + sfree(passphrase); if(secret == NULL) goto errde3; @@ -134,7 +134,7 @@ int pcpdecrypt(char *id, int useid, char *infile, char *outfile, char *passwd, i } else { dlen = pcp_decrypt_stream(ptx, pin, pout, NULL, symkey, verify, 0); - ucfree(symkey, 64); + sfree(symkey); } ps_close(pin); @@ -151,7 +151,7 @@ int pcpdecrypt(char *id, int useid, char *infile, char *outfile, char *passwd, i errde3: if(symkey != NULL) - ucfree(symkey, 64); + free(symkey); return 1; } @@ -177,15 +177,15 @@ int pcpencrypt(char *id, char *infile, char *outfile, char *passwd, plist_t *rec "Enter passphrase for symetric encryption", "Repeat passphrase", 1); } else { - passphrase = ucmalloc(strlen(passwd)+1); - strncpy(passphrase, passwd, strlen(passwd)); + passphrase = smalloc(strlen(passwd)+1); + memcpy(passphrase, passwd, strlen(passwd)+1); } byte *salt = ucmalloc(90); /* FIXME: use random salt, concat it with result afterwards */ char stsalt[] = PBP_COMPAT_SALT; memcpy(salt, stsalt, 90); symkey = pcp_scrypt(ptx, passphrase, strlen(passphrase), salt, 90); free(salt); - ucfree(passphrase, strlen(passphrase)); + sfree(passphrase); } else if(id != NULL && recipient == NULL) { /* lookup by id */ @@ -256,11 +256,11 @@ int pcpencrypt(char *id, char *infile, char *outfile, char *passwd, plist_t *rec "Enter passphrase to decrypt your secret key", NULL, 1); } else { - passphrase = ucmalloc(strlen(passwd)+1); - strncpy(passphrase, passwd, strlen(passwd)+1); + passphrase = smalloc(strlen(passwd)+1); + memcpy(passphrase, passwd, strlen(passwd)+1); } secret = pcpkey_decrypt(ptx, secret, passphrase); - ucfree(passphrase, strlen(passphrase)); + sfree(passphrase); if(secret == NULL) goto erren2; } @@ -297,7 +297,7 @@ int pcpencrypt(char *id, char *infile, char *outfile, char *passwd, plist_t *rec if(self == 1) { clen = pcp_encrypt_stream_sym(ptx, pin, pout, symkey, 0, NULL); - ucfree(symkey, 64); + sfree(symkey); } else { clen = pcp_encrypt_stream(ptx, pin, pout, secret, pubhash, signcrypt, anon); @@ -335,7 +335,7 @@ int pcpencrypt(char *id, char *infile, char *outfile, char *passwd, plist_t *rec pcphash_cleanpub(pubhash); if(symkey != NULL) - ucfree(symkey, 64); + sfree(symkey); erren3: diff --git a/src/keymgmt.c b/src/keymgmt.c index 925816d..87f0e1a 100644 --- a/src/keymgmt.c +++ b/src/keymgmt.c @@ -83,7 +83,7 @@ void pcp_keygen(char *passwd) { } else { passphrase = ucmalloc(strlen(passwd)+1); - strncpy(passphrase, passwd, strlen(passwd)+1); + memcpy(passphrase, passwd, strlen(passwd)+1); } if(strnlen(passphrase, 1024) > 0) @@ -234,12 +234,10 @@ void pcp_exportsecret(char *keyid, int useid, char *outfile, int armor, char *pa "Enter passphrase to decrypt your secret key", NULL, 1); key = pcpkey_decrypt(ptx, key, passphrase); if(key == NULL) { - memset(passphrase, 0, strlen(passphrase)); - free(passphrase); + sfree(passphrase); goto errexpse1; } - memset(passphrase, 0, strlen(passphrase)); - free(passphrase); + sfree(passphrase); } else { key = pcpkey_decrypt(ptx, key, passwd); @@ -259,8 +257,7 @@ void pcp_exportsecret(char *keyid, int useid, char *outfile, int armor, char *pa pcp_readpass(&passphrase, "Enter passphrase to encrypt the exported secret key", "Repeat passphrase", 1); exported_sk = pcp_export_secret(ptx, key, passphrase); - memset(passphrase, 0, strlen(passphrase)); - free(passphrase); + sfree(passphrase); } if(exported_sk != NULL) { @@ -349,8 +346,7 @@ void pcp_exportpublic(char *keyid, char *passwd, char *outfile, int format, int pcp_readpass(&passphrase, "Enter passphrase to decrypt your secret key", NULL, 1); sk = pcpkey_decrypt(ptx, sk, passphrase); - memset(passphrase, 0, strlen(passphrase)); - free(passphrase); + sfree(passphrase); } if(sk == NULL) { goto errpcpexpu1; @@ -454,7 +450,7 @@ void pcpedit_key(char *keyid) { char *passphrase; pcp_readpass(&passphrase, "Enter passphrase to decrypt the key", NULL, 1); key = pcpkey_decrypt(ptx, key, passphrase); - ucfree(passphrase, strlen(passphrase)); + sfree(passphrase); } if(key != NULL) { @@ -509,7 +505,7 @@ void pcpedit_key(char *keyid) { if(strnlen(passphrase, 1024) > 0) { key = pcpkey_encrypt(ptx, key, passphrase); - ucfree(passphrase, strlen(passphrase)); + sfree(passphrase); } if(key != NULL) { @@ -616,7 +612,7 @@ int pcp_import (vault_t *vault, FILE *in, char *passwd) { pcp_readpass(&passphrase, "Enter passphrase to decrypt the secret key file", NULL, 1); sk = pcp_import_secret(ptx, buf, bufsize, passphrase); - ucfree(passphrase, strlen(passphrase)); + sfree(passphrase); } if(sk == NULL) { @@ -645,7 +641,7 @@ int pcp_import (vault_t *vault, FILE *in, char *passwd) { if(strnlen(passphrase, 1024) > 0) { /* encrypt the key */ sk = pcpkey_encrypt(ptx, sk, passphrase); - ucfree(passphrase, strlen(passphrase)); + sfree(passphrase); } else { /* ask for confirmation if we shall store it in the clear */ diff --git a/src/keymgmt.h b/src/keymgmt.h index de506ae..5309daf 100644 --- a/src/keymgmt.h +++ b/src/keymgmt.h @@ -30,7 +30,6 @@ #include #include "randomart.h" -#include "mac.h" #include "key.h" #include "pcp.h" #include "vault.h" diff --git a/src/readpass.c b/src/readpass.c index 717cb70..ddd5986 100644 --- a/src/readpass.c +++ b/src/readpass.c @@ -107,10 +107,15 @@ retry: fclose(readfrom); /* Copy the password out. */ + char *p = smalloc(strlen(passbuf) + 1); + memcpy(p, passbuf, strlen(passbuf) + 1 ); + *passwd = p; + /* if ((*passwd = strdup(passbuf)) == NULL) { fatal(ptx, "Cannot allocate memory\n"); goto err1; } + */ /* Zero any stored passwords. */ memset(passbuf, 0, MAXPASSLEN); diff --git a/src/signature.c b/src/signature.c index cf62eca..935b559 100644 --- a/src/signature.c +++ b/src/signature.c @@ -62,12 +62,12 @@ int pcpsign(char *infile, char *outfile, char *passwd, int z85, int detach) { "Enter passphrase to decrypt your secret key", NULL, 1); } else { - passphrase = ucmalloc(strlen(passwd)+1); - strncpy(passphrase, passwd, strlen(passwd)+1); + passphrase = smalloc(strlen(passwd)+1); + memcpy(passphrase, passwd, strlen(passwd)+1); } secret = pcpkey_decrypt(ptx, secret, passphrase); - ucfree(passphrase, strlen(passwd)+1); + sfree(passphrase); if(secret == NULL) goto errs1; } diff --git a/tests/statictest.c b/tests/statictest.c index 22f6a6d..6fca11b 100644 --- a/tests/statictest.c +++ b/tests/statictest.c @@ -4,7 +4,7 @@ int main() { sodium_init(); unsigned char *t = ucmalloc(12); - if(pcp_sodium_verify_box(&t, cipher, cipher_len, nonce, secret_b, public_a) == 0) { + if(crypto_box_open_easy(t, cipher, cipher_len, nonce, public_a, secret_b) == 0) { if(memcmp(t, message, message_len) == 0) { printf("ok\n"); }