Revert "Improve search scope handling for resolvers"

This commit is contained in:
Daniel 2022-03-19 22:15:15 +01:00 committed by GitHub
parent b0b2fff5d7
commit 4151ac64ff
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 71 additions and 197 deletions

View file

@ -27,8 +27,6 @@ func sendResponse(
// Dropping query.
return nil
}
// Signify that we are a recursive resolver.
reply.RecursionAvailable = true
// Add extra RRs through a custom RRProvider.
for _, rrProvider := range rrProviders {

View file

@ -104,8 +104,6 @@ The format is: "protocol://ip:port?parameter=value&parameter=value"
- "empty": server replies with NXDomain status, but without any other record in any section
- "refused": server replies with Refused status
- "zeroip": server replies with an IP address, but it is zero
- "search": specify prioritized domains/TLDs for this resolver (delimited by ",")
- "search-only": use this resolver for domains in the "search" parameter only (no value)
`, `"`, "`"),
OptType: config.OptTypeStringArray,
ExpertiseLevel: config.ExpertiseLevelExpert,

View file

@ -14,6 +14,8 @@ import (
"github.com/safing/portbase/modules"
"github.com/safing/portbase/notifications"
"github.com/safing/portbase/utils/debug"
// Dependency.
_ "github.com/safing/portmaster/core/base"
"github.com/safing/portmaster/intel"
)

View file

@ -63,7 +63,6 @@ type Resolver struct {
// Special Options
VerifyDomain string
Search []string
SearchOnly bool
// logic interface
Conn ResolverConn `json:"-"`

View file

@ -12,27 +12,16 @@ import (
"golang.org/x/net/publicsuffix"
"github.com/safing/portbase/log"
"github.com/safing/portbase/utils"
"github.com/safing/portmaster/netenv"
"github.com/safing/portmaster/network/netutils"
)
const maxSearchDomains = 100
// Scope defines a domain scope and which resolvers can resolve it.
type Scope struct {
Domain string
Resolvers []*Resolver
}
const (
parameterName = "name"
parameterVerify = "verify"
parameterBlockedIf = "blockedif"
parameterSearch = "search"
parameterSearchOnly = "search-only"
)
var (
globalResolvers []*Resolver // all (global) resolvers
localResolvers []*Resolver // all resolvers that are in site-local or link-local IP ranges
@ -128,47 +117,31 @@ func createResolver(resolverURL, source string) (*Resolver, bool, error) {
return nil, true, nil // skip
}
// Get parameters and check if keys exist.
query := u.Query()
for key := range query {
switch key {
case parameterName,
parameterVerify,
parameterBlockedIf,
parameterSearch,
parameterSearchOnly:
// Known key, continue.
default:
// Unknown key, abort.
return nil, false, fmt.Errorf(`unknown parameter "%s"`, key)
}
}
// Check domain verification config.
verifyDomain := query.Get(parameterVerify)
verifyDomain := query.Get("verify")
if verifyDomain != "" && u.Scheme != ServerTypeDoT {
return nil, false, fmt.Errorf("domain verification only supported in DOT")
}
if verifyDomain == "" && u.Scheme == ServerTypeDoT {
return nil, false, fmt.Errorf("DOT must have a verify query parameter set")
}
// Check block detection type.
blockType := query.Get(parameterBlockedIf)
blockType := query.Get("blockedif")
if blockType == "" {
blockType = BlockDetectionZeroIP
}
switch blockType {
case BlockDetectionDisabled, BlockDetectionEmptyAnswer, BlockDetectionRefused, BlockDetectionZeroIP:
default:
return nil, false, fmt.Errorf("invalid value for upstream block detection (blockedif=)")
}
// Build resolver.
newResolver := &Resolver{
ConfigURL: resolverURL,
Info: &ResolverInfo{
Name: query.Get(parameterName),
Name: query.Get("name"),
Type: u.Scheme,
Source: source,
IP: ip,
@ -180,55 +153,22 @@ func createResolver(resolverURL, source string) (*Resolver, bool, error) {
UpstreamBlockDetection: blockType,
}
// Parse search domains.
searchDomains := query.Get(parameterSearch)
if searchDomains != "" {
err = configureSearchDomains(newResolver, strings.Split(searchDomains, ","), true)
if err != nil {
return nil, false, err
}
}
// Check if searchOnly is set and valid.
if query.Has(parameterSearchOnly) {
newResolver.SearchOnly = true
if query.Get(parameterSearchOnly) != "" {
return nil, false, fmt.Errorf("%s may only be used as an empty parameter", parameterSearchOnly)
}
if len(newResolver.Search) == 0 {
return nil, false, fmt.Errorf("cannot use %s without search scopes", parameterSearchOnly)
}
}
newResolver.Conn = resolverConnFactory(newResolver)
return newResolver, false, nil
}
func configureSearchDomains(resolver *Resolver, searches []string, hardfail bool) error {
// Check all search domains.
for i, value := range searches {
trimmedDomain := strings.ToLower(strings.Trim(value, "."))
err := checkSearchScope(trimmedDomain)
if err != nil {
if hardfail {
return fmt.Errorf("failed to validate search domain #%d: %w", i+1, err)
}
log.Warningf("resolver: skipping invalid search domain for resolver %s: %s", resolver, utils.SafeFirst16Chars(value))
} else {
resolver.Search = append(resolver.Search, fmt.Sprintf(".%s.", trimmedDomain))
func configureSearchDomains(resolver *Resolver, searches []string) {
// only allow searches for local resolvers
for _, value := range searches {
trimmedDomain := strings.Trim(value, ".")
if checkSearchScope(trimmedDomain) {
resolver.Search = append(resolver.Search, fmt.Sprintf(".%s.", strings.Trim(value, ".")))
}
}
// Limit search domains to mitigate exploitation via malicious local resolver.
if len(resolver.Search) > maxSearchDomains {
if hardfail {
return fmt.Errorf("too many search domains, for security reasons only %d search domains are supported", maxSearchDomains)
}
log.Warningf("resolver: limiting search domains for resolver %s to %d for security reasons", resolver, maxSearchDomains)
resolver.Search = resolver.Search[:maxSearchDomains]
// cap to mitigate exploitation via malicious local resolver
if len(resolver.Search) > 100 {
resolver.Search = resolver.Search[:100]
}
return nil
}
func getConfiguredResolvers(list []string) (resolvers []*Resolver) {
@ -264,7 +204,7 @@ func getSystemResolvers() (resolvers []*Resolver) {
}
if resolver.Info.IPScope.IsLAN() {
_ = configureSearchDomains(resolver, nameserver.Search, false)
configureSearchDomains(resolver, nameserver.Search)
}
resolvers = append(resolvers, resolver)
@ -415,48 +355,46 @@ func setScopedResolvers(resolvers []*Resolver) {
)
}
func checkSearchScope(searchDomain string) error {
// Sanity check the input.
func checkSearchScope(searchDomain string) (ok bool) {
// sanity check
if len(searchDomain) == 0 ||
searchDomain[0] == '.' ||
searchDomain[len(searchDomain)-1] == '.' {
return fmt.Errorf("invalid (1) search domain: %s", searchDomain)
return false
}
// Domains that are specifically listed in the special service domains may be
// diverted completely.
if domainInScope("."+searchDomain+".", specialServiceDomains) {
return nil
}
// add more subdomains to use official publicsuffix package for our cause
searchDomain = "*.*.*.*.*." + searchDomain
// In order to check if the search domain is too high up in the hierarchy, we
// need to add some more subdomains.
augmentedSearchDomain := "*.*.*.*.*." + searchDomain
// Get the public suffix of the search domain and if the TLD is managed by ICANN.
suffix, icann := publicsuffix.PublicSuffix(augmentedSearchDomain)
// Sanity check the result.
// get suffix
suffix, icann := publicsuffix.PublicSuffix(searchDomain)
// sanity check
if len(suffix) == 0 {
return fmt.Errorf("invalid (2) search domain: %s", searchDomain)
return false
}
// TLDs that are not managed by ICANN (ie. are unofficial) may be
// diverted completely.
// This might include special service domains like ".onion" and ".bit".
// inexistent (custom) tlds are okay
// this will include special service domains! (.onion, .bit, ...)
if !icann && !strings.Contains(suffix, ".") {
return nil
return true
}
// Build the eTLD+1 domain, which is the highest that may be diverted.
split := len(augmentedSearchDomain) - len(suffix) - 1
eTLDplus1 := augmentedSearchDomain[1+strings.LastIndex(augmentedSearchDomain[:split], "."):]
// check if suffix is a special service domain (may be handled fully by local nameserver)
if domainInScope("."+suffix+".", specialServiceDomains) {
return true
}
// Check if the scope is being violated by checking if the eTLD+1 contains a wildcard.
// build eTLD+1
split := len(searchDomain) - len(suffix) - 1
eTLDplus1 := searchDomain[1+strings.LastIndex(searchDomain[:split], "."):]
// scope check
//nolint:gosimple // want comment
if strings.Contains(eTLDplus1, "*") {
return fmt.Errorf(`search domain "%s" is dangerously high up the hierarchy, stay at or below "%s"`, searchDomain, eTLDplus1)
// oops, search domain is too high up the hierarchy
return false
}
return nil
return true
}
// IsResolverAddress returns whether the given ip and port match a configured resolver.

View file

@ -1,44 +1,40 @@
package resolver
import (
"testing"
"github.com/stretchr/testify/assert"
)
import "testing"
func TestCheckResolverSearchScope(t *testing.T) {
t.Parallel()
// should fail (invalid)
assert.Error(t, checkSearchScope("."))
assert.Error(t, checkSearchScope(".com."))
assert.Error(t, checkSearchScope("com."))
assert.Error(t, checkSearchScope(".com"))
test := func(t *testing.T, domain string, expectedResult bool) {
t.Helper()
// should fail (too high scope)
assert.Error(t, checkSearchScope("com"))
assert.Error(t, checkSearchScope("net"))
assert.Error(t, checkSearchScope("org"))
assert.Error(t, checkSearchScope("pvt.k12.ma.us"))
if checkSearchScope(domain) != expectedResult {
if expectedResult {
t.Errorf("domain %s failed scope test", domain)
} else {
t.Errorf("domain %s should fail scope test", domain)
}
}
}
// should fail (invalid)
test(t, ".", false)
test(t, ".com.", false)
test(t, "com.", false)
test(t, ".com", false)
// should succeed
assert.NoError(t, checkSearchScope("a.com"))
assert.NoError(t, checkSearchScope("b.a.com"))
assert.NoError(t, checkSearchScope("c.b.a.com"))
assert.NoError(t, checkSearchScope("test.pvt.k12.ma.us"))
test(t, "a.com", true)
test(t, "b.a.com", true)
test(t, "c.b.a.com", true)
assert.NoError(t, checkSearchScope("onion"))
assert.NoError(t, checkSearchScope("a.onion"))
assert.NoError(t, checkSearchScope("b.a.onion"))
assert.NoError(t, checkSearchScope("c.b.a.onion"))
test(t, "onion", true)
test(t, "a.onion", true)
test(t, "b.a.onion", true)
test(t, "c.b.a.onion", true)
assert.NoError(t, checkSearchScope("bit"))
assert.NoError(t, checkSearchScope("a.bit"))
assert.NoError(t, checkSearchScope("b.a.bit"))
assert.NoError(t, checkSearchScope("c.b.a.bit"))
assert.NoError(t, checkSearchScope("doesnotexist"))
assert.NoError(t, checkSearchScope("a.doesnotexist"))
assert.NoError(t, checkSearchScope("b.a.doesnotexist"))
assert.NoError(t, checkSearchScope("c.b.a.doesnotexist"))
test(t, "bit", true)
test(t, "a.bit", true)
test(t, "b.a.bit", true)
test(t, "c.b.a.bit", true)
}

View file

@ -91,62 +91,11 @@ var (
// Special-Service Domain Names
// Handling: Send to nameservers with matching search scope, then local and system assigned nameservers.
specialServiceDomains = []string{
// RFC7686: Tor Hidden Services, https://www.torproject.org/
// RFC7686: Tor Hidden Services
".onion.",
// I2P: Fully encrypted private network layer, https://geti2p.net/
".i2p.",
// Lokinet: Internal services on the decentralised network, https://lokinet.org/
".loki.",
// Namecoin: Blockchain based nameservice, https://www.namecoin.org/
".bit.",
// Ethereum Name Service (ENS): naming system based on the Ethereum blockchain, https://ens.domains/
".eth.",
// Unstoppable Domains: NFT based domain names, https://unstoppabledomains.com/
".888.",
".bitcoin.",
".coin.",
".crypto.",
".dao.",
".nft.",
".wallet.",
".x.",
".zil.",
// EmerDNS: Domain name registration on EmerCoin, https://emercoin.com/en/emerdns/
".bazar.",
".coin.",
".emc.",
".lib.",
// OpenNIC TLDs: Democratic alternative to ICANN, https://www.opennic.org/
".bbs.",
".chan.",
".dyn.",
".free.",
".fur.",
".geek.",
".glue.",
".gopher.",
".indy.",
".libre.",
".neo.",
".null.",
".o.",
".oss.",
".oz.",
".parody.",
".pirate.",
// NewNations: TLDs for countries/regions without a ccTLD, http://new-nations.net/
".ku.",
".te.",
".ti.",
".uu.",
}
)
@ -230,7 +179,6 @@ var (
errInsecureProtocol = errors.New("insecure protocols disabled")
errAssignedServer = errors.New("assigned (dhcp) nameservers disabled")
errMulticastDNS = errors.New("multicast DNS disabled")
errOutOfScope = errors.New("query out of scope for resolver")
)
func (q *Query) checkCompliance() error {
@ -288,10 +236,5 @@ func (resolver *Resolver) checkCompliance(_ context.Context, q *Query) error {
}
}
// Check if the resolver should only be used for the search scopes.
if resolver.SearchOnly && !domainInScope(q.dotPrefixedFQDN, resolver.Search) {
return errOutOfScope
}
return nil
}