Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dnsmasq: Add EDNS0 Upstream support #14349

Closed
wants to merge 1 commit into from

Conversation

db260179
Copy link
Contributor

@db260179 db260179 commented Jan 6, 2024

Forward client mac address and subnet on dns queries.

Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router.

This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

@github-actions github-actions bot added the core packages pull request/issue for core (in-tree) packages label Jan 6, 2024
@schuettecarsten
Copy link
Contributor

schuettecarsten commented Jan 15, 2024

Please make this configurable (default: on), as adding the requestors MAC address to upstream forwarded requests might have security or privacy implications. It is only useful if you use Pihole/Adguard but not for external upstream servers.

@KanjiMonster
Copy link
Member

Please make this configurable (default: on), as adding the requestors MAC address to upstream forwarded requests might have security or privacy implications. It is only useful if you use Pihole/Adguard but not for external upstream servers.

If this can have security or privacy implications the default should be off, not on.

@db260179
Copy link
Contributor Author

Please make this configurable (default: on), as adding the requestors MAC address to upstream forwarded requests might have security or privacy implications. It is only useful if you use Pihole/Adguard but not for external upstream servers.

If this can have security or privacy implications the default should be off, not on.

Yes i agree, this is just exposing the options, its up to the user to configure via uci or luci (patch coming for this)

@schuettecarsten
Copy link
Contributor

schuettecarsten commented Feb 20, 2024

Tested together with PiHole (development-v6), works.
I would also suggest to add an option for "--strip-subnet" and "--strip-mac":

append_bool "$cfg" strip_mac "--strip-mac"
append_bool "$cfg" strip_subnet "--strip-subnet"

I've also created a draft to add this to LuCI in dhcp.js (line 650):

s.taboption('forward', form.Flag, 'add_mac',
        _('Add requestor MAC address to forwards'),
        _('Add the MAC address of the requestor to DNS queries which are forwarded upstream.'));

s.taboption('forward', form.Flag, 'strip_mac',
        _('Remove MAC address before forwarding query'),
        _('Remove any MAC address information already in downstream queries before forwarding upstream.'));

o = s.taboption('forward', form.Value, 'add_subnet',
        _('Add subnet address to forwards'),
        _('Add a subnet address to the DNS queries which are forwarded upstream, e.g. {32,128} for full requestor address.'));
o.optional = true;

s.taboption('forward', form.Flag, 'strip_subnet',
        _('Remove subnet address before forwarding query'),
        _('Remove any subnet address already present in a downstream query before forwarding it upstream.'));

Forward client mac address and subnet on dns queries.

Pi-hole and Adguard use this feature to send the originators ip address/subnet so it can be logged and not just the nat address of the router.

This feature has been added since version 2.56 of dnsmasq and would be nice to expose this feature in openwrt.

Signed-off-by: David Bentham <db260179@gmail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Mar 7, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Mar 7, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Mar 7, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
@@ -972,12 +972,16 @@ dnsmasq_start()
append_bool "$cfg" noping "--no-ping"
append_bool "$cfg" rapidcommit "--dhcp-rapid-commit"
append_bool "$cfg" scriptarp "--script-arp"
append_bool "$cfg" add_mac "--add-mac"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter is NOT a boolean. This must change. The manpage says --add-mac[=base64|text].

So either --add-mac by itself, or --add-mac=base64 or --add-mac=text. There might appear other variations, but I doubt it. This param must handle a naked param, or with an option (which the user provides).

@@ -972,12 +972,16 @@ dnsmasq_start()
append_bool "$cfg" noping "--no-ping"
append_bool "$cfg" rapidcommit "--dhcp-rapid-commit"
append_bool "$cfg" scriptarp "--script-arp"
append_bool "$cfg" add_mac "--add-mac"
append_bool "$cfg" strip_mac "--strip-mac"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine.


append_bool "$cfg" filter_aaaa "--filter-AAAA"
append_bool "$cfg" filter_a "--filter-A"

append_parm "$cfg" logfacility "--log-facility"
config_get logfacility "$cfg" "logfacility"
append_parm "$cfg" add_subnet "--add-subnet"
Copy link
Contributor

@systemcrash systemcrash Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks OK. This must be able to supply a naked parameter as with --add-mac, or with any combination mentioned in the manpage: --add-subnet[[=[<IPv4 address>/]<IPv4 prefix length>][,[<IPv6 address>/]<IPv6 prefix length>]].

--add-mac by itself means: the address of the requestor will be used.

schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Mar 8, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Apr 8, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Apr 9, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Apr 13, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
@systemcrash
Copy link
Contributor

ping @db260179 - please resolve the code-change comments

@db260179
Copy link
Contributor Author

ping @db260179 - please resolve the code-change comments

When i get time to do this, busy at the moment

schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request May 13, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request May 14, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Jul 14, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
schuettecarsten added a commit to schuettecarsten/openwrt-luci that referenced this pull request Jul 17, 2024
The pull request openwrt/openwrt#14349 adds four
new options to configure EDNS0 upstream support. This patch adds the
new settings to DNSMASQ web UI.

Signed-off-by: Carsten Schuette <schuettecarsten@googlemail.com>
@schuettecarsten
Copy link
Contributor

I've created two new PRs for this, one for the openwrt core system and a luci patch.

@systemcrash
Copy link
Contributor

Please close this PR as outdated/abandoned.

@db260179 db260179 closed this Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core packages pull request/issue for core (in-tree) packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants