From c4940a5c888d85b4c477b6face5e9a618e64718d Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Sun, 30 Sep 2012 13:45:47 +0200 Subject: [PATCH 1/1] Add strict checks to hex to binary conversions. The main goal is to catch misuse of the obsolete PrivateKey and PublicKey statements. --- lib/utils.c | 10 ++++++---- lib/utils.h | 2 +- src/net_setup.c | 15 ++++++++++++--- src/protocol_auth.c | 15 ++++++++++++--- src/protocol_key.c | 8 +++++++- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/lib/utils.c b/lib/utils.c index 405097b..3b221f5 100644 --- a/lib/utils.c +++ b/lib/utils.c @@ -32,11 +32,13 @@ static int charhex2bin(char c) { return toupper(c) - 'A' + 10; } - -void hex2bin(char *src, char *dst, int length) { - int i; - for(i = 0; i < length; i++) +bool hex2bin(char *src, char *dst, int length) { + for(int i = 0; i < length; i++) { + if(!isxdigit(src[i * 2]) || !isxdigit(src[i * 2 + 1])) + return false; dst[i] = charhex2bin(src[i * 2]) * 16 + charhex2bin(src[i * 2 + 1]); + } + return true; } void bin2hex(char *src, char *dst, int length) { diff --git a/lib/utils.h b/lib/utils.h index f6ff705..2e4fcf8 100644 --- a/lib/utils.h +++ b/lib/utils.h @@ -21,7 +21,7 @@ #ifndef __TINC_UTILS_H__ #define __TINC_UTILS_H__ -extern void hex2bin(char *src, char *dst, int length); +extern bool hex2bin(char *src, char *dst, int length); extern void bin2hex(char *src, char *dst, int length); #ifdef HAVE_MINGW diff --git a/src/net_setup.c b/src/net_setup.c index eec438a..a28ab7a 100644 --- a/src/net_setup.c +++ b/src/net_setup.c @@ -66,7 +66,10 @@ bool read_rsa_public_key(connection_t *c) { /* First, check for simple PublicKey statement */ if(get_config_string(lookup_config(c->config_tree, "PublicKey"), &key)) { - BN_hex2bn(&c->rsa_key->n, key); + if(BN_hex2bn(&c->rsa_key->n, key) != strlen(key)) { + logger(LOG_ERR, "Invalid PublicKey for %s!", c->name); + return false; + } BN_hex2bn(&c->rsa_key->e, "FFFF"); free(key); return true; @@ -169,8 +172,14 @@ static bool read_rsa_private_key(void) { } myself->connection->rsa_key = RSA_new(); // RSA_blinding_on(myself->connection->rsa_key, NULL); - BN_hex2bn(&myself->connection->rsa_key->d, key); - BN_hex2bn(&myself->connection->rsa_key->n, pubkey); + if(BN_hex2bn(&myself->connection->rsa_key->d, key) != strlen(key)) { + logger(LOG_ERR, "Invalid PrivateKey for myself!"); + return false; + } + if(BN_hex2bn(&myself->connection->rsa_key->n, pubkey) != strlen(pubkey)) { + logger(LOG_ERR, "Invalid PublicKey for myself!"); + return false; + } BN_hex2bn(&myself->connection->rsa_key->e, "FFFF"); free(key); free(pubkey); diff --git a/src/protocol_auth.c b/src/protocol_auth.c index 0ef5468..3bd34a0 100644 --- a/src/protocol_auth.c +++ b/src/protocol_auth.c @@ -308,7 +308,10 @@ bool metakey_h(connection_t *c) { /* Convert the challenge from hexadecimal back to binary */ - hex2bin(buffer, buffer, len); + if(!hex2bin(buffer, buffer, len)) { + logger(LOG_ERR, "Got bad %s from %s(%s): %s", "METAKEY", c->name, c->hostname, "invalid key"); + return false; + } /* Decrypt the meta key */ @@ -426,7 +429,10 @@ bool challenge_h(connection_t *c) { /* Convert the challenge from hexadecimal back to binary */ - hex2bin(buffer, c->mychallenge, len); + if(!hex2bin(buffer, c->mychallenge, len)) { + logger(LOG_ERR, "Got bad %s from %s(%s): %s", "CHALLENGE", c->name, c->hostname, "invalid challenge"); + return false; + } c->allow_request = CHAL_REPLY; @@ -480,7 +486,10 @@ bool chal_reply_h(connection_t *c) { /* Convert the hash to binary format */ - hex2bin(hishash, hishash, c->outdigest->md_size); + if(!hex2bin(hishash, hishash, c->outdigest->md_size)) { + logger(LOG_ERR, "Got bad %s from %s(%s): %s", "CHAL_REPLY", c->name, c->hostname, "invalid hash"); + return false; + } /* Calculate the hash from the challenge we sent */ diff --git a/src/protocol_key.c b/src/protocol_key.c index 1a184fb..f2f317d 100644 --- a/src/protocol_key.c +++ b/src/protocol_key.c @@ -240,10 +240,16 @@ bool ans_key_h(connection_t *c) { return send_request(to->nexthop->connection, "%s", c->buffer); } + /* Don't use key material until every check has passed. */ + from->status.validkey = false; + /* Update our copy of the origin's packet key */ from->outkey = xrealloc(from->outkey, strlen(key) / 2); from->outkeylength = strlen(key) / 2; - hex2bin(key, from->outkey, from->outkeylength); + if(!hex2bin(key, from->outkey, from->outkeylength)) { + logger(LOG_ERR, "Got bad %s from %s(%s): %s", "ANS_KEY", from->name, from->hostname, "invalid key"); + return true; + } /* Check and lookup cipher and digest algorithms */ -- 2.25.1