From: Willem Toorop Date: Tue, 25 Mar 2025 15:46:42 +0000 (+0100) Subject: A bit better TSIG handling X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3674e4813c359ac2b597fa32d9b569794f991752;p=thirdparty%2Funbound.git A bit better TSIG handling --- diff --git a/daemon/worker.c b/daemon/worker.c index ac2f34700..cadf1e228 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -79,6 +79,7 @@ #include "respip/respip.h" #include "libunbound/context.h" #include "libunbound/libworker.h" +#include "sldns/parseutil.h" #include "sldns/sbuffer.h" #include "sldns/wire2str.h" #include "util/shm_side/shm_main.h" @@ -1520,27 +1521,33 @@ worker_handle_request(struct comm_point* c, void* arg, int error, if (check_result.value == RESPONSE_MESSAGE) { struct reply_info *rep = NULL; int r; - const char* tsig_name = "\x19" "foobar-example-dyn-update\x00"; - const size_t tsig_name_len = 27; + const char* tsig_name = "\x19""foobar-example-dyn-update\x00"; + const char* alg = "\x0b""hmac-sha256\x00"; const char* tsig_secret = "\x59\x2E\xD3\xD0\x84\xA8\x69\x5F\x8C\xCA\x07\xBE\x1B\xFC\x1E\x98\x74\xE7\xF6\x64\x30\x32\x10\xC6\x33\x09\x93\x94\x9D\xF1\x71\x74\x42\x27\xAB\xF5\x11\x59\x0D\x2E\x52\x2F\xBD\xA8\x7E\xD9\xEA\xD6\x8F\x3D\x6F\xD2\x60\x56\xD8\xD3\xCA\x02\xB7\x16\x1C\x43\x6D\xB8"; const size_t tsig_secret_len = 64; - log_err("RESPONSE received"); if (!worker_check_response(c->buffer, worker)) { - log_err("Not a suitable response to cache"); + verbose(VERB_ALGO, "Bad response"); + log_addr(VERB_CLIENT,"from",&repinfo->client_addr, repinfo->client_addrlen); comm_point_drop_reply(repinfo); return 0; } - if((r = reply_info_parse(c->buffer, worker->env.alloc, &qinfo, - &rep, worker->scratchpad, &edns))) { - log_err("bad response. Will not cache it (%d)", r); + if((r = tsig_verify(c->buffer, tsig_name, alg, tsig_secret, + tsig_secret_len, *worker->env.now))) { + verbose(VERB_ALGO, "worker tsig very of response: %s", + sldns_lookup_by_id(sldns_tsig_errors, r)? + sldns_lookup_by_id(sldns_tsig_errors, r)->name:"??"); + log_addr(VERB_CLIENT,"from",&repinfo->client_addr, repinfo->client_addrlen); comm_point_drop_reply(repinfo); return 0; } - if((r = tsig_verify(c->buffer, tsig_name, tsig_name_len, - tsig_secret, tsig_secret_len))) { - log_err("response had wrong TSIG. Will not cache it (%d)", r); + if((r = reply_info_parse(c->buffer, worker->env.alloc, &qinfo, + &rep, worker->scratchpad, &edns))) { + verbose(VERB_ALGO, "worker parse response: %s", + sldns_lookup_by_id(sldns_rcodes, r)? + sldns_lookup_by_id(sldns_rcodes, r)->name:"??"); + log_addr(VERB_CLIENT,"from",&repinfo->client_addr, repinfo->client_addrlen); comm_point_drop_reply(repinfo); return 0; } diff --git a/sldns/sbuffer.h b/sldns/sbuffer.h index 1b7fe370c..ce995a8f7 100644 --- a/sldns/sbuffer.h +++ b/sldns/sbuffer.h @@ -56,6 +56,18 @@ sldns_read_uint32(const void *src) #endif } +INLINE uint64_t +sldns_read_uint48(const void *src) +{ + const uint8_t *p = (const uint8_t *) src; + return ( ((uint64_t) p[0] << 40) + | ((uint64_t) p[1] << 32) + | ((uint64_t) p[2] << 24) + | ((uint64_t) p[3] << 16) + | ((uint64_t) p[4] << 8) + | (uint64_t) p[5]); +} + /* * Copy data allowing for unaligned accesses in network byte order * (big endian). @@ -693,6 +705,32 @@ sldns_buffer_read_u32(sldns_buffer *buffer) return result; } +/** + * returns the 6-byte integer value at the given position in the buffer + * \param[in] buffer the buffer + * \param[in] at position in the buffer + * \return 6 byte integer + */ +INLINE uint64_t +sldns_buffer_read_u48_at(sldns_buffer *buffer, size_t at) +{ + assert(sldns_buffer_available_at(buffer, at, 6)); + return sldns_read_uint48(buffer->_data + at); +} + +/** + * returns the 6-byte integer value at the current position in the buffer + * \param[in] buffer the buffer + * \return 6 byte integer + */ +INLINE uint64_t +sldns_buffer_read_u48(sldns_buffer *buffer) +{ + uint64_t result = sldns_buffer_read_u48_at(buffer, buffer->_position); + buffer->_position += 6; + return result; +} + /** * returns the status of the buffer * \param[in] buffer diff --git a/util/data/dname.c b/util/data/dname.c index f08760e2f..c135c9e5c 100644 --- a/util/data/dname.c +++ b/util/data/dname.c @@ -97,7 +97,7 @@ dname_valid(uint8_t* dname, size_t maxlen) /** compare uncompressed, noncanonical, registers are hints for speed */ int -query_dname_compare(register uint8_t* d1, register uint8_t* d2) +query_dname_compare(const register uint8_t* d1, const register uint8_t* d2) { register uint8_t lab1, lab2; log_assert(d1 && d2); diff --git a/util/data/dname.h b/util/data/dname.h index 6e4cf7ea3..7a82425e3 100644 --- a/util/data/dname.h +++ b/util/data/dname.h @@ -96,7 +96,7 @@ void pkt_dname_tolower(struct sldns_buffer* pkt, uint8_t* dname); * @return: -1, 0, or +1 depending on comparison results. * Sort order is first difference found. not the canonical ordering. */ -int query_dname_compare(uint8_t* d1, uint8_t* d2); +int query_dname_compare(const uint8_t* d1, const uint8_t* d2); /** * Determine correct, compressed, dname present in packet. diff --git a/util/tsig.c b/util/tsig.c index 58ae24e63..c380c7958 100644 --- a/util/tsig.c +++ b/util/tsig.c @@ -40,6 +40,7 @@ #include "util/data/msgparse.h" #include "util/data/dname.h" #include +#include /** @@ -47,24 +48,27 @@ * out 0 on success, an error code otherwise. */ int -tsig_verify(sldns_buffer* pkt, const uint8_t* name, size_t name_len, - const uint8_t* secret, size_t secret_len) +tsig_verify(sldns_buffer* pkt, const uint8_t* name, const uint8_t* alg, + const uint8_t* secret, size_t secret_len, uint64_t now) { size_t n_rrs; + size_t end_of_message; uint16_t rdlength; uint8_t mac[1024]; - uint16_t mac_size; + uint16_t mac_sz; uint8_t hmac_result[1024]; - size_t hmac_result_len; + unsigned int hmac_result_len; size_t pos; uint16_t other_len; size_t algname_size; + uint64_t time_signed; + uint16_t fudge; const EVP_MD* digester; assert(LDNS_QDCOUNT(sldns_buffer_begin(pkt)) == 1); if(LDNS_ARCOUNT(sldns_buffer_begin(pkt)) < 1) { - log_err("No TSIG found (not in ARCOUNT)"); + log_err("No TSIG found (ARCOUNT == 0)"); return -1; } LDNS_ARCOUNT_SET( sldns_buffer_begin(pkt) @@ -75,57 +79,66 @@ tsig_verify(sldns_buffer* pkt, const uint8_t* name, size_t name_len, sldns_buffer_rewind(pkt); sldns_buffer_skip(pkt, LDNS_HEADER_SIZE); - pkt_dname_len(pkt); /* skip qname */ - sldns_buffer_skip(pkt, 2 * sizeof(uint16_t)); - if(sldns_buffer_remaining(pkt) < sizeof(uint16_t)) + pkt_dname_len(pkt); /* skip qname */ + sldns_buffer_skip(pkt, 2 * sizeof(uint16_t)); /* skip type and class */ + if(!skip_pkt_rrs(pkt, n_rrs)) /* skip all rrs */ return -1; - if(!skip_pkt_rrs(pkt, n_rrs)) - return -1; - pkt_dname_len(pkt); /* skip TSIG name (TODO, compare with name) */ - pos = sldns_buffer_position(pkt); /* Append pos */ + end_of_message = sldns_buffer_position(pkt); + if(query_dname_compare(name, sldns_buffer_current(pkt))) + return LDNS_TSIG_ERROR_BADKEY; + pkt_dname_len(pkt); /* skip TSIG name */ + pos = sldns_buffer_position(pkt); /* Append pos */ if(sldns_buffer_read_u16(pkt) != LDNS_RR_TYPE_TSIG) { log_err("No TSIG found!"); return -1; } - sldns_buffer_skip(pkt, sizeof(uint16_t) + sizeof(uint32_t)); /* skip class + TTL */ - rdlength = sldns_buffer_read_u16(pkt); - algname_size = pkt_dname_len(pkt); - sldns_buffer_skip(pkt, 8); /* skip time signed and fudge */ - mac_size = sldns_buffer_read_u16(pkt); - memcpy(mac, sldns_buffer_current(pkt), mac_size); - sldns_buffer_skip(pkt, mac_size); + sldns_buffer_skip(pkt, sizeof(uint16_t) /* skip class */ + + sizeof(uint32_t) /* skip TTLS */ + + sizeof(uint16_t)); /* skip rdlength */ + if(query_dname_compare(alg, sldns_buffer_current(pkt))) + return LDNS_TSIG_ERROR_BADKEY; + algname_size = pkt_dname_len(pkt); /* skip alg name */ + time_signed = sldns_buffer_read_u48(pkt); /* time signed */ + fudge = sldns_buffer_read_u16(pkt); /* fudge */ + mac_sz = sldns_buffer_read_u16(pkt); /* mac size */ + memcpy(mac,sldns_buffer_current(pkt),mac_sz); /* copy mac */ + sldns_buffer_skip(pkt, mac_sz); /* skip mac */ /* Set ID to original ID */ sldns_buffer_write_u16_at(pkt, 0, sldns_buffer_read_u16(pkt)); - sldns_buffer_skip(pkt, sizeof(uint16_t)); /* error */ - other_len = sldns_buffer_read_u16(pkt); + sldns_buffer_skip(pkt, sizeof(uint16_t)); /* skip error */ + other_len = sldns_buffer_read_u16(pkt); /* other len */ - /* move CLASS and TTL -> TYPE position */ + /* move CLASS (uint16_t) and TTL (uint32_t) -> TYPE position */ memmove( sldns_buffer_at(pkt, pos) - , sldns_buffer_at(pkt, pos + 2) + , sldns_buffer_at(pkt, pos + sizeof(uint16_t) /* type */) , sizeof(uint16_t) + sizeof(uint32_t)); pos += sizeof(uint16_t) + sizeof(uint32_t); - /* Append Algorithm Name, Time signed, Fudge */ + /* Append Algorithm Name, Time signed (uint48_t), Fudge (uint16_t) */ memmove( sldns_buffer_at(pkt, pos) - , sldns_buffer_at(pkt, pos + 4) + , sldns_buffer_at(pkt, pos + sizeof(uint16_t) /* type */ + + sizeof(uint16_t) /* rdlength */) , algname_size + 6 /* sizeof(uint48_t) */ + sizeof(uint16_t)); pos += algname_size + 6 /* sizeof(uint48_t) */ + sizeof(uint16_t); - /* Append Error, Other Len, Other Data */ + /* Append Error (uint16_t), Other Len (uint16_t), Other Data */ memmove( sldns_buffer_at(pkt, pos) - , sldns_buffer_at(pkt, sldns_buffer_position(pkt) - 4) - , 4 + other_len); - pos += 4 + other_len; + , sldns_buffer_at(pkt, sldns_buffer_position(pkt) + - 2 * sizeof(uint16_t)) + , 2 * sizeof(uint16_t) + other_len); + pos += 2 * sizeof(uint16_t) + other_len; digester = EVP_sha256(); hmac_result_len = sizeof(hmac_result); HMAC( digester, secret, secret_len, sldns_buffer_begin(pkt), pos - , &hmac_result, &hmac_result_len); + , hmac_result, &hmac_result_len); if(memcmp(mac, hmac_result, hmac_result_len) == 0) { - log_err("Correct TSIG found (YAY!), MAC size: %d", mac_size); + return now > time_signed ? + ( time_signed - now > fudge ? LDNS_TSIG_ERROR_BADTIME : 0 ) + : now - time_signed > fudge ? LDNS_TSIG_ERROR_BADTIME : 0 ; + sldns_buffer_set_position(pkt, end_of_message); return 0; } - log_err("TSIG verification failed!"); - return -1; + return LDNS_TSIG_ERROR_BADSIG; } diff --git a/util/tsig.h b/util/tsig.h index 7b96f6da1..87a06b41d 100644 --- a/util/tsig.h +++ b/util/tsig.h @@ -48,7 +48,7 @@ * Verify pkt with the name (domain name), algorithm and key. * out 0 on success, an error code otherwise. */ -int tsig_verify(sldns_buffer* pkt, const uint8_t* name, size_t name_len, - const uint8_t* secret, size_t secret_len); +int tsig_verify(sldns_buffer* pkt, const uint8_t* name, const uint8_t* alg, + const uint8_t* secret, size_t secret_len, uint64_t now); #endif