A bit better TSIG handling

diff --git a/daemon/worker.c b/daemon/worker.c
index ac2f347007f6de76c5258d22a3338a173cccbd16..cadf1e228bffc2c641e6f07147d56df04d3512de 100644 (file)
--- 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 1b7fe370cc4e17f2976b8eaaf2b14ff3acad10ad..ce995a8f7e3583a57e8b4043ca9f5b56f800ef6c 100644 (file)
--- 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 f08760e2f9fc581461945362e7a7d1ea77471b53..c135c9e5cb6db1121fc12e052cb77f99401a44aa 100644 (file)
--- 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 6e4cf7ea3be7639360416630b129f0281509adef..7a82425e3a6a7aaae3797a51d0e2271b50a1d9ea 100644 (file)
--- 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 58ae24e63974fe1f7bb5541a33ff5a0d8d230964..c380c79587a283fdf5aa6010ae01efb8e54eff36 100644 (file)
--- a/util/tsig.c
+++ b/util/tsig.c
@@ -40,6 +40,7 @@
 #include "util/data/msgparse.h"
 #include "util/data/dname.h"
 #include <openssl/evp.h>
+#include <openssl/hmac.h>
 
 
 /** 
@@ -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 7b96f6da1ab8c6b9e832bef2b8bf26a513e38228..87a06b41d2af529a06d368e3ed026a6059ae63c5 100644 (file)
--- 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