From: Kirill Isakov Date: Sun, 15 Aug 2021 18:57:05 +0000 (+0600) Subject: Replace pointers to cipher_t/digest_t in connection_t with structs. X-Git-Url: https://git.tinc-vpn.org/git/browse?a=commitdiff_plain;h=bcac314fe2d758e85335d499dbb4300bfa8a599e;p=tinc Replace pointers to cipher_t/digest_t in connection_t with structs. Part of #294. --- diff --git a/src/Makefile.am b/src/Makefile.am index 792dd784..a1d34239 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -216,13 +216,13 @@ endif if OPENSSL tincd_SOURCES += \ - openssl/cipher.c \ + openssl/cipher.c openssl/cipher.h \ openssl/crypto.c \ openssl/digest.c openssl/digest.h \ openssl/prf.c \ openssl/rsa.c tinc_SOURCES += \ - openssl/cipher.c \ + openssl/cipher.c openssl/cipher.h \ openssl/crypto.c \ openssl/digest.c openssl/digest.h \ openssl/prf.c \ @@ -282,6 +282,13 @@ sptps_speed_SOURCES += \ endif endif +if WITH_LEGACY_PROTOCOL +tinc_SOURCES += digest.c +tincd_SOURCES += digest.c cipher.c +sptps_test_SOURCES += digest.c +sptps_speed_SOURCES += digest.c +endif + if MINIUPNPC tincd_SOURCES += upnp.h upnp.c tincd_LDADD = $(MINIUPNPC_LIBS) diff --git a/src/cipher.c b/src/cipher.c new file mode 100644 index 00000000..0bab3bd9 --- /dev/null +++ b/src/cipher.c @@ -0,0 +1,18 @@ +#include "cipher.h" +#include "xalloc.h" + +#ifndef DISABLE_LEGACY + +cipher_t *cipher_alloc() { + return xzalloc(sizeof(cipher_t)); +} + +void cipher_free(cipher_t **cipher) { + if(cipher && *cipher) { + cipher_close(*cipher); + free(*cipher); + *cipher = NULL; + } +} + +#endif // DISABLE_LEGACY diff --git a/src/cipher.h b/src/cipher.h index 68332be3..1224210c 100644 --- a/src/cipher.h +++ b/src/cipher.h @@ -28,10 +28,20 @@ #ifndef DISABLE_LEGACY +#ifdef HAVE_OPENSSL +#include "openssl/cipher.h" +#elif HAVE_LIBGCRYPT +#include "gcrypt/cipher.h" +#else +#error Incorrect cryptographic library, please reconfigure. +#endif + typedef struct cipher cipher_t; -extern cipher_t *cipher_open_by_name(const char *name) __attribute__((__malloc__)); -extern cipher_t *cipher_open_by_nid(int nid) __attribute__((__malloc__)); +extern cipher_t *cipher_alloc() __attribute__((__malloc__)); +extern void cipher_free(cipher_t **cipher); +extern bool cipher_open_by_name(cipher_t *cipher, const char *name); +extern bool cipher_open_by_nid(cipher_t *cipher, int nid); extern void cipher_close(cipher_t *cipher); extern size_t cipher_keylength(const cipher_t *cipher); extern size_t cipher_blocksize(const cipher_t *cipher); @@ -43,6 +53,6 @@ extern bool cipher_decrypt(cipher_t *cipher, const void *indata, size_t inlen, v extern int cipher_get_nid(const cipher_t *cipher); extern bool cipher_active(const cipher_t *cipher); -#endif +#endif // DISABLE_LEGACY -#endif +#endif // TINC_CIPHER_H diff --git a/src/connection.c b/src/connection.c index 0c5e7ef0..7d27e706 100644 --- a/src/connection.c +++ b/src/connection.c @@ -63,10 +63,10 @@ void free_connection(connection_t *c) { } #ifndef DISABLE_LEGACY - cipher_close(c->incipher); - digest_close(c->indigest); - cipher_close(c->outcipher); - digest_close(c->outdigest); + cipher_close(&c->incipher); + digest_close(&c->indigest); + cipher_close(&c->outcipher); + digest_close(&c->outdigest); rsa_free(c->rsa); #endif diff --git a/src/connection.h b/src/connection.h index b23e02b7..d9340fdb 100644 --- a/src/connection.h +++ b/src/connection.h @@ -78,10 +78,10 @@ typedef struct connection_t { #ifndef DISABLE_LEGACY rsa_t *rsa; /* his public RSA key */ - cipher_t *incipher; /* Cipher he will use to send data to us */ - cipher_t *outcipher; /* Cipher we will use to send data to him */ - digest_t *indigest; - digest_t *outdigest; + cipher_t incipher; /* Cipher he will use to send data to us */ + cipher_t outcipher; /* Cipher we will use to send data to him */ + digest_t indigest; + digest_t outdigest; uint64_t inbudget; uint64_t outbudget; #endif diff --git a/src/digest.c b/src/digest.c new file mode 100644 index 00000000..85bea7d5 --- /dev/null +++ b/src/digest.c @@ -0,0 +1,18 @@ +#include "digest.h" +#include "xalloc.h" + +#ifndef DISABLE_LEGACY + +digest_t *digest_alloc() { + return xzalloc(sizeof(digest_t)); +} + +void digest_free(digest_t **digest) { + if(digest && *digest) { + digest_close(*digest); + free(*digest); + *digest = NULL; + } +} + +#endif // DISABLE_LEGACY diff --git a/src/digest.h b/src/digest.h index 1cebf582..9fb12483 100644 --- a/src/digest.h +++ b/src/digest.h @@ -27,10 +27,20 @@ #ifndef DISABLE_LEGACY +#ifdef HAVE_OPENSSL +#include "openssl/digest.h" +#elif HAVE_LIBGCRYPT +#include "gcrypt/digest.h" +#else +#error Incorrect cryptographic library, please reconfigure. +#endif + typedef struct digest digest_t; -extern digest_t *digest_open_by_name(const char *name, size_t maclength) __attribute__((__malloc__)); -extern digest_t *digest_open_by_nid(int nid, size_t maclength) __attribute__((__malloc__)); +extern bool digest_open_by_name(digest_t *digest, const char *name, size_t maclength); +extern bool digest_open_by_nid(digest_t *digest, int nid, size_t maclength); +extern digest_t *digest_alloc() __attribute__((__malloc__)); +extern void digest_free(digest_t **digest); extern void digest_close(digest_t *digest); extern bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *outdata) __attribute__((__warn_unused_result__)); extern bool digest_verify(digest_t *digest, const void *indata, size_t inlen, const void *digestdata) __attribute__((__warn_unused_result__)); diff --git a/src/gcrypt/cipher.c b/src/gcrypt/cipher.c index 83065317..d8e4cc51 100644 --- a/src/gcrypt/cipher.c +++ b/src/gcrypt/cipher.c @@ -147,7 +147,8 @@ void cipher_close(cipher_t *cipher) { } free(cipher->key); - cipher->key = NULL; + + memset(cipher, 0, sizeof(*cipher)); } size_t cipher_keylength(const cipher_t *cipher) { diff --git a/src/gcrypt/digest.c b/src/gcrypt/digest.c index 3333f5f2..ad1f31d8 100644 --- a/src/gcrypt/digest.c +++ b/src/gcrypt/digest.c @@ -120,7 +120,7 @@ void digest_close(digest_t *digest) { gcry_md_close(digest->hmac); } - digest->hmac = NULL; + memset(digest, 0, sizeof(*digest)); } bool digest_set_key(digest_t *digest, const void *key, size_t len) { diff --git a/src/meta.c b/src/meta.c index 7a8baac4..f6a9ba18 100644 --- a/src/meta.c +++ b/src/meta.c @@ -78,7 +78,7 @@ bool send_meta(connection_t *c, const void *buffer, size_t length) { size_t outlen = length; - if(!cipher_encrypt(c->outcipher, buffer, length, buffer_prepare(&c->outbuf, length), &outlen, false) || outlen != length) { + if(!cipher_encrypt(&c->outcipher, buffer, length, buffer_prepare(&c->outbuf, length), &outlen, false) || outlen != length) { logger(DEBUG_ALWAYS, LOG_ERR, "Error while encrypting metadata to %s (%s)", c->name, c->hostname); return false; @@ -258,7 +258,7 @@ bool receive_meta(connection_t *c) { size_t outlen = inlen; - if(!cipher_decrypt(c->incipher, bufp, inlen, buffer_prepare(&c->inbuf, inlen), &outlen, false) || (size_t)inlen != outlen) { + if(!cipher_decrypt(&c->incipher, bufp, inlen, buffer_prepare(&c->inbuf, inlen), &outlen, false) || (size_t)inlen != outlen) { logger(DEBUG_ALWAYS, LOG_ERR, "Error while decrypting metadata from %s (%s)", c->name, c->hostname); return false; diff --git a/src/net_setup.c b/src/net_setup.c index d4993fa9..d2adc13f 100644 --- a/src/net_setup.c +++ b/src/net_setup.c @@ -820,10 +820,15 @@ static bool setup_myself(void) { if(!strcasecmp(cipher, "none")) { myself->incipher = NULL; - } else if(!(myself->incipher = cipher_open_by_name(cipher))) { - logger(DEBUG_ALWAYS, LOG_ERR, "Unrecognized cipher type!"); - free(cipher); - return false; + } else { + myself->incipher = cipher_alloc(); + + if(!cipher_open_by_name(myself->incipher, cipher)) { + logger(DEBUG_ALWAYS, LOG_ERR, "Unrecognized cipher type!"); + cipher_free(&myself->incipher); + free(cipher); + return false; + } } free(cipher); @@ -850,10 +855,15 @@ static bool setup_myself(void) { if(!strcasecmp(digest, "none")) { myself->indigest = NULL; - } else if(!(myself->indigest = digest_open_by_name(digest, maclength))) { - logger(DEBUG_ALWAYS, LOG_ERR, "Unrecognized digest type!"); - free(digest); - return false; + } else { + myself->indigest = digest_alloc(); + + if(!digest_open_by_name(myself->indigest, digest, maclength)) { + logger(DEBUG_ALWAYS, LOG_ERR, "Unrecognized digest type!"); + digest_free(&myself->indigest); + free(digest); + return false; + } } free(digest); diff --git a/src/node.c b/src/node.c index 4caf992d..7655cae9 100644 --- a/src/node.c +++ b/src/node.c @@ -95,10 +95,10 @@ void free_node(node_t *n) { sockaddrfree(&n->address); #ifndef DISABLE_LEGACY - cipher_close(n->incipher); - digest_close(n->indigest); - cipher_close(n->outcipher); - digest_close(n->outdigest); + cipher_free(&n->incipher); + digest_free(&n->indigest); + cipher_free(&n->outcipher); + digest_free(&n->outdigest); #endif ecdsa_free(n->ecdsa); diff --git a/src/openssl/cipher.c b/src/openssl/cipher.c index 45d101df..08b81de7 100644 --- a/src/openssl/cipher.c +++ b/src/openssl/cipher.c @@ -25,45 +25,38 @@ #include "../cipher.h" #include "../logger.h" -#include "../xalloc.h" -struct cipher { - EVP_CIPHER_CTX *ctx; - const EVP_CIPHER *cipher; -}; - -static cipher_t *cipher_open(const EVP_CIPHER *evp_cipher) { - cipher_t *cipher = xzalloc(sizeof(*cipher)); +static void cipher_open(cipher_t *cipher, const EVP_CIPHER *evp_cipher) { cipher->cipher = evp_cipher; cipher->ctx = EVP_CIPHER_CTX_new(); if(!cipher->ctx) { abort(); } - - return cipher; } -cipher_t *cipher_open_by_name(const char *name) { +bool cipher_open_by_name(cipher_t *cipher, const char *name) { const EVP_CIPHER *evp_cipher = EVP_get_cipherbyname(name); if(!evp_cipher) { logger(DEBUG_ALWAYS, LOG_ERR, "Unknown cipher name '%s'!", name); - return NULL; + return false; } - return cipher_open(evp_cipher); + cipher_open(cipher, evp_cipher); + return true; } -cipher_t *cipher_open_by_nid(int nid) { +bool cipher_open_by_nid(cipher_t *cipher, int nid) { const EVP_CIPHER *evp_cipher = EVP_get_cipherbynid(nid); if(!evp_cipher) { logger(DEBUG_ALWAYS, LOG_ERR, "Unknown cipher nid %d!", nid); - return NULL; + return false; } - return cipher_open(evp_cipher); + cipher_open(cipher, evp_cipher); + return true; } void cipher_close(cipher_t *cipher) { @@ -71,8 +64,11 @@ void cipher_close(cipher_t *cipher) { return; } - EVP_CIPHER_CTX_free(cipher->ctx); - free(cipher); + if(cipher->ctx) { + EVP_CIPHER_CTX_free(cipher->ctx); + } + + memset(cipher, 0, sizeof(*cipher)); } size_t cipher_keylength(const cipher_t *cipher) { diff --git a/src/openssl/cipher.h b/src/openssl/cipher.h new file mode 100644 index 00000000..6f3e6b7b --- /dev/null +++ b/src/openssl/cipher.h @@ -0,0 +1,11 @@ +#ifndef TINC_OPENSSL_CIPHER_H +#define TINC_OPENSSL_CIPHER_H + +#include + +struct cipher { + EVP_CIPHER_CTX *ctx; + const EVP_CIPHER *cipher; +}; + +#endif diff --git a/src/openssl/digest.c b/src/openssl/digest.c index 82364e71..71eaec7c 100644 --- a/src/openssl/digest.c +++ b/src/openssl/digest.c @@ -18,7 +18,6 @@ */ #include "../system.h" -#include "../xalloc.h" #include #include @@ -27,8 +26,7 @@ #include "../digest.h" #include "../logger.h" -static digest_t *digest_open(const EVP_MD *evp_md, size_t maclength) { - digest_t *digest = xzalloc(sizeof(*digest)); +static void digest_open(digest_t *digest, const EVP_MD *evp_md, size_t maclength) { digest->digest = evp_md; size_t digestlen = EVP_MD_size(digest->digest); @@ -38,11 +36,9 @@ static digest_t *digest_open(const EVP_MD *evp_md, size_t maclength) { } else { digest->maclength = maclength; } - - return digest; } -digest_t *digest_open_by_name(const char *name, size_t maclength) { +bool digest_open_by_name(digest_t *digest, const char *name, size_t maclength) { const EVP_MD *evp_md = EVP_get_digestbyname(name); if(!evp_md) { @@ -50,10 +46,11 @@ digest_t *digest_open_by_name(const char *name, size_t maclength) { return false; } - return digest_open(evp_md, maclength); + digest_open(digest, evp_md, maclength); + return true; } -digest_t *digest_open_by_nid(int nid, size_t maclength) { +bool digest_open_by_nid(digest_t *digest, int nid, size_t maclength) { const EVP_MD *evp_md = EVP_get_digestbynid(nid); if(!evp_md) { @@ -61,7 +58,8 @@ digest_t *digest_open_by_nid(int nid, size_t maclength) { return false; } - return digest_open(evp_md, maclength); + digest_open(digest, evp_md, maclength); + return true; } bool digest_set_key(digest_t *digest, const void *key, size_t len) { @@ -88,7 +86,7 @@ void digest_close(digest_t *digest) { HMAC_CTX_free(digest->hmac_ctx); } - free(digest); + memset(digest, 0, sizeof(*digest)); } bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *outdata) { diff --git a/src/openssl/prf.c b/src/openssl/prf.c index 62770859..5d597d9a 100644 --- a/src/openssl/prf.c +++ b/src/openssl/prf.c @@ -30,18 +30,19 @@ */ static bool prf_xor(int nid, const uint8_t *secret, size_t secretlen, uint8_t *seed, size_t seedlen, uint8_t *out, size_t outlen) { - digest_t *digest = digest_open_by_nid(nid, DIGEST_ALGO_SIZE); + digest_t digest = {0}; - if(!digest) { + if(!digest_open_by_nid(&digest, nid, DIGEST_ALGO_SIZE)) { + digest_close(&digest); return false; } - if(!digest_set_key(digest, secret, secretlen)) { - digest_close(digest); + if(!digest_set_key(&digest, secret, secretlen)) { + digest_close(&digest); return false; } - size_t len = digest_length(digest); + size_t len = digest_length(&digest); /* Data is what the "inner" HMAC function processes. It consists of the previous HMAC result plus the seed. @@ -55,14 +56,14 @@ static bool prf_xor(int nid, const uint8_t *secret, size_t secretlen, uint8_t *s while(outlen > 0) { /* Inner HMAC */ - if(!digest_create(digest, data, len + seedlen, data)) { - digest_close(digest); + if(!digest_create(&digest, data, len + seedlen, data)) { + digest_close(&digest); return false; } /* Outer HMAC */ - if(!digest_create(digest, data, len + seedlen, hash)) { - digest_close(digest); + if(!digest_create(&digest, data, len + seedlen, hash)) { + digest_close(&digest); return false; } @@ -76,7 +77,7 @@ static bool prf_xor(int nid, const uint8_t *secret, size_t secretlen, uint8_t *s outlen -= i; } - digest_close(digest); + digest_close(&digest); return true; } diff --git a/src/protocol_auth.c b/src/protocol_auth.c index e457b19e..d4bb4074 100644 --- a/src/protocol_auth.c +++ b/src/protocol_auth.c @@ -509,18 +509,24 @@ bool send_metakey(connection_t *c) { */ size_t keylen = cipher_keylength(myself->incipher); + const char *cipher_name; if(keylen <= 16) { - c->outcipher = cipher_open_by_name("aes-128-cfb"); + cipher_name = "aes-128-cfb"; } else if(keylen <= 24) { - c->outcipher = cipher_open_by_name("aes-192-cfb"); + cipher_name = "aes-192-cfb"; } else { - c->outcipher = cipher_open_by_name("aes-256-cfb"); + cipher_name = "aes-256-cfb"; } - c->outbudget = cipher_budget(c->outcipher); + if(!cipher_open_by_name(&c->outcipher, cipher_name)) { + return false; + } + + c->outbudget = cipher_budget(&c->outcipher); - if(!(c->outdigest = digest_open_by_name("sha256", DIGEST_ALGO_SIZE))) { + if(!digest_open_by_name(&c->outdigest, "sha256", DIGEST_ALGO_SIZE)) { + cipher_close(&c->outcipher); return false; } @@ -545,7 +551,7 @@ bool send_metakey(connection_t *c) { key[0] &= 0x7F; - if(!cipher_set_key_from_rsa(c->outcipher, key, len, true)) { + if(!cipher_set_key_from_rsa(&c->outcipher, key, len, true)) { return false; } @@ -573,8 +579,8 @@ bool send_metakey(connection_t *c) { /* Send the meta key */ bool result = send_request(c, "%d %d %d %d %d %s", METAKEY, - cipher_get_nid(c->outcipher), - digest_get_nid(c->outdigest), c->outmaclength, + cipher_get_nid(&c->outcipher), + digest_get_nid(&c->outdigest), c->outmaclength, c->outcompression, hexkey); c->status.encryptout = true; @@ -623,7 +629,7 @@ bool metakey_h(connection_t *c, const char *request) { /* Check and lookup cipher and digest algorithms */ if(cipher) { - if(!(c->incipher = cipher_open_by_nid(cipher)) || !cipher_set_key_from_rsa(c->incipher, key, len, false)) { + if(!cipher_open_by_nid(&c->incipher, cipher) || !cipher_set_key_from_rsa(&c->incipher, key, len, false)) { logger(DEBUG_ALWAYS, LOG_ERR, "Error during initialisation of cipher from %s (%s)", c->name, c->hostname); return false; } @@ -632,10 +638,10 @@ bool metakey_h(connection_t *c, const char *request) { return false; } - c->inbudget = cipher_budget(c->incipher); + c->inbudget = cipher_budget(&c->incipher); if(digest) { - if(!(c->indigest = digest_open_by_nid(digest, DIGEST_ALGO_SIZE))) { + if(!digest_open_by_nid(&c->indigest, digest, DIGEST_ALGO_SIZE)) { logger(DEBUG_ALWAYS, LOG_ERR, "Error during initialisation of digest from %s (%s)", c->name, c->hostname); return false; } @@ -709,12 +715,12 @@ bool challenge_h(connection_t *c, const char *request) { bool send_chal_reply(connection_t *c) { const size_t len = rsa_size(myself->connection->rsa); - size_t digestlen = digest_length(c->indigest); + size_t digestlen = digest_length(&c->indigest); char digest[digestlen * 2 + 1]; /* Calculate the hash from the challenge we received */ - if(!digest_create(c->indigest, c->mychallenge, len, digest)) { + if(!digest_create(&c->indigest, c->mychallenge, len, digest)) { return false; } @@ -745,7 +751,7 @@ bool chal_reply_h(connection_t *c, const char *request) { /* Check if the length of the hash is all right */ - if(inlen != digest_length(c->outdigest)) { + if(inlen != digest_length(&c->outdigest)) { logger(DEBUG_ALWAYS, LOG_ERR, "Possible intruder %s (%s): %s", c->name, c->hostname, "wrong challenge reply length"); return false; } @@ -753,7 +759,7 @@ bool chal_reply_h(connection_t *c, const char *request) { /* Verify the hash */ - if(!digest_verify(c->outdigest, c->hischallenge, rsa_size(c->rsa), hishash)) { + if(!digest_verify(&c->outdigest, c->hischallenge, rsa_size(c->rsa), hishash)) { logger(DEBUG_ALWAYS, LOG_ERR, "Possible intruder %s (%s): %s", c->name, c->hostname, "wrong challenge reply"); return false; } diff --git a/src/protocol_key.c b/src/protocol_key.c index a957780b..f045009f 100644 --- a/src/protocol_key.c +++ b/src/protocol_key.c @@ -340,13 +340,13 @@ bool send_ans_key(node_t *to) { randomize(key, keylen); - cipher_close(to->incipher); - digest_close(to->indigest); + cipher_free(&to->incipher); + digest_free(&to->indigest); if(myself->incipher) { - to->incipher = cipher_open_by_nid(cipher_get_nid(myself->incipher)); + to->incipher = cipher_alloc(); - if(!to->incipher) { + if(!cipher_open_by_nid(to->incipher, cipher_get_nid(myself->incipher))) { abort(); } @@ -356,10 +356,11 @@ bool send_ans_key(node_t *to) { } if(myself->indigest) { - to->indigest = digest_open_by_nid(digest_get_nid(myself->indigest), - digest_length(myself->indigest)); + to->indigest = digest_alloc(); - if(!to->indigest) { + if(!digest_open_by_nid(to->indigest, + digest_get_nid(myself->indigest), + digest_length(myself->indigest))) { abort(); } @@ -459,8 +460,8 @@ bool ans_key_h(connection_t *c, const char *request) { #ifndef DISABLE_LEGACY /* Don't use key material until every check has passed. */ - cipher_close(from->outcipher); - digest_close(from->outdigest); + cipher_free(&from->outcipher); + digest_free(&from->outdigest); #endif if(!from->status.sptps) { @@ -555,7 +556,10 @@ bool ans_key_h(connection_t *c, const char *request) { /* Check and lookup cipher and digest algorithms */ if(cipher) { - if(!(from->outcipher = cipher_open_by_nid(cipher))) { + from->outcipher = cipher_alloc(); + + if(!cipher_open_by_nid(from->outcipher, cipher)) { + cipher_free(&from->outcipher); logger(DEBUG_ALWAYS, LOG_ERR, "Node %s (%s) uses unknown cipher!", from->name, from->hostname); return false; } @@ -564,7 +568,10 @@ bool ans_key_h(connection_t *c, const char *request) { } if(digest) { - if(!(from->outdigest = digest_open_by_nid(digest, maclength))) { + from->outdigest = digest_alloc(); + + if(!digest_open_by_nid(from->outdigest, digest, maclength)) { + digest_free(&from->outdigest); logger(DEBUG_ALWAYS, LOG_ERR, "Node %s (%s) uses unknown digest!", from->name, from->hostname); return false; }