Re: [Dnsmasq-discuss] [PATCH v4] Connection track mark based DNS query filtering.
On 18.02.21, 22:10, "Dnsmasq-discuss on behalf of Geert Stappers" wrote: > On Thu, Feb 18, 2021 at 12:11:55AM +0100, Etan Kissling wrote: > > On 17.02.21, 23:41, Geert Stappers" wrote: > > > > +#if defined(HAVE_CONNTRACK) && defined(HAVE_UBUS) > > > > > > One of many > > > > Sorry, I don't understand the comment for these. > > The sorry should come from me. I was way too short with text. > Thing I trying to tell is that the wish is to avoid #if > conditionals. Reason I remember is reducing the amount > of different binaries. But HAVE_CONNTRACK is already > present in current source. No new binaries are created with this patch. The existing HAVE_CONNTRACK symbol is used to guard accessing Netfilter connection track marks. The existing HAVE_UBUS symbol is used to guard OpenWrt specific code. Furthermore, even when those symbols are defined, all new code only activates when the configuration option to enable the feature is set: if (option_bool(OPT_CMARK_ALST_EN)) Thanks Etan ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
[Dnsmasq-discuss] [PATCH v4] Connection track mark based DNS query filtering.
This extends query filtering support beyond what is currently possible
with the `--ipset` configuration option, by adding support for:
1) Specifying allowlists on a per-client basis, based on their
associated Linux connection track mark.
2) Dynamic configuration of allowlists via Ubus.
3) Reporting when a DNS query resolves or is rejected via Ubus.
4) DNS name patterns containing wildcards.
Disallowed queries are not forwarded; they are rejected
with a REFUSED error code.
Signed-off-by: Etan Kissling
---
v2: Rebase to v2.83, and fix compilation when HAVE_UBUS not present.
v3: Rebase to v2.84test2.
v4: Rebase to v2.84rc2 (update copyright notice).
Makefile | 2 +-
man/dnsmasq.8 | 31 +++-
src/dnsmasq.h | 25 +++-
src/forward.c | 121 +++-
src/option.c | 134 ++
src/pattern.c | 386 ++
src/rfc1035.c | 82 +++
src/ubus.c| 182
8 files changed, 955 insertions(+), 8 deletions(-)
create mode 100644 src/pattern.c
diff --git a/Makefile b/Makefile
index e4c3f5c..506e56b 100644
--- a/Makefile
+++ b/Makefile
@@ -79,7 +79,7 @@ copts_conf = .copts_$(sum)
objs = cache.o rfc1035.o util.o option.o forward.o network.o \
dnsmasq.o dhcp.o lease.o rfc2131.o netlink.o dbus.o bpf.o \
helper.o tftp.o log.o conntrack.o dhcp6.o rfc3315.o \
- dhcp-common.o outpacket.o radv.o slaac.o auth.o ipset.o \
+ dhcp-common.o outpacket.o radv.o slaac.o auth.o ipset.o pattern.o \
domain.o dnssec.o blockdata.o tables.o loop.o inotify.o \
poll.o rrfilter.o edns0.o arp.o crypto.o dump.o ubus.o \
metrics.o hash_questions.o
diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
index ac7c9fa..04d666d 100644
--- a/man/dnsmasq.8
+++ b/man/dnsmasq.8
@@ -368,7 +368,10 @@ provides service at that name, rather than the default
which is
.TP
.B --enable-ubus[=]
Enable dnsmasq UBus interface. It sends notifications via UBus on
-DHCPACK and DHCPRELEASE events. Furthermore it offers metrics.
+DHCPACK and DHCPRELEASE events. Furthermore it offers metrics
+and allows configuration of Linux connection track mark based filtering.
+When DNS query filtering based on Linux connection track marks is enabled
+UBus notifications are generated for each resolved or filtered DNS query.
Requires that dnsmasq has been built with UBus support. If the service
name is given, dnsmasq provides service at that namespace, rather than
the default which is
@@ -533,6 +536,32 @@ These IP sets must already exist. See
.BR ipset (8)
for more details.
.TP
+.B --connmark-allowlist-enable[=]
+Enables filtering of incoming DNS queries with associated Linux connection
track marks
+according to individual allowlists configured via a series of
\fB--connmark-allowlist\fP
+options. Disallowed queries are not forwarded; they are rejected with a
REFUSED error code.
+DNS queries are only allowed if they do not have an associated Linux connection
+track mark, or if the queried domains match the configured DNS patterns for the
+associated Linux connection track mark. If no allowlist is configured for a
+Linux connection track mark, all DNS queries associated with that mark are
rejected.
+If a mask is specified, Linux connection track marks are first bitwise ANDed
+with the given mask before being processed.
+.TP
+.B --connmark-allowlist=[/][,[/...]]
+Configures the DNS patterns that are allowed in DNS queries associated with
+the given Linux connection track mark.
+If a mask is specified, Linux connection track marks are first bitwise ANDed
+with the given mask before they are compared to the given connection track
mark.
+Patterns follow the syntax of DNS names, but additionally allow the wildcard
+character "*" to be used up to twice per label to match 0 or more characters
+within that label. Note that the wildcard never matches a dot (e.g.,
"*.example.com"
+matches "api.example.com" but not "api.us.example.com"). Patterns must be
+fully qualified, i.e., consist of at least two labels. The final label must
not be
+fully numeric, and must not be the "local" pseudo-TLD. A pattern must end with
at least
+two literal (non-wildcard) labels.
+Instead of a pattern, "*" can be specified to disable allowlist filtering
+for a given Linux connection track mark entirely.
+.TP
.B \-m, --mx-host=[[,],]
Return an MX record named pointing to the given hostname (if
given), or
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index e770454..b48e433 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -273,7 +273,8 @@ struct event_desc {
#define OPT_IGNORE_CLID59
#define OPT_SINGLE_PORT60
#define OPT_LEASE_RENEW61
-#define OPT_LAST 62
+#define OPT_CMARK_ALST_EN 62
+#define OPT_LAST 63
#define OPTION_BITS (sizeof(unsigned int)*8)
#define OPTION_SIZE ( (OPT_LAST/OPTION_BITS)+((OPT_LAST%OPTION_BITS)!=0) )
@@ -567,6 +568,12 @@ struct ipsets {
struct ipsets *next;
};
+struct allowlist {
+
Re: [Dnsmasq-discuss] [PATCH v4] Connection track mark based DNS query filtering.
On Thu, Feb 18, 2021 at 12:11:55AM +0100, Etan Kissling wrote: > On 17.02.21, 23:41, Geert Stappers" wrote: > > > +#if defined(HAVE_CONNTRACK) && defined(HAVE_UBUS) > > > > One of many > > > > +# ifdef HAVE_CONNTRACK > > > > One of many > > Sorry, I don't understand the comment for these. The sorry should come from me. I was way too short with text. Thing I trying to tell is that the wish is to avoid #if conditionals. Reason I remember is reducing the amount of different binaries. But HAVE_CONNTRACK is already present in current source. > As those features need libraries that are only present when the > corresponding defines are set, the usage code also needs to be guarded. True .. > > Do know that it is _not_ up to me to decide on this patch. > > > > Thing I'm saying is that it got some human attention. > > Thanks for taking your time to look into it. Appreciate the comments! ;-) Groeten Geert Stappers P.S. Simon, we can't tell if you are already reviewing the patch. In case you did, share with us your verdict. Do know that it is better to reject a patch than leaving the patchsubmitter in vain. -- Silence is hard to parse ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH v4] Connection track mark based DNS query filtering.
On 17.02.21, 23:41, "Dnsmasq-discuss on behalf of Geert Stappers"
wrote:
> > @@ -567,6 +568,12 @@ struct ipsets {
> >struct ipsets *next;
> > };
> >
> > +struct allowlist {
> > + uint32_t mark, mask;
> > + char **patterns;
> > + struct allowlist *next;
> > +};
> > +
>
> I think the missing '# ifdef HAVE_CONNTRACK' will trigger "unused struct"
> warnings ...
> >struct ipsets *ipsets;
> > + uint32_t allowlist_mask;
>
> I think the missing '# ifdef HAVE_CONNTRACK' will trigger "unused uint32_t"
> warnings ...
I have tested compilation with both no-conntrack and HAVE_CONNTRACK on
Raspberry Pi OS, and encountered no compile warnings. Likewise, I have
tested with both HAVE_CONNTRACK and HAVE_UBUS on OpenWrt (for Ubus).
Technically, you are right that this could be guarded as you suggest.
I was thinking about guarding this, but other structs that are only used
optionally, such as the `struct ipsets` directly above this are not
guarded with a similar check.
Besides being consistent with the existing code style, this makes it a
tiny bit less error-prone when having a partial re-compile where some
files use a stale version of the headers with different #ifdefs, which
can introduce very subtle bugs at development time when switching cfg
because the memory layout in the struct would change.
> > +#if defined(HAVE_CONNTRACK) && defined(HAVE_UBUS)
>
> One of many
> > +# ifdef HAVE_CONNTRACK
>
> One of many
Sorry, I don't understand the comment for these. As those features need
libraries that are only present when the corresponding defines are set,
the usage code also needs to be guarded.
Also, as this introduces a new feature I wanted to minimize impact on
any existing installations that do not already use optional features.
Code is only compiled with -DHAVE_CONNTRACK (and -DHAVE_UBUS), and only
activated when the config file enables it (default = disabled).
> snip .
>
> > + if (0);
> > +#ifdef HAVE_CONNTRACK
> > + else if (!allowed)
> > +{
> > + m = setup_reply(header, n, /* addrp: */ NULL, /* flags: */ 0, /*
> > ttl: */ 0);
> > + if (m >= 1)
> > + {
> > + send_from(listen->fd, option_bool(OPT_NOWILD) ||
> > option_bool(OPT_CLEVERBIND),
> > + (char *)header, m, _addr, _addr, if_index);
> > + daemon->metrics[METRIC_DNS_LOCAL_ANSWERED]++;
> > + }
> > +}
> > +#endif
> > #ifdef HAVE_AUTH
> > - if (auth_dns)
> > + else if (auth_dns)
>
> That extra elsefeels odd.
You snipped one line too much at the top, I re-added it here.
If (0);
The previous logic was:
if (auth_dns)
The new intended logic is:
if (!allowed)
else if (auth_dns)
Because the allowed case is in an #ifdef, the logic is like this:
if (0);
#ifdef HAVE_CONNTRACK
else if (!allowed)
#endif
else if (auth_dns)
In the case where HAVE_CONNTRACK is not defined, this becomes:
if (0);
else if (auth_dns)
which is equivalent to:
if (auth_dns)
In the case where HAVE_CONNTRACK is defined, this becomes:
if (0);
else if (!allowed)
else if (auth_dns)
which is equivalent to:
if (!allowed)
else if (auth_dns)
> > - else
> > #endif
> > + else
>
> That swap of lines feels odd.
This can be explained in a similar way to the one above, it is just some
trickery to stack regular C ifs and pre-processor ifs.
> Do know that it is _not_ up to me to decide on this patch.
>
> Thing I'm saying is that it got some human attention.
Thanks for taking your time to look into it. Appreciate the comments!
> Regards
> Geert Stappers
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
[Dnsmasq-discuss] [PATCH v4] Connection track mark based DNS query filtering.
This extends query filtering support beyond what is currently possible
with the `--ipset` configuration option, by adding support for:
1) Specifying allowlists on a per-client basis, based on their
associated Linux connection track mark.
2) Dynamic configuration of allowlists via Ubus.
3) Reporting when a DNS query resolves or is rejected via Ubus.
4) DNS name patterns containing wildcards.
Disallowed queries are not forwarded; they are rejected
with a REFUSED error code.
Signed-off-by: Etan Kissling
---
v2: Rebase to v2.83, and fix compilation when HAVE_UBUS not present.
v3: Rebase to v2.84test2.
v4: Rebase to v2.84rc2 (update copyright notice).
Makefile | 2 +-
man/dnsmasq.8 | 31 +++-
src/dnsmasq.h | 25 +++-
src/forward.c | 121 +++-
src/option.c | 134 ++
src/pattern.c | 386 ++
src/rfc1035.c | 82 +++
src/ubus.c| 182
8 files changed, 955 insertions(+), 8 deletions(-)
create mode 100644 src/pattern.c
diff --git a/Makefile b/Makefile
index e4c3f5c..506e56b 100644
--- a/Makefile
+++ b/Makefile
@@ -79,7 +79,7 @@ copts_conf = .copts_$(sum)
objs = cache.o rfc1035.o util.o option.o forward.o network.o \
dnsmasq.o dhcp.o lease.o rfc2131.o netlink.o dbus.o bpf.o \
helper.o tftp.o log.o conntrack.o dhcp6.o rfc3315.o \
- dhcp-common.o outpacket.o radv.o slaac.o auth.o ipset.o \
+ dhcp-common.o outpacket.o radv.o slaac.o auth.o ipset.o pattern.o \
domain.o dnssec.o blockdata.o tables.o loop.o inotify.o \
poll.o rrfilter.o edns0.o arp.o crypto.o dump.o ubus.o \
metrics.o hash_questions.o
diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
index ac7c9fa..04d666d 100644
--- a/man/dnsmasq.8
+++ b/man/dnsmasq.8
@@ -368,7 +368,10 @@ provides service at that name, rather than the default
which is
.TP
.B --enable-ubus[=]
Enable dnsmasq UBus interface. It sends notifications via UBus on
-DHCPACK and DHCPRELEASE events. Furthermore it offers metrics.
+DHCPACK and DHCPRELEASE events. Furthermore it offers metrics
+and allows configuration of Linux connection track mark based filtering.
+When DNS query filtering based on Linux connection track marks is enabled
+UBus notifications are generated for each resolved or filtered DNS query.
Requires that dnsmasq has been built with UBus support. If the service
name is given, dnsmasq provides service at that namespace, rather than
the default which is
@@ -533,6 +536,32 @@ These IP sets must already exist. See
.BR ipset (8)
for more details.
.TP
+.B --connmark-allowlist-enable[=]
+Enables filtering of incoming DNS queries with associated Linux connection
track marks
+according to individual allowlists configured via a series of
\fB--connmark-allowlist\fP
+options. Disallowed queries are not forwarded; they are rejected with a
REFUSED error code.
+DNS queries are only allowed if they do not have an associated Linux connection
+track mark, or if the queried domains match the configured DNS patterns for the
+associated Linux connection track mark. If no allowlist is configured for a
+Linux connection track mark, all DNS queries associated with that mark are
rejected.
+If a mask is specified, Linux connection track marks are first bitwise ANDed
+with the given mask before being processed.
+.TP
+.B --connmark-allowlist=[/][,[/...]]
+Configures the DNS patterns that are allowed in DNS queries associated with
+the given Linux connection track mark.
+If a mask is specified, Linux connection track marks are first bitwise ANDed
+with the given mask before they are compared to the given connection track
mark.
+Patterns follow the syntax of DNS names, but additionally allow the wildcard
+character "*" to be used up to twice per label to match 0 or more characters
+within that label. Note that the wildcard never matches a dot (e.g.,
"*.example.com"
+matches "api.example.com" but not "api.us.example.com"). Patterns must be
+fully qualified, i.e., consist of at least two labels. The final label must
not be
+fully numeric, and must not be the "local" pseudo-TLD. A pattern must end with
at least
+two literal (non-wildcard) labels.
+Instead of a pattern, "*" can be specified to disable allowlist filtering
+for a given Linux connection track mark entirely.
+.TP
.B \-m, --mx-host=[[,],]
Return an MX record named pointing to the given hostname (if
given), or
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index e770454..b48e433 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -273,7 +273,8 @@ struct event_desc {
#define OPT_IGNORE_CLID59
#define OPT_SINGLE_PORT60
#define OPT_LEASE_RENEW61
-#define OPT_LAST 62
+#define OPT_CMARK_ALST_EN 62
+#define OPT_LAST 63
#define OPTION_BITS (sizeof(unsigned int)*8)
#define OPTION_SIZE ( (OPT_LAST/OPTION_BITS)+((OPT_LAST%OPTION_BITS)!=0) )
@@ -567,6 +568,12 @@ struct ipsets {
struct ipsets *next;
};
+struct allowlist {
+
[Dnsmasq-discuss] [PATCH v4] Connection track mark based DNS query filtering.
This extends query filtering support beyond what is currently possible
with the `--ipset` configuration option, by adding support for:
1) Specifying allowlists on a per-client basis, based on their
associated Linux connection track mark.
2) Dynamic configuration of allowlists via Ubus.
3) Reporting when a DNS query resolves or is rejected via Ubus.
4) DNS name patterns containing wildcards.
Disallowed queries are not forwarded; they are rejected
with a REFUSED error code.
Signed-off-by: Etan Kissling
---
v2: Rebase to v2.83, and fix compilation when HAVE_UBUS not present.
v3: Rebase to v2.84test2.
v4: Rebase to v2.84rc2 (update copyright notice).
Makefile | 2 +-
man/dnsmasq.8 | 31 +++-
src/dnsmasq.h | 25 +++-
src/forward.c | 121 +++-
src/option.c | 134 ++
src/pattern.c | 386 ++
src/rfc1035.c | 82 +++
src/ubus.c| 182
8 files changed, 955 insertions(+), 8 deletions(-)
create mode 100644 src/pattern.c
diff --git a/Makefile b/Makefile
index e4c3f5c..506e56b 100644
--- a/Makefile
+++ b/Makefile
@@ -79,7 +79,7 @@ copts_conf = .copts_$(sum)
objs = cache.o rfc1035.o util.o option.o forward.o network.o \
dnsmasq.o dhcp.o lease.o rfc2131.o netlink.o dbus.o bpf.o \
helper.o tftp.o log.o conntrack.o dhcp6.o rfc3315.o \
- dhcp-common.o outpacket.o radv.o slaac.o auth.o ipset.o \
+ dhcp-common.o outpacket.o radv.o slaac.o auth.o ipset.o pattern.o \
domain.o dnssec.o blockdata.o tables.o loop.o inotify.o \
poll.o rrfilter.o edns0.o arp.o crypto.o dump.o ubus.o \
metrics.o hash_questions.o
diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
index ac7c9fa..04d666d 100644
--- a/man/dnsmasq.8
+++ b/man/dnsmasq.8
@@ -368,7 +368,10 @@ provides service at that name, rather than the default
which is
.TP
.B --enable-ubus[=]
Enable dnsmasq UBus interface. It sends notifications via UBus on
-DHCPACK and DHCPRELEASE events. Furthermore it offers metrics.
+DHCPACK and DHCPRELEASE events. Furthermore it offers metrics
+and allows configuration of Linux connection track mark based filtering.
+When DNS query filtering based on Linux connection track marks is enabled
+UBus notifications are generated for each resolved or filtered DNS query.
Requires that dnsmasq has been built with UBus support. If the service
name is given, dnsmasq provides service at that namespace, rather than
the default which is
@@ -533,6 +536,32 @@ These IP sets must already exist. See
.BR ipset (8)
for more details.
.TP
+.B --connmark-allowlist-enable[=]
+Enables filtering of incoming DNS queries with associated Linux connection
track marks
+according to individual allowlists configured via a series of
\fB--connmark-allowlist\fP
+options. Disallowed queries are not forwarded; they are rejected with a
REFUSED error code.
+DNS queries are only allowed if they do not have an associated Linux connection
+track mark, or if the queried domains match the configured DNS patterns for the
+associated Linux connection track mark. If no allowlist is configured for a
+Linux connection track mark, all DNS queries associated with that mark are
rejected.
+If a mask is specified, Linux connection track marks are first bitwise ANDed
+with the given mask before being processed.
+.TP
+.B --connmark-allowlist=[/][,[/...]]
+Configures the DNS patterns that are allowed in DNS queries associated with
+the given Linux connection track mark.
+If a mask is specified, Linux connection track marks are first bitwise ANDed
+with the given mask before they are compared to the given connection track
mark.
+Patterns follow the syntax of DNS names, but additionally allow the wildcard
+character "*" to be used up to twice per label to match 0 or more characters
+within that label. Note that the wildcard never matches a dot (e.g.,
"*.example.com"
+matches "api.example.com" but not "api.us.example.com"). Patterns must be
+fully qualified, i.e., consist of at least two labels. The final label must
not be
+fully numeric, and must not be the "local" pseudo-TLD. A pattern must end with
at least
+two literal (non-wildcard) labels.
+Instead of a pattern, "*" can be specified to disable allowlist filtering
+for a given Linux connection track mark entirely.
+.TP
.B \-m, --mx-host=[[,],]
Return an MX record named pointing to the given hostname (if
given), or
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index e770454..b48e433 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -273,7 +273,8 @@ struct event_desc {
#define OPT_IGNORE_CLID59
#define OPT_SINGLE_PORT60
#define OPT_LEASE_RENEW61
-#define OPT_LAST 62
+#define OPT_CMARK_ALST_EN 62
+#define OPT_LAST 63
#define OPTION_BITS (sizeof(unsigned int)*8)
#define OPTION_SIZE ( (OPT_LAST/OPTION_BITS)+((OPT_LAST%OPTION_BITS)!=0) )
@@ -567,6 +568,12 @@ struct ipsets {
struct ipsets *next;
};
+struct allowlist {
+
