[Dnsmasq-discuss] Patch to cache SRV records - updated version.
Jeremy Allison
jra at google.com
Thu Dec 20 20:20:41 GMT 2018
On Thu, 20 Dec 2018 11:53:11 -0800
Jeremy Allison <jra at google.com> wrote:
> I know dnsmasq doesn't cache SRV records by design:
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=579536;msg=9
>
> However, when used with Samba code (winbindd) and
> other Windows-integrating technology (MIT krb5)
> there are a lot of SRV record queries that make
> the whole integration stack slow if SRV records
> are not cached.
>
> With that in mind, here is a patch to cache
> SRV records positively and negatively inside
> dnsmasq.
>
> I'm sending here it so that people who might need
> this functionality have a central place to find
> it (it might end up being used in ChromeOS, depending
> on test results / review).
Sigh. Found a bug when testing. free_mx_srv_record()
wasn't checking for NULL pointers on free(),
which can be the case for negative cache
records.
Updated version attached.
Jeremy.
-------------- next part --------------
From d9758e1b340ceb7a46c965a53dc040193d80956d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at google.com>
Date: Tue, 18 Dec 2018 10:44:08 -0800
Subject: [PATCH 1/6] Widen 'flags' field in cache to unsigned int from
unsigned short.
Ensure we still only cache unsigned short bits (0xFFFF).
Signed-off-by: Jeremy Allison <jra at google.com>
---
src/cache.c | 23 ++++++++++++++---------
src/dnsmasq.h | 6 ++++--
src/rfc1035.c | 2 +-
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index 8662f54..cd72a0c 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -27,7 +27,7 @@ static int bignames_left, hash_size;
static void make_non_terminals(struct crec *source);
static struct crec *really_insert(char *name, struct all_addr *addr,
- time_t now, unsigned long ttl, unsigned short flags);
+ time_t now, unsigned long ttl, unsigned int flags);
/* type->string mapping: this is also used by the name-hash function as a mixing table. */
static const struct {
@@ -330,7 +330,7 @@ static int is_expired(time_t now, struct crec *crecp)
return 1;
}
-static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t now, unsigned short flags,
+static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t now, unsigned int flags,
struct crec **target_crec, unsigned int *target_uid)
{
/* Scan and remove old entries.
@@ -465,7 +465,7 @@ void cache_start_insert(void)
}
struct crec *cache_insert(char *name, struct all_addr *addr,
- time_t now, unsigned long ttl, unsigned short flags)
+ time_t now, unsigned long ttl, unsigned int flags)
{
/* Don't log DNSSEC records here, done elsewhere */
if (flags & (F_IPV4 | F_IPV6 | F_CNAME))
@@ -483,7 +483,7 @@ struct crec *cache_insert(char *name, struct all_addr *addr,
static struct crec *really_insert(char *name, struct all_addr *addr,
- time_t now, unsigned long ttl, unsigned short flags)
+ time_t now, unsigned long ttl, unsigned int flags)
{
struct crec *new, *target_crec = NULL;
union bigname *big_name = NULL;
@@ -495,6 +495,11 @@ static struct crec *really_insert(char *name, struct all_addr *addr,
if (insert_error)
return NULL;
+ /*
+ * Ensure we only cache unsigned short bits.
+ */
+ flags &= DNSMASQ_CACHE_MASK;
+
/* First remove any expired entries and entries for the name/address we
are currently inserting. */
if ((new = cache_scan_free(name, addr, now, flags, &target_crec, &target_uid)))
@@ -656,7 +661,7 @@ void cache_end_insert(void)
{
char *name = cache_get_name(new_chain);
ssize_t m = strlen(name);
- unsigned short flags = new_chain->flags;
+ unsigned int flags = new_chain->flags;
#ifdef HAVE_DNSSEC
u16 class = new_chain->uid;
#endif
@@ -717,7 +722,7 @@ int cache_recv_insert(time_t now, int fd)
struct all_addr addr;
unsigned long ttl;
time_t ttd;
- unsigned short flags;
+ unsigned int flags;
struct crec *crecp = NULL;
cache_start_insert();
@@ -856,7 +861,7 @@ struct crec *cache_find_by_name(struct crec *crecp, char *name, time_t now, unsi
/* first search, look for relevant entries and push to top of list
also free anything which has expired */
struct crec *next, **up, **insert = NULL, **chainp = &ans;
- unsigned short ins_flags = 0;
+ unsigned int ins_flags = 0;
for (up = hash_bucket(name), crecp = *up; crecp; crecp = next)
{
@@ -1140,7 +1145,7 @@ int read_hostsfile(char *filename, unsigned int index, int cache_size, struct cr
FILE *f = fopen(filename, "r");
char *token = daemon->namebuff, *domain_suffix = NULL;
int addr_count = 0, name_count = cache_size, lineno = 0;
- unsigned short flags = 0;
+ unsigned int flags = 0;
struct all_addr addr;
int atnl, addrlen = 0;
@@ -1435,7 +1440,7 @@ void cache_add_dhcp_entry(char *host_name, int prot,
struct all_addr *host_address, time_t ttd)
{
struct crec *crec = NULL, *fail_crec = NULL;
- unsigned short flags = F_IPV4;
+ unsigned int flags = F_IPV4;
int in_hosts = 0;
size_t addrlen = sizeof(struct in_addr);
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index f8803f5..43f1974 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -433,7 +433,7 @@ struct crec {
time_t ttd; /* time to die */
/* used as class if DNSKEY/DS, index to source for F_HOSTS */
unsigned int uid;
- unsigned short flags;
+ unsigned int flags;
union {
char sname[SMALLDNAME];
union bigname *bname;
@@ -461,6 +461,8 @@ struct crec {
#define F_DS (1u<<14)
#define F_DNSSECOK (1u<<15)
+#define DNSMASQ_CACHE_MASK (0xFFFF)
+
/* below here are only valid as args to log_query: cache
entries are limited to 16 bits */
#define F_UPSTREAM (1u<<16)
@@ -1149,7 +1151,7 @@ void cache_end_insert(void);
void cache_start_insert(void);
int cache_recv_insert(time_t now, int fd);
struct crec *cache_insert(char *name, struct all_addr *addr,
- time_t now, unsigned long ttl, unsigned short flags);
+ time_t now, unsigned long ttl, unsigned int flags);
void cache_reload(void);
void cache_add_dhcp_entry(char *host_name, int prot, struct all_addr *host_address, time_t ttd);
struct in_addr a_record_from_hosts(char *name, time_t now);
diff --git a/src/rfc1035.c b/src/rfc1035.c
index 19e2a8b..dd7e085 100644
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -1272,7 +1272,7 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
unsigned int qtype, qclass;
struct all_addr addr;
int nameoffset;
- unsigned short flag;
+ unsigned int flag;
int q, ans, anscount = 0, addncount = 0;
int dryrun = 0;
struct crec *crecp;
--
2.20.1.415.g653613c723-goog
From f79997c20ccfb6459948e8b752182963c336fe4a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at google.com>
Date: Tue, 18 Dec 2018 10:52:26 -0800
Subject: [PATCH 2/6] Change cache_XXX() interface to take a 'void *' parameter
instead of 'struct all_addr *'.
This allows us to pass in a different type when caching SRV records later.
Signed-off-by: Jeremy Allison <jra at google.com>
---
src/cache.c | 37 +++++++++++++++++++++++--------------
src/dnsmasq.h | 2 +-
2 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index cd72a0c..32fc1ec 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -26,7 +26,7 @@ static union bigname *big_free = NULL;
static int bignames_left, hash_size;
static void make_non_terminals(struct crec *source);
-static struct crec *really_insert(char *name, struct all_addr *addr,
+static struct crec *really_insert(char *name, void *vaddr,
time_t now, unsigned long ttl, unsigned int flags);
/* type->string mapping: this is also used by the name-hash function as a mixing table. */
@@ -330,7 +330,7 @@ static int is_expired(time_t now, struct crec *crecp)
return 1;
}
-static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t now, unsigned int flags,
+static struct crec *cache_scan_free(char *name, void *vaddr, time_t now, unsigned int flags,
struct crec **target_crec, unsigned int *target_uid)
{
/* Scan and remove old entries.
@@ -381,14 +381,19 @@ static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t no
#ifdef HAVE_DNSSEC
/* Deletion has to be class-sensitive for DS and DNSKEY */
- if ((flags & crecp->flags & (F_DNSKEY | F_DS)) && crecp->uid == addr->addr.dnssec.class)
+ if (flags & crecp->flags & (F_DNSKEY | F_DS))
{
- if (crecp->flags & F_CONFIG)
- return crecp;
- *up = crecp->hash_next;
- cache_unlink(crecp);
- cache_free(crecp);
- continue;
+ struct all_addr *addr = (struct all_addr *)vaddr;
+ /* Deletion has to be class-sensitive for DS and DNSKEY */
+ if (crecp->uid == addr->addr.dnssec.class)
+ {
+ if (crecp->flags & F_CONFIG)
+ return crecp;
+ *up = crecp->hash_next;
+ cache_unlink(crecp);
+ cache_free(crecp);
+ continue;
+ }
}
#endif
}
@@ -428,7 +433,7 @@ static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t no
else if (!(crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)) &&
(flags & crecp->flags & F_REVERSE) &&
(flags & crecp->flags & (F_IPV4 | F_IPV6)) &&
- memcmp(&crecp->addr.addr, addr, addrlen) == 0)
+ memcmp(&crecp->addr.addr, vaddr, addrlen) == 0)
{
*up = crecp->hash_next;
cache_unlink(crecp);
@@ -464,12 +469,13 @@ void cache_start_insert(void)
insert_error = 0;
}
-struct crec *cache_insert(char *name, struct all_addr *addr,
+struct crec *cache_insert(char *name, void *vaddr,
time_t now, unsigned long ttl, unsigned int flags)
{
/* Don't log DNSSEC records here, done elsewhere */
if (flags & (F_IPV4 | F_IPV6 | F_CNAME))
{
+ struct all_addr *addr = (struct all_addr *)vaddr;
log_query(flags | F_UPSTREAM, name, addr, NULL);
/* Don't mess with TTL for DNSSEC records. */
if (daemon->max_cache_ttl != 0 && daemon->max_cache_ttl < ttl)
@@ -478,13 +484,14 @@ struct crec *cache_insert(char *name, struct all_addr *addr,
ttl = daemon->min_cache_ttl;
}
- return really_insert(name, addr, now, ttl, flags);
+ return really_insert(name, vaddr, now, ttl, flags);
}
-static struct crec *really_insert(char *name, struct all_addr *addr,
+static struct crec *really_insert(char *name, void *vaddr,
time_t now, unsigned long ttl, unsigned int flags)
{
+ struct all_addr *addr = NULL;
struct crec *new, *target_crec = NULL;
union bigname *big_name = NULL;
int freed_all = flags & F_REVERSE;
@@ -500,9 +507,11 @@ static struct crec *really_insert(char *name, struct all_addr *addr,
*/
flags &= DNSMASQ_CACHE_MASK;
+ addr = (struct all_addr *)vaddr;
+
/* First remove any expired entries and entries for the name/address we
are currently inserting. */
- if ((new = cache_scan_free(name, addr, now, flags, &target_crec, &target_uid)))
+ if ((new = cache_scan_free(name, vaddr, now, flags, &target_crec, &target_uid)))
{
/* We're trying to insert a record over one from
/etc/hosts or DHCP, or other config. If the
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 43f1974..172af2e 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1150,7 +1150,7 @@ struct crec *cache_find_by_name(struct crec *crecp,
void cache_end_insert(void);
void cache_start_insert(void);
int cache_recv_insert(time_t now, int fd);
-struct crec *cache_insert(char *name, struct all_addr *addr,
+struct crec *cache_insert(char *name, void *vaddr,
time_t now, unsigned long ttl, unsigned int flags);
void cache_reload(void);
void cache_add_dhcp_entry(char *host_name, int prot, struct all_addr *host_address, time_t ttd);
--
2.20.1.415.g653613c723-goog
From 8e3e2f3ec8fb23c623fd8bbd704746e33ceb00bf Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at google.com>
Date: Tue, 18 Dec 2018 11:04:07 -0800
Subject: [PATCH 3/6] Add utility function free_mx_srv_record().
Not yet used.
Signed-off-by: Jeremy Allison <jra at google.com>
---
src/dnsmasq.h | 1 +
src/util.c | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 172af2e..78b9850 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1279,6 +1279,7 @@ int read_write(int fd, unsigned char *packet, int size, int rw);
int wildcard_match(const char* wildcard, const char* match);
int wildcard_matchn(const char* wildcard, const char* match, int num);
+void free_mx_srv_record(struct mx_srv_record* rec);
/* log.c */
void die(char *message, char *arg1, int exit_code) ATTRIBUTE_NORETURN;
diff --git a/src/util.c b/src/util.c
index 079fe44..850b4cd 100644
--- a/src/util.c
+++ b/src/util.c
@@ -741,3 +741,11 @@ int wildcard_matchn(const char* wildcard, const char* match, int num)
return (!num) || (*wildcard == *match);
}
+
+void free_mx_srv_record(struct mx_srv_record* rec)
+{
+ if (rec->name != NULL)
+ free(rec->name);
+ if (rec->target != NULL)
+ free(rec->target);
+}
--
2.20.1.415.g653613c723-goog
From 881c0fbb35c4a0a2812f15f742b2522120157e84 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at google.com>
Date: Tue, 18 Dec 2018 11:12:09 -0800
Subject: [PATCH 4/6] Add the ability to cache SRV records.
Not yet used.
Signed-off-by: Jeremy Allison <jra at google.com>
---
src/cache.c | 16 +++++++++++++---
src/dnsmasq.h | 8 +++++---
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/cache.c b/src/cache.c
index 32fc1ec..71e3cfd 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -230,6 +230,9 @@ static void cache_free(struct crec *crecp)
crecp->flags &= ~F_BIGNAME;
}
+ if (crecp->flags & F_SRV)
+ free_mx_srv_record(&crecp->addr.srv);
+
#ifdef HAVE_DNSSEC
cache_blockdata_free(crecp);
#endif
@@ -492,6 +495,7 @@ static struct crec *really_insert(char *name, void *vaddr,
time_t now, unsigned long ttl, unsigned int flags)
{
struct all_addr *addr = NULL;
+ struct mx_srv_record *srv = NULL;
struct crec *new, *target_crec = NULL;
union bigname *big_name = NULL;
int freed_all = flags & F_REVERSE;
@@ -503,11 +507,15 @@ static struct crec *really_insert(char *name, void *vaddr,
return NULL;
/*
- * Ensure we only cache unsigned short bits.
+ * Ensure we only cache unsigned short bits
+ * plus F_SRV.
*/
flags &= DNSMASQ_CACHE_MASK;
- addr = (struct all_addr *)vaddr;
+ if (flags & F_SRV)
+ srv = (struct mx_srv_record *)vaddr;
+ else
+ addr = (struct all_addr *)vaddr;
/* First remove any expired entries and entries for the name/address we
are currently inserting. */
@@ -629,7 +637,9 @@ static struct crec *really_insert(char *name, void *vaddr,
else
*cache_get_name(new) = 0;
- if (addr)
+ if (srv)
+ new->addr.srv = *srv;
+ else if (addr)
{
#ifdef HAVE_DNSSEC
if (flags & (F_DS | F_DNSKEY))
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 78b9850..798be50 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -429,6 +429,7 @@ struct crec {
unsigned char algo;
unsigned char digest;
} ds;
+ struct mx_srv_record srv; /* For SRV record caching. */
} addr;
time_t ttd; /* time to die */
/* used as class if DNSKEY/DS, index to source for F_HOSTS */
@@ -461,10 +462,11 @@ struct crec {
#define F_DS (1u<<14)
#define F_DNSSECOK (1u<<15)
-#define DNSMASQ_CACHE_MASK (0xFFFF)
+#define F_SRV (1u<<30)
-/* below here are only valid as args to log_query: cache
- entries are limited to 16 bits */
+#define DNSMASQ_CACHE_MASK (0xFFFF|F_SRV)
+
+/* below here are only valid as args to log_query */
#define F_UPSTREAM (1u<<16)
#define F_RRNAME (1u<<17)
#define F_SERVER (1u<<18)
--
2.20.1.415.g653613c723-goog
From 6646374b9e6e8303aa0092cf901042a06fb0dc3a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at google.com>
Date: Tue, 18 Dec 2018 11:19:53 -0800
Subject: [PATCH 5/6] Add positive and negative SRV record return insertion
into cache.
Not yet looked up.
Signed-off-by: Jeremy Allison <jra at google.com>
---
src/rfc1035.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/src/rfc1035.c b/src/rfc1035.c
index dd7e085..e5a7f48 100644
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -722,6 +722,87 @@ int extract_addresses(struct dns_header *header, size_t qlen, char *name, time_t
cache_insert(NULL, &addr, now, ttl, name_encoding | F_REVERSE | F_NEG | flags | (secure ? F_DNSSECOK : 0));
}
}
+ else if (qtype == T_SRV)
+ {
+ /* SRV records */
+ if (!(flags & F_NXDOMAIN))
+ {
+ if (!(p1 = skip_questions(header, qlen)))
+ return 0;
+
+ for (j = 0; j < ntohs(header->ancount); j++)
+ {
+ /*
+ * Get the name from the answer. This is
+ * the name we'll use as the name to cache.
+ */
+ if (!extract_name(header, qlen, &p1, name, 1, 0))
+ return 0;
+
+ GETSHORT(aqtype, p1);
+ GETSHORT(aqclass, p1);
+ GETLONG(attl, p1);
+
+ if ((daemon->max_ttl != 0) && (attl > daemon->max_ttl) && !is_sign)
+ {
+ (p1) -= 4;
+ PUTLONG(daemon->max_ttl, p1);
+ }
+
+ GETSHORT(ardlen, p1);
+ endrr = p1+ardlen;
+
+ if (aqclass == C_IN && aqtype == T_SRV)
+ {
+ struct mx_srv_record srv;
+
+ GETSHORT(srv.priority, p1);
+ GETSHORT(srv.weight, p1);
+ GETSHORT(srv.srvport, p1);
+
+ srv.name = strdup(name);
+ if (srv.name == NULL)
+ return 0;
+
+ /*
+ * Now get the target name to cache.
+ */
+ if (!extract_name(header, qlen, &p1, name, 1, 0))
+ {
+ free_mx_srv_record(&srv);
+ return 0;
+ }
+ srv.target = strdup(name);
+ if (srv.target == NULL)
+ {
+ free_mx_srv_record(&srv);
+ return 0;
+ }
+
+ srv.offset = 0;
+ srv.next = NULL;
+ srv.issrv = 1;
+ cache_insert(srv.name, &srv, now, attl, flags | F_SRV | F_FORWARD);
+ }
+
+ p1 = endrr;
+ if (!CHECK_LEN(header, p1, qlen, 0))
+ return 0; /* bad packet */
+ }
+ }
+
+ /* Negative SRV caching. */
+ if ((flags & F_NXDOMAIN) && !option_bool(OPT_NO_NEG))
+ {
+ if (!searched_soa)
+ {
+ searched_soa = 1;
+ ttl = find_soa(header, qlen, NULL, doctored);
+ }
+ if (ttl)
+ cache_insert(name, NULL, now, ttl, F_SRV | F_FORWARD | F_NEG | flags | (secure ? F_DNSSECOK : 0));
+ }
+ }
else
{
/* everything other than PTR */
--
2.20.1.415.g653613c723-goog
From 9002b6d6f2523eac0822a00687dc35b7e8cbc6fd Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at google.com>
Date: Tue, 18 Dec 2018 11:31:51 -0800
Subject: [PATCH 6/6] Add looking up of positive and negative SRV records from
the cache.
Signed-off-by: Jeremy Allison <jra at google.com>
---
src/rfc1035.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/src/rfc1035.c b/src/rfc1035.c
index e5a7f48..26fa571 100644
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -1918,6 +1918,62 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
*up = move;
move->next = NULL;
}
+
+ if (!found)
+ {
+ /* Look for SRV records in the cache. */
+ crecp = NULL;
+ while (1)
+ {
+ struct mx_srv_record *srv = NULL;
+
+ crecp = cache_find_by_name(crecp,
+ name,
+ now,
+ flag | F_SRV);
+ if (!crecp)
+ break;
+
+ if (crecp->flags & F_NEG)
+ {
+ /* Found a negative SRV cache entry. */
+ ans = 1;
+ auth = 0;
+ found = 1;
+ if (crecp->flags & F_NXDOMAIN)
+ nxdomain = 1;
+ if (!dryrun)
+ log_query(crecp->flags, name, NULL, NULL);
+ break;
+ }
+
+ srv = &crecp->addr.srv;
+
+ if (add_resource_record(header,
+ limit,
+ &trunc,
+ nameoffset,
+ &ansp,
+ crec_ttl(crecp, now),
+ &nameoffset,
+ T_SRV,
+ C_IN,
+ "sssd",
+ srv->priority,
+ srv->weight,
+ srv->srvport,
+ srv->target))
+ {
+ ans = 1;
+ anscount++;
+ found = 1;
+ log_query((crecp->flags & ~F_FORWARD) | F_RRNAME,
+ cache_get_name(crecp),
+ NULL,
+ srv->target);
+ }
+ }
+ }
if (!found && option_bool(OPT_FILTER) && (qtype == T_SRV || (qtype == T_ANY && strchr(name, '_'))))
{
--
2.20.1.415.g653613c723-goog
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20181220/f15ae694/attachment-0001.sig>
More information about the Dnsmasq-discuss
mailing list