From c845bc109c85e6fb350096c63e13ef8e617ee29b Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Fri, 18 Dec 2009 01:15:25 +0100 Subject: [PATCH] Fix packet authentication. This wasn't working at all, since we didn't do HMAC but just a plain hash. Also, verification of packets failed because it was checking the whole packet, not the packet minus the HMAC. --- src/gcrypt/digest.c | 34 ++++++++++++++++++++++++++++------ src/gcrypt/digest.h | 2 ++ src/net_packet.c | 41 ++++++++++++++++++++--------------------- src/openssl/digest.c | 35 +++++++++++++++++++++++++++-------- src/openssl/digest.h | 3 +++ src/protocol_key.c | 10 +++++++--- 6 files changed, 87 insertions(+), 38 deletions(-) diff --git a/src/gcrypt/digest.c b/src/gcrypt/digest.c index 34f9d280..066600f9 100644 --- a/src/gcrypt/digest.c +++ b/src/gcrypt/digest.c @@ -87,6 +87,7 @@ static bool digest_open(digest_t *digest, int algo, int maclength) { digest->maclength = maclength; digest->algo = algo; + digest->hmac = NULL; return true; } @@ -118,24 +119,45 @@ bool digest_open_sha1(digest_t *digest, int maclength) { } void digest_close(digest_t *digest) { + if(digest->hmac) + gcry_md_close(digest->hmac); + digest->hmac = NULL; +} + +bool digest_set_key(digest_t *digest, const void *key, size_t len) { + if(!digest->hmac) + gcry_md_open(&digest->hmac, digest->algo, GCRY_MD_FLAG_HMAC); + if(!digest->hmac) + return false; + + return !gcry_md_setkey(digest->hmac, key, len); } bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *outdata) { unsigned int len = gcry_md_get_algo_dlen(digest->algo); - char tmpdata[len]; - gcry_md_hash_buffer(digest->algo, tmpdata, indata, inlen); - memcpy(outdata, tmpdata, digest->maclength); + if(digest->hmac) { + char *tmpdata; + gcry_md_reset(digest->hmac); + gcry_md_write(digest->hmac, indata, inlen); + tmpdata = gcry_md_read(digest->hmac, digest->algo); + if(!tmpdata) + return false; + memcpy(outdata, tmpdata, digest->maclength); + } else { + char tmpdata[len]; + gcry_md_hash_buffer(digest->algo, tmpdata, indata, inlen); + memcpy(outdata, tmpdata, digest->maclength); + } return true; } bool digest_verify(digest_t *digest, const void *indata, size_t inlen, const void *cmpdata) { - unsigned int len = gcry_md_get_algo_dlen(digest->algo); + unsigned int len = digest->maclength; char outdata[len]; - gcry_md_hash_buffer(digest->algo, outdata, indata, inlen); - return !memcmp(cmpdata, outdata, digest->maclength); + return digest_create(digest, indata, inlen, outdata) && !memcmp(cmpdata, outdata, len); } int digest_get_nid(const digest_t *digest) { diff --git a/src/gcrypt/digest.h b/src/gcrypt/digest.h index f70fecce..1188496a 100644 --- a/src/gcrypt/digest.h +++ b/src/gcrypt/digest.h @@ -28,6 +28,7 @@ typedef struct digest { int algo; int nid; int maclength; + gcry_md_hd_t hmac; } digest_t; extern bool digest_open_by_name(struct digest *, const char *name, int maclength); @@ -36,6 +37,7 @@ extern bool digest_open_sha1(struct digest *, int maclength); extern void digest_close(struct digest *); extern bool digest_create(struct digest *, const void *indata, size_t inlen, void *outdata); extern bool digest_verify(struct digest *, const void *indata, size_t inlen, const void *digestdata); +extern bool digest_set_key(struct digest *, const void *key, size_t len); extern int digest_get_nid(const struct digest *); extern size_t digest_length(const struct digest *); extern bool digest_active(const struct digest *); diff --git a/src/net_packet.c b/src/net_packet.c index aaa0c720..df6f4826 100644 --- a/src/net_packet.c +++ b/src/net_packet.c @@ -195,7 +195,7 @@ static bool try_mac(node_t *n, const vpn_packet_t *inpkt) { if(!digest_active(&n->indigest) || inpkt->len < sizeof inpkt->seqno + digest_length(&n->indigest)) return false; - return digest_verify(&n->indigest, &inpkt->seqno, inpkt->len, (const char *)&inpkt->seqno + inpkt->len); + return digest_verify(&n->indigest, &inpkt->seqno, inpkt->len - n->indigest.maclength, (const char *)&inpkt->seqno + inpkt->len - n->indigest.maclength); } static void receive_udppacket(node_t *n, vpn_packet_t *inpkt) { @@ -222,11 +222,13 @@ static void receive_udppacket(node_t *n, vpn_packet_t *inpkt) { /* Check the message authentication code */ - if(digest_active(&n->indigest) && !digest_verify(&n->indigest, &inpkt->seqno, inpkt->len, (const char *)&inpkt->seqno + inpkt->len)) { - ifdebug(TRAFFIC) logger(LOG_DEBUG, "Got unauthenticated packet from %s (%s)", n->name, n->hostname); - return; + if(digest_active(&n->indigest)) { + inpkt->len -= n->indigest.maclength; + if(!digest_verify(&n->indigest, &inpkt->seqno, inpkt->len, (const char *)&inpkt->seqno + inpkt->len)) { + ifdebug(TRAFFIC) logger(LOG_DEBUG, "Got unauthenticated packet from %s (%s)", n->name, n->hostname); + return; + } } - /* Decrypt the packet */ if(cipher_active(&n->incipher)) { @@ -501,31 +503,28 @@ void broadcast_packet(const node_t *from, vpn_packet_t *packet) { static node_t *try_harder(const sockaddr_t *from, const vpn_packet_t *pkt) { splay_node_t *node; - edge_t *e; - node_t *n = NULL; + node_t *n, *found = NULL; static time_t last_hard_try = 0; time_t now = time(NULL); - for(node = edge_weight_tree->head; node; node = node->next) { - e = node->data; - - if(sockaddrcmp_noport(from, &e->address)) { - if(last_hard_try == now) - continue; - last_hard_try = now; - } + if(last_hard_try == now) + return NULL; + else + last_hard_try = now; - if(!n) - n = e->to; + for(node = node_tree->head; node; node = node->next) { + n = node->data; - if(!try_mac(e->to, pkt)) + if(n == myself || !digest_active(&n->indigest)) continue; - n = e->to; - break; + if(try_mac(n, pkt)) { + found = n; + break; + } } - return n; + return found; } void handle_incoming_vpn_data(int sock, short events, void *data) { diff --git a/src/openssl/digest.c b/src/openssl/digest.c index ff350daa..f20a9755 100644 --- a/src/openssl/digest.c +++ b/src/openssl/digest.c @@ -18,8 +18,11 @@ */ #include "system.h" +#include "utils.h" +#include "xalloc.h" #include +#include #include "digest.h" #include "logger.h" @@ -35,6 +38,7 @@ static void set_maclength(digest_t *digest, int maclength) { bool digest_open_by_name(digest_t *digest, const char *name, int maclength) { digest->digest = EVP_get_digestbyname(name); + digest->key = NULL; if(!digest->digest) { logger(LOG_DEBUG, "Unknown digest name '%s'!", name); @@ -47,6 +51,7 @@ bool digest_open_by_name(digest_t *digest, const char *name, int maclength) { bool digest_open_by_nid(digest_t *digest, int nid, int maclength) { digest->digest = EVP_get_digestbynid(nid); + digest->key = NULL; if(!digest->digest) { logger(LOG_DEBUG, "Unknown digest nid %d!", nid); @@ -59,25 +64,39 @@ bool digest_open_by_nid(digest_t *digest, int nid, int maclength) { bool digest_open_sha1(digest_t *digest, int maclength) { digest->digest = EVP_sha1(); + digest->key = NULL; set_maclength(digest, maclength); return true; } +bool digest_set_key(digest_t *digest, const void *key, size_t len) { + digest->key = xrealloc(digest->key, len); + memcpy(digest->key, key, len); + digest->keylength = len; +} + void digest_close(digest_t *digest) { + if(digest->key) + free(digest->key); + digest->key = NULL; } bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *outdata) { size_t len = EVP_MD_size(digest->digest); unsigned char tmpdata[len]; - EVP_MD_CTX ctx; - - if(!EVP_DigestInit(&ctx, digest->digest) - || !EVP_DigestUpdate(&ctx, indata, inlen) - || !EVP_DigestFinal(&ctx, tmpdata, NULL)) { - logger(LOG_DEBUG, "Error creating digest: %s", ERR_error_string(ERR_get_error(), NULL)); - return false; + if(digest->key) { + HMAC(digest->digest, digest->key, digest->keylength, indata, inlen, tmpdata, NULL); + } else { + EVP_MD_CTX ctx; + + if(!EVP_DigestInit(&ctx, digest->digest) + || !EVP_DigestUpdate(&ctx, indata, inlen) + || !EVP_DigestFinal(&ctx, tmpdata, NULL)) { + logger(LOG_DEBUG, "Error creating digest: %s", ERR_error_string(ERR_get_error(), NULL)); + return false; + } } memcpy(outdata, tmpdata, digest->maclength); @@ -85,7 +104,7 @@ bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *out } bool digest_verify(digest_t *digest, const void *indata, size_t inlen, const void *cmpdata) { - size_t len = EVP_MD_size(digest->digest); + size_t len = digest->maclength; unsigned char outdata[len]; return digest_create(digest, indata, inlen, outdata) && !memcmp(cmpdata, outdata, digest->maclength); diff --git a/src/openssl/digest.h b/src/openssl/digest.h index 258bcf6c..f3855c92 100644 --- a/src/openssl/digest.h +++ b/src/openssl/digest.h @@ -27,6 +27,8 @@ typedef struct digest { const EVP_MD *digest; int maclength; + int keylength; + char *key; } digest_t; extern bool digest_open_by_name(struct digest *, const char *name, int maclength); @@ -35,6 +37,7 @@ extern bool digest_open_sha1(struct digest *, int maclength); extern void digest_close(struct digest *); extern bool digest_create(struct digest *, const void *indata, size_t inlen, void *outdata); extern bool digest_verify(struct digest *, const void *indata, size_t inlen, const void *digestdata); +extern bool digest_set_key(struct digest *, const void *key, size_t len); extern int digest_get_nid(const struct digest *); extern size_t digest_length(const struct digest *); extern bool digest_active(const struct digest *); diff --git a/src/protocol_key.c b/src/protocol_key.c index aae5516c..3b022679 100644 --- a/src/protocol_key.c +++ b/src/protocol_key.c @@ -139,6 +139,7 @@ bool send_ans_key(node_t *to) { randomize(key, keylen); cipher_set_key(&to->incipher, key, true); + digest_set_key(&to->indigest, key, keylen); bin2hex(key, key, keylen); key[keylen * 2] = '\0'; @@ -160,7 +161,7 @@ bool ans_key_h(connection_t *c, char *request) { char from_name[MAX_STRING_SIZE]; char to_name[MAX_STRING_SIZE]; char key[MAX_STRING_SIZE]; - int cipher, digest, maclength, compression; + int cipher, digest, maclength, compression, keylen; node_t *from, *to; if(sscanf(request, "%*d "MAX_STRING" "MAX_STRING" "MAX_STRING" %d %d %d %d", @@ -209,7 +210,9 @@ bool ans_key_h(connection_t *c, char *request) { return false; } - if(strlen(key) / 2 != cipher_keylength(&from->outcipher)) { + keylen = strlen(key) / 2; + + if(keylen != cipher_keylength(&from->outcipher)) { logger(LOG_ERR, "Node %s (%s) uses wrong keylength!", from->name, from->hostname); return false; } @@ -233,8 +236,9 @@ bool ans_key_h(connection_t *c, char *request) { /* Update our copy of the origin's packet key */ - hex2bin(key, key, cipher_keylength(&from->outcipher)); + hex2bin(key, key, keylen); cipher_set_key(&from->outcipher, key, false); + digest_set_key(&from->outdigest, key, keylen); from->status.validkey = true; from->status.waitingforkey = false; -- 2.20.1