Hello,
we discovered our dnsmasq were using also privileged source ports when
sending queries. Interesting enough, it has right to do it, because it
has to listen also on privileged port. It never drops such privilege.
It was fixed in commit [1]. But my question is, why is there even custom
generator or random ports, when OS can do it itself? And usually far
better? So I dug a bit into it and came with patch, that would use
random ports from OS by default.
When I tested it, I got the same results when skipping bind() call on
random ports at all. Is there some reason, why dnsmasq does not follow
OS policy for source outgoing port and choses its own range by itself?
1.
http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=baf553db0cdb50707ddab464fb3eff7786ea576c
--
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com PGP: 65C6C973
From 57b19ecc3fdd1f7357ea4472bfa4fe731e28c3cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Thu, 9 Aug 2018 18:17:26 +0200
Subject: [PATCH 1/2] Use OS random ports by default
Unless max-port or min-port is given, let OS allocate random ports for
DNS queries. Randomize similar to --query-port=0, but for each query
separately. Would use port according to system policy.
---
src/dnsmasq.c | 2 +-
src/network.c | 15 ++++++++++++---
src/option.c | 4 +++-
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index 7a8e891..02fa003 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -230,7 +230,7 @@ int main (int argc, char **argv)
die(_("Ubus not available: set HAVE_UBUS in src/config.h"), NULL, EC_BADCONF);
#endif
- if (daemon->max_port < daemon->min_port)
+ if (daemon->max_port >= 0 && daemon->max_port < daemon->min_port)
die(_("max_port cannot be smaller than min_port"), NULL, EC_BADCONF);
now = dnsmasq_time();
diff --git a/src/network.c b/src/network.c
index b405458..e1b60d1 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1138,18 +1138,27 @@ int random_sock(int family)
if ((fd = socket(family, SOCK_DGRAM, 0)) != -1)
{
union mysockaddr addr;
- unsigned int ports_avail = ((unsigned short)daemon->max_port - (unsigned short)daemon->min_port) + 1;
- int tries = ports_avail < 30 ? 3 * ports_avail : 100;
+ unsigned short ports_avail = 0;
+ int tries = 100;
+ unsigned short port = 0;
memset(&addr, 0, sizeof(addr));
addr.sa.sa_family = family;
+ if (daemon->max_port >= 0)
+ {
+ ports_avail = ((unsigned short)daemon->max_port - (unsigned short)daemon->min_port) + 1;
+ if (ports_avail < 30)
+ tries = 3 * ports_avail;
+ }
+
/* don't loop forever if all ports in use. */
if (fix_fd(fd))
while(tries--)
{
- unsigned short port = htons(daemon->min_port + (rand16() % ((unsigned short)ports_avail)));
+ if (ports_avail)
+ port = htons(daemon->min_port + (rand16() % ports_avail));
if (family == AF_INET)
{
diff --git a/src/option.c b/src/option.c
index c203826..f77f5aa 100644
--- a/src/option.c
+++ b/src/option.c
@@ -2620,6 +2620,8 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
case LOPT_MINPORT: /* --min-port */
if (!atoi_check16(arg, &daemon->min_port))
ret_err(gen_err);
+ if (daemon->max_port < 0)
+ daemon->max_port = MAX_PORT;
break;
case LOPT_MAXPORT: /* --max-port */
@@ -4698,7 +4700,7 @@ void read_opts(int argc, char **argv, char *compile_opts)
daemon->soa_refresh = SOA_REFRESH;
daemon->soa_retry = SOA_RETRY;
daemon->soa_expiry = SOA_EXPIRY;
- daemon->max_port = MAX_PORT;
+ daemon->max_port = -1;
daemon->min_port = MIN_PORT;
#ifndef NO_ID
--
2.14.4
From b54e4550f8ebf3ee5aaaca41e8f7ccf278539bcb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Thu, 9 Aug 2018 20:57:07 +0200
Subject: [PATCH 2/2] Simplify random ports generator
Do not bind random port with any address at all, just leave socket
unbound. Rely on sendto() to connect it first time.
---
src/network.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/network.c b/src/network.c
index e1b60d1..71f5186 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1138,27 +1138,25 @@ int random_sock(int family)
if ((fd = socket(family, SOCK_DGRAM, 0)) != -1)
{
union mysockaddr addr;
- unsigned short ports_avail = 0;
- int tries = 100;
- unsigned short port = 0;
+ unsigned short ports_avail;
+ int tries;
+ unsigned short port;
+
+ if (!fix_fd(fd))
+ goto close_fd;
+
+ if (daemon->max_port < 0)
+ return fd;
memset(&addr, 0, sizeof(addr));
addr.sa.sa_family = family;
-
- if (daemon->max_port >= 0)
- {
- ports_avail = ((unsigned short)daemon->max_port - (unsigned short)daemon->min_port) + 1;
- if (ports_avail < 30)
- tries = 3 * ports_avail;
- }
+ ports_avail = ((unsigned short)daemon->max_port - (unsigned short)daemon->min_port) + 1;
+ tries = (ports_avail < 30) ? 3 * ports_avail : 100;
/* don't loop forever if all ports in use. */
-
- if (fix_fd(fd))
while(tries--)
{
- if (ports_avail)
- port = htons(daemon->min_port + (rand16() % ports_avail));
+ port = htons(daemon->min_port + (rand16() % ports_avail));
if (family == AF_INET)
{
@@ -1185,7 +1183,7 @@ int random_sock(int family)
if (errno != EADDRINUSE && errno != EACCES)
break;
}
-
+close_fd:
close(fd);
}
--
2.14.4
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
