Wipe (some) secrets from memory after use
authorKirill Isakov <bootctl@gmail.com>
Fri, 22 Apr 2022 12:33:52 +0000 (18:33 +0600)
committerGuus Sliepen <guus@tinc-vpn.org>
Fri, 22 Apr 2022 20:22:18 +0000 (22:22 +0200)
to lessen the amount of sensitive information ending up in swap, core
dumps, or in the hands of any remote attackers.

While there still remaings a lot interesting data in configuration trees,
connection_t structs, etc, this is considered a good practice nevertheless.

Some bedtime reading:

- http://www.daemonology.net/blog/2014-09-04-how-to-zero-a-buffer.html
- http://www.daemonology.net/blog/2014-09-06-zeroing-buffers-is-insufficient.html
- https://github.com/jedisct1/libsodium/blob/be58b2e6664389d9c7993b55291402934b43b3ca/src/libsodium/sodium/utils.c#L78:L101

33 files changed:
src/chacha-poly1305/chacha-poly1305.c
src/chacha-poly1305/chacha-poly1305.h
src/conf.c
src/ed25519/ecdh.c
src/ed25519/ecdsa.c
src/ed25519/ecdsagen.c
src/edge.c
src/gcrypt/cipher.c
src/gcrypt/digest.c
src/gcrypt/rsa.c
src/gcrypt/rsagen.c
src/have.h
src/invitation.c
src/keys.c
src/meson.build
src/net_setup.c
src/node.c
src/openssl/rsa.c
src/protocol.c
src/protocol_auth.c
src/protocol_key.c
src/script.c
src/sptps.c
src/sptps.h
src/sptps_keypair.c
src/sptps_test.c
src/utils.c
src/utils.h
src/xalloc.h
test/unit/meson.build
test/unit/test_memzero_null.c [new file with mode: 0644]
test/unit/test_random_noinit.c
test/unit/test_xalloc.c [new file with mode: 0644]

index d13fec4..77d531a 100644 (file)
@@ -10,16 +10,14 @@ struct chacha_poly1305_ctx {
 };
 
 chacha_poly1305_ctx_t *chacha_poly1305_init(void) {
-       chacha_poly1305_ctx_t *ctx = xzalloc(sizeof(*ctx));
-       return ctx;
+       return xzalloc(sizeof(chacha_poly1305_ctx_t));
 }
 
 void chacha_poly1305_exit(chacha_poly1305_ctx_t *ctx) {
-       free(ctx);
+       xzfree(ctx, sizeof(chacha_poly1305_ctx_t));
 }
 
-bool chacha_poly1305_set_key(chacha_poly1305_ctx_t *ctx, const void *vkey) {
-       const uint8_t *key = vkey;
+bool chacha_poly1305_set_key(chacha_poly1305_ctx_t *ctx, const uint8_t *key) {
        chacha_keysetup(&ctx->main_ctx, key, 256);
        chacha_keysetup(&ctx->header_ctx, key + 32, 256);
        return true;
index af7eaf5..b558e45 100644 (file)
@@ -7,7 +7,7 @@ typedef struct chacha_poly1305_ctx chacha_poly1305_ctx_t;
 
 extern chacha_poly1305_ctx_t *chacha_poly1305_init(void);
 extern void chacha_poly1305_exit(chacha_poly1305_ctx_t *);
-extern bool chacha_poly1305_set_key(chacha_poly1305_ctx_t *ctx, const void *key);
+extern bool chacha_poly1305_set_key(chacha_poly1305_ctx_t *ctx, const uint8_t *key);
 
 extern bool chacha_poly1305_encrypt(chacha_poly1305_ctx_t *ctx, uint64_t seqnr, const void *indata, size_t inlen, void *outdata, size_t *outlen);
 extern bool chacha_poly1305_decrypt(chacha_poly1305_ctx_t *ctx, uint64_t seqnr, const void *indata, size_t inlen, void *outdata, size_t *outlen);
index a40fdfa..c8cb141 100644 (file)
@@ -98,7 +98,7 @@ config_t *new_config(void) {
 
 void free_config(config_t *cfg) {
        free(cfg->variable);
-       free(cfg->value);
+       free_string(cfg->value);
        free(cfg->file);
        free(cfg);
 }
index 469f502..cfb2077 100644 (file)
@@ -36,16 +36,17 @@ ecdh_t *ecdh_generate_public(void *pubkey) {
        uint8_t seed[32];
        randomize(seed, sizeof(seed));
        ed25519_create_keypair(pubkey, ecdh->private, seed);
+       memzero(seed, sizeof(seed));
 
        return ecdh;
 }
 
 bool ecdh_compute_shared(ecdh_t *ecdh, const void *pubkey, void *shared) {
        ed25519_key_exchange(shared, pubkey, ecdh->private);
-       free(ecdh);
+       ecdh_free(ecdh);
        return true;
 }
 
 void ecdh_free(ecdh_t *ecdh) {
-       free(ecdh);
+       xzfree(ecdh, sizeof(ecdh_t));
 }
index f0ce2c1..28daf4a 100644 (file)
@@ -47,7 +47,7 @@ ecdsa_t *ecdsa_set_base64_public_key(const char *p) {
 
        if(len != 32) {
                logger(DEBUG_ALWAYS, LOG_ERR, "Invalid format of public key! len = %lu", (unsigned long)len);
-               free(ecdsa);
+               ecdsa_free(ecdsa);
                return 0;
        }
 
@@ -64,6 +64,7 @@ char *ecdsa_get_base64_public_key(ecdsa_t *ecdsa) {
 // Read PEM ECDSA keys
 
 static bool read_pem(FILE *fp, const char *type, void *vbuf, size_t size) {
+       const size_t buflen = size;
        char line[1024];
        bool data = false;
        size_t typelen = strlen(type);
@@ -90,16 +91,10 @@ static bool read_pem(FILE *fp, const char *type, void *vbuf, size_t size) {
                size_t linelen = strcspn(line, "\r\n");
                size_t len = b64decode_tinc(line, line, linelen);
 
-               if(!len) {
-                       logger(DEBUG_ALWAYS, LOG_ERR, "Invalid base64 data in PEM file\n");
+               if(!len || len > size) {
+                       logger(DEBUG_ALWAYS, LOG_ERR, "%s base64 data in PEM file\n", len ? "Too much" : "Invalid");
                        errno = EINVAL;
-                       return false;
-               }
-
-               if(len > size) {
-                       logger(DEBUG_ALWAYS, LOG_ERR, "Too much base64 data in PEM file\n");
-                       errno = EINVAL;
-                       return false;
+                       goto exit;
                }
 
                memcpy(buf, line, len);
@@ -114,7 +109,13 @@ static bool read_pem(FILE *fp, const char *type, void *vbuf, size_t size) {
                } else {
                        errno = ENOENT;
                }
+       }
+
+exit:
+       memzero(line, sizeof(line));
 
+       if(size) {
+               memzero(vbuf, buflen);
                return false;
        }
 
@@ -128,8 +129,8 @@ ecdsa_t *ecdsa_read_pem_public_key(FILE *fp) {
                return ecdsa;
        }
 
-       free(ecdsa);
-       return 0;
+       ecdsa_free(ecdsa);
+       return NULL;
 }
 
 ecdsa_t *ecdsa_read_pem_private_key(FILE *fp) {
@@ -139,8 +140,8 @@ ecdsa_t *ecdsa_read_pem_private_key(FILE *fp) {
                return ecdsa;
        }
 
-       free(ecdsa);
-       return 0;
+       ecdsa_free(ecdsa);
+       return NULL;
 }
 
 size_t ecdsa_size(ecdsa_t *ecdsa) {
@@ -164,5 +165,5 @@ bool ecdsa_active(ecdsa_t *ecdsa) {
 }
 
 void ecdsa_free(ecdsa_t *ecdsa) {
-       free(ecdsa);
+       xzfree(ecdsa, sizeof(ecdsa_t));
 }
index bc14fd2..1edc945 100644 (file)
@@ -40,6 +40,7 @@ ecdsa_t *ecdsa_generate(void) {
        uint8_t seed[32];
        randomize(seed, sizeof(seed));
        ed25519_create_keypair(ecdsa->public, ecdsa->private, seed);
+       memzero(seed, sizeof(seed));
 
        return ecdsa;
 }
@@ -60,6 +61,8 @@ static bool write_pem(FILE *fp, const char *type, void *vbuf, size_t size) {
                size -= todo;
        }
 
+       memzero(base64, sizeof(base64));
+
        fprintf(fp, "-----END %s-----\n", type);
        return !ferror(fp);
 }
index c521d0e..baa6d26 100644 (file)
@@ -71,10 +71,12 @@ edge_t *new_edge(void) {
 }
 
 void free_edge(edge_t *e) {
-       sockaddrfree(&e->address);
-       sockaddrfree(&e->local_address);
+       if(e) {
+               sockaddrfree(&e->address);
+               sockaddrfree(&e->local_address);
 
-       free(e);
+               free(e);
+       }
 }
 
 void edge_add(edge_t *e) {
index 37f232f..2813da6 100644 (file)
@@ -106,7 +106,7 @@ static bool cipher_open(cipher_t *cipher, cipher_algo_t algo, cipher_mode_t mode
 
        cipher->keylen = gcry_cipher_get_algo_keylen(algo);
        cipher->blklen = gcry_cipher_get_algo_blklen(algo);
-       cipher->key = xmalloc(cipher->keylen + cipher->blklen);
+       cipher->key = xmalloc(cipher_keylength(cipher));
        cipher->padding = mode == GCRY_CIPHER_MODE_ECB || mode == GCRY_CIPHER_MODE_CBC;
 
        return true;
@@ -137,13 +137,15 @@ bool cipher_open_by_nid(cipher_t *cipher, nid_t nid) {
 }
 
 void cipher_close(cipher_t *cipher) {
+       if(!cipher) {
+               return;
+       }
+
        if(cipher->handle) {
                gcry_cipher_close(cipher->handle);
-               cipher->handle = NULL;
        }
 
-       free(cipher->key);
-
+       xzfree(cipher->key, cipher_keylength(cipher));
        memset(cipher, 0, sizeof(*cipher));
 }
 
@@ -184,7 +186,7 @@ size_t cipher_blocksize(const cipher_t *cipher) {
 bool cipher_set_key(cipher_t *cipher, void *key, bool encrypt) {
        (void)encrypt;
 
-       memcpy(cipher->key, key, cipher->keylen + cipher->blklen);
+       memcpy(cipher->key, key, cipher_keylength(cipher));
 
        gcry_cipher_setkey(cipher->handle, cipher->key, cipher->keylen);
        gcry_cipher_setiv(cipher->handle, cipher->key + cipher->keylen, cipher->blklen);
index 50446f6..cfea1f0 100644 (file)
@@ -111,6 +111,10 @@ bool digest_open_by_nid(digest_t *digest, nid_t nid, size_t maclength) {
 }
 
 void digest_close(digest_t *digest) {
+       if(!digest) {
+               return;
+       }
+
        if(digest->hmac) {
                gcry_md_close(digest->hmac);
        }
index 1162ddf..83f177b 100644 (file)
@@ -130,7 +130,7 @@ rsa_t *rsa_set_hex_public_key(const char *n, const char *e) {
 
        if(err) {
                logger(DEBUG_ALWAYS, LOG_ERR, "Error while reading RSA public key: %s", gcry_strerror(errno));
-               free(rsa);
+               rsa_free(rsa);
                return false;
        }
 
@@ -152,8 +152,8 @@ rsa_t *rsa_set_hex_private_key(const char *n, const char *e, const char *d) {
 
        if(err) {
                logger(DEBUG_ALWAYS, LOG_ERR, "Error while reading RSA public key: %s", gcry_strerror(errno));
-               free(rsa);
-               return false;
+               rsa_free(rsa);
+               return NULL;
        }
 
        return rsa;
@@ -177,7 +177,7 @@ rsa_t *rsa_read_pem_public_key(FILE *fp) {
                        || !ber_read_mpi(&derp, &derlen, &rsa->e)
                        || derlen) {
                logger(DEBUG_ALWAYS, LOG_ERR, "Error while decoding RSA public key");
-               free(rsa);
+               rsa_free(rsa);
                return NULL;
        }
 
@@ -207,10 +207,11 @@ rsa_t *rsa_read_pem_private_key(FILE *fp) {
                        || !ber_read_mpi(&derp, &derlen, NULL) // u
                        || derlen) {
                logger(DEBUG_ALWAYS, LOG_ERR, "Error while decoding RSA private key");
-               free(rsa);
-               return NULL;
+               rsa_free(rsa);
+               rsa = NULL;
        }
 
+       memzero(derbuf, sizeof(derbuf));
        return rsa;
 }
 
@@ -218,19 +219,27 @@ size_t rsa_size(const rsa_t *rsa) {
        return (gcry_mpi_get_nbits(rsa->n) + 7) / 8;
 }
 
+static bool check(gcry_error_t err) {
+       if(err) {
+               logger(DEBUG_ALWAYS, LOG_ERR, "gcrypt error %s/%s", gcry_strsource(err), gcry_strerror(err));
+       }
+
+       return !err;
+}
+
 /* Well, libgcrypt has functions to handle RSA keys, but they suck.
  * So we just use libgcrypt's mpi functions, and do the math ourselves.
  */
 
-// TODO: get rid of this macro, properly clean up gcry_ structures after use
-#define check(foo) { gcry_error_t err = (foo); if(err) {logger(DEBUG_ALWAYS, LOG_ERR, "gcrypt error %s/%s at %s:%d", gcry_strsource(err), gcry_strerror(err), __FILE__, __LINE__); return false; }}
+static bool rsa_powm(const gcry_mpi_t ed, const gcry_mpi_t n, const void *in, size_t len, void *out) {
+       gcry_mpi_t inmpi = NULL;
 
-bool rsa_public_encrypt(rsa_t *rsa, const void *in, size_t len, void *out) {
-       gcry_mpi_t inmpi;
-       check(gcry_mpi_scan(&inmpi, GCRYMPI_FMT_USG, in, len, NULL));
+       if(!check(gcry_mpi_scan(&inmpi, GCRYMPI_FMT_USG, in, len, NULL))) {
+               return false;
+       }
 
-       gcry_mpi_t outmpi = gcry_mpi_new(len * 8);
-       gcry_mpi_powm(outmpi, inmpi, rsa->e, rsa->n);
+       gcry_mpi_t outmpi = gcry_mpi_snew(len * 8);
+       gcry_mpi_powm(outmpi, inmpi, ed, n);
 
        size_t out_bytes = (gcry_mpi_get_nbits(outmpi) + 7) / 8;
        size_t pad = len - MIN(out_bytes, len);
@@ -240,28 +249,20 @@ bool rsa_public_encrypt(rsa_t *rsa, const void *in, size_t len, void *out) {
                *pout++ = 0;
        }
 
-       check(gcry_mpi_print(GCRYMPI_FMT_USG, pout, len, NULL, outmpi));
-
-       return true;
-}
+       bool ok = check(gcry_mpi_print(GCRYMPI_FMT_USG, pout, len, NULL, outmpi));
 
-bool rsa_private_decrypt(rsa_t *rsa, const void *in, size_t len, void *out) {
-       gcry_mpi_t inmpi;
-       check(gcry_mpi_scan(&inmpi, GCRYMPI_FMT_USG, in, len, NULL));
-
-       gcry_mpi_t outmpi = gcry_mpi_new(len * 8);
-       gcry_mpi_powm(outmpi, inmpi, rsa->d, rsa->n);
-
-       size_t pad = len - (gcry_mpi_get_nbits(outmpi) + 7) / 8;
-       unsigned char *pout = out;
+       gcry_mpi_release(outmpi);
+       gcry_mpi_release(inmpi);
 
-       for(; pad; --pad) {
-               *pout++ = 0;
-       }
+       return ok;
+}
 
-       check(gcry_mpi_print(GCRYMPI_FMT_USG, pout, len, NULL, outmpi));
+bool rsa_public_encrypt(rsa_t *rsa, const void *in, size_t len, void *out) {
+       return rsa_powm(rsa->e, rsa->n, in, len, out);
+}
 
-       return true;
+bool rsa_private_decrypt(rsa_t *rsa, const void *in, size_t len, void *out) {
+       return rsa_powm(rsa->d, rsa->n, in, len, out);
 }
 
 void rsa_free(rsa_t *rsa) {
index 01bb137..8576555 100644 (file)
@@ -26,6 +26,7 @@
 #include "pem.h"
 #include "../rsagen.h"
 #include "../xalloc.h"
+#include "../utils.h"
 
 // ASN.1 tags.
 typedef enum {
@@ -240,7 +241,9 @@ bool rsa_write_pem_private_key(rsa_t *rsa, FILE *fp) {
        gcry_mpi_release(params[dq]);
        gcry_mpi_release(params[u]);
 
-       return pem_encode(fp, "RSA PRIVATE KEY", derbuf, derlen);
+       bool success = pem_encode(fp, "RSA PRIVATE KEY", derbuf, derlen);
+       memzero(derbuf, sizeof(derbuf));
+       return success;
 }
 
 static gcry_mpi_t find_mpi(const gcry_sexp_t rsa, const char *token) {
index 6c9d675..d176098 100644 (file)
@@ -29,6 +29,8 @@
 #define _CRT_NONSTDC_NO_WARNINGS
 #endif
 
+#define __STDC_WANT_LIB_EXT1__ 1
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
index cff9e72..5db1e98 100644 (file)
@@ -314,6 +314,7 @@ static bool copy_config_replacing_port(FILE *out, const char *filename, const ch
                }
        }
 
+       memzero(line, sizeof(line));
        fclose(in);
        return true;
 }
@@ -521,6 +522,7 @@ int cmd_invite(int argc, char *argv[]) {
        int ifd = open(filename, O_RDWR | O_CREAT | O_EXCL, 0600);
 
        if(!ifd) {
+               memzero(cookie, sizeof(cookie));
                fprintf(stderr, "Could not create invitation file %s: %s\n", filename, strerror(errno));
                return 1;
        }
@@ -536,9 +538,17 @@ int cmd_invite(int argc, char *argv[]) {
        char *port = NULL;
 
        if(!get_my_hostname(&address, &port)) {
+               memzero(cookie, sizeof(cookie));
                return 1;
        }
 
+       // Create an URL from the local address, key hash and cookie
+       char *url;
+       xasprintf(&url, "%s/%s%s", address, hash, cookie);
+
+       memzero(cookie, sizeof(cookie));
+       free(address);
+
        // Fill in the details.
        fprintf(f, "Name = %s\n", argv[1]);
 
@@ -577,14 +587,10 @@ int cmd_invite(int argc, char *argv[]) {
 
        if(!appended) {
                fprintf(stderr, "Could not append my config to invitation file: %s.\n", strerror(errno));
-               free(address);
+               free_string(url);
                return 1;
        }
 
-       // Create an URL from the local address, key hash and cookie
-       char *url;
-       xasprintf(&url, "%s/%s%s", address, hash, cookie);
-
        // Call the inviation-created script
        environment_t env;
        environment_init(&env);
@@ -595,8 +601,7 @@ int cmd_invite(int argc, char *argv[]) {
        environment_exit(&env);
 
        puts(url);
-       free(url);
-       free(address);
+       free_string(url);
 
        return 0;
 }
@@ -972,9 +977,9 @@ make_names:
                return false;
        }
 
-       char *b64key = ecdsa_get_base64_public_key(key);
+       char *b64_pubkey = ecdsa_get_base64_public_key(key);
 
-       if(!b64key) {
+       if(!b64_pubkey) {
                return false;
        }
 
@@ -994,10 +999,10 @@ make_names:
 
        fclose(f);
 
-       fprintf(fh, "Ed25519PublicKey = %s\n", b64key);
+       fprintf(fh, "Ed25519PublicKey = %s\n", b64_pubkey);
 
-       sptps_send_record(&sptps, 1, b64key, strlen(b64key));
-       free(b64key);
+       sptps_send_record(&sptps, 1, b64_pubkey, strlen(b64_pubkey));
+       free(b64_pubkey);
        ecdsa_free(key);
 
 #ifndef DISABLE_LEGACY
@@ -1299,13 +1304,13 @@ int cmd_join(int argc, char *argv[]) {
                return 1;
        }
 
-       char *b64key = ecdsa_get_base64_public_key(key);
+       char *b64_pubkey = ecdsa_get_base64_public_key(key);
 
        // Connect to the tinc daemon mentioned in the URL.
        struct addrinfo *ai = str2addrinfo(address, port, SOCK_STREAM);
 
        if(!ai) {
-               free(b64key);
+               free(b64_pubkey);
                ecdsa_free(key);
                return 1;
        }
@@ -1320,7 +1325,7 @@ next:
 
                if(!aip) {
                        freeaddrinfo(ai);
-                       free(b64key);
+                       free(b64_pubkey);
                        ecdsa_free(key);
                        return 1;
                }
@@ -1346,13 +1351,13 @@ next:
        fprintf(stderr, "Connected to %s port %s...\n", address, port);
 
        // Tell him we have an invitation, and give him our throw-away key.
-       ssize_t len = snprintf(line, sizeof(line), "0 ?%s %d.%d\n", b64key, PROT_MAJOR, PROT_MINOR);
+       ssize_t len = snprintf(line, sizeof(line), "0 ?%s %d.%d\n", b64_pubkey, PROT_MAJOR, PROT_MINOR);
 
        if(len <= 0 || (size_t)len >= sizeof(line)) {
                abort();
        }
 
-       if(!sendline(sock, "0 ?%s %d.%d", b64key, PROT_MAJOR, 1)) {
+       if(!sendline(sock, "0 ?%s %d.%d", b64_pubkey, PROT_MAJOR, 1)) {
                fprintf(stderr, "Error sending request to %s port %s: %s\n", address, port, strerror(errno));
                closesocket(sock);
                goto next;
@@ -1371,8 +1376,8 @@ next:
        ai = NULL;
        aip = NULL;
 
-       free(b64key);
-       b64key = NULL;
+       free(b64_pubkey);
+       b64_pubkey = NULL;
 
        // Check if the hash of the key he gave us matches the hash in the URL.
        char *fingerprint = line + 2;
index 2db2b15..ac99ac4 100644 (file)
@@ -227,13 +227,13 @@ rsa_t *read_rsa_private_key(splay_tree_t *config_tree, char **keyfile) {
        if(get_config_string(rsa_priv_conf, &d)) {
                if(!get_config_string(lookup_config(config_tree, "PublicKey"), &n)) {
                        logger(DEBUG_ALWAYS, LOG_ERR, "PrivateKey used but no PublicKey found!");
-                       free(d);
+                       free_string(d);
                        return NULL;
                }
 
                key = rsa_set_hex_private_key(n, "FFFF", d);
                free(n);
-               free(d);
+               free_string(d);
 
                if(key && keyfile && rsa_priv_conf->file) {
                        *keyfile = xstrdup(rsa_priv_conf->file);
index c8e783b..f84fb4d 100644 (file)
@@ -73,8 +73,11 @@ endif
 check_functions = [
   'asprintf',
   'daemon',
+  'explicit_bzero',
+  'explicit_memset',
   'fchmod',
   'gettimeofday',
+  'memset_s',
   'mlockall',
   'putenv',
   'strsignal',
index b0b8760..40cdaf6 100644 (file)
@@ -267,7 +267,7 @@ bool setup_myself_reloadable(void) {
                        proxytype = PROXY_EXEC;
                } else {
                        logger(DEBUG_ALWAYS, LOG_ERR, "Unknown proxy type %s!", proxy);
-                       free(proxy);
+                       free_string(proxy);
                        return false;
                }
 
@@ -277,10 +277,10 @@ bool setup_myself_reloadable(void) {
                free(proxyport);
                proxyport = NULL;
 
-               free(proxyuser);
+               free_string(proxyuser);
                proxyuser = NULL;
 
-               free(proxypass);
+               free_string(proxypass);
                proxypass = NULL;
 
                switch(proxytype) {
@@ -291,7 +291,7 @@ bool setup_myself_reloadable(void) {
                case PROXY_EXEC:
                        if(!space || !*space) {
                                logger(DEBUG_ALWAYS, LOG_ERR, "Argument expected for proxy type exec!");
-                               free(proxy);
+                               free_string(proxy);
                                return false;
                        }
 
@@ -312,7 +312,7 @@ bool setup_myself_reloadable(void) {
                                logger(DEBUG_ALWAYS, LOG_ERR, "Host and port argument expected for proxy!");
                                proxyport = NULL;
                                proxyhost = NULL;
-                               free(proxy);
+                               free_string(proxy);
                                return false;
                        }
 
@@ -338,7 +338,7 @@ bool setup_myself_reloadable(void) {
                        break;
                }
 
-               free(proxy);
+               free_string(proxy);
        }
 
        bool choice;
index 53013d2..23a0610 100644 (file)
@@ -89,6 +89,10 @@ node_t *new_node(void) {
 }
 
 void free_node(node_t *n) {
+       if(!n) {
+               return;
+       }
+
        splay_empty_tree(&n->subnet_tree);
        splay_empty_tree(&n->edge_tree);
 
index a4e4ac6..56ebf27 100644 (file)
@@ -113,7 +113,7 @@ exit:
        if(!rsa)
 #endif
        {
-               BN_free(bn_d);
+               BN_clear_free(bn_d);
                BN_free(bn_e);
                BN_free(bn_n);
        }
index 3c006d5..bf165a7 100644 (file)
@@ -80,8 +80,10 @@ static int past_request_compare(const past_request_t *a, const past_request_t *b
 }
 
 static void free_past_request(past_request_t *r) {
-       free((char *)r->request);
-       free(r);
+       if(r) {
+               free((char *)r->request);
+               free(r);
+       }
 }
 
 static splay_tree_t past_request_tree = {
index 2225457..b13d901 100644 (file)
@@ -554,9 +554,10 @@ bool send_metakey(connection_t *c) {
        }
 
        const size_t len = rsa_size(ctx->rsa);
+       const size_t hexkeylen = HEX_SIZE(len);
        char *key = alloca(len);
        char *enckey = alloca(len);
-       char *hexkey = alloca(2 * len + 1);
+       char *hexkey = alloca(hexkeylen);
 
        /* Create a random key */
 
@@ -576,12 +577,14 @@ bool send_metakey(connection_t *c) {
 
        if(!cipher_set_key_from_rsa(&ctx->out.cipher, key, len, true)) {
                free_legacy_ctx(ctx);
+               memzero(key, len);
                return false;
        }
 
        if(debug_level >= DEBUG_SCARY_THINGS) {
                bin2hex(key, hexkey, len);
                logger(DEBUG_SCARY_THINGS, LOG_DEBUG, "Generated random meta key (unencrypted): %s", hexkey);
+               memzero(hexkey, hexkeylen);
        }
 
        /* Encrypt the random data
@@ -591,7 +594,10 @@ bool send_metakey(connection_t *c) {
           with a length equal to that of the modulus of the RSA key.
         */
 
-       if(!rsa_public_encrypt(ctx->rsa, key, len, enckey)) {
+       bool encrypted = rsa_public_encrypt(ctx->rsa, key, len, enckey);
+       memzero(key, len);
+
+       if(!encrypted) {
                free_legacy_ctx(ctx);
                logger(DEBUG_ALWAYS, LOG_ERR, "Error during encryption of meta key for %s (%s)", c->name, c->hostname);
                return false;
@@ -631,6 +637,11 @@ bool metakey_h(connection_t *c, const char *request) {
                return false;
        }
 
+       if(!cipher || !digest) {
+               logger(DEBUG_ALWAYS, LOG_ERR, "Possible intruder %s (%s): cipher %d, digest %d", c->name, c->hostname, cipher, digest);
+               return false;
+       }
+
        /* Convert the challenge from hexadecimal back to binary */
 
        size_t inlen = hex2bin(hexkey, enckey, len);
@@ -652,26 +663,26 @@ bool metakey_h(connection_t *c, const char *request) {
        if(debug_level >= DEBUG_SCARY_THINGS) {
                bin2hex(key, hexkey, len);
                logger(DEBUG_SCARY_THINGS, LOG_DEBUG, "Received random meta key (unencrypted): %s", hexkey);
+               // Hopefully the user knew what he was doing leaking session keys into logs. We'll do the right thing here anyway.
+               memzero(hexkey, HEX_SIZE(len));
        }
 
        /* Check and lookup cipher and digest algorithms */
 
-       if(!cipher || !digest) {
-               logger(DEBUG_ALWAYS, LOG_ERR, "Possible intruder %s (%s): cipher %d, digest %d", c->name, c->hostname, cipher, digest);
-               return false;
-       }
-
        if(!init_crypto_by_nid(&c->legacy->in, cipher, digest)) {
+               memzero(key, len);
                logger(DEBUG_ALWAYS, LOG_ERR, "Error during initialisation of cipher or digest from %s (%s)", c->name, c->hostname);
                return false;
        }
 
-       if(!cipher_set_key_from_rsa(&c->legacy->in.cipher, key, len, false)) {
+       bool key_set = cipher_set_key_from_rsa(&c->legacy->in.cipher, key, len, false);
+       memzero(key, len);
+
+       if(!key_set) {
                logger(DEBUG_ALWAYS, LOG_ERR, "Error setting RSA key for %s (%s)", c->name, c->hostname);
                return false;
        }
 
-
        c->status.decryptin = true;
 
        c->allow_request = CHALLENGE;
index 740d2fb..09acd6a 100644 (file)
@@ -33,7 +33,7 @@
 #include "utils.h"
 #include "compression.h"
 #include "random.h"
-#include "legacy.h"
+#include "xalloc.h"
 
 void send_key_changed(void) {
 #ifndef DISABLE_LEGACY
@@ -341,7 +341,8 @@ bool send_ans_key(node_t *to) {
        return false;
 #else
        size_t keylen = myself->incipher ? cipher_keylength(myself->incipher) : 1;
-       char *key = alloca(keylen * 2 + 1);
+       size_t keyhexlen = HEX_SIZE(keylen);
+       char *key = alloca(keyhexlen);
 
        randomize(key, keylen);
 
@@ -388,12 +389,16 @@ bool send_ans_key(node_t *to) {
 
        to->status.validkey_in = true;
 
-       return send_request(to->nexthop->connection, "%d %s %s %s %d %d %lu %d", ANS_KEY,
-                           myself->name, to->name, key,
-                           cipher_get_nid(to->incipher),
-                           digest_get_nid(to->indigest),
-                           (unsigned long)digest_length(to->indigest),
-                           to->incompression);
+       bool sent = send_request(to->nexthop->connection, "%d %s %s %s %d %d %lu %d", ANS_KEY,
+                                myself->name, to->name, key,
+                                cipher_get_nid(to->incipher),
+                                digest_get_nid(to->indigest),
+                                (unsigned long)digest_length(to->indigest),
+                                to->incompression);
+
+       memzero(key, keyhexlen);
+
+       return sent;
 #endif
 }
 
index 0b16c86..3f44bf9 100644 (file)
@@ -134,7 +134,7 @@ void environment_init(environment_t *env) {
 
 void environment_exit(environment_t *env) {
        for(int i = 0; i < env->n; i++) {
-               free(env->entries[i]);
+               free_string(env->entries[i]);
        }
 
        free(env->entries);
index a0483c3..42eaba0 100644 (file)
 #include "chacha-poly1305/chacha-poly1305.h"
 #include "ecdh.h"
 #include "ecdsa.h"
-#include "logger.h"
 #include "prf.h"
 #include "sptps.h"
 #include "random.h"
+#include "xalloc.h"
 
 unsigned int sptps_replaywin = 16;
 
@@ -90,6 +90,22 @@ static void warning(sptps_t *s, const char *format, ...) {
        va_end(ap);
 }
 
+static sptps_kex_t *new_sptps_kex(void) {
+       return xzalloc(sizeof(sptps_kex_t));
+}
+
+static void free_sptps_kex(sptps_kex_t *kex) {
+       xzfree(kex, sizeof(sptps_kex_t));
+}
+
+static sptps_key_t *new_sptps_key(void) {
+       return xzalloc(sizeof(sptps_key_t));
+}
+
+static void free_sptps_key(sptps_key_t *key) {
+       xzfree(key, sizeof(sptps_key_t));
+}
+
 // Send a record (datagram version, accepts all record types, handles encryption and authentication).
 static bool send_record_priv_datagram(sptps_t *s, uint8_t type, const void *data, uint16_t len) {
        uint8_t *buffer = alloca(len + 21UL);
@@ -154,49 +170,49 @@ bool sptps_send_record(sptps_t *s, uint8_t type, const void *data, uint16_t len)
 
 // Send a Key EXchange record, containing a random nonce and an ECDHE public key.
 static bool send_kex(sptps_t *s) {
-       size_t keylen = ECDH_SIZE;
-
        // Make room for our KEX message, which we will keep around since send_sig() needs it.
        if(s->mykex) {
                return false;
        }
 
-       s->mykex = realloc(s->mykex, 1 + 32 + keylen);
-
-       if(!s->mykex) {
-               return error(s, errno, strerror(errno));
-       }
+       s->mykex = new_sptps_kex();
 
        // Set version byte to zero.
-       s->mykex[0] = SPTPS_VERSION;
+       s->mykex->version = SPTPS_VERSION;
 
        // Create a random nonce.
-       randomize(s->mykex + 1, 32);
+       randomize(s->mykex->nonce, ECDH_SIZE);
 
        // Create a new ECDH public key.
-       if(!(s->ecdh = ecdh_generate_public(s->mykex + 1 + 32))) {
+       if(!(s->ecdh = ecdh_generate_public(s->mykex->pubkey))) {
                return error(s, EINVAL, "Failed to generate ECDH public key");
        }
 
-       return send_record_priv(s, SPTPS_HANDSHAKE, s->mykex, 1 + 32 + keylen);
+       return send_record_priv(s, SPTPS_HANDSHAKE, s->mykex, sizeof(sptps_kex_t));
+}
+
+static size_t sigmsg_len(size_t labellen) {
+       return 1 + 2 * sizeof(sptps_kex_t) + labellen;
+}
+
+static void fill_msg(uint8_t *msg, bool initiator, const sptps_kex_t *kex0, const sptps_kex_t *kex1, const sptps_t *s) {
+       *msg = initiator, msg++;
+       memcpy(msg, kex0, sizeof(*kex0)), msg += sizeof(*kex0);
+       memcpy(msg, kex1, sizeof(*kex1)), msg += sizeof(*kex1);
+       memcpy(msg, s->label, s->labellen);
 }
 
 // Send a SIGnature record, containing an Ed25519 signature over both KEX records.
 static bool send_sig(sptps_t *s) {
-       size_t keylen = ECDH_SIZE;
-       size_t siglen = ecdsa_size(s->mykey);
-
        // Concatenate both KEX messages, plus tag indicating if it is from the connection originator, plus label
-       const size_t msglen = (1 + 32 + keylen) * 2 + 1 + s->labellen;
+       size_t msglen = sigmsg_len(s->labellen);
        uint8_t *msg = alloca(msglen);
-       uint8_t *sig = alloca(siglen);
-
-       msg[0] = s->initiator;
-       memcpy(msg + 1, s->mykex, 1 + 32 + keylen);
-       memcpy(msg + 1 + 33 + keylen, s->hiskex, 1 + 32 + keylen);
-       memcpy(msg + 1 + 2 * (33 + keylen), s->label, s->labellen);
+       fill_msg(msg, s->initiator, s->mykex, s->hiskex, s);
 
        // Sign the result.
+       size_t siglen = ecdsa_size(s->mykey);
+       uint8_t *sig = alloca(siglen);
+
        if(!ecdsa_sign(s->mykey, msg, msglen, sig)) {
                return error(s, EINVAL, "Failed to sign SIG record");
        }
@@ -218,30 +234,27 @@ static bool generate_key_material(sptps_t *s, const uint8_t *shared, size_t len)
        }
 
        // Allocate memory for key material
-       size_t keylen = 2 * CHACHA_POLY1305_KEYLEN;
+       s->key = new_sptps_key();
 
-       s->key = realloc(s->key, keylen);
+       // Create the HMAC seed, which is "key expansion" + session label + server nonce + client nonce
+       const size_t msglen = sizeof("key expansion") - 1;
+       const size_t seedlen = msglen + s->labellen + ECDH_SIZE * 2;
+       uint8_t *seed = alloca(seedlen);
 
-       if(!s->key) {
-               return error(s, errno, strerror(errno));
-       }
+       uint8_t *ptr = seed;
+       memcpy(ptr, "key expansion", msglen);
+       ptr += msglen;
 
-       // Create the HMAC seed, which is "key expansion" + session label + server nonce + client nonce
-       uint8_t *seed = alloca(s->labellen + 64 + 13);
-       memcpy(seed, "key expansion", 13);
+       memcpy(ptr, (s->initiator ? s->mykex : s->hiskex)->nonce, ECDH_SIZE);
+       ptr += ECDH_SIZE;
 
-       if(s->initiator) {
-               memcpy(seed + 13, s->mykex + 1, 32);
-               memcpy(seed + 45, s->hiskex + 1, 32);
-       } else {
-               memcpy(seed + 13, s->hiskex + 1, 32);
-               memcpy(seed + 45, s->mykex + 1, 32);
-       }
+       memcpy(ptr, (s->initiator ? s->hiskex : s->mykex)->nonce, ECDH_SIZE);
+       ptr += ECDH_SIZE;
 
-       memcpy(seed + 77, s->label, s->labellen);
+       memcpy(ptr, s->label, s->labellen);
 
        // Use PRF to generate the key material
-       if(!prf(shared, len, seed, s->labellen + 64 + 13, s->key, keylen)) {
+       if(!prf(shared, len, seed, seedlen, s->key->both, sizeof(sptps_key_t))) {
                return error(s, EINVAL, "Failed to generate key material");
        }
 
@@ -261,17 +274,13 @@ static bool receive_ack(sptps_t *s, const uint8_t *data, uint16_t len) {
                return error(s, EIO, "Invalid ACK record length");
        }
 
-       if(s->initiator) {
-               if(!chacha_poly1305_set_key(s->incipher, s->key)) {
-                       return error(s, EINVAL, "Failed to set counter");
-               }
-       } else {
-               if(!chacha_poly1305_set_key(s->incipher, s->key + CHACHA_POLY1305_KEYLEN)) {
-                       return error(s, EINVAL, "Failed to set counter");
-               }
+       uint8_t *key = s->initiator ? s->key->key0 : s->key->key1;
+
+       if(!chacha_poly1305_set_key(s->incipher, key)) {
+               return error(s, EINVAL, "Failed to set counter");
        }
 
-       free(s->key);
+       free_sptps_key(s->key);
        s->key = NULL;
        s->instate = true;
 
@@ -281,24 +290,21 @@ static bool receive_ack(sptps_t *s, const uint8_t *data, uint16_t len) {
 // Receive a Key EXchange record, respond by sending a SIG record.
 static bool receive_kex(sptps_t *s, const uint8_t *data, uint16_t len) {
        // Verify length of the HELLO record
-       if(len != 1 + 32 + ECDH_SIZE) {
+       if(len != sizeof(sptps_kex_t)) {
                return error(s, EIO, "Invalid KEX record length");
        }
 
-       // Ignore version number for now.
+       if(*data != SPTPS_VERSION) {
+               return error(s, EINVAL, "Received incorrect version %d", *data);
+       }
 
        // Make a copy of the KEX message, send_sig() and receive_sig() need it
        if(s->hiskex) {
                return error(s, EINVAL, "Received a second KEX message before first has been processed");
        }
 
-       s->hiskex = realloc(s->hiskex, len);
-
-       if(!s->hiskex) {
-               return error(s, errno, strerror(errno));
-       }
-
-       memcpy(s->hiskex, data, len);
+       s->hiskex = new_sptps_kex();
+       memcpy(s->hiskex, data, sizeof(sptps_kex_t));
 
        if(s->initiator) {
                return send_sig(s);
@@ -309,22 +315,15 @@ static bool receive_kex(sptps_t *s, const uint8_t *data, uint16_t len) {
 
 // Receive a SIGnature record, verify it, if it passed, compute the shared secret and calculate the session keys.
 static bool receive_sig(sptps_t *s, const uint8_t *data, uint16_t len) {
-       size_t keylen = ECDH_SIZE;
-       size_t siglen = ecdsa_size(s->hiskey);
-
        // Verify length of KEX record.
-       if(len != siglen) {
+       if(len != ecdsa_size(s->hiskey)) {
                return error(s, EIO, "Invalid KEX record length");
        }
 
        // Concatenate both KEX messages, plus tag indicating if it is from the connection originator
-       const size_t msglen = (1 + 32 + keylen) * 2 + 1 + s->labellen;
+       const size_t msglen = sigmsg_len(s->labellen);
        uint8_t *msg = alloca(msglen);
-
-       msg[0] = !s->initiator;
-       memcpy(msg + 1, s->hiskex, 1 + 32 + keylen);
-       memcpy(msg + 1 + 33 + keylen, s->mykex, 1 + 32 + keylen);
-       memcpy(msg + 1 + 2 * (33 + keylen), s->label, s->labellen);
+       fill_msg(msg, !s->initiator, s->hiskex, s->mykex, s);
 
        // Verify signature.
        if(!ecdsa_verify(s->hiskey, msg, msglen, data)) {
@@ -334,14 +333,18 @@ static bool receive_sig(sptps_t *s, const uint8_t *data, uint16_t len) {
        // Compute shared secret.
        uint8_t shared[ECDH_SHARED_SIZE];
 
-       if(!ecdh_compute_shared(s->ecdh, s->hiskex + 1 + 32, shared)) {
+       if(!ecdh_compute_shared(s->ecdh, s->hiskex->pubkey, shared)) {
+               memzero(shared, sizeof(shared));
                return error(s, EINVAL, "Failed to compute ECDH shared secret");
        }
 
        s->ecdh = NULL;
 
        // Generate key material from shared secret.
-       if(!generate_key_material(s, shared, sizeof(shared))) {
+       bool generated = generate_key_material(s, shared, sizeof(shared));
+       memzero(shared, sizeof(shared));
+
+       if(!generated) {
                return false;
        }
 
@@ -349,10 +352,10 @@ static bool receive_sig(sptps_t *s, const uint8_t *data, uint16_t len) {
                return false;
        }
 
-       free(s->mykex);
-       free(s->hiskex);
-
+       free_sptps_kex(s->mykex);
        s->mykex = NULL;
+
+       free_sptps_kex(s->hiskex);
        s->hiskex = NULL;
 
        // Send cipher change record
@@ -361,14 +364,10 @@ static bool receive_sig(sptps_t *s, const uint8_t *data, uint16_t len) {
        }
 
        // TODO: only set new keys after ACK has been set/received
-       if(s->initiator) {
-               if(!chacha_poly1305_set_key(s->outcipher, s->key + CHACHA_POLY1305_KEYLEN)) {
-                       return error(s, EINVAL, "Failed to set key");
-               }
-       } else {
-               if(!chacha_poly1305_set_key(s->outcipher, s->key)) {
-                       return error(s, EINVAL, "Failed to set key");
-               }
+       uint8_t *key = s->initiator ? s->key->key1 : s->key->key0;
+
+       if(!chacha_poly1305_set_key(s->outcipher, key)) {
+               return error(s, EINVAL, "Failed to set key");
        }
 
        return true;
@@ -759,9 +758,9 @@ bool sptps_stop(sptps_t *s) {
        chacha_poly1305_exit(s->outcipher);
        ecdh_free(s->ecdh);
        free(s->inbuf);
-       free(s->mykex);
-       free(s->hiskex);
-       free(s->key);
+       free_sptps_kex(s->mykex);
+       free_sptps_kex(s->hiskex);
+       free_sptps_key(s->key);
        free(s->label);
        free(s->late);
        memset(s, 0, sizeof(*s));
index 96edc36..95209e5 100644 (file)
@@ -47,6 +47,26 @@ typedef enum sptps_state_t {
        SPTPS_ACK = 4,           // Waiting for an ACKnowledgement record
 } sptps_state_t;
 
+PACKED(struct sptps_kex_t {
+       uint8_t version;
+       uint8_t nonce[ECDH_SIZE];
+       uint8_t pubkey[ECDH_SIZE];
+});
+
+typedef struct sptps_kex_t sptps_kex_t;
+
+STATIC_ASSERT(sizeof(sptps_kex_t) == 65, "sptps_kex_t has invalid size");
+
+typedef union sptps_key_t {
+       struct {
+               uint8_t key0[CHACHA_POLY1305_KEYLEN];
+               uint8_t key1[CHACHA_POLY1305_KEYLEN];
+       };
+       uint8_t both[CHACHA_POLY1305_KEYLEN * 2];
+} sptps_key_t;
+
+STATIC_ASSERT(sizeof(sptps_key_t) == 128, "sptps_key_t has invalid size");
+
 typedef struct sptps {
        bool initiator;
        bool datagram;
@@ -72,9 +92,9 @@ typedef struct sptps {
        ecdsa_t *hiskey;
        ecdh_t *ecdh;
 
-       uint8_t *mykex;
-       uint8_t *hiskex;
-       uint8_t *key;
+       sptps_kex_t *mykex;
+       sptps_kex_t *hiskex;
+       sptps_key_t *key;
        uint8_t *label;
        size_t labellen;
 
index 17d26f9..7fcfee6 100644 (file)
@@ -62,14 +62,14 @@ static int generate_keypair(char *argv[]) {
        if(fp) {
                if(!ecdsa_write_pem_private_key(key, fp)) {
                        fprintf(stderr, "Could not write ECDSA private key\n");
-                       free(key);
+                       ecdsa_free(key);
                        return 1;
                }
 
                fclose(fp);
        } else {
                fprintf(stderr, "Could not open '%s' for writing: %s\n", argv[1], strerror(errno));
-               free(key);
+               ecdsa_free(key);
                return 1;
        }
 
@@ -80,12 +80,12 @@ static int generate_keypair(char *argv[]) {
                        fprintf(stderr, "Could not write ECDSA public key\n");
                }
 
-               free(key);
+               ecdsa_free(key);
                fclose(fp);
                return 0;
        } else {
                fprintf(stderr, "Could not open '%s' for writing: %s\n", argv[2], strerror(errno));
-               free(key);
+               ecdsa_free(key);
                return 1;
        }
 }
index 249f2e4..acc692a 100644 (file)
@@ -543,14 +543,14 @@ static int run_test(int argc, char *argv[]) {
 
        if(!fp) {
                fprintf(stderr, "Could not open %s: %s\n", argv[2], strerror(errno));
-               free(mykey);
+               ecdsa_free(mykey);
                return 1;
        }
 
        ecdsa_t *hiskey = NULL;
 
        if(!(hiskey = ecdsa_read_pem_public_key(fp))) {
-               free(mykey);
+               ecdsa_free(mykey);
                return 1;
        }
 
@@ -563,8 +563,8 @@ static int run_test(int argc, char *argv[]) {
        sptps_t s;
 
        if(!sptps_start(&s, &sock, initiator, datagram, mykey, hiskey, "sptps_test", 10, send_data, receive_record)) {
-               free(mykey);
-               free(hiskey);
+               ecdsa_free(mykey);
+               ecdsa_free(hiskey);
                return 1;
        }
 
@@ -575,8 +575,8 @@ static int run_test(int argc, char *argv[]) {
 
                if(in < 0) {
                        fprintf(stderr, "Could not init stdin reader thread\n");
-                       free(mykey);
-                       free(hiskey);
+                       ecdsa_free(mykey);
+                       ecdsa_free(hiskey);
                        return 1;
                }
        }
@@ -603,8 +603,8 @@ static int run_test(int argc, char *argv[]) {
                FD_SET(sock, &fds);
 
                if(select(max_fd + 1, &fds, NULL, NULL, NULL) <= 0) {
-                       free(mykey);
-                       free(hiskey);
+                       ecdsa_free(mykey);
+                       ecdsa_free(hiskey);
                        return 1;
                }
 
@@ -617,8 +617,8 @@ static int run_test(int argc, char *argv[]) {
 
                        if(len < 0) {
                                fprintf(stderr, "Could not read from stdin: %s\n", strerror(errno));
-                               free(mykey);
-                               free(hiskey);
+                               ecdsa_free(mykey);
+                               ecdsa_free(hiskey);
                                return 1;
                        }
 
@@ -649,8 +649,8 @@ static int run_test(int argc, char *argv[]) {
                                        sptps_send_record(&s, 0, buf, len);
                                }
                        } else if(!sptps_send_record(&s, buf[0] == '!' ? 1 : 0, buf, (len == 1 && buf[0] == '\n') ? 0 : buf[0] == '*' ? sizeof(buf) : (size_t)len)) {
-                               free(mykey);
-                               free(hiskey);
+                               ecdsa_free(mykey);
+                               ecdsa_free(hiskey);
                                return 1;
                        }
                }
@@ -660,8 +660,8 @@ static int run_test(int argc, char *argv[]) {
 
                        if(len < 0) {
                                fprintf(stderr, "Could not read from socket: %s\n", sockstrerror(sockerrno));
-                               free(mykey);
-                               free(hiskey);
+                               ecdsa_free(mykey);
+                               ecdsa_free(hiskey);
                                return 1;
                        }
 
@@ -691,8 +691,8 @@ static int run_test(int argc, char *argv[]) {
 
                                if(!done) {
                                        if(!datagram) {
-                                               free(mykey);
-                                               free(hiskey);
+                                               ecdsa_free(mykey);
+                                               ecdsa_free(hiskey);
                                                return 1;
                                        }
                                }
@@ -705,8 +705,8 @@ static int run_test(int argc, char *argv[]) {
 
        bool stopped = sptps_stop(&s);
 
-       free(mykey);
-       free(hiskey);
+       ecdsa_free(mykey);
+       ecdsa_free(hiskey);
        closesocket(sock);
 
        return !stopped;
index c2f4333..c395169 100644 (file)
@@ -18,6 +18,8 @@
     51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 
+#include <assert.h>
+
 #include "logger.h"
 #include "system.h"
 #include "utils.h"
index be629ee..0818047 100644 (file)
@@ -26,6 +26,7 @@
 #include "crypto.h"
 
 #define B64_SIZE(len) ((len) * 4 / 3 + 5)
+#define HEX_SIZE(len) ((len) * 2 + 1)
 
 extern size_t hex2bin(const char *src, void *dst, size_t length);
 extern size_t bin2hex(const void *src, char *dst, size_t length);
index 96b9592..c8cfbeb 100644 (file)
@@ -23,6 +23,8 @@
 
 #include "system.h"
 
+#include <assert.h>
+
 static inline void *xmalloc(size_t n) ATTR_MALLOC;
 static inline void *xmalloc(size_t n) {
        void *p = malloc(n);
@@ -96,4 +98,51 @@ static inline int xasprintf(char **strp, const char *fmt, ...) {
        return result;
 }
 
+// Zero out a block of memory containing sensitive information using whatever secure
+// erase function is available on the platform (or an unreliable fallback if none are).
+// The pointer must not be NULL. Length can be zero, in which case the call is a noop.
+static inline void memzero(void *buf, size_t buflen) ATTR_NONNULL;
+static inline void memzero(void *buf, size_t buflen) {
+       assert(buf);
+
+       if(!buflen) {
+               return;
+       }
+
+#if defined(HAVE_EXPLICIT_BZERO)
+       explicit_bzero(buf, buflen);
+#elif defined(HAVE_EXPLICIT_MEMSET)
+       explicit_memset(buf, 0, buflen);
+#elif defined(HAVE_MEMSET_S)
+       errno_t err = memset_s(buf, buflen, 0, buflen);
+       assert(err == 0);
+#elif defined(HAVE_WINDOWS)
+       SecureZeroMemory(buf, buflen);
+#else
+       volatile uint8_t *p = buf;
+
+       while(buflen--) {
+               *p++ = 0;
+       }
+
 #endif
+}
+
+// Zero out a buffer of size `len` located at `ptr` and free() it.
+// Does nothing if called on NULL.
+static inline void xzfree(void *ptr, size_t len) {
+       if(ptr) {
+               memzero(ptr, len);
+               free(ptr);
+       }
+}
+
+// Zero out a NULL-terminated string using memzero() and then free it.
+// Does nothing if called on NULL.
+static inline void free_string(char *str) {
+       if(str) {
+               xzfree(str, strlen(str));
+       }
+}
+
+#endif // TINC_XALLOC_H
index bebd093..eaa9d18 100644 (file)
@@ -47,6 +47,13 @@ tests = {
   'utils': {
     'code': 'test_utils.c',
   },
+  'xalloc': {
+    'code': 'test_xalloc.c',
+  },
+  'memzero_null': {
+    'code': 'test_memzero_null.c',
+    'fail': true,
+  },
   'splay_tree': {
     'code': 'test_splay_tree.c',
     'link': link_tinc,
diff --git a/test/unit/test_memzero_null.c b/test/unit/test_memzero_null.c
new file mode 100644 (file)
index 0000000..b7ae136
--- /dev/null
@@ -0,0 +1,12 @@
+// Test that memzero() with NULL pointer crashes the program
+
+#include "config.h"
+#undef HAVE_ATTR_NONNULL
+
+#include "unittest.h"
+#include "../../src/xalloc.h"
+
+int main(void) {
+       memzero(NULL, 1);
+       return 0;
+}
index d02f453..aeedda9 100644 (file)
@@ -9,14 +9,8 @@ int main(void) {
 #else
 #include "../../src/random.h"
 
-static void on_abort(int sig) {
-       (void)sig;
-       exit(1);
-}
-
 int main(void) {
-       signal(SIGABRT, on_abort);
-       u_int8_t buf[16];
+       uint8_t buf[16];
        randomize(buf, sizeof(buf));
        return 0;
 }
diff --git a/test/unit/test_xalloc.c b/test/unit/test_xalloc.c
new file mode 100644 (file)
index 0000000..08f7a72
--- /dev/null
@@ -0,0 +1,61 @@
+#include "unittest.h"
+#include "../../src/xalloc.h"
+
+static const uint8_t ref[] = {0, 1, 2, 3, 4, 5, 6, 7};
+
+static void test_memzero_wipes_partial(void **state) {
+       (void)state;
+
+       uint8_t buf[sizeof(ref)];
+       memcpy(buf, ref, sizeof(buf));
+
+       const size_t len = 2;
+       memzero(buf, len);
+       assert_int_equal(0, buf[0]);
+       assert_int_equal(0, buf[1]);
+
+       assert_memory_equal(&buf[len], &ref[len], sizeof(ref) - len);
+}
+
+static void test_memzero_wipes_buffer(void **state) {
+       (void)state;
+
+       uint8_t buf[sizeof(ref)];
+       uint8_t zero[sizeof(ref)] = {0};
+
+       memcpy(buf, ref, sizeof(buf));
+       assert_memory_equal(ref, buf, sizeof(buf));
+
+       memzero(buf, sizeof(buf));
+       assert_memory_not_equal(buf, ref, sizeof(buf));
+       assert_memory_equal(zero, buf, sizeof(buf));
+}
+
+static void test_memzero_zerolen_does_not_change_memory(void **state) {
+       (void)state;
+
+       uint8_t buf[sizeof(ref)];
+
+       memcpy(buf, ref, sizeof(buf));
+       assert_memory_equal(ref, buf, sizeof(buf));
+
+       memzero(buf, 0);
+       assert_memory_equal(ref, buf, sizeof(buf));
+}
+
+// This test will fail under ASAN if xzfree forgets to call free() or overflows the buffer
+static void test_xzfree_frees_memory(void **state) {
+       (void)state;
+
+       xzfree(xmalloc(64), 64);
+}
+
+int main(void) {
+       const struct CMUnitTest tests[] = {
+               cmocka_unit_test(test_memzero_wipes_partial),
+               cmocka_unit_test(test_memzero_wipes_buffer),
+               cmocka_unit_test(test_memzero_zerolen_does_not_change_memory),
+               cmocka_unit_test(test_xzfree_frees_memory),
+       };
+       return cmocka_run_group_tests(tests, NULL, NULL);
+}