cotugno/AdGuardHome
2
Fork 0
mirror of https://github.com/AdguardTeam/AdGuardHome.git synced 2024-11-15 09:58:42 -07:00
Code Issues Packages Projects Releases Wiki Activity

Pull request #1770: 5567-extract-subnet-arpa

Browse Source 
Merge in DNS/adguard-home from 5567-extract-subnet-arpa to master

Updates #5567.

Squashed commit of the following:

commit 288fb405b82eff2a95d75f8c557100908a998a08
Merge: e16b3ce5 9f7a582d
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Mar 17 14:01:39 2023 +0300

    Merge branch 'master' into 5567-extract-subnet-arpa

commit e16b3ce57ba41a9f4a7743dbdb93c2320e650140
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Mar 17 13:58:58 2023 +0300

    dnsforward: use netip

commit 265b08c5f82f8df555ab1a5f01c2e9ef8caef64a
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Mar 16 19:11:49 2023 +0300

    dnsforward: imp tests more

commit 53a839cb6dd924cabf0552386f76aa8775c88983
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Mar 16 19:09:15 2023 +0300

    dnsforward: imp naming in tests

commit 74dcccbdda217422260579e331289003a024695e
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Mar 16 18:59:12 2023 +0300

    dnsforward: imp code & tests more

commit da8badfaa75a0a67c10ce6f347e551dcfd4c0589
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Mar 15 14:52:48 2023 +0300

    all: log changes

commit c491cbfb3fd8d716303224c1f73329a47087753a
Merge: 74a93179 2b5e4850
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Mar 15 14:44:31 2023 +0300

    Merge branch 'master' into 5567-extract-subnet-arpa

commit 74a93179d7fb7f005455ce02f7f0c16b796c3914
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Wed Mar 15 14:42:55 2023 +0300

    dnsforward: imp code, docs

commit 17df1a0ce461335649c6dab65c984eb0cce0bdf0
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Mar 14 19:49:10 2023 +0300

    dnsforward: extract subnet from arpa
This commit is contained in:
Eugene Burkov 2023-03-17 17:10:33 +03:00
parent 9f7a582dbc
commit 48431f8b86
5 changed files with 239 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
CHANGELOG.md
View File

@ -28,7 +28,10 @@ NOTE: Add new changes BELOW THIS COMMENT.
- The ability to manage safesearch for each service by using the new
`safe_search` field ([#1163]).
### Changed
### Changed
- ARPA domain names containing a subnet within private networks now also
considered private, behaving closer to [RFC 6761][rfc6761] ([#5567]).
#### Configuration Changes
@ -65,8 +68,11 @@ In this release, the schema version has changed from 17 to 19.
([#5584]).
[#1163]: https://github.com/AdguardTeam/AdGuardHome/issues/1163
[#5567]: https://github.com/AdguardTeam/AdGuardHome/issues/5567
[#5584]: https://github.com/AdguardTeam/AdGuardHome/issues/5584
[rfc6761]: https://www.rfc-editor.org/rfc/rfc6761
<!--
NOTE: Add new changes ABOVE THIS COMMENT.
-->

112
internal/dnsforward/dns.go
View File

@ -4,6 +4,7 @@ import (
"encoding/binary"
"net"
"net/netip"
"strconv"
"strings"
"time"
@ -37,6 +38,8 @@ type dnsContext struct {
// was parsed successfully and belongs to one of the locally served IP
// ranges. It is also filled with unmapped version of the address if it's
// within DNS64 prefixes.
//
// TODO(e.burkov): Use netip.Addr when we switch to netip more fully.
unreversedReqIP net.IP
// err is the error returned from a processing function.
@ -442,6 +445,88 @@ func (s *Server) processDHCPHosts(dctx *dnsContext) (rc resultCode) {
return resultCodeSuccess
}
// indexFirstV4Label returns the index at which the reversed IPv4 address
// starts, assuiming the domain is pre-validated ARPA domain having in-addr and
// arpa labels removed.
func indexFirstV4Label(domain string) (idx int) {
idx = len(domain)
for labelsNum := 0; labelsNum < net.IPv4len && idx > 0; labelsNum++ {
curIdx := strings.LastIndexByte(domain[:idx-1], '.') + 1
_, parseErr := strconv.ParseUint(domain[curIdx:idx-1], 10, 8)
if parseErr != nil {
return idx
}
idx = curIdx
}
return idx
}
// indexFirstV6Label returns the index at which the reversed IPv6 address
// starts, assuiming the domain is pre-validated ARPA domain having ip6 and arpa
// labels removed.
func indexFirstV6Label(domain string) (idx int) {
idx = len(domain)
for labelsNum := 0; labelsNum < net.IPv6len*2 && idx > 0; labelsNum++ {
curIdx := idx - len("a.")
if curIdx > 1 && domain[curIdx-1] != '.' {
return idx
}
nibble := domain[curIdx]
if (nibble < '0' || nibble > '9') && (nibble < 'a' || nibble > 'f') {
return idx
}
idx = curIdx
}
return idx
}
// extractARPASubnet tries to convert a reversed ARPA address being a part of
// domain to an IP network. domain must be an FQDN.
//
// TODO(e.burkov): Move to golibs.
func extractARPASubnet(domain string) (pref netip.Prefix, err error) {
err = netutil.ValidateDomainName(strings.TrimSuffix(domain, "."))
if err != nil {
// Don't wrap the error since it's informative enough as is.
return netip.Prefix{}, err
}
const (
v4Suffix = "in-addr.arpa."
v6Suffix = "ip6.arpa."
)
domain = strings.ToLower(domain)
var idx int
switch {
case strings.HasSuffix(domain, v4Suffix):
idx = indexFirstV4Label(domain[:len(domain)-len(v4Suffix)])
case strings.HasSuffix(domain, v6Suffix):
idx = indexFirstV6Label(domain[:len(domain)-len(v6Suffix)])
default:
return netip.Prefix{}, &netutil.AddrError{
Err: netutil.ErrNotAReversedSubnet,
Kind: netutil.AddrKindARPA,
Addr: domain,
}
}
var subnet *net.IPNet
subnet, err = netutil.SubnetFromReversedAddr(domain[idx:])
if err != nil {
// Don't wrap the error since it's informative enough as is.
return netip.Prefix{}, err
}
return netutil.IPNetToPrefixNoMapped(subnet)
}
// processRestrictLocal responds with NXDOMAIN to PTR requests for IP addresses
// in locally served network from external clients.
func (s *Server) processRestrictLocal(dctx *dnsContext) (rc resultCode) {
@ -453,34 +538,29 @@ func (s *Server) processRestrictLocal(dctx *dnsContext) (rc resultCode) {
return resultCodeSuccess
}
ip, err := netutil.IPFromReversedAddr(q.Name)
subnet, err := extractARPASubnet(q.Name)
if err != nil {
log.Debug("dnsforward: parsing reversed addr: %s", err)
if errors.Is(err, netutil.ErrNotAReversedSubnet) {
log.Debug("dnsforward: request is not for arpa domain")
// DNS-Based Service Discovery uses PTR records having not an ARPA
// format of the domain name in question. Those shouldn't be
// invalidated. See http://www.dns-sd.org/ServerStaticSetup.html and
// RFC 2782.
name := strings.TrimSuffix(q.Name, ".")
if err = netutil.ValidateSRVDomainName(name); err != nil {
log.Debug("dnsforward: validating service domain: %s", err)
return resultCodeError
return resultCodeSuccess
}
log.Debug("dnsforward: request is not for arpa domain")
log.Debug("dnsforward: parsing reversed addr: %s", err)
return resultCodeSuccess
return resultCodeError
}
// Restrict an access to local addresses for external clients. We also
// assume that all the DHCP leases we give are locally served or at least
// shouldn't be accessible externally.
if !s.privateNets.Contains(ip) {
subnetAddr := subnet.Addr()
addrData := subnetAddr.AsSlice()
if !s.privateNets.Contains(addrData) {
return resultCodeSuccess
}
log.Debug("dnsforward: addr %s is from locally served network", ip)
log.Debug("dnsforward: addr %s is from locally served network", subnetAddr)
if !dctx.isLocalClient {
log.Debug("dnsforward: %q requests an internal ip", pctx.Addr)
@ -491,7 +571,7 @@ func (s *Server) processRestrictLocal(dctx *dnsContext) (rc resultCode) {
}
// Do not perform unreversing ever again.
dctx.unreversedReqIP = ip
dctx.unreversedReqIP = addrData
// There is no need to filter request from external addresses since this
// code is only executed when the request is for locally served ARPA

126
internal/dnsforward/dns_test.go
View File

@ -605,3 +605,129 @@ func TestIPStringFromAddr(t *testing.T) {
assert.Empty(t, ipStringFromAddr(nil))
})
}
// TODO(e.burkov): Add fuzzing when moving to golibs.
func TestExtractARPASubnet(t *testing.T) {
const (
v4Suf = `in-addr.arpa.`
v4Part = `2.1.` + v4Suf
v4Whole = `4.3.` + v4Part
v6Suf = `ip6.arpa.`
v6Part = `4.3.2.1.0.0.0.0.0.0.0.0.0.0.0.0.` + v6Suf
v6Whole = `f.e.d.c.0.0.0.0.0.0.0.0.0.0.0.0.` + v6Part
)
v4Pref := netip.MustParsePrefix("1.2.3.4/32")
v4PrefPart := netip.MustParsePrefix("1.2.0.0/16")
v6Pref := netip.MustParsePrefix("::1234:0:0:0:cdef/128")
v6PrefPart := netip.MustParsePrefix("0:0:0:1234::/64")
testCases := []struct {
want netip.Prefix
name string
domain string
wantErr string
}{{
want: netip.Prefix{},
name: "not_an_arpa",
domain: "some.domain.name.",
wantErr: `bad arpa domain name "some.domain.name.": ` +
`not a reversed ip network`,
}, {
want: netip.Prefix{},
name: "bad_domain_name",
domain: "abc.123.",
wantErr: `bad domain name "abc.123": ` +
`bad top-level domain name label "123": all octets are numeric`,
}, {
want: v4Pref,
name: "whole_v4",
domain: v4Whole,
wantErr: "",
}, {
want: v4PrefPart,
name: "partial_v4",
domain: v4Part,
wantErr: "",
}, {
want: v4Pref,
name: "whole_v4_within_domain",
domain: "a." + v4Whole,
wantErr: "",
}, {
want: v4Pref,
name: "whole_v4_additional_label",
domain: "5." + v4Whole,
wantErr: "",
}, {
want: v4PrefPart,
name: "partial_v4_within_domain",
domain: "a." + v4Part,
wantErr: "",
}, {
want: v4PrefPart,
name: "overflow_v4",
domain: "256." + v4Part,
wantErr: "",
}, {
want: v4PrefPart,
name: "overflow_v4_within_domain",
domain: "a.256." + v4Part,
wantErr: "",
}, {
want: netip.Prefix{},
name: "empty_v4",
domain: v4Suf,
wantErr: `bad arpa domain name "in-addr.arpa": ` +
`not a reversed ip network`,
}, {
want: netip.Prefix{},
name: "empty_v4_within_domain",
domain: "a." + v4Suf,
wantErr: `bad arpa domain name "in-addr.arpa": ` +
`not a reversed ip network`,
}, {
want: v6Pref,
name: "whole_v6",
domain: v6Whole,
wantErr: "",
}, {
want: v6PrefPart,
name: "partial_v6",
domain: v6Part,
}, {
want: v6Pref,
name: "whole_v6_within_domain",
domain: "g." + v6Whole,
wantErr: "",
}, {
want: v6Pref,
name: "whole_v6_additional_label",
domain: "1." + v6Whole,
wantErr: "",
}, {
want: v6PrefPart,
name: "partial_v6_within_domain",
domain: "label." + v6Part,
wantErr: "",
}, {
want: netip.Prefix{},
name: "empty_v6",
domain: v6Suf,
wantErr: `bad arpa domain name "ip6.arpa": not a reversed ip network`,
}, {
want: netip.Prefix{},
name: "empty_v6_within_domain",
domain: "g." + v6Suf,
wantErr: `bad arpa domain name "ip6.arpa": not a reversed ip network`,
}}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
subnet, err := extractARPASubnet(tc.domain)
testutil.AssertErrorMsg(t, tc.wantErr, err)
assert.Equal(t, tc.want, subnet)
})
}
}

6
internal/dnsforward/http.go
View File

@ -388,15 +388,15 @@ func ValidateUpstreamsPrivate(upstreams []string, privateNets netutil.SubnetSet)
var errs []error
for _, domain := range keys {
var subnet *net.IPNet
subnet, err = netutil.SubnetFromReversedAddr(domain)
var subnet netip.Prefix
subnet, err = extractARPASubnet(domain)
if err != nil {
errs = append(errs, err)
continue
}
if !privateNets.Contains(subnet.IP) {
if !privateNets.Contains(subnet.Addr().AsSlice()) {
errs = append(
errs,
fmt.Errorf("arpa domain %q should point to a locally-served network", domain),

10
internal/dnsforward/http_test.go
View File

@ -212,7 +212,7 @@ func TestDNSForwardHTTP_handleSetConfig(t *testing.T) {
}, {
name: "local_ptr_upstreams_bad",
wantSet: `validating private upstream servers: checking domain-specific upstreams: ` +
`bad arpa domain name "non.arpa": not a reversed ip network`,
`bad arpa domain name "non.arpa.": not a reversed ip network`,
}, {
name: "local_ptr_upstreams_null",
wantSet: "",
@ -373,7 +373,7 @@ func TestValidateUpstreamsPrivate(t *testing.T) {
}, {
name: "not_arpa_subnet",
wantErr: `checking domain-specific upstreams: ` +
`bad arpa domain name "hello.world": not a reversed ip network`,
`bad arpa domain name "hello.world.": not a reversed ip network`,
u: "[/hello.world/]#",
}, {
name: "non-private_arpa_address",
@ -389,8 +389,12 @@ func TestValidateUpstreamsPrivate(t *testing.T) {
name: "several_bad",
wantErr: `checking domain-specific upstreams: 2 errors: ` +
`"arpa domain \"1.2.3.4.in-addr.arpa.\" should point to a locally-served network", ` +
`"bad arpa domain name \"non.arpa\": not a reversed ip network"`,
`"bad arpa domain name \"non.arpa.\": not a reversed ip network"`,
u: "[/non.arpa/1.2.3.4.in-addr.arpa/127.in-addr.arpa/]#",
}, {
name: "partial_good",
wantErr: "",
u: "[/a.1.2.3.10.in-addr.arpa/a.10.in-addr.arpa/]#",
}}
for _, tc := range testCases {