archive
/
tailscale
mirror of https://github.com/tailscale/tailscale/
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

tsdns: copy name when loewrcasing.

Browse Source 
The previous approach modifies name in-place in the request slice to avoid an allocation.
This is incorrect: the question section of a DNS request
must be copied verbatim, without any such modification.
Software may rely on it (we rely on other resolvers doing it it in tsdns/forwarder).

Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
pull/701/head
Dmytro Shynkevych 4 years ago
parent 1886dfdca3
commit 6c71e5b851
No known key found for this signature in database
GPG Key ID: FF5E2F3DAD97EA23
2 changed files with 61 additions and 42 deletions
Show Stats Download Patch File Download Diff File

62
wgengine/tsdns/tsdns.go
Unescape Escape View File

@ -7,10 +7,10 @@
package tsdns package tsdns
import ( import (
"bytes"
"encoding/hex" "encoding/hex"
"errors" "errors"
"net" "net"
"strings"
"sync" "sync"
"time" "time"
@ -59,7 +59,7 @@ type Packet struct {
type Resolver struct { type Resolver struct {
logf logger.Logf logf logger.Logf
// rootDomain is <root> in <mynode>.<mydomain>.<root>. // rootDomain is <root> in <mynode>.<mydomain>.<root>.
rootDomain []byte rootDomain string
// forwarder is // forwarder is
forwarder *forwarder forwarder *forwarder
@ -100,7 +100,7 @@ func NewResolver(config ResolverConfig) *Resolver {
responses: make(chan Packet), responses: make(chan Packet),
errors: make(chan error), errors: make(chan error),
closed: make(chan struct{}), closed: make(chan struct{}),
rootDomain: []byte(config.RootDomain), rootDomain: config.RootDomain,
} }
if config.Forward { if config.Forward {
@ -293,14 +293,6 @@ func (r *Resolver) parseQuery(query []byte, resp *response) error {
return err return err
} }
// Lowercase the name: DOMAIN.COM. should resolve the same as domain.com.
name := resp.Question.Name.Data[:resp.Question.Name.Length]
for i, b := range name {
if 'A' <= b && b <= 'Z' {
name[i] = b - 'A' + 'a'
}
}
return nil return nil
} }
@ -402,19 +394,34 @@ func marshalResponse(resp *response) ([]byte, error) {
return builder.Finish() return builder.Finish()
} }
var ( const (
rdnsv4Suffix = []byte(".in-addr.arpa.") rdnsv4Suffix = ".in-addr.arpa."
rdnsv6Suffix = []byte(".ip6.arpa.") rdnsv6Suffix = ".ip6.arpa."
) )
// rawNameToLower converts a raw DNS name to a string, lowercasing it.
func rawNameToLower(name []byte) string {
var sb strings.Builder
sb.Grow(len(name))
for _, b := range name {
if 'A' <= b && b <= 'Z' {
b = b - 'A' + 'a'
}
sb.WriteByte(b)
}
return sb.String()
}
// ptrNameToIPv4 transforms a PTR name representing an IPv4 address to said address. // ptrNameToIPv4 transforms a PTR name representing an IPv4 address to said address.
// Such names are IPv4 labels in reverse order followed by .in-addr.arpa. // Such names are IPv4 labels in reverse order followed by .in-addr.arpa.
// For example, // For example,
// 4.3.2.1.in-addr.arpa // 4.3.2.1.in-addr.arpa
// is transformed to // is transformed to
// 1.2.3.4 // 1.2.3.4
func rdnsNameToIPv4(name []byte) (ip netaddr.IP, ok bool) { func rdnsNameToIPv4(name string) (ip netaddr.IP, ok bool) {
name = bytes.TrimSuffix(name, rdnsv4Suffix) name = strings.TrimSuffix(name, rdnsv4Suffix)
ip, err := netaddr.ParseIP(string(name)) ip, err := netaddr.ParseIP(string(name))
if err != nil { if err != nil {
return netaddr.IP{}, false return netaddr.IP{}, false
@ -432,11 +439,11 @@ func rdnsNameToIPv4(name []byte) (ip netaddr.IP, ok bool) {
// b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa. // b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.
// is transformed to // is transformed to
// 2001:db8::567:89ab // 2001:db8::567:89ab
func rdnsNameToIPv6(name []byte) (ip netaddr.IP, ok bool) { func rdnsNameToIPv6(name string) (ip netaddr.IP, ok bool) {
var b [32]byte var b [32]byte
var ipb [16]byte var ipb [16]byte
name = bytes.TrimSuffix(name, rdnsv6Suffix) name = strings.TrimSuffix(name, rdnsv6Suffix)
// 32 nibbles and 31 dots between them. // 32 nibbles and 31 dots between them.
if len(name) != 63 { if len(name) != 63 {
return netaddr.IP{}, false return netaddr.IP{}, false
@ -470,16 +477,14 @@ func rdnsNameToIPv6(name []byte) (ip netaddr.IP, ok bool) {
// respondReverse returns a DNS response to a PTR query. // respondReverse returns a DNS response to a PTR query.
// It is assumed that resp.Question is populated by respond before this is called. // It is assumed that resp.Question is populated by respond before this is called.
func (r *Resolver) respondReverse(query []byte, resp *response) ([]byte, error) { func (r *Resolver) respondReverse(query []byte, name string, resp *response) ([]byte, error) {
name := resp.Question.Name.Data[:resp.Question.Name.Length]
var ip netaddr.IP var ip netaddr.IP
var ok bool var ok bool
var err error var err error
switch { switch {
case bytes.HasSuffix(name, rdnsv4Suffix): case strings.HasSuffix(name, rdnsv4Suffix):
ip, ok = rdnsNameToIPv4(name) ip, ok = rdnsNameToIPv4(name)
case bytes.HasSuffix(name, rdnsv6Suffix): case strings.HasSuffix(name, rdnsv6Suffix):
ip, ok = rdnsNameToIPv6(name) ip, ok = rdnsNameToIPv6(name)
default: default:
return nil, errNotOurName return nil, errNotOurName
@ -489,7 +494,7 @@ func (r *Resolver) respondReverse(query []byte, resp *response) ([]byte, error)
// To avoid frustrating users, just log and delegate. // To avoid frustrating users, just log and delegate.
if !ok { if !ok {
// Without this conversion, escape analysis rules that resp escapes. // Without this conversion, escape analysis rules that resp escapes.
r.logf("parsing rdns: malformed name: %s", resp.Question.Name.String()) r.logf("parsing rdns: malformed name: %s", name)
return nil, errNotOurName return nil, errNotOurName
} }
@ -518,24 +523,23 @@ func (r *Resolver) respond(query []byte) ([]byte, error) {
resp.Header.RCode = dns.RCodeFormatError resp.Header.RCode = dns.RCodeFormatError
return marshalResponse(resp) return marshalResponse(resp)
} }
rawName := resp.Question.Name.Data[:resp.Question.Name.Length]
name := rawNameToLower(rawName)
// Always try to handle reverse lookups; delegate inside when not found. // Always try to handle reverse lookups; delegate inside when not found.
// This way, queries for exitent nodes do not leak, // This way, queries for exitent nodes do not leak,
// but we behave gracefully if non-Tailscale nodes exist in CGNATRange. // but we behave gracefully if non-Tailscale nodes exist in CGNATRange.
if resp.Question.Type == dns.TypePTR { if resp.Question.Type == dns.TypePTR {
return r.respondReverse(query, resp) return r.respondReverse(query, name, resp)
} }
// Delegate forward lookups when not a subdomain of rootDomain. // Delegate forward lookups when not a subdomain of rootDomain.
// We do this on bytes because Name.String() allocates. if !strings.HasSuffix(name, r.rootDomain) {
rawName := resp.Question.Name.Data[:resp.Question.Name.Length]
if !bytes.HasSuffix(rawName, r.rootDomain) {
return nil, errNotOurName return nil, errNotOurName
} }
switch resp.Question.Type { switch resp.Question.Type {
case dns.TypeA, dns.TypeAAAA, dns.TypeALL: case dns.TypeA, dns.TypeAAAA, dns.TypeALL:
name := resp.Question.Name.String()
resp.IP, resp.Header.RCode, err = r.Resolve(name) resp.IP, resp.Header.RCode, err = r.Resolve(name)
default: default:
resp.Header.RCode = dns.RCodeNotImplemented resp.Header.RCode = dns.RCodeNotImplemented

41
wgengine/tsdns/tsdns_test.go
Unescape Escape View File

@ -122,8 +122,7 @@ func TestRDNSNameToIPv4(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
name := []byte(tt.input) ip, ok := rdnsNameToIPv4(tt.input)
ip, ok := rdnsNameToIPv4(name)
if ok != tt.wantOK { if ok != tt.wantOK {
t.Errorf("ok = %v; want %v", ok, tt.wantOK) t.Errorf("ok = %v; want %v", ok, tt.wantOK)
} else if ok && ip != tt.wantIP { } else if ok && ip != tt.wantIP {
@ -168,8 +167,7 @@ func TestRDNSNameToIPv6(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
name := []byte(tt.input) ip, ok := rdnsNameToIPv6(tt.input)
ip, ok := rdnsNameToIPv6(name)
if ok != tt.wantOK { if ok != tt.wantOK {
t.Errorf("ok = %v; want %v", ok, tt.wantOK) t.Errorf("ok = %v; want %v", ok, tt.wantOK)
} else if ok && ip != tt.wantIP { } else if ok && ip != tt.wantIP {
@ -467,7 +465,7 @@ func TestConcurrentSetUpstreams(t *testing.T) {
wg.Wait() wg.Wait()
} }
var validIPv4Response = []byte{ var ipv4Response = []byte{
0x00, 0x00, // transaction id: 0 0x00, 0x00, // transaction id: 0
0x84, 0x00, // flags: response, authoritative, no error 0x84, 0x00, // flags: response, authoritative, no error
0x00, 0x01, // one question 0x00, 0x01, // one question
@ -484,7 +482,7 @@ var validIPv4Response = []byte{
0x01, 0x02, 0x03, 0x04, // A: 1.2.3.4 0x01, 0x02, 0x03, 0x04, // A: 1.2.3.4
} }
var validIPv6Response = []byte{ var ipv6Response = []byte{
0x00, 0x00, // transaction id: 0 0x00, 0x00, // transaction id: 0
0x84, 0x00, // flags: response, authoritative, no error 0x84, 0x00, // flags: response, authoritative, no error
0x00, 0x01, // one question 0x00, 0x01, // one question
@ -502,7 +500,24 @@ var validIPv6Response = []byte{
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0xb, 0xc, 0xd, 0xe, 0xf, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0xb, 0xc, 0xd, 0xe, 0xf,
} }
var validPTRResponse = []byte{ var ipv4UppercaseResponse = []byte{
0x00, 0x00, // transaction id: 0
0x84, 0x00, // flags: response, authoritative, no error
0x00, 0x01, // one question
0x00, 0x01, // one answer
0x00, 0x00, 0x00, 0x00, // no authority or additional RRs
// Question:
0x05, 0x54, 0x45, 0x53, 0x54, 0x31, 0x03, 0x49, 0x50, 0x4e, 0x03, 0x44, 0x45, 0x56, 0x00, // name
0x00, 0x01, 0x00, 0x01, // type A, class IN
// Answer:
0x05, 0x54, 0x45, 0x53, 0x54, 0x31, 0x03, 0x49, 0x50, 0x4e, 0x03, 0x44, 0x45, 0x56, 0x00, // name
0x00, 0x01, 0x00, 0x01, // type A, class IN
0x00, 0x00, 0x02, 0x58, // TTL: 600
0x00, 0x04, // length: 4 bytes
0x01, 0x02, 0x03, 0x04, // A: 1.2.3.4
}
var ptrResponse = []byte{
0x00, 0x00, // transaction id: 0 0x00, 0x00, // transaction id: 0
0x84, 0x00, // flags: response, authoritative, no error 0x84, 0x00, // flags: response, authoritative, no error
0x00, 0x01, // one question 0x00, 0x01, // one question
@ -548,10 +563,10 @@ func TestFull(t *testing.T) {
request []byte request []byte
response []byte response []byte
}{ }{
{"ipv4", dnspacket("test1.ipn.dev.", dns.TypeA), validIPv4Response}, {"ipv4", dnspacket("test1.ipn.dev.", dns.TypeA), ipv4Response},
{"ipv6", dnspacket("test2.ipn.dev.", dns.TypeAAAA), validIPv6Response}, {"ipv6", dnspacket("test2.ipn.dev.", dns.TypeAAAA), ipv6Response},
{"upper", dnspacket("TEST1.IPN.DEV.", dns.TypeA), validIPv4Response}, {"upper", dnspacket("TEST1.IPN.DEV.", dns.TypeA), ipv4UppercaseResponse},
{"ptr", dnspacket("4.3.2.1.in-addr.arpa.", dns.TypePTR), validPTRResponse}, {"ptr", dnspacket("4.3.2.1.in-addr.arpa.", dns.TypePTR), ptrResponse},
{"error", dnspacket("test3.ipn.dev.", dns.TypeA), nxdomainResponse}, {"error", dnspacket("test3.ipn.dev.", dns.TypeA), nxdomainResponse},
} }
@ -584,8 +599,8 @@ func TestAllocs(t *testing.T) {
query []byte query []byte
want int want int
}{ }{
// The only alloc is the slice created by dns.NewBuilder. // Name lowercasing and response slice created by dns.NewBuilder.
{"forward", dnspacket("test1.ipn.dev.", dns.TypeA), 1}, {"forward", dnspacket("test1.ipn.dev.", dns.TypeA), 2},
// 3 extra allocs in rdnsNameToIPv4 and one in marshalPTRRecord (dns.NewName). // 3 extra allocs in rdnsNameToIPv4 and one in marshalPTRRecord (dns.NewName).
{"reverse", dnspacket("4.3.2.1.in-addr.arpa.", dns.TypePTR), 5}, {"reverse", dnspacket("4.3.2.1.in-addr.arpa.", dns.TypePTR), 5},
} }

Write Preview
Loading…
Cancel
Save