From 48520b107f4ce6df32d772a75e13c1858b8800f6 Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Sat, 20 Sep 2014 13:41:32 +0100 Subject: [PATCH 01/54] Typo in the readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1274b9f..3f09a1c 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ An authoritative DNS nameserver that queries an [etcd](http://github.com/coreos/ - Support for TTLs - Global default on all records - Individual TTL values for individual records -- Runtime and application metrics are captured regularly for monitoring (stdout or grahite) +- Runtime and application metrics are captured regularly for monitoring (stdout or graphite) - Incoming query filters #### Production Readyness From a100666f3edbd6ee0264f60465e637b667391aa8 Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Sun, 21 Sep 2014 01:45:59 +0100 Subject: [PATCH 02/54] [WIP] --- format.go | 23 +++++++ main.go | 13 ++++ resolver.go | 20 +----- server.go | 98 ++++++++++++++++++++--------- update.go | 167 +++++++++++++++++++++++++++++++++++++++++++++++++ update_test.go | 1 + 6 files changed, 274 insertions(+), 48 deletions(-) create mode 100644 format.go create mode 100644 update.go create mode 100644 update_test.go diff --git a/format.go b/format.go new file mode 100644 index 0000000..acc89f8 --- /dev/null +++ b/format.go @@ -0,0 +1,23 @@ +package main + +import ( + "bytes" + "strings" +) + +// nameToKey returns a string representing the etcd version of a domain, replacing dots with slashes +// and reversing it (foo.net. -> /net/foo) +func nameToKey(name string, suffix string) string { + segments := strings.Split(name, ".") + + var keyBuffer bytes.Buffer + for i := len(segments) - 1; i >= 0; i-- { + if len(segments[i]) > 0 { + keyBuffer.WriteString("/") + keyBuffer.WriteString(segments[i]) + } + } + + keyBuffer.WriteString(suffix) + return keyBuffer.String() +} diff --git a/main.go b/main.go index 5245b6d..f28d0d2 100644 --- a/main.go +++ b/main.go @@ -30,6 +30,7 @@ var ( DefaultTtl uint32 `short:"t" long:"default-ttl" description:"Default TTL to return on records without an explicit TTL" default:"300"` Accept []string `long:"accept" description:"Limit DNS queries to a set of domain:[type,...] pairs"` Reject []string `long:"reject" description:"Limit DNS queries to a set of domain:[type,...] pairs"` + TsigSecret []string `short:"s" long:"tsig" description:"Transaction signature secret in the format name:secret"` } ) @@ -79,6 +80,17 @@ func main() { logger.Printf("Metric logging disabled") } + // Parse the tsig arguments + tsigSecret := map[string]string{} + for _, arg := range Options.TsigSecret { + components := strings.SplitN(arg, ":", 2) + if len(components) != 2 { + logger.Printf("Failed to parse TSIG argument") + continue + } + tsigSecret[dns.Fqdn(components[0])] = components[1] + } + // Start up the DNS resolver server server := &Server{ addr: Options.ListenAddress, @@ -87,6 +99,7 @@ func main() { rTimeout: time.Duration(5) * time.Second, wTimeout: time.Duration(5) * time.Second, defaultTtl: Options.DefaultTtl, + tsigSecret: tsigSecret, queryFilterer: &QueryFilterer{acceptFilters: parseFilters(Options.Accept), rejectFilters: parseFilters(Options.Reject)}} diff --git a/resolver.go b/resolver.go index e146e0f..5659804 100644 --- a/resolver.go +++ b/resolver.go @@ -1,7 +1,6 @@ package main import ( - "bytes" "fmt" "github.com/coreos/go-etcd/etcd" "github.com/miekg/dns" @@ -246,7 +245,7 @@ func (r *Resolver) AnswerQuestion(answers chan dns.RR, errors chan error, q dns. for rrType, _ := range converters { go func(rrType uint16) { - defer func() { recover() }() + defer recover() defer wg.Done() results, err := r.LookupAnswersForType(q.Name, rrType) @@ -325,23 +324,6 @@ func (r *Resolver) LookupAnswersForType(name string, rrType uint16) (answers []d return } -// nameToKey returns a string representing the etcd version of a domain, replacing dots with slashes -// and reversing it (foo.net. -> /net/foo) -func nameToKey(name string, suffix string) string { - segments := strings.Split(name, ".") - - var keyBuffer bytes.Buffer - for i := len(segments) - 1; i >= 0; i-- { - if len(segments[i]) > 0 { - keyBuffer.WriteString("/") - keyBuffer.WriteString(segments[i]) - } - } - - keyBuffer.WriteString(suffix) - return keyBuffer.String() -} - // Map of conversion functions that turn individual etcd nodes into dns.RR answers var converters = map[uint16]func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { diff --git a/server.go b/server.go index 3538f8a..c987216 100644 --- a/server.go +++ b/server.go @@ -15,51 +15,86 @@ type Server struct { rTimeout time.Duration wTimeout time.Duration defaultTtl uint32 + tsigSecret map[string]string queryFilterer *QueryFilterer } type Handler struct { resolver *Resolver queryFilterer *QueryFilterer + updateManager *DynamicUpdateManager // Metrics requestCounter metrics.Counter acceptCounter metrics.Counter rejectCounter metrics.Counter + // authFailCounter metrics.Counter + // authSuccessCounter metrics.Counter responseTimer metrics.Timer } func (h *Handler) Handle(response dns.ResponseWriter, req *dns.Msg) { h.requestCounter.Inc(1) h.responseTimer.Time(func() { - debugMsg("Handling incoming query for domain " + req.Question[0].Name) - - // Lookup the dns record for the request - // This method will add any answers to the message - var msg *dns.Msg - if h.queryFilterer.ShouldAcceptQuery(req) != true { - debugMsg("Query not accepted") - - h.rejectCounter.Inc(1) - - msg = new(dns.Msg) - msg.SetReply(req) - msg.SetRcode(req, dns.RcodeNameError) - msg.Authoritative = true - msg.RecursionAvailable = false - - // Add a useful TXT record - header := dns.RR_Header{Name: req.Question[0].Name, - Class: dns.ClassINET, - Rrtype: dns.TypeTXT} - msg.Ns = []dns.RR{&dns.TXT{header, []string{"Rejected query based on matched filters"}}} + debugMsg("Incoming message with opcode " + dns.OpcodeToString[req.MsgHdr.Opcode]) + + var res *dns.Msg + if req.MsgHdr.Opcode == dns.OpcodeQuery { + // TODO(tarnfeld): Support for multiple questions? + debugMsg("Handling incoming query for domain " + req.Question[0].Name) + + // Lookup the dns record for the request + // This method will add any answers to the message + if h.queryFilterer.ShouldAcceptQuery(req) != true { + debugMsg("Query not accepted") + + h.rejectCounter.Inc(1) + + res = new(dns.Msg) + res.SetReply(req) + res.SetRcode(req, dns.RcodeNameError) + res.Authoritative = true + res.RecursionAvailable = false + + // Add a useful TXT record + header := dns.RR_Header{Name: req.Question[0].Name, + Class: dns.ClassINET, + Rrtype: dns.TypeTXT} + res.Ns = []dns.RR{&dns.TXT{header, []string{"Rejected query based on matched filters"}}} + } else { + h.acceptCounter.Inc(1) + res = h.resolver.Lookup(req) + } + } else if req.MsgHdr.Opcode == dns.OpcodeUpdate { + zone := req.Question[0].Name + debugMsg("Handling incoming update for zone " + zone) + + res = new(dns.Msg) + res.SetReply(req) + + // Authenticate the request + if req.IsTsig() != nil && response.TsigStatus() == nil { + sig := req.IsTsig() + debugMsg("Authenticated update request") + + // Verify the tsig is for the correct zone + if sig.Hdr.Name != zone { + res.SetRcode(req, dns.RcodeBadSig) + } else { + res = h.updateManager.Update(zone, req) + } + } else { + debugMsg("Authentication failed") + res.SetRcode(req, dns.RcodeNotAuth) + } } else { - h.acceptCounter.Inc(1) - msg = h.resolver.Lookup(req) + res = new(dns.Msg) + res.SetReply(req) + res.SetRcode(req, dns.RcodeNotImplemented) } - if msg != nil { - err := response.WriteMsg(msg) + if res != nil { + err := response.WriteMsg(res) if err != nil { debugMsg("Error writing message: ", err) } @@ -94,20 +129,23 @@ func (s *Server) Run() { metrics.Register("request.handler.udp.filter_rejects", udpRejectCounter) resolver := Resolver{etcd: s.etcd, defaultTtl: s.defaultTtl} + updateManager := DynamicUpdateManager{etcd: s.etcd} tcpDNShandler := &Handler{ resolver: &resolver, requestCounter: tcpRequestCounter, acceptCounter: tcpAcceptCounter, rejectCounter: tcpRejectCounter, responseTimer: tcpResponseTimer, - queryFilterer: s.queryFilterer} + queryFilterer: s.queryFilterer, + updateManager: &updateManager} udpDNShandler := &Handler{ resolver: &resolver, requestCounter: udpRequestCounter, acceptCounter: udpAcceptCounter, rejectCounter: udpRejectCounter, responseTimer: udpResponseTimer, - queryFilterer: s.queryFilterer} + queryFilterer: s.queryFilterer, + updateManager: &updateManager} udpHandler := dns.NewServeMux() tcpHandler := dns.NewServeMux() @@ -119,14 +157,16 @@ func (s *Server) Run() { Net: "tcp", Handler: tcpHandler, ReadTimeout: s.rTimeout, - WriteTimeout: s.wTimeout} + WriteTimeout: s.wTimeout, + TsigSecret: s.tsigSecret} udpServer := &dns.Server{Addr: s.Addr(), Net: "udp", Handler: udpHandler, UDPSize: 65535, ReadTimeout: s.rTimeout, - WriteTimeout: s.wTimeout} + WriteTimeout: s.wTimeout, + TsigSecret: s.tsigSecret} go s.start(udpServer) go s.start(tcpServer) diff --git a/update.go b/update.go new file mode 100644 index 0000000..3f821f4 --- /dev/null +++ b/update.go @@ -0,0 +1,167 @@ +package main + +import ( + "github.com/coreos/go-etcd/etcd" + "github.com/miekg/dns" + "fmt" +) + +type DynamicUpdateManager struct { + etcd *etcd.Client + etcdPrefix string +} + +func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) { + msg = new(dns.Msg) + msg.SetReply(req) + + // Verify the updates are within the zone + for _, rr := range req.Answer { + if dns.CompareDomainName(rr.Header().Name, zone) != dns.CountLabel(zone) { + msg.SetRcode(req, dns.RcodeNotZone) + return msg + } + } + for _, rr := range req.Ns { + if dns.CompareDomainName(rr.Header().Name, zone) != dns.CountLabel(zone) { + msg.SetRcode(req, dns.RcodeNotZone) + return msg + } + } + + // Ensure we recover from any panic to help ensure we unlock the locked domains + defer func() { + if r := recover(); r != nil { + debugMsg("[PANIC] " + fmt.Sprint(r)) + msg.SetRcode(req, dns.RcodeServerFailure) + } + }() + + // Attempt to acquire a lock on all of the domains referenced in the update + // If any lock attempt fails, all acquired locks will be released and no + // update will be applied. + for _, rr := range req.Answer { + lock := lockDomain(u.etcd, rr.Header().Name) + defer lock.Unlock() + } + for _, rr := range req.Ns { + lock := lockDomain(u.etcd, rr.Header().Name) + defer lock.Unlock() + } + + // Validate the prerequisites of the update request + validationStatus := validatePrerequisites(req.Answer) + msg.SetRcode(req, validationStatus) + + // If we failed to validate prerequisites, return the error + if validationStatus != dns.RcodeSuccess { + return msg + } + + // Perform the updates to the domain name system + msg.SetRcode(req, performUpdate(req.Ns)) + + return +} + +// validatePrerequisites will perform all necessary validation checks against +// update prerequisites and return the relevent status is validation fails, +// otherwise NOERROR(0) will be returned. +func validatePrerequisites(rr []dns.RR) (rcode int) { + for _, record := range rr { + header := record.Header() + if header.Ttl != 0 { + return dns.RcodeFormatError + } + + if header.Class == dns.ClassANY { + if header.Rdlength != 0 { + return dns.RcodeFormatError + } + if header.Rrtype == dns.TypeANY { + // TODO (dns.TypeANY, header.Name) dns.RcodeNameError + } else { + // TODO (header.Rrtype, header.Name) dns.RcodeNXRrset + } + } else if header.Class == dns.ClassNone { + if header.Rdlength != 0 { + return dns.RcodeFormatError + } + if header.Rrtype == dns.TypeANY { + // TODO (dns.TypeANY, header.Name) dns.RcodeYXDomain + } else { + // TODO (header.Rrtype, header.Name) dns.RcodeYXRrset + } + } else if header.Class == dns.ClassINET { + // Compare rr with the same type+name in the db (value comparison) + } else { + return dns.RcodeFormatError + } + } + + return dns.RcodeSuccess +} + +// performUpdate will commit the requested updates to the database +// It is assumed by this point all prerequisites have been validated and all +// domains are locked. +func performUpdate(rr []dns.RR) (rcode int) { + return dns.RcodeSuccess +} + +type DomainLock struct { + etcd *etcd.Client + domain string + index uint64 + lockPath string + lockExpiry uint64 +} + +func (l *DomainLock) Unlock() error { + debugMsg("Unlocking " + l.domain + " from " + l.lockPath) + + _, err := l.etcd.CompareAndDelete(l.lockPath, "", l.index) + if err != nil { + debugMsg(err) + } + + return err +} + +func (l *DomainLock) Lock() error { + if l.index > 0 || l.lockPath != "" { + return nil + } + + l.lockPath = nameToKey(l.domain, "/._UPDATE_LOCK") + debugMsg("Locking " + l.domain + " at " + l.lockPath) + + response, err := l.etcd.Create(l.lockPath, "", l.lockExpiry) + if err != nil { + panic("Failed to acquire lock on domain " + l.domain) + } + + l.index = response.Node.CreatedIndex + return err +} + +// IsLocked will return true if +func (l *DomainLock) IsLocked() (locked bool) { + locked = false + + // Verify that the domain is locked and that it's *our* lock + if l.lockPath != "" { + response, err := l.etcd.Get(l.lockPath, false, false) + if err == nil { + locked = response.Node.CreatedIndex == l.index + } + } + + return +} + +func lockDomain(etcd *etcd.Client, domain string) (lock *DomainLock) { + lock = &DomainLock{etcd: etcd, domain: domain, lockExpiry: 30} + defer lock.Lock() + return lock +} diff --git a/update_test.go b/update_test.go new file mode 100644 index 0000000..06ab7d0 --- /dev/null +++ b/update_test.go @@ -0,0 +1 @@ +package main From 159702b803fa7577bab3a15f25646879fb6a860c Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Mon, 22 Sep 2014 10:36:21 +0100 Subject: [PATCH 03/54] WIP --- filter.go | 25 ++++++ format.go | 23 ----- lock.go | 75 +++++++++++++++++ lock_test.go | 1 + main.go | 29 +------ record.go | 232 +++++++++++++++++++++++++++++++++++++++++++++++++++ resolver.go | 166 +----------------------------------- server.go | 2 +- update.go | 164 +++++++++++++++++------------------- 9 files changed, 415 insertions(+), 302 deletions(-) delete mode 100644 format.go create mode 100644 lock.go create mode 100644 lock_test.go create mode 100644 record.go diff --git a/filter.go b/filter.go index 6c89a59..1355236 100644 --- a/filter.go +++ b/filter.go @@ -68,3 +68,28 @@ func (f *QueryFilterer) ShouldAcceptQuery(req *dns.Msg) bool { return accepted } + +// parseFilters will convert a string into a Query Filter structure. The accepted +// format for input is [domain]:[type,type,...]. For example... +// +// - "domain:A,AAAA" # Match all A and AAAA queries within `domain` +// - ":TXT" # Matches only TXT queries for any domain +// - "domain:" # Matches any query within `domain` +func parseFilters(filters []string) []QueryFilter { + parsedFilters := make([]QueryFilter, 0) + for _, filter := range filters { + components := strings.Split(filter, ":") + if len(components) != 2 { + logger.Printf("Expected only one colon ([domain]:[type,type...])") + continue + } + + domain := dns.Fqdn(components[0]) + types := strings.Split(components[1], ",") + + debugMsg("Adding filter with domain '" + domain + "' and types '" + strings.Join(types, ",") + "'") + parsedFilters = append(parsedFilters, QueryFilter{domain, types}) + } + + return parsedFilters +} diff --git a/format.go b/format.go deleted file mode 100644 index acc89f8..0000000 --- a/format.go +++ /dev/null @@ -1,23 +0,0 @@ -package main - -import ( - "bytes" - "strings" -) - -// nameToKey returns a string representing the etcd version of a domain, replacing dots with slashes -// and reversing it (foo.net. -> /net/foo) -func nameToKey(name string, suffix string) string { - segments := strings.Split(name, ".") - - var keyBuffer bytes.Buffer - for i := len(segments) - 1; i >= 0; i-- { - if len(segments[i]) > 0 { - keyBuffer.WriteString("/") - keyBuffer.WriteString(segments[i]) - } - } - - keyBuffer.WriteString(suffix) - return keyBuffer.String() -} diff --git a/lock.go b/lock.go new file mode 100644 index 0000000..1c65a40 --- /dev/null +++ b/lock.go @@ -0,0 +1,75 @@ +package main + +type DomainLock struct { + etcd *etcd.Client + domain string + index uint64 + lockPath string + lockExpiry uint64 +} + +func (l *DomainLock) Lock(shouldPanic bool) error { + if l.index > 0 || l.lockPath != "" { + return + } + + l.lockPath = nameToKey(l.domain, "/._UPDATE_LOCK") + debugMsg("Locking " + l.domain + " at " + l.lockPath) + + response, err := l.etcd.Create(l.lockPath, "", l.lockExpiry) + if err != nil { + debugMsg("Failed to acquire lock on domain " + l.domain) + debugMsg(err) + + if shouldPanic { + panic("Failed to acquire lock on domain " + l.domain) + } + + return err + } + + l.index = response.Node.CreatedIndex + return nil +} + +func (l *DomainLock) Unlock(shouldPanic bool) error { + debugMsg("Unlocking " + l.domain + " from " + l.lockPath) + + _, err := l.etcd.CompareAndDelete(l.lockPath, "", l.index) + if err != nil { + debugMsg("Failed to unlock domain " + l.domain) + debugMsg(err) + + if shouldPanic { + panic("Failed to unlock domain " + l.domain) + } + } + + return err +} + +// IsLocked will return true if the lock is still acquired by this instance +// Since locks may have an expiry, it is possible for the lock to expire and be +// acquired by another party +func (l *DomainLock) IsLocked() (locked bool) { + locked = false + + // Verify that the domain is locked and that it's *our* lock + if l.lockPath != "" { + response, err := l.etcd.Get(l.lockPath, false, false) + if err == nil { + locked = response.Node.CreatedIndex == l.index + } + } + + return +} + +// lockDomain will lock the given domain in the given etcd cluster, and return +// the DomainLock struct pre-populated such that calling DomainLock.Unlock() +// will release the lock. +func lockDomain(etcd *etcd.Client, domain string) (lock *DomainLock) { + lock = &DomainLock{etcd: etcd, domain: domain, lockExpiry: 30} + defer lock.Lock() + return lock +} diff --git a/lock_test.go b/lock_test.go new file mode 100644 index 0000000..06ab7d0 --- /dev/null +++ b/lock_test.go @@ -0,0 +1 @@ +package main diff --git a/main.go b/main.go index f28d0d2..4cfde30 100644 --- a/main.go +++ b/main.go @@ -30,7 +30,7 @@ var ( DefaultTtl uint32 `short:"t" long:"default-ttl" description:"Default TTL to return on records without an explicit TTL" default:"300"` Accept []string `long:"accept" description:"Limit DNS queries to a set of domain:[type,...] pairs"` Reject []string `long:"reject" description:"Limit DNS queries to a set of domain:[type,...] pairs"` - TsigSecret []string `short:"s" long:"tsig" description:"Transaction signature secret in the format name:secret"` + TsigSecret []string `short:"s" long:"tsig" description:"Transaction signature secret in the format zone:secret"` } ) @@ -80,7 +80,7 @@ func main() { logger.Printf("Metric logging disabled") } - // Parse the tsig arguments + // Parse the tsig arguments, these are formatted as "zone:secret" tsigSecret := map[string]string{} for _, arg := range Options.TsigSecret { components := strings.SplitN(arg, ":", 2) @@ -129,31 +129,6 @@ func debugMsg(v ...interface{}) { } } -// parseFilters will convert a string into a Query Filter structure. The accepted -// format for input is [domain]:[type,type,...]. For example... -// -// - "domain:A,AAAA" # Match all A and AAAA queries within `domain` -// - ":TXT" # Matches only TXT queries for any domain -// - "domain:" # Matches any query within `domain` -func parseFilters(filters []string) []QueryFilter { - parsedFilters := make([]QueryFilter, 0) - for _, filter := range filters { - components := strings.Split(filter, ":") - if len(components) != 2 { - logger.Printf("Expected only one colon ([domain]:[type,type...])") - continue - } - - domain := dns.Fqdn(components[0]) - types := strings.Split(components[1], ",") - - debugMsg("Adding filter with domain '" + domain + "' and types '" + strings.Join(types, ",") + "'") - parsedFilters = append(parsedFilters, QueryFilter{domain, types}) - } - - return parsedFilters -} - func init() { runtime.GOMAXPROCS(runtime.NumCPU()) } diff --git a/record.go b/record.go new file mode 100644 index 0000000..5d99b0b --- /dev/null +++ b/record.go @@ -0,0 +1,232 @@ +package main + +import ( + "github.com/coreos/go-etcd/etcd" + "github.com/miekg/dns" + "bytes" + "strings" +) + +type EtcdRecord struct { + node *etcd.Node + ttl uint32 +} + +// convertNodeToRR will convert an etcd node with a raw value into a dns.RR +// record, returning an error if the conversion fails +func convertNodeToRR(node *etd.Node, header dns.RR_Header) (rr dns.RR, err error) { + return convertersToRR[header.Rrtype](node, header) +} + +// convertRRToNode will convert a DNS RR and it's type specific values to an +// etcd node with a raw value and key path, returning an error if the conversion +// fails +func convertRRToNode(rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return convertersFromRR[header.Rrtype](rr, header) +} + +// nameToKey returns a string representing the etcd version of a domain, replacing dots with slashes +// and reversing it (foo.net. -> /net/foo) +func nameToKey(name string, suffix string) string { + segments := strings.Split(name, ".") + + var keyBuffer bytes.Buffer + for i := len(segments) - 1; i >= 0; i-- { + if len(segments[i]) > 0 { + keyBuffer.WriteString("/") + keyBuffer.WriteString(segments[i]) + } + } + + keyBuffer.WriteString(suffix) + return keyBuffer.String() +} + +// Map of conversion functions that turn individual etcd nodes into dns.RR answers +var convertersToRR = map[uint16]func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + + dns.TypeA: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + + ip := net.ParseIP(node.Value) + if ip == nil { + err = &NodeConversionError{ + Node: node, + Message: fmt.Sprintf("Failed to parse %s as IP Address", node.Value), + AttemptedType: dns.TypeA, + } + } else if ip.To4() == nil { + err = &NodeConversionError{ + Node: node, + Message: fmt.Sprintf("Value %s isn't an IPv4 address", node.Value), + AttemptedType: dns.TypeA, + } + } else { + rr = &dns.A{header, ip} + } + + return + }, + + dns.TypeAAAA: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + + ip := net.ParseIP(node.Value) + if ip == nil { + err = &NodeConversionError{ + Node: node, + Message: fmt.Sprintf("Failed to parse IP Address %s", node.Value), + AttemptedType: dns.TypeAAAA} + } else if ip.To16() == nil { + err = &NodeConversionError{ + Node: node, + Message: fmt.Sprintf("Value %s isn't an IPv6 address", node.Value), + AttemptedType: dns.TypeA} + } else { + rr = &dns.AAAA{header, ip} + } + return + }, + + dns.TypeTXT: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + rr = &dns.TXT{header, []string{node.Value}} + return + }, + + dns.TypeCNAME: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + rr = &dns.CNAME{header, dns.Fqdn(node.Value)} + return + }, + + dns.TypeNS: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + rr = &dns.NS{header, dns.Fqdn(node.Value)} + return + }, + + dns.TypePTR: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + labels, ok := dns.IsDomainName(node.Value) + + if (ok && labels > 0) { + rr = &dns.PTR{header, dns.Fqdn(node.Value)} + } else { + err = &NodeConversionError{ + Node: node, + Message: fmt.Sprintf("Value '%s' isn't a valid domain name", node.Value), + AttemptedType: dns.TypePTR} + } + return + }, + + dns.TypeSRV: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + parts := strings.SplitN(node.Value, "\t", 4) + + if len(parts) != 4 { + err = &NodeConversionError{ + Node: node, + Message: fmt.Sprintf("Value %s isn't valid for SRV", node.Value), + AttemptedType: dns.TypeSRV} + } else { + + priority, err := strconv.ParseUint(parts[0], 10, 16) + if err != nil { + return nil, err + } + + weight, err := strconv.ParseUint(parts[1], 10, 16) + if err != nil { + return nil, err + } + + port, err := strconv.ParseUint(parts[2], 10, 16) + if err != nil { + return nil, err + } + + target := dns.Fqdn(parts[3]) + + rr = &dns.SRV{ + header, + uint16(priority), + uint16(weight), + uint16(port), + target} + } + return + }, + + dns.TypeSOA: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { + parts := strings.SplitN(node.Value, "\t", 6) + + if len(parts) < 6 { + err = &NodeConversionError{ + Node: node, + Message: fmt.Sprintf("Value %s isn't valid for SOA", node.Value), + AttemptedType: dns.TypeSOA} + } else { + refresh, err := strconv.ParseUint(parts[2], 10, 32) + if err != nil { + return nil, err + } + + retry, err := strconv.ParseUint(parts[3], 10, 32) + if err != nil { + return nil, err + } + + expire, err := strconv.ParseUint(parts[4], 10, 32) + if err != nil { + return nil, err + } + + minttl, err := strconv.ParseUint(parts[5], 10, 32) + if err != nil { + return nil, err + } + + rr = &dns.SOA{ + Hdr: header, + Ns: dns.Fqdn(parts[0]), + Mbox: dns.Fqdn(parts[1]), + Refresh: uint32(refresh), + Retry: uint32(retry), + Expire: uint32(expire), + Minttl: uint32(minttl)} + } + + return + }, +} + +// Map of conversion functions that turn dns.RR answers into individual etcd nodes +var convertersFromRR = map[uint16]func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + + dns.TypeA: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return nil, nil + }, + + dns.TypeAAAA: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return nil, nil + }, + + dns.TypeTXT: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return nil, nil + }, + + dns.TypeCNAME: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return nil, nil + }, + + dns.TypeNS: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return nil, nil + }, + + dns.TypePTR: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return nil, nil + }, + + dns.TypeSRV: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return nil, nil + }, + + dns.TypeSOA: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + return nil, nil + }, +} diff --git a/resolver.go b/resolver.go index 5659804..6f739c9 100644 --- a/resolver.go +++ b/resolver.go @@ -18,11 +18,6 @@ type Resolver struct { defaultTtl uint32 } -type EtcdRecord struct { - node *etcd.Node - ttl uint32 -} - // GetFromStorage looks up a key in etcd and returns a slice of nodes. It supports two storage structures; // - File: /foo/bar/.A -> "value" // - Directory: /foo/bar/.A/0 -> "value-0" @@ -241,9 +236,9 @@ func (r *Resolver) AnswerQuestion(answers chan dns.RR, errors chan error, q dns. debugMsg("Answering question ", q) if q.Qtype == dns.TypeANY { - wg.Add(len(converters)) + wg.Add(len(convertersToRR)) - for rrType, _ := range converters { + for rrType, _ := range convertersToRR { go func(rrType uint16) { defer recover() defer wg.Done() @@ -258,7 +253,7 @@ func (r *Resolver) AnswerQuestion(answers chan dns.RR, errors chan error, q dns. } }(rrType) } - } else if _, ok := converters[q.Qtype]; ok { + } else if _, ok := convertersToRR[q.Qtype]; ok { wg.Add(1) go func() { @@ -311,7 +306,7 @@ func (r *Resolver) LookupAnswersForType(name string, rrType uint16) (answers []d for i, node := range nodes { header := dns.RR_Header{Name: name, Class: dns.ClassINET, Rrtype: rrType, Ttl: node.ttl} - answer, err := converters[rrType](node.node, header) + answer, err := convertersToRR[rrType](node.node, header) if err != nil { debugMsg("Error converting type: ", err) @@ -323,156 +318,3 @@ func (r *Resolver) LookupAnswersForType(name string, rrType uint16) (answers []d return } - -// Map of conversion functions that turn individual etcd nodes into dns.RR answers -var converters = map[uint16]func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - - dns.TypeA: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - - ip := net.ParseIP(node.Value) - if ip == nil { - err = &NodeConversionError{ - Node: node, - Message: fmt.Sprintf("Failed to parse %s as IP Address", node.Value), - AttemptedType: dns.TypeA, - } - } else if ip.To4() == nil { - err = &NodeConversionError{ - Node: node, - Message: fmt.Sprintf("Value %s isn't an IPv4 address", node.Value), - AttemptedType: dns.TypeA, - } - } else { - rr = &dns.A{header, ip} - } - - return - }, - - dns.TypeAAAA: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - - ip := net.ParseIP(node.Value) - if ip == nil { - err = &NodeConversionError{ - Node: node, - Message: fmt.Sprintf("Failed to parse IP Address %s", node.Value), - AttemptedType: dns.TypeAAAA} - } else if ip.To16() == nil { - err = &NodeConversionError{ - Node: node, - Message: fmt.Sprintf("Value %s isn't an IPv6 address", node.Value), - AttemptedType: dns.TypeA} - } else { - rr = &dns.AAAA{header, ip} - } - return - }, - - dns.TypeTXT: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - rr = &dns.TXT{header, []string{node.Value}} - return - }, - - dns.TypeCNAME: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - rr = &dns.CNAME{header, dns.Fqdn(node.Value)} - return - }, - - dns.TypeNS: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - rr = &dns.NS{header, dns.Fqdn(node.Value)} - return - }, - - dns.TypePTR: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - labels, ok := dns.IsDomainName(node.Value) - - if (ok && labels > 0) { - rr = &dns.PTR{header, dns.Fqdn(node.Value)} - } else { - err = &NodeConversionError{ - Node: node, - Message: fmt.Sprintf("Value '%s' isn't a valid domain name", node.Value), - AttemptedType: dns.TypePTR} - } - return - }, - - dns.TypeSRV: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - parts := strings.SplitN(node.Value, "\t", 4) - - if len(parts) != 4 { - err = &NodeConversionError{ - Node: node, - Message: fmt.Sprintf("Value %s isn't valid for SRV", node.Value), - AttemptedType: dns.TypeSRV} - } else { - - priority, err := strconv.ParseUint(parts[0], 10, 16) - if err != nil { - return nil, err - } - - weight, err := strconv.ParseUint(parts[1], 10, 16) - if err != nil { - return nil, err - } - - port, err := strconv.ParseUint(parts[2], 10, 16) - if err != nil { - return nil, err - } - - target := dns.Fqdn(parts[3]) - - rr = &dns.SRV{ - header, - uint16(priority), - uint16(weight), - uint16(port), - target} - } - return - }, - - dns.TypeSOA: func (node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - parts := strings.SplitN(node.Value, "\t", 6) - - if len(parts) < 6 { - err = &NodeConversionError{ - Node: node, - Message: fmt.Sprintf("Value %s isn't valid for SOA", node.Value), - AttemptedType: dns.TypeSOA} - } else { - refresh, err := strconv.ParseUint(parts[2], 10, 32) - if err != nil { - return nil, err - } - - retry, err := strconv.ParseUint(parts[3], 10, 32) - if err != nil { - return nil, err - } - - expire, err := strconv.ParseUint(parts[4], 10, 32) - if err != nil { - return nil, err - } - - minttl, err := strconv.ParseUint(parts[5], 10, 32) - if err != nil { - return nil, err - } - - rr = &dns.SOA{ - Hdr: header, - Ns: dns.Fqdn(parts[0]), - Mbox: dns.Fqdn(parts[1]), - Refresh: uint32(refresh), - Retry: uint32(retry), - Expire: uint32(expire), - Minttl: uint32(minttl)} - } - - return - }, -} diff --git a/server.go b/server.go index c987216..2ba56a6 100644 --- a/server.go +++ b/server.go @@ -129,7 +129,7 @@ func (s *Server) Run() { metrics.Register("request.handler.udp.filter_rejects", udpRejectCounter) resolver := Resolver{etcd: s.etcd, defaultTtl: s.defaultTtl} - updateManager := DynamicUpdateManager{etcd: s.etcd} + updateManager := DynamicUpdateManager{etcd: s.etcd, resolver: &resolver} tcpDNShandler := &Handler{ resolver: &resolver, requestCounter: tcpRequestCounter, diff --git a/update.go b/update.go index 3f821f4..9916847 100644 --- a/update.go +++ b/update.go @@ -9,27 +9,34 @@ import ( type DynamicUpdateManager struct { etcd *etcd.Client etcdPrefix string + resolver *Resolver } +// Update will perform the necessary logic to update the DNS database with +// the changes described in the RFC-2136 formatted DNS message given. +// The return value will be the response message to send back to the client. +// It is assumed at this level the client has already authenticated and proven +// their right to update records in the given zone. func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) { + + rrsets := []dns.RR{req.Answer, req.Ns} msg = new(dns.Msg) msg.SetReply(req) - // Verify the updates are within the zone - for _, rr := range req.Answer { - if dns.CompareDomainName(rr.Header().Name, zone) != dns.CountLabel(zone) { - msg.SetRcode(req, dns.RcodeNotZone) - return msg - } - } - for _, rr := range req.Ns { - if dns.CompareDomainName(rr.Header().Name, zone) != dns.CountLabel(zone) { - msg.SetRcode(req, dns.RcodeNotZone) - return msg + // Verify the updates are within the zone we're modifying, since cross + // zone updates are invalid. + for _, rrs := range rrsets { + for _, rr := range rrs { + if dns.CompareDomainName(rr.Header().Name, zone) != dns.CountLabel(zone) { + debugMsg("Domain " + rr.Header().Name + " is not in the " + zone + " zone") + msg.SetRcode(req, dns.RcodeNotZone) + return + } } } - // Ensure we recover from any panic to help ensure we unlock the locked domains + // Ensure we recover from any panicking goroutine, this helps ensure we don't + // leave any acquired locks around if possible defer func() { if r := recover(); r != nil { debugMsg("[PANIC] " + fmt.Sprint(r)) @@ -40,34 +47,35 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) // Attempt to acquire a lock on all of the domains referenced in the update // If any lock attempt fails, all acquired locks will be released and no // update will be applied. - for _, rr := range req.Answer { - lock := lockDomain(u.etcd, rr.Header().Name) - defer lock.Unlock() - } - for _, rr := range req.Ns { - lock := lockDomain(u.etcd, rr.Header().Name) - defer lock.Unlock() + for _, rrs := range rrsets { + for _, rr := range rrs { + lock := lockDomain(u.etcd, rr.Header().Name) + defer lock.Unlock() + } } - // Validate the prerequisites of the update request - validationStatus := validatePrerequisites(req.Answer) - msg.SetRcode(req, validationStatus) - - // If we failed to validate prerequisites, return the error + // Validate the prerequisites of the update, returning immediately if they + // are not satisfied. + validationStatus := validatePrerequisites(req.Answer, u.resolver) if validationStatus != dns.RcodeSuccess { - return msg + msg.SetRcode(req, validationStatus) + return } // Perform the updates to the domain name system + // This is not inside any kind of transaction, so a failure here *can* + // result in a partially updated zone. + // TODO(tarnfeld): Figure out a way of rolling back changes, perhaps make + // use of the etcd indexes? msg.SetRcode(req, performUpdate(req.Ns)) return } // validatePrerequisites will perform all necessary validation checks against -// update prerequisites and return the relevent status is validation fails, +// update prerequisites and return the relevant status is validation fails, // otherwise NOERROR(0) will be returned. -func validatePrerequisites(rr []dns.RR) (rcode int) { +func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { for _, record := range rr { header := record.Header() if header.Ttl != 0 { @@ -77,23 +85,46 @@ func validatePrerequisites(rr []dns.RR) (rcode int) { if header.Class == dns.ClassANY { if header.Rdlength != 0 { return dns.RcodeFormatError - } - if header.Rrtype == dns.TypeANY { - // TODO (dns.TypeANY, header.Name) dns.RcodeNameError + } else if header.Rrtype == dns.TypeANY { + if answers, ok := resolver.LookupAnswersForType(header.Name, dns.TypeANY); len(answers) > 0 { + if ok != nil { + return dns.RcodeServerFailure + } + return dns.RcodeNameError + } } else { - // TODO (header.Rrtype, header.Name) dns.RcodeNXRrset + if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); len(answers) > 0 { + if ok != nil { + return dns.RcodeServerFailure + } + return dns.RcodeNXRrset + } } } else if header.Class == dns.ClassNone { if header.Rdlength != 0 { return dns.RcodeFormatError - } - if header.Rrtype == dns.TypeANY { - // TODO (dns.TypeANY, header.Name) dns.RcodeYXDomain + } else if header.Rrtype == dns.TypeANY { + if answers, ok := resolver.LookupAnswersForType(header.Name, dns.TypeANY)); len(answers) == 0 { + if ok != nil { + return dns.RcodeServerFailure + } + return dns.RcodeYXDomain + } } else { - // TODO (header.Rrtype, header.Name) dns.RcodeYXRrset + if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype)); len(answers) == 0 { + if ok != nil { + return dns.RcodeServerFailure + } + return dns.RcodeYXRrset + } } } else if header.Class == dns.ClassINET { - // Compare rr with the same type+name in the db (value comparison) + if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); answers != rr { + if ok != nil { + return dns.RcodeServerFailure + } + return false // TODO(tarnfekd): What error type should this be? + } } else { return dns.RcodeFormatError } @@ -109,59 +140,14 @@ func performUpdate(rr []dns.RR) (rcode int) { return dns.RcodeSuccess } -type DomainLock struct { - etcd *etcd.Client - domain string - index uint64 - lockPath string - lockExpiry uint64 -} - -func (l *DomainLock) Unlock() error { - debugMsg("Unlocking " + l.domain + " from " + l.lockPath) - - _, err := l.etcd.CompareAndDelete(l.lockPath, "", l.index) - if err != nil { - debugMsg(err) - } - - return err -} - -func (l *DomainLock) Lock() error { - if l.index > 0 || l.lockPath != "" { - return nil - } - - l.lockPath = nameToKey(l.domain, "/._UPDATE_LOCK") - debugMsg("Locking " + l.domain + " at " + l.lockPath) - - response, err := l.etcd.Create(l.lockPath, "", l.lockExpiry) - if err != nil { - panic("Failed to acquire lock on domain " + l.domain) - } - - l.index = response.Node.CreatedIndex - return err -} - -// IsLocked will return true if -func (l *DomainLock) IsLocked() (locked bool) { - locked = false - - // Verify that the domain is locked and that it's *our* lock - if l.lockPath != "" { - response, err := l.etcd.Get(l.lockPath, false, false) - if err == nil { - locked = response.Node.CreatedIndex == l.index - } - } - - return +// nameInUser will return true if the name in the dns.RR_Header given is +// already in use, or false if not. +func nameInUse(header dns.RR_Header) []dns.RR { + return false } -func lockDomain(etcd *etcd.Client, domain string) (lock *DomainLock) { - lock = &DomainLock{etcd: etcd, domain: domain, lockExpiry: 30} - defer lock.Lock() - return lock +// nameInUser will return true if the name AND type in the dns.RR_Header given +// is are already in use. +func nameAndTypeInUse(header dns.RR_Header) bool { + return false } From ffca58da10ae1251302fea188600c9fbc05c67a5 Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Tue, 23 Sep 2014 09:36:08 +0100 Subject: [PATCH 04/54] Fixed compiler errors --- lock.go | 10 +++++++--- record.go | 25 ++++++++++++++----------- resolver.go | 2 -- update.go | 23 +++++++++++------------ 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/lock.go b/lock.go index 1c65a40..8ac9af2 100644 --- a/lock.go +++ b/lock.go @@ -1,5 +1,9 @@ package main +import ( + "github.com/coreos/go-etcd/etcd" +) + type DomainLock struct { etcd *etcd.Client domain string @@ -10,7 +14,7 @@ type DomainLock struct { func (l *DomainLock) Lock(shouldPanic bool) error { if l.index > 0 || l.lockPath != "" { - return + return nil } l.lockPath = nameToKey(l.domain, "/._UPDATE_LOCK") @@ -66,10 +70,10 @@ func (l *DomainLock) IsLocked() (locked bool) { } // lockDomain will lock the given domain in the given etcd cluster, and return -// the DomainLock struct pre-populated such that calling DomainLock.Unlock() +// the DomainLock struct pre-populated such that calling Domain Unlock() // will release the lock. func lockDomain(etcd *etcd.Client, domain string) (lock *DomainLock) { lock = &DomainLock{etcd: etcd, domain: domain, lockExpiry: 30} - defer lock.Lock() + defer lock.Lock(true) return lock } diff --git a/record.go b/record.go index 5d99b0b..e07b8c3 100644 --- a/record.go +++ b/record.go @@ -5,6 +5,9 @@ import ( "github.com/miekg/dns" "bytes" "strings" + "fmt" + "strconv" + "net" ) type EtcdRecord struct { @@ -14,14 +17,14 @@ type EtcdRecord struct { // convertNodeToRR will convert an etcd node with a raw value into a dns.RR // record, returning an error if the conversion fails -func convertNodeToRR(node *etd.Node, header dns.RR_Header) (rr dns.RR, err error) { +func convertNodeToRR(node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { return convertersToRR[header.Rrtype](node, header) } // convertRRToNode will convert a DNS RR and it's type specific values to an // etcd node with a raw value and key path, returning an error if the conversion // fails -func convertRRToNode(rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { +func convertRRToNode(rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return convertersFromRR[header.Rrtype](rr, header) } @@ -196,37 +199,37 @@ var convertersToRR = map[uint16]func (node *etcd.Node, header dns.RR_Header) (rr } // Map of conversion functions that turn dns.RR answers into individual etcd nodes -var convertersFromRR = map[uint16]func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { +var convertersFromRR = map[uint16]func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - dns.TypeA: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + dns.TypeA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return nil, nil }, - dns.TypeAAAA: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + dns.TypeAAAA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return nil, nil }, - dns.TypeTXT: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + dns.TypeTXT: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return nil, nil }, - dns.TypeCNAME: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + dns.TypeCNAME: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return nil, nil }, - dns.TypeNS: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + dns.TypeNS: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return nil, nil }, - dns.TypePTR: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + dns.TypePTR: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return nil, nil }, - dns.TypeSRV: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + dns.TypeSRV: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return nil, nil }, - dns.TypeSOA: func (rr *dns.RR, header dns.RR_Header) (node etcd.Node, err error) { + dns.TypeSOA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { return nil, nil }, } diff --git a/resolver.go b/resolver.go index 6f739c9..951f1ac 100644 --- a/resolver.go +++ b/resolver.go @@ -1,11 +1,9 @@ package main import ( - "fmt" "github.com/coreos/go-etcd/etcd" "github.com/miekg/dns" "github.com/rcrowley/go-metrics" - "net" "strconv" "strings" "sync" diff --git a/update.go b/update.go index 9916847..7b38ec8 100644 --- a/update.go +++ b/update.go @@ -19,7 +19,7 @@ type DynamicUpdateManager struct { // their right to update records in the given zone. func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) { - rrsets := []dns.RR{req.Answer, req.Ns} + rrsets := [][]dns.RR{req.Answer, req.Ns} msg = new(dns.Msg) msg.SetReply(req) @@ -50,7 +50,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) for _, rrs := range rrsets { for _, rr := range rrs { lock := lockDomain(u.etcd, rr.Header().Name) - defer lock.Unlock() + defer lock.Unlock(true) } } @@ -100,18 +100,18 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { return dns.RcodeNXRrset } } - } else if header.Class == dns.ClassNone { + } else if header.Class == dns.ClassNONE { if header.Rdlength != 0 { return dns.RcodeFormatError } else if header.Rrtype == dns.TypeANY { - if answers, ok := resolver.LookupAnswersForType(header.Name, dns.TypeANY)); len(answers) == 0 { + if answers, ok := resolver.LookupAnswersForType(header.Name, dns.TypeANY); len(answers) == 0 { if ok != nil { return dns.RcodeServerFailure } return dns.RcodeYXDomain } } else { - if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype)); len(answers) == 0 { + if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); len(answers) == 0 { if ok != nil { return dns.RcodeServerFailure } @@ -119,12 +119,11 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { } } } else if header.Class == dns.ClassINET { - if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); answers != rr { - if ok != nil { - return dns.RcodeServerFailure - } - return false // TODO(tarnfekd): What error type should this be? - } + // if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); answers != rr { + // if ok != nil { + // return dns.RcodeServerFailure + // } + // } } else { return dns.RcodeFormatError } @@ -143,7 +142,7 @@ func performUpdate(rr []dns.RR) (rcode int) { // nameInUser will return true if the name in the dns.RR_Header given is // already in use, or false if not. func nameInUse(header dns.RR_Header) []dns.RR { - return false + return nil } // nameInUser will return true if the name AND type in the dns.RR_Header given From 43abcd9e8d79b98ac851b60ceaa7757490104abf Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Thu, 25 Sep 2014 09:12:25 +0100 Subject: [PATCH 05/54] Add a boolean argument to prevent resolving CNAMEs In some situations when querying for answers for a given domain, it may not be desireable to resolve the domain to a CNAME if one exists. This is indeed the requirement for dynamic updates, as an rrtype is always explicit. --- resolver.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/resolver.go b/resolver.go index 951f1ac..7e54ef4 100644 --- a/resolver.go +++ b/resolver.go @@ -140,7 +140,7 @@ func (r *Resolver) Lookup(req *dns.Msg) (msg *dns.Msg) { errors := make(chan error) if q.Qclass == dns.ClassINET { - r.AnswerQuestion(answers, errors, q, &wait) + r.AnswerQuestion(answers, errors, q, &wait, true) } // Spawn a goroutine to close the channel as soon as all of the things @@ -161,7 +161,7 @@ func (r *Resolver) Lookup(req *dns.Msg) (msg *dns.Msg) { Qtype: q.Qtype, Qclass: q.Qclass} - r.AnswerQuestion(answers, errors, question, &wait) + r.AnswerQuestion(answers, errors, question, &wait, true) wait.Wait() if len(answers) > 0 { @@ -225,7 +225,7 @@ func (r *Resolver) Lookup(req *dns.Msg) (msg *dns.Msg) { // the way. The function will return immediately, and spawn off a bunch of goroutines // to do the work, when using this function one should use a WaitGroup to know when all work // has been completed. -func (r *Resolver) AnswerQuestion(answers chan dns.RR, errors chan error, q dns.Question, wg *sync.WaitGroup) { +func (r *Resolver) AnswerQuestion(answers chan dns.RR, errors chan error, q dns.Question, wg *sync.WaitGroup, resolveAliases boolean) { typeStr := strings.ToLower(dns.TypeToString[q.Qtype]) type_counter := metrics.GetOrRegisterCounter("resolver.answers.type." + typeStr, metrics.DefaultRegistry) @@ -265,7 +265,7 @@ func (r *Resolver) AnswerQuestion(answers chan dns.RR, errors chan error, q dns. for _, rr := range records { answers <- rr } - } else { + } else if resolveAliases { cnames, err := r.LookupAnswersForType(q.Name, dns.TypeCNAME) if err != nil { errors <- err From da29d03a7dcf947a9a47458c9309b6b4c2df5aea Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Mon, 29 Sep 2014 09:26:09 +0100 Subject: [PATCH 06/54] Ensure there is no leading slash from nameToKey --- record.go | 8 +++++++- record_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ resolver.go | 2 +- resolver_test.go | 19 ------------------- 4 files changed, 54 insertions(+), 21 deletions(-) create mode 100644 record_test.go diff --git a/record.go b/record.go index e07b8c3..f3e9bf2 100644 --- a/record.go +++ b/record.go @@ -34,10 +34,16 @@ func nameToKey(name string, suffix string) string { segments := strings.Split(name, ".") var keyBuffer bytes.Buffer + var writtenSegment bool for i := len(segments) - 1; i >= 0; i-- { if len(segments[i]) > 0 { - keyBuffer.WriteString("/") + // We never want to write a leading slash + if writtenSegment { + keyBuffer.WriteString("/") + } + keyBuffer.WriteString(segments[i]) + writtenSegment = true } } diff --git a/record_test.go b/record_test.go new file mode 100644 index 0000000..b979fd2 --- /dev/null +++ b/record_test.go @@ -0,0 +1,46 @@ +package main + +import ( + "testing" +) + +func TestRecord(t *testing.T) { + // Enable debug logging + log_debug = true +} + +func TestNameToKey(t *testing.T) { + + key := nameToKey("foo.disco.net", "") + if key != "net/disco/foo" { + t.Error("Expected key to be /net/disco/foo but got " + key) + t.Fatal() + } +} + +func TestNameToKeyFQDN(t *testing.T) { + + key := nameToKey("foo.disco.net.", "") + if key != "net/disco/foo" { + t.Error("Expected key to be /net/disco/foo but got " + key) + t.Fatal() + } +} + +func TestNameToKeyWithSuffix(t *testing.T) { + + key := nameToKey("foo.disco.net", "/.A") + if key != "net/disco/foo/.A" { + t.Error("Expected key to be /net/disco/foo/.A but got " + key) + t.Fatal() + } +} + +func TestNameToKeyFQDNWithSuffix(t *testing.T) { + + key := nameToKey("foo.disco.net.", "/.A") + if key != "net/disco/foo/.A" { + t.Error("Expected key to be /net/disco/foo/.A but got " + key) + t.Fatal() + } +} diff --git a/resolver.go b/resolver.go index 7e54ef4..e82d920 100644 --- a/resolver.go +++ b/resolver.go @@ -26,7 +26,7 @@ func (r *Resolver) GetFromStorage(key string) (nodes []*EtcdRecord, err error) { error_counter := metrics.GetOrRegisterCounter("resolver.etcd.query_error_count", metrics.DefaultRegistry) counter.Inc(1) - debugMsg("Querying etcd for " + key) + debugMsg("Querying etcd for /" + r.etcdPrefix + key) response, err := r.etcd.Get(r.etcdPrefix + key, true, true) if err != nil { diff --git a/resolver_test.go b/resolver_test.go index caab100..84c2bd2 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -80,25 +80,6 @@ func TestGetFromStorageNestedKeys(t *testing.T) { } } -func TestNameToKeyConverter(t *testing.T) { - var key string - - key = nameToKey("foo.net.", "") - if key != "/net/foo" { - t.Error("Expected key /net/foo") - } - - key = nameToKey("foo.net", "") - if key != "/net/foo" { - t.Error("Expected key /net/foo") - } - - key = nameToKey("foo.net.", "/.A") - if key != "/net/foo/.A" { - t.Error("Expected key /net/foo/.A") - } -} - /** * Test that the right authority is being returned for different types of DNS * queries. From e5e685eb8bbd747494cce0db1607b450ea09e3fc Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Mon, 29 Sep 2014 09:26:35 +0100 Subject: [PATCH 07/54] Fixed type error --- resolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resolver.go b/resolver.go index e82d920..4e22d69 100644 --- a/resolver.go +++ b/resolver.go @@ -225,7 +225,7 @@ func (r *Resolver) Lookup(req *dns.Msg) (msg *dns.Msg) { // the way. The function will return immediately, and spawn off a bunch of goroutines // to do the work, when using this function one should use a WaitGroup to know when all work // has been completed. -func (r *Resolver) AnswerQuestion(answers chan dns.RR, errors chan error, q dns.Question, wg *sync.WaitGroup, resolveAliases boolean) { +func (r *Resolver) AnswerQuestion(answers chan dns.RR, errors chan error, q dns.Question, wg *sync.WaitGroup, resolveAliases bool) { typeStr := strings.ToLower(dns.TypeToString[q.Qtype]) type_counter := metrics.GetOrRegisterCounter("resolver.answers.type." + typeStr, metrics.DefaultRegistry) From ddc71c8e096c2cd3f92b877c95537ad41b0ba0cf Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Mon, 29 Sep 2014 09:26:49 +0100 Subject: [PATCH 08/54] Added helper methods for domain/rrset validation checks --- resolver.go | 44 +++++++++++++++++++++++++++++ resolver_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/resolver.go b/resolver.go index 4e22d69..4307d2d 100644 --- a/resolver.go +++ b/resolver.go @@ -316,3 +316,47 @@ func (r *Resolver) LookupAnswersForType(name string, rrType uint16) (answers []d return } + +// NameExists will return true if the given domain name exists and has any +// resource records in the database. If an error occurs while querying for +// data the function will return false and an error. +func (r *Resolver) NameExists(name string) (exists bool, err error) { + wg := sync.WaitGroup{} + answers := make(chan dns.RR) + errors := make(chan error) + + question := dns.Question{dns.Fqdn(name), dns.TypeANY, dns.ClassINET} + r.AnswerQuestion(answers, errors, question, &wg, true) + + go func() { + wg.Wait() + close(answers) + close(errors) + }() + + select { + case _, ok := <-answers: + if ok { + return true, nil + } + case err, ok := <-errors: + if ok { + return false, err + } + } + + return false, nil +} + +func (r *Resolver) RRSetExists(name string, rrType uint16) (exists bool, err error) { + answers, err := r.LookupAnswersForType(dns.Fqdn(name), rrType) + if err != nil { + return false, err + } + + return len(answers) > 0, nil +} + +func (r *Resolver) MatchRR(rr dns.RR) (matches, exists bool, err error) { + return false, false, nil +} diff --git a/resolver_test.go b/resolver_test.go index 84c2bd2..e24325d 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -881,3 +881,75 @@ func TestLookupAnswerForSRVInvalidValues(t *testing.T) { } } } + +func TestNameExistsDoesExist(t *testing.T) { + + resolver.etcdPrefix = "TestNameExistsDoesExist/" + client.Set("TestNameExistsDoesExist/net/disco/bar/.A", "127.0.0.1", 0) + + exists, err := resolver.NameExists("bar.disco.net") + if exists != true { + t.Error("Expected domain to exist (true), got (false)") + t.Fatal() + } + + if err != nil { + t.Error("Expected error to be nil") + t.Fatal() + } +} + +func TestNameExistsDoesNotExist(t *testing.T) { + + resolver.etcdPrefix = "TestNameExistsDoesNotExist/" + exists, err := resolver.NameExists("bar.disco.net") + if exists != false { + t.Error("Expected domain to not exist (false), got (true)") + t.Fatal() + } + + if err != nil { + t.Error("Expected error to be nil") + t.Fatal() + } +} + +func TestRRSetExistsDoesExist(t *testing.T) { + + resolver.etcdPrefix = "TestRRSetExistsDoesExist/" + client.Set("TestRRSetExistsDoesExist/net/disco/bar/.A", "127.0.0.1", 0) + + exists, err := resolver.RRSetExists("bar.disco.net", dns.TypeA) + if exists != true { + t.Error("Expected RRset to exist (true), got (false)") + t.Fatal() + } + + if err != nil { + t.Error("Expected error to be nil") + t.Fatal() + } +} + +func TestRRSetExistsDoesNotExist(t *testing.T) { + + resolver.etcdPrefix = "TestRRSetExistsDoesNotExist/" + exists, err := resolver.RRSetExists("bar.disco.net", dns.TypeA) + if exists != false { + t.Error("Expected RRset to not exist (false), got (true)") + t.Fatal() + } + + if err != nil { + t.Error("Expected error to be nil") + t.Fatal() + } +} + +func TestMatchRRMatches(t *testing.T) { + +} + +func TestMatchRRDoesNotMatch(t *testing.T) { + +} From 1f68dca3064c33c21ef22353c2baf89b8f1565b0 Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Mon, 29 Sep 2014 09:27:08 +0100 Subject: [PATCH 09/54] Added more useful debug logging --- update.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/update.go b/update.go index 7b38ec8..3e9393f 100644 --- a/update.go +++ b/update.go @@ -58,6 +58,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) // are not satisfied. validationStatus := validatePrerequisites(req.Answer, u.resolver) if validationStatus != dns.RcodeSuccess { + debugMsg("Validation of prerequisites failed") msg.SetRcode(req, validationStatus) return } @@ -90,6 +91,7 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { if ok != nil { return dns.RcodeServerFailure } + debugMsg("Domain that should exist does not ", header.Name) return dns.RcodeNameError } } else { @@ -97,6 +99,7 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { if ok != nil { return dns.RcodeServerFailure } + debugMsg("RRset that should exist does not ", header.Name) return dns.RcodeNXRrset } } @@ -108,6 +111,7 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { if ok != nil { return dns.RcodeServerFailure } + debugMsg("Domain that should not exist does ", header.Name) return dns.RcodeYXDomain } } else { @@ -115,10 +119,12 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { if ok != nil { return dns.RcodeServerFailure } + debugMsg("RRset that should not exist does ", header.Name) return dns.RcodeYXRrset } } } else if header.Class == dns.ClassINET { + // TODO(tarnfeld): Perform strict comparisons between the resource records // if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); answers != rr { // if ok != nil { // return dns.RcodeServerFailure From 683576dcd7f40c4d6708276f5067d0ef4677d57d Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Fri, 21 Nov 2014 15:03:50 +0000 Subject: [PATCH 10/54] Added some basic test cases for panic based locks --- lock_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/lock_test.go b/lock_test.go index 06ab7d0..e662b7d 100644 --- a/lock_test.go +++ b/lock_test.go @@ -1 +1,47 @@ package main + +import ( + "testing" +) + +func TestLock(t *testing.T) { + // Enable debug logging + log_debug = true +} + +func TestSimpleLockUnlock(t *testing.T) { + lock := lockDomain(client, "discodns.net") + + if lock.IsLocked() != true { + t.Error("Expected lock to be locked, it was not") + t.Fatal() + } + + lock.Unlock(false) + + if lock.IsLocked() != false { + t.Error("Expected lock to be unlocked, it was not") + t.Fatal() + } +} + +func TestConflictingLock(t *testing.T) { + lockA := lockDomain(client, "discodns.net") + if lockA.IsLocked() != true { + t.Error("Expected lock to be locked, it was not") + t.Fatal() + } + + // Defer a function to handle the panic when we request overlapping + // locks. We do this here because if the lock fails to be acquired above + // the test should fail exceptionally. + defer func() { + p := recover() + if p == nil { + t.Error("Expected a panic, got nil") + t.Fatal() + } + }() + + lockDomain(client, "discodns.net") +} From 68381334dd62a23eb620fb10141b38ab0754b8f9 Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Tue, 3 Feb 2015 10:30:23 +0000 Subject: [PATCH 11/54] Added MOTD for makefile --- Makefile | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 8344559..98274da 100644 --- a/Makefile +++ b/Makefile @@ -1,28 +1,39 @@ -all:: clean build +all:: clean build test build:: get compile -clean: +motd: + @echo + @echo ' ___ __' + @echo ' ____/ (_)_____________ ____/ /___ _____' + @echo ' / __ / / ___/ ___/ __ \/ __ / __ \/ ___/' + @echo '/ /_/ / (__ ) /__/ /_/ / /_/ / / / (__ )' + @echo '\__,_/_/____/\___/\____/\__,_/_/ /_/____/' + @echo + @echo '© Copyright DueDil 2015. Licensed under MIT.' + @echo + +clean: motd @echo "\033[34m●\033[39m Cleaning out the build folder ./build" rm -rf build/* @echo "\033[32m✔\033[39m Cleaned ./build" -get: +get: motd @echo "\033[34m●\033[39m Downloading go packages" go get -d @echo "\033[32m✔\033[39m Finished downloading packages" -compile: +compile: motd get @echo "\033[34m●\033[39m Building into ./build" mkdir -p build/bin go build -o build/bin/discodns *.go @echo "\033[32m✔\033[39m Successfully built into ./build" -test: +test: motd @echo "\033[34m●\033[39m Running tests" go test @echo "\033[32m✔\033[39m Tests passed" -install: +install: motd compile @echo "\033[34m●\033[39m Installing into /usr/local/bin" cp build/bin/discodns /usr/local/bin/ @echo "\033[32m✔\033[39m Successfully installed into /usr/local/bin/discodns" From d67d220f66f6ca808339df20e7eab3c0663b58da Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Tue, 3 Feb 2015 10:30:29 +0000 Subject: [PATCH 12/54] Test race conditions too --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 98274da..e417282 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ compile: motd get test: motd @echo "\033[34m●\033[39m Running tests" - go test + go test -race ./ @echo "\033[32m✔\033[39m Tests passed" install: motd compile From a65461d6253d7f2dcff22a0bc122c7c7fe2d65bc Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Tue, 3 Feb 2015 10:30:50 +0000 Subject: [PATCH 13/54] Text alignment --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3f09a1c..06183a8 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ discodns -====== +======== [![Build Status](https://travis-ci.org/duedil-ltd/discodns.png?branch=master)](https://travis-ci.org/duedil-ltd/discodns) From 1b93f297c39dc11ac0ce3edd2bd68ffba8fa7b24 Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Tue, 3 Feb 2015 10:31:32 +0000 Subject: [PATCH 14/54] Progress with add/delete dynamic updates --- error.go | 2 +- record.go | 66 ++++++++++++++++++++++++++++------------------- update.go | 66 ++++++++++++++++++++++++++++++++++++----------- update_test.go | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 161 insertions(+), 42 deletions(-) diff --git a/error.go b/error.go index ab3d8be..5f9882a 100644 --- a/error.go +++ b/error.go @@ -14,7 +14,7 @@ type NodeConversionError struct { func (e *NodeConversionError) Error() string { return fmt.Sprintf( - "Unable to convert etc Node into a RR of type %d ('%s'): %s. Node details: %+v", + "Unable to convert etcd Node into a RR of type %d ('%s'): %s. Node details: %+v", e.AttemptedType, dns.TypeToString[e.AttemptedType], e.Message, diff --git a/record.go b/record.go index f3e9bf2..a2f188a 100644 --- a/record.go +++ b/record.go @@ -18,14 +18,16 @@ type EtcdRecord struct { // convertNodeToRR will convert an etcd node with a raw value into a dns.RR // record, returning an error if the conversion fails func convertNodeToRR(node *etcd.Node, header dns.RR_Header) (rr dns.RR, err error) { - return convertersToRR[header.Rrtype](node, header) + rr, err = convertersToRR[header.Rrtype](node, header) + return } // convertRRToNode will convert a DNS RR and it's type specific values to an // etcd node with a raw value and key path, returning an error if the conversion // fails -func convertRRToNode(rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return convertersFromRR[header.Rrtype](rr, header) +func convertRRToNode(rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + node, err = convertersFromRR[header.Rrtype](rr, header) + return } // nameToKey returns a string representing the etcd version of a domain, replacing dots with slashes @@ -205,37 +207,49 @@ var convertersToRR = map[uint16]func (node *etcd.Node, header dns.RR_Header) (rr } // Map of conversion functions that turn dns.RR answers into individual etcd nodes -var convertersFromRR = map[uint16]func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { +var convertersFromRR = map[uint16]func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - dns.TypeA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return nil, nil - }, + dns.TypeANY: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + node = &etcd.Node{Key: nameToKey(header.Name, "")} - dns.TypeAAAA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return nil, nil + return }, - dns.TypeTXT: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return nil, nil - }, + dns.TypeA: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + if record, ok := rr.(*dns.A); ok { + node = &etcd.Node{ + Key: nameToKey(header.Name, "/.A"), + Value: record.A.String()} + } - dns.TypeCNAME: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return nil, nil + return }, - dns.TypeNS: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return nil, nil - }, + // dns.TypeAAAA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // panic("Not implemented") + // }, - dns.TypePTR: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return nil, nil - }, + // dns.TypeTXT: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // panic("Not implemented") + // }, - dns.TypeSRV: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return nil, nil - }, + // dns.TypeCNAME: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // panic("Not implemented") + // }, - dns.TypeSOA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - return nil, nil - }, + // dns.TypeNS: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // panic("Not implemented") + // }, + + // dns.TypePTR: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // panic("Not implemented") + // }, + + // dns.TypeSRV: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // panic("Not implemented") + // }, + + // dns.TypeSOA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // panic("Not implemented") + // }, } diff --git a/update.go b/update.go index 3e9393f..99bb3d9 100644 --- a/update.go +++ b/update.go @@ -4,6 +4,7 @@ import ( "github.com/coreos/go-etcd/etcd" "github.com/miekg/dns" "fmt" + "strconv" ) type DynamicUpdateManager struct { @@ -18,7 +19,7 @@ type DynamicUpdateManager struct { // It is assumed at this level the client has already authenticated and proven // their right to update records in the given zone. func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) { - + rrsets := [][]dns.RR{req.Answer, req.Ns} msg = new(dns.Msg) msg.SetReply(req) @@ -60,7 +61,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) if validationStatus != dns.RcodeSuccess { debugMsg("Validation of prerequisites failed") msg.SetRcode(req, validationStatus) - return + return } // Perform the updates to the domain name system @@ -68,7 +69,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) // result in a partially updated zone. // TODO(tarnfeld): Figure out a way of rolling back changes, perhaps make // use of the etcd indexes? - msg.SetRcode(req, performUpdate(req.Ns)) + msg.SetRcode(req, performUpdate(u.etcdPrefix, u.etcd, req.Ns)) return } @@ -141,18 +142,53 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { // performUpdate will commit the requested updates to the database // It is assumed by this point all prerequisites have been validated and all // domains are locked. -func performUpdate(rr []dns.RR) (rcode int) { - return dns.RcodeSuccess -} +func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode int) { + for _, rr := range records { + header := rr.Header() + if _, ok := convertersFromRR[header.Rrtype]; ok != true { + panic("Record converter does exist for " + dns.TypeToString[header.Rrtype]) + } -// nameInUser will return true if the name in the dns.RR_Header given is -// already in use, or false if not. -func nameInUse(header dns.RR_Header) []dns.RR { - return nil -} + node, err := convertRRToNode(rr, *header) + if err != nil { + panic("Got error when converting node") + } else if node == nil { + panic("Got NIL after successfully converting node") + } + + // Prepend the etcd prefix, if we're given one + node.Key = prefix + node.Key + + if header.Class == dns.ClassANY { + debugMsg("Deleting all RRs from key " + node.Key) + _, err := etcd.Delete(node.Key, true) + if err != nil { + debugMsg(err) + panic("Failed to delete RRs from key " + node.Key) + } -// nameInUser will return true if the name AND type in the dns.RR_Header given -// is are already in use. -func nameAndTypeInUse(header dns.RR_Header) bool { - return false + } else if header.Class == dns.ClassNONE { // Delete an RR + debugMsg("Delete specific RR: " + rr.String()) + } else { // Insert RR + debugMsg("Inserting " + node.Value + " to " + node.Key) + + // Insert the record into etcd + _, err = etcd.Set(node.Key, node.Value, 0) + if err != nil { + debugMsg(err) + panic("Failed to insert record into etcd") + } + + // Insert the TTL record if one has been requested + if header.Ttl > 0 { + ttl := strconv.FormatInt(int64(header.Ttl), 10) + _, err = etcd.Set(node.Key + "/.ttl", ttl, 0) + if err != nil { + panic("Failed to insert ttl into etcd") + } + } + } + } + + return dns.RcodeSuccess } diff --git a/update_test.go b/update_test.go index 06ab7d0..00e9d58 100644 --- a/update_test.go +++ b/update_test.go @@ -1 +1,70 @@ package main + +import ( + "github.com/miekg/dns" + "net" + "testing" +) + +func TestInsertNewRecordNoPrerequsites(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestRecordNoPrerequsites/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + record := &dns.A{ + Hdr: dns.RR_Header{Name: "disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, + A: net.ParseIP("1.2.3.4")} + + msg := &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) + msg.Insert([]dns.RR{record}) + + result := manager.Update("disco.net.", msg) + + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("Failed to insert new DNS record") + t.Fatal() + } + + answers, err := resolver.LookupAnswersForType("disco.net.", dns.TypeA) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) != 1 { + t.Error("Expected exactly one answer for discodns.net.") + t.Fatal() + } +} + +func TestDeleteRecordNoPrerequsites(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestRecordNoPrerequsites/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + // record := &dns.A{ + // Hdr: dns.RR_Header{Name: "disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, + // A: net.ParseIP("1.2.3.4")} + + record := &dns.ANY{Hdr: dns.RR_Header{Name: "disco.net."}} + + msg := &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) + msg.RemoveName([]dns.RR{record}) + + result := manager.Update("disco.net.", msg) + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("Failed to remove DNS record") + t.Fatal() + } + + answers, err := resolver.LookupAnswersForType("disco.net.", dns.TypeA) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) > 0 { + t.Error("Expected zero answers for discodns.net.") + t.Fatal() + } +} From bfbe460d6438f6af47767477d32006c97bd441de Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Fri, 6 Feb 2015 12:59:00 +0000 Subject: [PATCH 15/54] Make sure we send a TSIG response back --- server.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server.go b/server.go index 2ba56a6..3bb6cb4 100644 --- a/server.go +++ b/server.go @@ -73,15 +73,17 @@ func (h *Handler) Handle(response dns.ResponseWriter, req *dns.Msg) { res.SetReply(req) // Authenticate the request - if req.IsTsig() != nil && response.TsigStatus() == nil { + tsig := req.IsTsig() + if tsig != nil && response.TsigStatus() == nil { sig := req.IsTsig() debugMsg("Authenticated update request") - + // Verify the tsig is for the correct zone if sig.Hdr.Name != zone { res.SetRcode(req, dns.RcodeBadSig) } else { res = h.updateManager.Update(zone, req) + res.SetTsig(tsig.Header().Name, dns.HmacMD5, 300, time.Now().Unix()) } } else { debugMsg("Authentication failed") From d18eb08404669cabedc9a058c12d017e1fb82694 Mon Sep 17 00:00:00 2001 From: Tom Arnfeld Date: Mon, 9 Feb 2015 13:12:03 +0000 Subject: [PATCH 16/54] Create new records as keys inside the type directory Previously we were creating new records as /net/foo/.A -> value however because there can be multiple records, we should create /net/foo/.A/X where X is a value created by etcd for us. --- update.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/update.go b/update.go index 99bb3d9..af940d8 100644 --- a/update.go +++ b/update.go @@ -173,7 +173,7 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in debugMsg("Inserting " + node.Value + " to " + node.Key) // Insert the record into etcd - _, err = etcd.Set(node.Key, node.Value, 0) + response, err := etcd.Create(node.Key, node.Value, 0) if err != nil { debugMsg(err) panic("Failed to insert record into etcd") @@ -182,7 +182,7 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in // Insert the TTL record if one has been requested if header.Ttl > 0 { ttl := strconv.FormatInt(int64(header.Ttl), 10) - _, err = etcd.Set(node.Key + "/.ttl", ttl, 0) + _, err = etcd.Set(response.Node.Key + "/.ttl", ttl, 0) if err != nil { panic("Failed to insert ttl into etcd") } From eb36f88638f88610f4d46447115a6f8d634c3bf8 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 10 Feb 2015 00:01:29 +0000 Subject: [PATCH 17/54] Fix creating directories for records: d18eb08 doesn't function as it was meant to, `Create` is just `Set` with a `prevExist` check --- update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/update.go b/update.go index af940d8..dfc2879 100644 --- a/update.go +++ b/update.go @@ -173,7 +173,7 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in debugMsg("Inserting " + node.Value + " to " + node.Key) // Insert the record into etcd - response, err := etcd.Create(node.Key, node.Value, 0) + response, err := etcd.CreateInOrder(node.Key, node.Value, 0) if err != nil { debugMsg(err) panic("Failed to insert record into etcd") From 8a395e69a822ede94e62a7e36c31deecbfea7347 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 9 Feb 2015 19:25:25 +0000 Subject: [PATCH 18/54] Fix 'given peers are not reachable' test errs: These are actually a badly-reported etcd 'unsupported protocol scheme' error in disguise --- resolver_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resolver_test.go b/resolver_test.go index fcda13c..d1b377a 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -8,7 +8,7 @@ import ( ) var ( - client = etcd.NewClient([]string{"127.0.0.1:4001"}) + client = etcd.NewClient([]string{"http://127.0.0.1:4001"}) resolver = &Resolver{etcd: client} ) From 0e36d77ae0d6236108a85ec1f3b03ad9fd3eeb5e Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Fri, 6 Feb 2015 12:21:12 +0000 Subject: [PATCH 19/54] Typo --- update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/update.go b/update.go index dfc2879..0d5ba99 100644 --- a/update.go +++ b/update.go @@ -146,7 +146,7 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in for _, rr := range records { header := rr.Header() if _, ok := convertersFromRR[header.Rrtype]; ok != true { - panic("Record converter does exist for " + dns.TypeToString[header.Rrtype]) + panic("Record converter doesn't exist for " + dns.TypeToString[header.Rrtype]) } node, err := convertRRToNode(rr, *header) From 8bcd5720d69ab3a9fae6f2d02e61340e7dbeb84f Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Fri, 6 Feb 2015 12:27:44 +0000 Subject: [PATCH 20/54] Correct signature for not-yet-implementeds --- record.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/record.go b/record.go index a2f188a..14c9c54 100644 --- a/record.go +++ b/record.go @@ -225,31 +225,31 @@ var convertersFromRR = map[uint16]func (rr dns.RR, header dns.RR_Header) (node * return }, - // dns.TypeAAAA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // dns.TypeAAAA: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") // }, - // dns.TypeTXT: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // dns.TypeTXT: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") // }, - // dns.TypeCNAME: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // dns.TypeCNAME: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") // }, - // dns.TypeNS: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // dns.TypeNS: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") // }, - // dns.TypePTR: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // dns.TypePTR: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") // }, - // dns.TypeSRV: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // dns.TypeSRV: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") // }, - // dns.TypeSOA: func (rr *dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + // dns.TypeSOA: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") // }, } From 5a8ba10419be64f59c5d0254672f6e079b837a11 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 9 Feb 2015 15:41:44 +0000 Subject: [PATCH 21/54] Use correct TTL etcd keyname: ...and debug if it fails --- update.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/update.go b/update.go index 0d5ba99..ca8d35a 100644 --- a/update.go +++ b/update.go @@ -182,8 +182,9 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in // Insert the TTL record if one has been requested if header.Ttl > 0 { ttl := strconv.FormatInt(int64(header.Ttl), 10) - _, err = etcd.Set(response.Node.Key + "/.ttl", ttl, 0) + _, err = etcd.Set(response.Node.Key + ".ttl", ttl, 0) if err != nil { + debugMsg(err) panic("Failed to insert ttl into etcd") } } From 1f9ea6badbfbb6a9dcd2de9ceba18795b519402b Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Fri, 6 Feb 2015 13:00:14 +0000 Subject: [PATCH 22/54] Implement update record converters: SRV, TXT, PTR --- record.go | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/record.go b/record.go index 14c9c54..cebd687 100644 --- a/record.go +++ b/record.go @@ -229,9 +229,14 @@ var convertersFromRR = map[uint16]func (rr dns.RR, header dns.RR_Header) (node * // panic("Not implemented") // }, - // dns.TypeTXT: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - // panic("Not implemented") - // }, + dns.TypeTXT: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + if record, ok := rr.(*dns.TXT); ok { + node = &etcd.Node{ + Key: nameToKey(header.Name, "/.TXT"), + Value: strings.Join(record.Txt, "\n")} + } + return + }, // dns.TypeCNAME: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") @@ -241,13 +246,25 @@ var convertersFromRR = map[uint16]func (rr dns.RR, header dns.RR_Header) (node * // panic("Not implemented") // }, - // dns.TypePTR: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - // panic("Not implemented") - // }, + dns.TypePTR: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + if record, ok := rr.(*dns.PTR); ok { + node = &etcd.Node{ + Key: nameToKey(header.Name, "/.PTR"), + Value: record.Ptr} + } - // dns.TypeSRV: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - // panic("Not implemented") - // }, + return + }, + + dns.TypeSRV: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + if record, ok := rr.(*dns.SRV); ok { + node = &etcd.Node{ + Key: nameToKey(header.Name, "/.SRV"), + Value: fmt.Sprintf("%d\t%d\t%d\t%s", record.Priority, record.Weight, record.Port, record.Target)} + } + + return + }, // dns.TypeSOA: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") From 5b091247a46f1f49c6a7bbf0fefdba2efb3c4246 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 9 Feb 2015 19:38:16 +0000 Subject: [PATCH 23/54] Clean up any stale locks in etcd before tests: when your tests are failing, locks can get left behind and cause you spurious subsequent failures --- lock_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lock_test.go b/lock_test.go index e662b7d..af348d3 100644 --- a/lock_test.go +++ b/lock_test.go @@ -10,6 +10,9 @@ func TestLock(t *testing.T) { } func TestSimpleLockUnlock(t *testing.T) { + // Ensure a clean test environment + client.Delete("net/discodns/._UPDATE_LOCK", true) + lock := lockDomain(client, "discodns.net") if lock.IsLocked() != true { @@ -23,9 +26,13 @@ func TestSimpleLockUnlock(t *testing.T) { t.Error("Expected lock to be unlocked, it was not") t.Fatal() } + } func TestConflictingLock(t *testing.T) { + // Ensure a clean test environment + client.Delete("net/discodns/._UPDATE_LOCK", true) + lockA := lockDomain(client, "discodns.net") if lockA.IsLocked() != true { t.Error("Expected lock to be locked, it was not") From d0a67d62c606a45794d36bfaf5461b5fbddd2d7f Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 10 Feb 2015 00:44:50 +0000 Subject: [PATCH 24/54] Use etcd prefixes for locks: This unmasks a hidden failure in TestDeleteRecordNoPrerequsites --- lock.go | 7 ++++--- lock_test.go | 12 +++++------- update.go | 2 +- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lock.go b/lock.go index 8ac9af2..b12089f 100644 --- a/lock.go +++ b/lock.go @@ -6,6 +6,7 @@ import ( type DomainLock struct { etcd *etcd.Client + etcdPrefix string domain string index uint64 lockPath string @@ -17,7 +18,7 @@ func (l *DomainLock) Lock(shouldPanic bool) error { return nil } - l.lockPath = nameToKey(l.domain, "/._UPDATE_LOCK") + l.lockPath = l.etcdPrefix + nameToKey(l.domain, "/._UPDATE_LOCK") debugMsg("Locking " + l.domain + " at " + l.lockPath) response, err := l.etcd.Create(l.lockPath, "", l.lockExpiry) @@ -72,8 +73,8 @@ func (l *DomainLock) IsLocked() (locked bool) { // lockDomain will lock the given domain in the given etcd cluster, and return // the DomainLock struct pre-populated such that calling Domain Unlock() // will release the lock. -func lockDomain(etcd *etcd.Client, domain string) (lock *DomainLock) { - lock = &DomainLock{etcd: etcd, domain: domain, lockExpiry: 30} +func lockDomain(etcd *etcd.Client, domain string, etcdPrefix string) (lock *DomainLock) { + lock = &DomainLock{etcd: etcd, domain: domain, lockExpiry: 30, etcdPrefix: etcdPrefix} defer lock.Lock(true) return lock } diff --git a/lock_test.go b/lock_test.go index af348d3..48548b4 100644 --- a/lock_test.go +++ b/lock_test.go @@ -10,10 +10,9 @@ func TestLock(t *testing.T) { } func TestSimpleLockUnlock(t *testing.T) { - // Ensure a clean test environment - client.Delete("net/discodns/._UPDATE_LOCK", true) + client.Delete("TestSimpleLockUnlock/", true) - lock := lockDomain(client, "discodns.net") + lock := lockDomain(client, "discodns.net", "TestSimpleLockUnlock") if lock.IsLocked() != true { t.Error("Expected lock to be locked, it was not") @@ -30,10 +29,9 @@ func TestSimpleLockUnlock(t *testing.T) { } func TestConflictingLock(t *testing.T) { - // Ensure a clean test environment - client.Delete("net/discodns/._UPDATE_LOCK", true) + client.Delete("TestConflictingLock/", true) - lockA := lockDomain(client, "discodns.net") + lockA := lockDomain(client, "discodns.net", "TestConflictingLock/") if lockA.IsLocked() != true { t.Error("Expected lock to be locked, it was not") t.Fatal() @@ -50,5 +48,5 @@ func TestConflictingLock(t *testing.T) { } }() - lockDomain(client, "discodns.net") + lockDomain(client, "discodns.net", "TestConflictingLock/") } diff --git a/update.go b/update.go index ca8d35a..1e83b8d 100644 --- a/update.go +++ b/update.go @@ -50,7 +50,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) // update will be applied. for _, rrs := range rrsets { for _, rr := range rrs { - lock := lockDomain(u.etcd, rr.Header().Name) + lock := lockDomain(u.etcd, rr.Header().Name, u.etcdPrefix) defer lock.Unlock(true) } } From 9b87d5cc2a6310bad9b83a858fe6bf91e7bb5ea5 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 10 Feb 2015 00:50:03 +0000 Subject: [PATCH 25/54] Add (failing) test for multiple records: Multiple records of different types at the same key fails due to the locking approach, despite being a legitimate usecase --- update_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/update_test.go b/update_test.go index 00e9d58..871b098 100644 --- a/update_test.go +++ b/update_test.go @@ -68,3 +68,48 @@ func TestDeleteRecordNoPrerequsites(t *testing.T) { t.Fatal() } } + +func TestInsertMultipleRecords(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestInsertMultipleRecords/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + client.Delete("TestInsertMultipleRecords/", true) + + record1 := &dns.SRV{ + Hdr: dns.RR_Header{Name: "disco.net.", Rrtype: dns.TypeSRV, Class: dns.ClassINET}, + Port: 80, Priority: 100, Weight: 100, Target: "foo.disco.net"} + + record2 := &dns.TXT{ + Hdr: dns.RR_Header{Name: "disco.net.", Rrtype: dns.TypeTXT, Class: dns.ClassINET}, + Txt: []string{"lol"}} + + msg := &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) + msg.Insert([]dns.RR{record1, record2}) + + result := manager.Update("disco.net.", msg) + + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("Failed to add DNS records") + t.Fatal() + } + + srvAnswers, err := resolver.LookupAnswersForType("disco.net.", dns.TypeSRV) + if err != nil { + t.Error("Caught error retrieving SRV") + t.Fatal() + } + if len(srvAnswers) != 1 { + t.Error("Expected one SRV response") + t.Fatal() + } + txtAnswers, err := resolver.LookupAnswersForType("disco.net.", dns.TypeTXT) + if err != nil { + t.Error("Caught error retrieving txt") + t.Fatal() + } + if len(txtAnswers) != 1 { + t.Error("Expected one TXT response") + t.Fatal() + } +} From 6d6729b58a79e1d6e7d2d49f19829185cfb0bed3 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Fri, 20 Feb 2015 09:40:38 +0000 Subject: [PATCH 26/54] Fix NameExists to use simpler flow from master --- resolver.go | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/resolver.go b/resolver.go index 968e621..61a1bf3 100644 --- a/resolver.go +++ b/resolver.go @@ -340,31 +340,15 @@ func (r *Resolver) LookupAnswersForType(name string, rrType uint16) (answers []d // resource records in the database. If an error occurs while querying for // data the function will return false and an error. func (r *Resolver) NameExists(name string) (exists bool, err error) { - wg := sync.WaitGroup{} - answers := make(chan dns.RR) - errors := make(chan error) question := dns.Question{dns.Fqdn(name), dns.TypeANY, dns.ClassINET} - r.AnswerQuestion(answers, errors, question, &wg, true) + aChan, eChan := r.AnswerQuestion(question, true) + answers, errors := chansGather(aChan, eChan) - go func() { - wg.Wait() - close(answers) - close(errors) - }() - - select { - case _, ok := <-answers: - if ok { - return true, nil - } - case err, ok := <-errors: - if ok { - return false, err - } + if len(errors) > 0 { + return false, errors[0] } - - return false, nil + return len(answers) > 0, nil } func (r *Resolver) RRSetExists(name string, rrType uint16) (exists bool, err error) { From ac8fd0eb757638882972cecfe6a686107a9f1a44 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Fri, 20 Feb 2015 12:53:07 +0000 Subject: [PATCH 27/54] Fix some merge issues --- resolver.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resolver.go b/resolver.go index 61a1bf3..0d8a29a 100644 --- a/resolver.go +++ b/resolver.go @@ -249,7 +249,7 @@ func (r *Resolver) AnswerQuestion(q dns.Question, resolveAliases bool) (answers close(answers) close(errors) }() - for rrType, _ := range converters { + for rrType, _ := range convertersToRR { go func(rrType uint16) { defer recover() defer wg.Done() @@ -343,7 +343,7 @@ func (r *Resolver) NameExists(name string) (exists bool, err error) { question := dns.Question{dns.Fqdn(name), dns.TypeANY, dns.ClassINET} aChan, eChan := r.AnswerQuestion(question, true) - answers, errors := chansGather(aChan, eChan) + answers, errors := gatherFromChannels(aChan, eChan) if len(errors) > 0 { return false, errors[0] From c037341cb6c45c880c5339cf0bdbfbaf6e395a40 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 18 Feb 2015 15:25:07 +0000 Subject: [PATCH 28/54] Add CLI option for unauthenticated updates: By starting with `--unauth=my.zone.net`, update requests that match the zone but don't give a TSIG secret will be allowed to update anyway. --- main.go | 11 +++++++++++ server.go | 17 +++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index 4cfde30..b4121a4 100644 --- a/main.go +++ b/main.go @@ -31,6 +31,7 @@ var ( Accept []string `long:"accept" description:"Limit DNS queries to a set of domain:[type,...] pairs"` Reject []string `long:"reject" description:"Limit DNS queries to a set of domain:[type,...] pairs"` TsigSecret []string `short:"s" long:"tsig" description:"Transaction signature secret in the format zone:secret"` + TsigFreeZones []string `long:"unauth" description:"Zone names that can be updated without TSIG authentication"` } ) @@ -91,6 +92,15 @@ func main() { tsigSecret[dns.Fqdn(components[0])] = components[1] } + // create a unique list of zone names that allow unauthenticated access + tsigFreeZones := make(map[string]struct{}, len(Options.TsigFreeZones)) + for _, zone := range Options.TsigFreeZones { + if zone[len(zone)-1] != '.' { + zone = zone + "." + } + tsigFreeZones[zone] = struct{}{} + } + // Start up the DNS resolver server server := &Server{ addr: Options.ListenAddress, @@ -100,6 +110,7 @@ func main() { wTimeout: time.Duration(5) * time.Second, defaultTtl: Options.DefaultTtl, tsigSecret: tsigSecret, + tsigFreeZones: tsigFreeZones, queryFilterer: &QueryFilterer{acceptFilters: parseFilters(Options.Accept), rejectFilters: parseFilters(Options.Reject)}} diff --git a/server.go b/server.go index 3bb6cb4..a50db62 100644 --- a/server.go +++ b/server.go @@ -16,6 +16,7 @@ type Server struct { wTimeout time.Duration defaultTtl uint32 tsigSecret map[string]string + tsigFreeZones map[string]struct{} queryFilterer *QueryFilterer } @@ -23,6 +24,7 @@ type Handler struct { resolver *Resolver queryFilterer *QueryFilterer updateManager *DynamicUpdateManager + tsigFreeZones map[string]struct{} // Metrics requestCounter metrics.Counter @@ -86,8 +88,13 @@ func (h *Handler) Handle(response dns.ResponseWriter, req *dns.Msg) { res.SetTsig(tsig.Header().Name, dns.HmacMD5, 300, time.Now().Unix()) } } else { - debugMsg("Authentication failed") - res.SetRcode(req, dns.RcodeNotAuth) + if _, ok := h.tsigFreeZones[zone]; ok { + debugMsg("allowing unauthenticated update") + res = h.updateManager.Update(zone, req) + } else { + debugMsg("Update authentication failed") + res.SetRcode(req, dns.RcodeNotAuth) + } } } else { res = new(dns.Msg) @@ -139,7 +146,8 @@ func (s *Server) Run() { rejectCounter: tcpRejectCounter, responseTimer: tcpResponseTimer, queryFilterer: s.queryFilterer, - updateManager: &updateManager} + updateManager: &updateManager, + tsigFreeZones: s.tsigFreeZones} udpDNShandler := &Handler{ resolver: &resolver, requestCounter: udpRequestCounter, @@ -147,7 +155,8 @@ func (s *Server) Run() { rejectCounter: udpRejectCounter, responseTimer: udpResponseTimer, queryFilterer: s.queryFilterer, - updateManager: &updateManager} + updateManager: &updateManager, + tsigFreeZones: s.tsigFreeZones} udpHandler := dns.NewServeMux() tcpHandler := dns.NewServeMux() From 6232c767d243e6adccca0c26f1960e80035e9670 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 18 Feb 2015 14:55:19 +0000 Subject: [PATCH 29/54] Use consistent etcd keys for new records: This allows easy repeated updates for the same key/value without an extra step of checking whether any of the records in the dir have the name content. And it allows faff-free TTL updates --- update.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/update.go b/update.go index 1e83b8d..dccf5bc 100644 --- a/update.go +++ b/update.go @@ -1,6 +1,8 @@ package main import ( + "crypto/md5" + "encoding/hex" "github.com/coreos/go-etcd/etcd" "github.com/miekg/dns" "fmt" @@ -172,8 +174,13 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in } else { // Insert RR debugMsg("Inserting " + node.Value + " to " + node.Key) - // Insert the record into etcd - response, err := etcd.CreateInOrder(node.Key, node.Value, 0) + // Insert the record into etcd. Use MD5 of the node value as the + // 'sub-key'. This makes duplicates impossible without sacrificing + // TTL updates or extra pre-update lookup faff + hasher := md5.New() + hasher.Write([]byte(node.Value)) + subkey := hex.EncodeToString(hasher.Sum(nil)) + response, err := etcd.Set(node.Key + "/" + subkey, node.Value, 0) if err != nil { debugMsg(err) panic("Failed to insert record into etcd") From 0bd0d05e61c62d3c5f4fab23e15686a5b1e4f92e Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 10 Feb 2015 01:00:22 +0000 Subject: [PATCH 30/54] Replace panic-based locking with blocking locks: Removing all tests for now, they will be replaced with equivalents --- lock.go | 183 +++++++++++++++++++++++++++++++++++---------------- lock_test.go | 53 +-------------- update.go | 25 ++++--- 3 files changed, 145 insertions(+), 116 deletions(-) diff --git a/lock.go b/lock.go index b12089f..53ecf8a 100644 --- a/lock.go +++ b/lock.go @@ -1,80 +1,151 @@ package main import ( + "code.google.com/p/go-uuid/uuid" + "errors" "github.com/coreos/go-etcd/etcd" + "time" ) -type DomainLock struct { - etcd *etcd.Client - etcdPrefix string - domain string - index uint64 - lockPath string - lockExpiry uint64 -} - -func (l *DomainLock) Lock(shouldPanic bool) error { - if l.index > 0 || l.lockPath != "" { - return nil - } +const ( + ETCD_LOCK_TTL = 10 + ETCD_LOCK_HEARTBEAT = 5 +) - l.lockPath = l.etcdPrefix + nameToKey(l.domain, "/._UPDATE_LOCK") - debugMsg("Locking " + l.domain + " at " + l.lockPath) +// EtcdKeyLock represents a lock on a single key. Its semantics: once asked to +// Acquire, it will try to grab hold of the key in etcd, if it doesnt exist. +// If it does exist, then the lock waits for the other party to release it. +// Once Acquired, it will hold on to the lock indefinitely until Abandoned +type EtcdKeyLock struct { + uuid string + key string + etcdClient *etcd.Client + killChan chan bool + killed bool +} - response, err := l.etcd.Create(l.lockPath, "", l.lockExpiry) - if err != nil { - debugMsg("Failed to acquire lock on domain " + l.domain) - debugMsg(err) +func NewEtcdKeyLock(etcdClient *etcd.Client, key string) *EtcdKeyLock { + uuid := uuid.New() + return &EtcdKeyLock{uuid: uuid, key: key, etcdClient: etcdClient, killChan: make(chan bool)} +} - if shouldPanic { - panic("Failed to acquire lock on domain " + l.domain) +// Start the process of trying to acquire a key lock. Returns a channel that +// will be sent true when the lock is aquired then closed. Callers can use this +// as their signal to proceed. The lock will kept indefinitely until abandoned. +func (l *EtcdKeyLock) Acquire() chan bool { + inner_acq := make(chan bool) + acquired := make(chan bool) + go func() { + _, ok := <-inner_acq + if ok { + acquired <- true + go heartbeat(l, nil) + go removeWhenCancelled(l) } + close(acquired) + }() + go tryCreate(l, inner_acq) + return acquired +} - return err +// Abandons the lock. This just means closing the internal cancellation +// channel, causing all the child goros to do whatever they need to do. +func (l *EtcdKeyLock) Abandon() { + if !l.killed { + l.killed = true + close(l.killChan) } - - l.index = response.Node.CreatedIndex - return nil } -func (l *DomainLock) Unlock(shouldPanic bool) error { - debugMsg("Unlocking " + l.domain + " from " + l.lockPath) - - _, err := l.etcd.CompareAndDelete(l.lockPath, "", l.index) - if err != nil { - debugMsg("Failed to unlock domain " + l.domain) - debugMsg(err) - - if shouldPanic { - panic("Failed to unlock domain " + l.domain) +// Blocking version of Acquire, hiding the channels from callers who just want +// to synchronously wait +func (l *EtcdKeyLock) WaitForAcquire(timeout int) (bool, error) { + timeoutKiller := time.AfterFunc(time.Duration(timeout) * time.Second, func(){ + l.Abandon() + }) + acq := l.Acquire() + ok, open := <- acq + if ok && open { + stopped := timeoutKiller.Stop() + // Stopped == false means the timer already fired: this shouldn't be + // possible (we shouldn't have been able to get an OK message in that + // case). Erroring mostly out of paranoia: I'm positive this race can't + // happen (famous last words though) + if !stopped { + return false, errors.New("Acquired a lock that was also killed by a timeout: this should not be possible!") } + return true, nil + } else { + return false, errors.New("Couldn't aqcuire lock in time") } - - return err } -// IsLocked will return true if the lock is still acquired by this instance -// Since locks may have an expiry, it is possible for the lock to expire and be -// acquired by another party -func (l *DomainLock) IsLocked() (locked bool) { - locked = false - - // Verify that the domain is locked and that it's *our* lock - if l.lockPath != "" { - response, err := l.etcd.Get(l.lockPath, false, false) - if err == nil { - locked = response.Node.CreatedIndex == l.index +// The internals of trying to get a lock: Try to PUT to the lock key iff it +// doesn't exist. If that suceeds, the lock is owned; signal the chan and +// return. If it fails, watch the etcd key until it changes. When it does +// change, try again. Repeat indefinitely until cancelled. +func tryCreate(l *EtcdKeyLock, acq chan bool) { + defer close(acq) + for { + select { + case _, chOpen := <-l.killChan: + if !chOpen { + return + } + default: + _, err := l.etcdClient.Create(l.key, l.uuid, ETCD_LOCK_TTL) + if err == nil { + acq <- true + return + } else { + err, cast := err.(*etcd.EtcdError) + if cast && err.ErrorCode == 105 { + // Watch until it changes. (The current index is given to + // make sure we don't miss any changes in between) + _, err := l.etcdClient.Watch(l.key, err.Index+1, false, nil, l.killChan) + if err == nil { + // Skip the sleep and attempt a retry asap + continue + } + } + // if not created and not watching, pause briefly + time.Sleep(1 * time.Second) + } } } +} - return +func heartbeat(l *EtcdKeyLock, ping chan bool) { + if ping == nil { + ping = make(chan bool) + } + defer close(ping) + for { + time.Sleep(ETCD_LOCK_HEARTBEAT * time.Second) + select { + case _, chOpen := <-l.killChan: + if !chOpen { + return + } + default: + _, err := l.etcdClient.Set(l.key, l.uuid, ETCD_LOCK_TTL) + // non-blocking write on the ping channel + select { + case ping <- (err == nil): + default: + } + } + } } -// lockDomain will lock the given domain in the given etcd cluster, and return -// the DomainLock struct pre-populated such that calling Domain Unlock() -// will release the lock. -func lockDomain(etcd *etcd.Client, domain string, etcdPrefix string) (lock *DomainLock) { - lock = &DomainLock{etcd: etcd, domain: domain, lockExpiry: 30, etcdPrefix: etcdPrefix} - defer lock.Lock(true) - return lock +func removeWhenCancelled(l *EtcdKeyLock) { + defer func() { + l.etcdClient.CompareAndDelete(l.key, l.uuid, 0) + }() + for { + _, chOpen := <-l.killChan + if !chOpen { + return + } + } } diff --git a/lock_test.go b/lock_test.go index 48548b4..8666c87 100644 --- a/lock_test.go +++ b/lock_test.go @@ -1,52 +1,5 @@ package main -import ( - "testing" -) - -func TestLock(t *testing.T) { - // Enable debug logging - log_debug = true -} - -func TestSimpleLockUnlock(t *testing.T) { - client.Delete("TestSimpleLockUnlock/", true) - - lock := lockDomain(client, "discodns.net", "TestSimpleLockUnlock") - - if lock.IsLocked() != true { - t.Error("Expected lock to be locked, it was not") - t.Fatal() - } - - lock.Unlock(false) - - if lock.IsLocked() != false { - t.Error("Expected lock to be unlocked, it was not") - t.Fatal() - } - -} - -func TestConflictingLock(t *testing.T) { - client.Delete("TestConflictingLock/", true) - - lockA := lockDomain(client, "discodns.net", "TestConflictingLock/") - if lockA.IsLocked() != true { - t.Error("Expected lock to be locked, it was not") - t.Fatal() - } - - // Defer a function to handle the panic when we request overlapping - // locks. We do this here because if the lock fails to be acquired above - // the test should fail exceptionally. - defer func() { - p := recover() - if p == nil { - t.Error("Expected a panic, got nil") - t.Fatal() - } - }() - - lockDomain(client, "discodns.net", "TestConflictingLock/") -} +// import ( +// "testing" +// ) diff --git a/update.go b/update.go index 1e83b8d..a31b92b 100644 --- a/update.go +++ b/update.go @@ -36,8 +36,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) } } - // Ensure we recover from any panicking goroutine, this helps ensure we don't - // leave any acquired locks around if possible + // Ensure we recover from any panicking goroutine defer func() { if r := recover(); r != nil { debugMsg("[PANIC] " + fmt.Sprint(r)) @@ -45,14 +44,20 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) } }() - // Attempt to acquire a lock on all of the domains referenced in the update - // If any lock attempt fails, all acquired locks will be released and no - // update will be applied. - for _, rrs := range rrsets { - for _, rr := range rrs { - lock := lockDomain(u.etcd, rr.Header().Name, u.etcdPrefix) - defer lock.Unlock(true) - } + // Attempt to acquire the dns-updates lock key. + // TODO (orls): This means all updates from all running instances are + // applied fully serially; this is less than ideal, the spec says they + // should be serial only when conflicting with one another. But...this is + // easier than building full transactions isolation mgmt :) For a low + // frequency of updates, this should fine. + lock := NewEtcdKeyLock(u.etcd, u.etcdPrefix + "._DISCODNS_UPDATE_LOCK") + defer lock.Abandon() + // block until locked or timed-out + _, err := lock.WaitForAcquire(30) + if err != nil { + debugMsg("Failed to acquire or keep the update lock: ", err) + msg.SetRcode(req, dns.RcodeServerFailure) + return } // Validate the prerequisites of the update, returning immediately if they From dd801398247daf888460a3ac05e48c0eab79de47 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 18 Feb 2015 00:21:26 +0000 Subject: [PATCH 31/54] Add simple lock test --- lock_test.go | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/lock_test.go b/lock_test.go index 8666c87..b59a1c3 100644 --- a/lock_test.go +++ b/lock_test.go @@ -1,5 +1,32 @@ package main -// import ( -// "testing" -// ) +import ( + "testing" + "time" +) + +func TestSimpleLockUnlock(t *testing.T) { + testKey := "TestSimpleLockUnlock/.lock" + client.Delete(testKey, true) + + lock := NewEtcdKeyLock(client, testKey) + locked, err := lock.WaitForAcquire(1) + + if !locked || err != nil { + t.Error("Expected to acquire lock, failed") + t.Fatal() + } + _, err = client.Get(testKey, false, true) + if err != nil { + t.Error("Lock claimed to succeed but etcd record missing/broken") + t.Fatal() + } + + lock.Abandon() + time.Sleep(500 * time.Millisecond) + _, err = client.Get(testKey, false, true) + if err == nil { + t.Error("Lock abandoned, but key exists") + t.Fatal() + } +} From 5536ccc6faebfe0b9374aa3f764bd8b01942c0b3 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 18 Feb 2015 01:00:01 +0000 Subject: [PATCH 32/54] Add conflicting lock test --- lock_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lock_test.go b/lock_test.go index b59a1c3..cd909e5 100644 --- a/lock_test.go +++ b/lock_test.go @@ -30,3 +30,30 @@ func TestSimpleLockUnlock(t *testing.T) { t.Fatal() } } + +func TestConflictingLock(t *testing.T) { + testKey := "TestConflictingLock/.lock" + client.Delete(testKey, true) + + lock_a := NewEtcdKeyLock(client, testKey) + lock_a.WaitForAcquire(30) + + lock_b := NewEtcdKeyLock(client, testKey) + b_locked, b_err := lock_b.WaitForAcquire(1) + if b_locked || b_err == nil { + t.Error("Expected second lock to timeout") + t.Fatal() + } + + lock_c := NewEtcdKeyLock(client, testKey) + go func(){ + time.Sleep(500 * time.Millisecond) + lock_a.Abandon() + }() + + c_locked, c_err := lock_c.WaitForAcquire(5) + if !c_locked || c_err != nil { + t.Error("Expected third lock to succeed in time") + t.Fatal() + } +} From c81e5fb888a31324b5a3a6f3ee3d7b77641f1515 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 23 Feb 2015 14:34:37 +0000 Subject: [PATCH 33/54] Annotate prereq conditions with their RFC meanings --- update.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/update.go b/update.go index c908e07..0717065 100644 --- a/update.go +++ b/update.go @@ -99,10 +99,12 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { if ok != nil { return dns.RcodeServerFailure } + // RFC Meaning: "Name is in use" debugMsg("Domain that should exist does not ", header.Name) return dns.RcodeNameError } } else { + // RFC Meaning: "RRset exists (value independent)" if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); len(answers) > 0 { if ok != nil { return dns.RcodeServerFailure @@ -119,10 +121,12 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { if ok != nil { return dns.RcodeServerFailure } + // RFC Meaning: "Name is not in use" debugMsg("Domain that should not exist does ", header.Name) return dns.RcodeYXDomain } } else { + // RFC meaning: "RRset does not exist" if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); len(answers) == 0 { if ok != nil { return dns.RcodeServerFailure @@ -132,6 +136,12 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { } } } else if header.Class == dns.ClassINET { + if header.Rrtype == dns.TypeANY { + return dns.RcodeFormatError + } else { + // RFC Meaning: "RRset exists (value dependent)" + } + // TODO(tarnfeld): Perform strict comparisons between the resource records // if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); answers != rr { // if ok != nil { From 36a19cc255315b96a4f12979bc3cad98cc9a2aba Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 23 Feb 2015 16:29:05 +0000 Subject: [PATCH 34/54] Tests for Name is/is not in use --- update_test.go | 102 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/update_test.go b/update_test.go index 871b098..c375171 100644 --- a/update_test.go +++ b/update_test.go @@ -113,3 +113,105 @@ func TestInsertMultipleRecords(t *testing.T) { t.Fatal() } } + +func TestPrerequisites_NameInUse(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestPrerequisites_NameInUse/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + client.Delete("TestPrerequisites_NameInUse/", true) + client.Set("TestPrerequisites_NameInUse/net/disco/foo/.A", "1.1.1.1", 0) + + recordToAdd := &dns.A{ + Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, + A: net.ParseIP("1.2.3.4")} + + prereq_fail := &dns.ANY{ Hdr: dns.RR_Header{Name: "foofoo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} + prereq_ok := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} + + msg_1 := &dns.Msg{} + msg_1.Question = append(msg_1.Question, dns.Question{Name: "bar.disco.net."}) + msg_1.Insert([]dns.RR{recordToAdd}) + msg_1.NameUsed([]dns.RR{prereq_fail}) + + result_1 := manager.Update("disco.net.", msg_1) + if result_1.Rcode != dns.RcodeNameError { + debugMsg(result_1) + t.Error("expected update to fail with NXDOMAIN, actually got", dns.RcodeToString[result_1.Rcode]) + t.Fatal() + } + + msg_2 := &dns.Msg{} + msg_2.Question = append(msg_2.Question, dns.Question{Name: "bar.disco.net."}) + msg_2.Insert([]dns.RR{recordToAdd}) + msg_2.NameUsed([]dns.RR{prereq_fail, prereq_ok}) + + result_2 := manager.Update("disco.net.", msg_2) + if result_2.Rcode != dns.RcodeNameError { + debugMsg(result_2) + t.Error("expected update to fail with NXDOMAIN, actually got", dns.RcodeToString[result_2.Rcode]) + t.Fatal() + } + + msg_3 := &dns.Msg{} + msg_3.Question = append(msg_3.Question, dns.Question{Name: "bar.disco.net."}) + msg_3.Insert([]dns.RR{recordToAdd}) + msg_3.NameUsed([]dns.RR{prereq_ok}) + + result_3 := manager.Update("disco.net.", msg_3) + if result_3.Rcode != dns.RcodeSuccess { + debugMsg(result_3) + t.Error("Failed to add DNS record with name-in-use prereq, got", dns.RcodeToString[result_3.Rcode]) + t.Fatal() + } +} + +func TestPrerequisites_NameNotInUse(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestPrerequisites_NameNotInUse/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + client.Delete("TestPrerequisites_NameNotInUse/", true) + client.Set("TestPrerequisites_NameNotInUse/net/disco/foo/.A", "1.1.1.1", 0) + + recordToAdd := &dns.A{ + Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, + A: net.ParseIP("1.2.3.4")} + + prereq_fail := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} + prereq_ok := &dns.ANY{ Hdr: dns.RR_Header{Name: "foofoo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} + + msg_1 := &dns.Msg{} + msg_1.Question = append(msg_1.Question, dns.Question{Name: "bar.disco.net."}) + msg_1.Insert([]dns.RR{recordToAdd}) + msg_1.NameNotUsed([]dns.RR{prereq_fail}) + + result_1 := manager.Update("disco.net.", msg_1) + if result_1.Rcode != dns.RcodeYXDomain { + debugMsg(result_1) + t.Error("expected update to fail with RcodeYXDomain, actually got", dns.RcodeToString[result_1.Rcode]) + t.Fatal() + } + + msg_2 := &dns.Msg{} + msg_2.Question = append(msg_2.Question, dns.Question{Name: "bar.disco.net."}) + msg_2.Insert([]dns.RR{recordToAdd}) + msg_2.NameNotUsed([]dns.RR{prereq_fail, prereq_ok}) + + result_2 := manager.Update("disco.net.", msg_2) + if result_2.Rcode != dns.RcodeYXDomain { + debugMsg(result_2) + t.Error("expected update to fail with RcodeYXDomain, actually got", dns.RcodeToString[result_2.Rcode]) + t.Fatal() + } + + msg_3 := &dns.Msg{} + msg_3.Question = append(msg_3.Question, dns.Question{Name: "bar.disco.net."}) + msg_3.Insert([]dns.RR{recordToAdd}) + msg_3.NameNotUsed([]dns.RR{prereq_ok}) + + result_3 := manager.Update("disco.net.", msg_3) + if result_3.Rcode != dns.RcodeSuccess { + debugMsg(result_3) + t.Error("Failed to add DNS record with name-not-in-use prereq, got", dns.RcodeToString[result_3.Rcode]) + t.Fatal() + } +} From 18a38b256e0799accfb3fdd874a5891346e40a3e Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 23 Feb 2015 16:29:38 +0000 Subject: [PATCH 35/54] Fix prereq checks for name-(not-)in-use --- update.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/update.go b/update.go index 0717065..531d5ab 100644 --- a/update.go +++ b/update.go @@ -95,11 +95,12 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { if header.Rdlength != 0 { return dns.RcodeFormatError } else if header.Rrtype == dns.TypeANY { - if answers, ok := resolver.LookupAnswersForType(header.Name, dns.TypeANY); len(answers) > 0 { - if ok != nil { - return dns.RcodeServerFailure - } // RFC Meaning: "Name is in use" + exists, err := resolver.NameExists(header.Name) + if err != nil { + return dns.RcodeServerFailure + } + if !exists { debugMsg("Domain that should exist does not ", header.Name) return dns.RcodeNameError } @@ -117,11 +118,12 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { if header.Rdlength != 0 { return dns.RcodeFormatError } else if header.Rrtype == dns.TypeANY { - if answers, ok := resolver.LookupAnswersForType(header.Name, dns.TypeANY); len(answers) == 0 { - if ok != nil { - return dns.RcodeServerFailure - } // RFC Meaning: "Name is not in use" + exists, err := resolver.NameExists(header.Name) + if err != nil { + return dns.RcodeServerFailure + } + if exists { debugMsg("Domain that should not exist does ", header.Name) return dns.RcodeYXDomain } From 383a06ca95ce48af09007184a595292b34cf1689 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 23 Feb 2015 19:20:39 +0000 Subject: [PATCH 36/54] Tests for RRset exists (value independent) --- update_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/update_test.go b/update_test.go index c375171..7373eaf 100644 --- a/update_test.go +++ b/update_test.go @@ -215,3 +215,73 @@ func TestPrerequisites_NameNotInUse(t *testing.T) { t.Fatal() } } + +func TestPrerequisites_ValueIndependentRRSet(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestPrerequisites_ValueIndependentRRSet/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + client.Delete("TestPrerequisites_ValueIndependentRRSet/", true) + client.Set("TestPrerequisites_ValueIndependentRRSet/net/disco/foo/.A", "1.1.1.1", 0) + client.Set("TestPrerequisites_ValueIndependentRRSet/net/disco/bar/.A", "1.1.1.1", 0) + client.Set("TestPrerequisites_ValueIndependentRRSet/net/disco/bar/.PTR", "bar.disco.net", 0) + + recordToAdd := &dns.A{ + Hdr: dns.RR_Header{Name: "baz.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, + A: net.ParseIP("1.2.3.4")} + + prereq_foo_a := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}} + prereq_foo_ptr := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypePTR, Class: dns.ClassINET}} + prereq_bar_a := &dns.ANY{ Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}} + prereq_bar_ptr := &dns.ANY{ Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypePTR, Class: dns.ClassINET}} + + // same name, many types, expecting failure + msg_1 := &dns.Msg{} + msg_1.Question = append(msg_1.Question, dns.Question{Name: "bar.disco.net."}) + msg_1.Insert([]dns.RR{recordToAdd}) + msg_1.RRsetUsed([]dns.RR{prereq_foo_a, prereq_foo_ptr}) + + result_1 := manager.Update("disco.net.", msg_1) + if result_1.Rcode != dns.RcodeNXRrset { + debugMsg(result_1) + t.Error("expected update to fail with NXRRSET, actually got", dns.RcodeToString[result_1.Rcode]) + t.Fatal() + } + + // many names, same type, expecting failure + msg_2 := &dns.Msg{} + msg_2.Question = append(msg_2.Question, dns.Question{Name: "bar.disco.net."}) + msg_2.Insert([]dns.RR{recordToAdd}) + msg_2.RRsetUsed([]dns.RR{prereq_foo_ptr, prereq_bar_ptr}) + + result_2 := manager.Update("disco.net.", msg_2) + if result_2.Rcode != dns.RcodeNXRrset { + debugMsg(result_2) + t.Error("expected update to fail with NXRRSET, actually got", dns.RcodeToString[result_2.Rcode]) + t.Fatal() + } + + // same name, many types, expecting success + msg_3 := &dns.Msg{} + msg_3.Question = append(msg_3.Question, dns.Question{Name: "bar.disco.net."}) + msg_3.Insert([]dns.RR{recordToAdd}) + msg_3.RRsetUsed([]dns.RR{prereq_bar_a, prereq_bar_ptr}) + + result_3 := manager.Update("disco.net.", msg_3) + if result_3.Rcode != dns.RcodeSuccess { + debugMsg(result_3) + t.Error("Failed to add DNS record with rr-exists prereqs, got", dns.RcodeToString[result_3.Rcode]) + t.Fatal() + } + // many names, same type, expecting success + msg_4 := &dns.Msg{} + msg_4.Question = append(msg_4.Question, dns.Question{Name: "bar.disco.net."}) + msg_4.Insert([]dns.RR{recordToAdd}) + msg_4.RRsetUsed([]dns.RR{prereq_foo_a, prereq_bar_a}) + + result_4 := manager.Update("disco.net.", msg_4) + if result_4.Rcode != dns.RcodeSuccess { + debugMsg(result_4) + t.Error("Failed to add DNS record with rr-exists prereqs, got", dns.RcodeToString[result_4.Rcode]) + t.Fatal() + } +} From 0233539da8cf32cdcd3775b0cdbb86c4026b4e3e Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 23 Feb 2015 19:21:05 +0000 Subject: [PATCH 37/54] Use the rrset-exists helper in prereq checks --- resolver.go | 1 + update.go | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/resolver.go b/resolver.go index 0d8a29a..7ffc906 100644 --- a/resolver.go +++ b/resolver.go @@ -351,6 +351,7 @@ func (r *Resolver) NameExists(name string) (exists bool, err error) { return len(answers) > 0, nil } +// RRSetExists returns true if RRs exist for the given name and type (value independent) func (r *Resolver) RRSetExists(name string, rrType uint16) (exists bool, err error) { answers, err := r.LookupAnswersForType(dns.Fqdn(name), rrType) if err != nil { diff --git a/update.go b/update.go index 531d5ab..39faa8a 100644 --- a/update.go +++ b/update.go @@ -106,11 +106,12 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { } } else { // RFC Meaning: "RRset exists (value independent)" - if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); len(answers) > 0 { - if ok != nil { - return dns.RcodeServerFailure - } - debugMsg("RRset that should exist does not ", header.Name) + exists, err := resolver.RRSetExists(header.Name, header.Rrtype) + if err != nil { + return dns.RcodeServerFailure + } + if !exists { + debugMsg("RRset that should exist does not ", header.Name, header.Rrtype) return dns.RcodeNXRrset } } @@ -129,10 +130,11 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { } } else { // RFC meaning: "RRset does not exist" - if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); len(answers) == 0 { - if ok != nil { - return dns.RcodeServerFailure - } + exists, err := resolver.RRSetExists(header.Name, header.Rrtype) + if err != nil { + return dns.RcodeServerFailure + } + if exists { debugMsg("RRset that should not exist does ", header.Name) return dns.RcodeYXRrset } From 8dc8889341aab889576a314ef2e9215a389f0f0b Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 23 Feb 2015 21:23:01 +0000 Subject: [PATCH 38/54] Add strict-rr-match helper to resolver --- resolver.go | 28 ++++++++++++++++++++++++++-- resolver_test.go | 7 ------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/resolver.go b/resolver.go index 7ffc906..b9df866 100644 --- a/resolver.go +++ b/resolver.go @@ -361,6 +361,30 @@ func (r *Resolver) RRSetExists(name string, rrType uint16) (exists bool, err err return len(answers) > 0, nil } -func (r *Resolver) MatchRR(rr dns.RR) (matches, exists bool, err error) { - return false, false, nil +// RRSetMatches checks that the set of records in the DNS for the given name +// and type *exactly* match the given RRs: their data must match, and there must +// be no more or less RRs +func (r *Resolver) RRSetMatches(name string, rrType uint16, rrs []dns.RR) (matches bool, err error) { + answers, err := r.LookupAnswersForType(dns.Fqdn(name), rrType) + if err != nil { + return false, err + } + if len(answers) != len(rrs) { + return false, nil + } + matched := 0 + // I'm sure theres a neater/faster way than comparing all to all, but meh + for _, rr := range rrs { + for _, answer := range answers { + // TTLS are explicitly excluded from comparison + cmp := dns.Copy(answer) + cmp.Header().Ttl = 0 + // TODO(orls): is string() enough? is there any relevant info not in the string reprs? + if cmp.String() == rr.String() { + matched++ + break + } + } + } + return matched == len(rrs), nil } diff --git a/resolver_test.go b/resolver_test.go index 1774db3..a4a4356 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -970,10 +970,3 @@ func TestRRSetExistsDoesNotExist(t *testing.T) { } } -func TestMatchRRMatches(t *testing.T) { - -} - -func TestMatchRRDoesNotMatch(t *testing.T) { - -} From b8fa209bcb339f841c39e4eea43b16825034f954 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Mon, 23 Feb 2015 21:29:31 +0000 Subject: [PATCH 39/54] Validate RRset matches in prereq checks --- update.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/update.go b/update.go index 39faa8a..9098b91 100644 --- a/update.go +++ b/update.go @@ -81,10 +81,17 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) return } +// internal utility struct for making a map of RRsets a bit neater to construct +type matchKey struct { + name string + rrType uint16 +} + // validatePrerequisites will perform all necessary validation checks against // update prerequisites and return the relevant status is validation fails, // otherwise NOERROR(0) will be returned. func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { + rrSetsToMatch := make(map[matchKey][]dns.RR) for _, record := range rr { header := record.Header() if header.Ttl != 0 { @@ -144,19 +151,24 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { return dns.RcodeFormatError } else { // RFC Meaning: "RRset exists (value dependent)" + mKey := matchKey{name: header.Name, rrType: header.Rrtype} + rrSetsToMatch[mKey] = append(rrSetsToMatch[mKey], record) } - - // TODO(tarnfeld): Perform strict comparisons between the resource records - // if answers, ok := resolver.LookupAnswersForType(header.Name, header.Rrtype); answers != rr { - // if ok != nil { - // return dns.RcodeServerFailure - // } - // } } else { return dns.RcodeFormatError } } + for matchKey, rrs := range rrSetsToMatch { + matched, err := resolver.RRSetMatches(matchKey.name, matchKey.rrType, rrs) + if err != nil { + return dns.RcodeServerFailure + } + if !matched { + return dns.RcodeNXRrset + } + } + return dns.RcodeSuccess } From 82b05066646751244c15859d76fa1e68d57c5281 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 24 Feb 2015 00:23:58 +0000 Subject: [PATCH 40/54] Factor out test boilerplate via reflection --- update_test.go | 166 ++++++++++++------------------------------------- 1 file changed, 40 insertions(+), 126 deletions(-) diff --git a/update_test.go b/update_test.go index 7373eaf..f46d837 100644 --- a/update_test.go +++ b/update_test.go @@ -4,6 +4,7 @@ import ( "github.com/miekg/dns" "net" "testing" + "reflect" ) func TestInsertNewRecordNoPrerequsites(t *testing.T) { @@ -114,55 +115,50 @@ func TestInsertMultipleRecords(t *testing.T) { } } -func TestPrerequisites_NameInUse(t *testing.T) { - manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestPrerequisites_NameInUse/", resolver: resolver} - resolver.etcdPrefix = manager.etcdPrefix - - client.Delete("TestPrerequisites_NameInUse/", true) - client.Set("TestPrerequisites_NameInUse/net/disco/foo/.A", "1.1.1.1", 0) +// Internal utility to save boilerplate. Creates a message with the given +// prereqs and tries to perform an update +func _prereqsTestHelper(t *testing.T, manager *DynamicUpdateManager, prereqMethod string, expected int, prereqs []dns.RR) { recordToAdd := &dns.A{ - Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, + Hdr: dns.RR_Header{Name: "baz.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, A: net.ParseIP("1.2.3.4")} - prereq_fail := &dns.ANY{ Hdr: dns.RR_Header{Name: "foofoo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} - prereq_ok := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} + msg := &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net.", Qclass: dns.ClassINET}) + msg.Insert([]dns.RR{recordToAdd}) + reflPrereqs := reflect.ValueOf(prereqs) + v := reflect.ValueOf(msg) + m := v.MethodByName(prereqMethod) + m.Call([]reflect.Value{reflPrereqs}) - msg_1 := &dns.Msg{} - msg_1.Question = append(msg_1.Question, dns.Question{Name: "bar.disco.net."}) - msg_1.Insert([]dns.RR{recordToAdd}) - msg_1.NameUsed([]dns.RR{prereq_fail}) + var errorMsg string + if (expected == dns.RcodeSuccess) { + errorMsg = "Failed to add DNS record with `" + prereqMethod +"` prereq, got" + } else { + errorMsg = "Expected update with `" + prereqMethod +"` prereqs to fail with " + dns.RcodeToString[expected] + ", got" + } - result_1 := manager.Update("disco.net.", msg_1) - if result_1.Rcode != dns.RcodeNameError { - debugMsg(result_1) - t.Error("expected update to fail with NXDOMAIN, actually got", dns.RcodeToString[result_1.Rcode]) + result := manager.Update("disco.net.", msg) + if result.Rcode != expected { + debugMsg(result) + t.Error(errorMsg, dns.RcodeToString[result.Rcode]) t.Fatal() } +} - msg_2 := &dns.Msg{} - msg_2.Question = append(msg_2.Question, dns.Question{Name: "bar.disco.net."}) - msg_2.Insert([]dns.RR{recordToAdd}) - msg_2.NameUsed([]dns.RR{prereq_fail, prereq_ok}) +func TestPrerequisites_NameInUse(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestPrerequisites_NameInUse/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix - result_2 := manager.Update("disco.net.", msg_2) - if result_2.Rcode != dns.RcodeNameError { - debugMsg(result_2) - t.Error("expected update to fail with NXDOMAIN, actually got", dns.RcodeToString[result_2.Rcode]) - t.Fatal() - } + client.Delete("TestPrerequisites_NameInUse/", true) + client.Set("TestPrerequisites_NameInUse/net/disco/foo/.A", "1.1.1.1", 0) - msg_3 := &dns.Msg{} - msg_3.Question = append(msg_3.Question, dns.Question{Name: "bar.disco.net."}) - msg_3.Insert([]dns.RR{recordToAdd}) - msg_3.NameUsed([]dns.RR{prereq_ok}) + prereq_fail := &dns.ANY{ Hdr: dns.RR_Header{Name: "foofoo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} + prereq_ok := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} - result_3 := manager.Update("disco.net.", msg_3) - if result_3.Rcode != dns.RcodeSuccess { - debugMsg(result_3) - t.Error("Failed to add DNS record with name-in-use prereq, got", dns.RcodeToString[result_3.Rcode]) - t.Fatal() - } + _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeNameError, []dns.RR{prereq_fail}) + _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeNameError, []dns.RR{prereq_fail, prereq_ok}) + _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeSuccess, []dns.RR{prereq_ok}) } func TestPrerequisites_NameNotInUse(t *testing.T) { @@ -172,48 +168,12 @@ func TestPrerequisites_NameNotInUse(t *testing.T) { client.Delete("TestPrerequisites_NameNotInUse/", true) client.Set("TestPrerequisites_NameNotInUse/net/disco/foo/.A", "1.1.1.1", 0) - recordToAdd := &dns.A{ - Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, - A: net.ParseIP("1.2.3.4")} - prereq_fail := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} prereq_ok := &dns.ANY{ Hdr: dns.RR_Header{Name: "foofoo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} - msg_1 := &dns.Msg{} - msg_1.Question = append(msg_1.Question, dns.Question{Name: "bar.disco.net."}) - msg_1.Insert([]dns.RR{recordToAdd}) - msg_1.NameNotUsed([]dns.RR{prereq_fail}) - - result_1 := manager.Update("disco.net.", msg_1) - if result_1.Rcode != dns.RcodeYXDomain { - debugMsg(result_1) - t.Error("expected update to fail with RcodeYXDomain, actually got", dns.RcodeToString[result_1.Rcode]) - t.Fatal() - } - - msg_2 := &dns.Msg{} - msg_2.Question = append(msg_2.Question, dns.Question{Name: "bar.disco.net."}) - msg_2.Insert([]dns.RR{recordToAdd}) - msg_2.NameNotUsed([]dns.RR{prereq_fail, prereq_ok}) - - result_2 := manager.Update("disco.net.", msg_2) - if result_2.Rcode != dns.RcodeYXDomain { - debugMsg(result_2) - t.Error("expected update to fail with RcodeYXDomain, actually got", dns.RcodeToString[result_2.Rcode]) - t.Fatal() - } - - msg_3 := &dns.Msg{} - msg_3.Question = append(msg_3.Question, dns.Question{Name: "bar.disco.net."}) - msg_3.Insert([]dns.RR{recordToAdd}) - msg_3.NameNotUsed([]dns.RR{prereq_ok}) - - result_3 := manager.Update("disco.net.", msg_3) - if result_3.Rcode != dns.RcodeSuccess { - debugMsg(result_3) - t.Error("Failed to add DNS record with name-not-in-use prereq, got", dns.RcodeToString[result_3.Rcode]) - t.Fatal() - } + _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeYXDomain, []dns.RR{prereq_fail}) + _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeYXDomain, []dns.RR{prereq_fail, prereq_ok}) + _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeSuccess, []dns.RR{prereq_ok}) } func TestPrerequisites_ValueIndependentRRSet(t *testing.T) { @@ -225,63 +185,17 @@ func TestPrerequisites_ValueIndependentRRSet(t *testing.T) { client.Set("TestPrerequisites_ValueIndependentRRSet/net/disco/bar/.A", "1.1.1.1", 0) client.Set("TestPrerequisites_ValueIndependentRRSet/net/disco/bar/.PTR", "bar.disco.net", 0) - recordToAdd := &dns.A{ - Hdr: dns.RR_Header{Name: "baz.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, - A: net.ParseIP("1.2.3.4")} - prereq_foo_a := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}} prereq_foo_ptr := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypePTR, Class: dns.ClassINET}} prereq_bar_a := &dns.ANY{ Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}} prereq_bar_ptr := &dns.ANY{ Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypePTR, Class: dns.ClassINET}} // same name, many types, expecting failure - msg_1 := &dns.Msg{} - msg_1.Question = append(msg_1.Question, dns.Question{Name: "bar.disco.net."}) - msg_1.Insert([]dns.RR{recordToAdd}) - msg_1.RRsetUsed([]dns.RR{prereq_foo_a, prereq_foo_ptr}) - - result_1 := manager.Update("disco.net.", msg_1) - if result_1.Rcode != dns.RcodeNXRrset { - debugMsg(result_1) - t.Error("expected update to fail with NXRRSET, actually got", dns.RcodeToString[result_1.Rcode]) - t.Fatal() - } - + _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_a, prereq_foo_ptr}) // many names, same type, expecting failure - msg_2 := &dns.Msg{} - msg_2.Question = append(msg_2.Question, dns.Question{Name: "bar.disco.net."}) - msg_2.Insert([]dns.RR{recordToAdd}) - msg_2.RRsetUsed([]dns.RR{prereq_foo_ptr, prereq_bar_ptr}) - - result_2 := manager.Update("disco.net.", msg_2) - if result_2.Rcode != dns.RcodeNXRrset { - debugMsg(result_2) - t.Error("expected update to fail with NXRRSET, actually got", dns.RcodeToString[result_2.Rcode]) - t.Fatal() - } - + _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_ptr, prereq_bar_ptr}) // same name, many types, expecting success - msg_3 := &dns.Msg{} - msg_3.Question = append(msg_3.Question, dns.Question{Name: "bar.disco.net."}) - msg_3.Insert([]dns.RR{recordToAdd}) - msg_3.RRsetUsed([]dns.RR{prereq_bar_a, prereq_bar_ptr}) - - result_3 := manager.Update("disco.net.", msg_3) - if result_3.Rcode != dns.RcodeSuccess { - debugMsg(result_3) - t.Error("Failed to add DNS record with rr-exists prereqs, got", dns.RcodeToString[result_3.Rcode]) - t.Fatal() - } + _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_bar_a, prereq_bar_ptr}) // many names, same type, expecting success - msg_4 := &dns.Msg{} - msg_4.Question = append(msg_4.Question, dns.Question{Name: "bar.disco.net."}) - msg_4.Insert([]dns.RR{recordToAdd}) - msg_4.RRsetUsed([]dns.RR{prereq_foo_a, prereq_bar_a}) - - result_4 := manager.Update("disco.net.", msg_4) - if result_4.Rcode != dns.RcodeSuccess { - debugMsg(result_4) - t.Error("Failed to add DNS record with rr-exists prereqs, got", dns.RcodeToString[result_4.Rcode]) - t.Fatal() - } + _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_foo_a, prereq_bar_a}) } From f1458570167d4dfa2fffb8f4744ed7ab210863c9 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 24 Feb 2015 01:15:34 +0000 Subject: [PATCH 41/54] Make test faliures report right line --- update_test.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/update_test.go b/update_test.go index f46d837..b69e38d 100644 --- a/update_test.go +++ b/update_test.go @@ -117,7 +117,7 @@ func TestInsertMultipleRecords(t *testing.T) { // Internal utility to save boilerplate. Creates a message with the given // prereqs and tries to perform an update -func _prereqsTestHelper(t *testing.T, manager *DynamicUpdateManager, prereqMethod string, expected int, prereqs []dns.RR) { +func _prereqsTestHelper(t *testing.T, manager *DynamicUpdateManager, prereqMethod string, expected int, prereqs []dns.RR) (pass bool) { recordToAdd := &dns.A{ Hdr: dns.RR_Header{Name: "baz.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, @@ -142,8 +142,9 @@ func _prereqsTestHelper(t *testing.T, manager *DynamicUpdateManager, prereqMetho if result.Rcode != expected { debugMsg(result) t.Error(errorMsg, dns.RcodeToString[result.Rcode]) - t.Fatal() + return false } + return true } func TestPrerequisites_NameInUse(t *testing.T) { @@ -156,9 +157,9 @@ func TestPrerequisites_NameInUse(t *testing.T) { prereq_fail := &dns.ANY{ Hdr: dns.RR_Header{Name: "foofoo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} prereq_ok := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} - _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeNameError, []dns.RR{prereq_fail}) - _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeNameError, []dns.RR{prereq_fail, prereq_ok}) - _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeSuccess, []dns.RR{prereq_ok}) + if ! _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeNameError, []dns.RR{prereq_fail}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeNameError, []dns.RR{prereq_fail, prereq_ok}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "NameUsed", dns.RcodeSuccess, []dns.RR{prereq_ok}) { t.Fatal() } } func TestPrerequisites_NameNotInUse(t *testing.T) { @@ -171,9 +172,9 @@ func TestPrerequisites_NameNotInUse(t *testing.T) { prereq_fail := &dns.ANY{ Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} prereq_ok := &dns.ANY{ Hdr: dns.RR_Header{Name: "foofoo.disco.net.", Rrtype: dns.TypeANY, Class: dns.ClassINET}} - _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeYXDomain, []dns.RR{prereq_fail}) - _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeYXDomain, []dns.RR{prereq_fail, prereq_ok}) - _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeSuccess, []dns.RR{prereq_ok}) + if ! _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeYXDomain, []dns.RR{prereq_fail}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeYXDomain, []dns.RR{prereq_fail, prereq_ok}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "NameNotUsed", dns.RcodeSuccess, []dns.RR{prereq_ok}) { t.Fatal() } } func TestPrerequisites_ValueIndependentRRSet(t *testing.T) { @@ -191,11 +192,11 @@ func TestPrerequisites_ValueIndependentRRSet(t *testing.T) { prereq_bar_ptr := &dns.ANY{ Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypePTR, Class: dns.ClassINET}} // same name, many types, expecting failure - _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_a, prereq_foo_ptr}) + if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_a, prereq_foo_ptr}) { t.Fatal() } // many names, same type, expecting failure - _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_ptr, prereq_bar_ptr}) + if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_ptr, prereq_bar_ptr}) { t.Fatal() } // same name, many types, expecting success - _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_bar_a, prereq_bar_ptr}) + if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_bar_a, prereq_bar_ptr}) { t.Fatal() } // many names, same type, expecting success - _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_foo_a, prereq_bar_a}) + if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_foo_a, prereq_bar_a}) { t.Fatal() } } From 16786e0cc483c202ebb88836748a515bc1828f1b Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 24 Feb 2015 01:21:39 +0000 Subject: [PATCH 42/54] Tests for RRset exists (value dependent) --- update_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/update_test.go b/update_test.go index b69e38d..bb88464 100644 --- a/update_test.go +++ b/update_test.go @@ -200,3 +200,28 @@ func TestPrerequisites_ValueIndependentRRSet(t *testing.T) { // many names, same type, expecting success if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_foo_a, prereq_bar_a}) { t.Fatal() } } + +func TestPrerequisites_ValueDependentRRSet(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestPrerequisites_ValueDependentRRSet/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + client.Delete("TestPrerequisites_ValueDependentRRSet/", true) + client.Set("TestPrerequisites_ValueDependentRRSet/net/disco/foo/.A", "1.1.1.1", 0) + client.Set("TestPrerequisites_ValueDependentRRSet/net/disco/bar/.A", "1.1.1.1", 0) + client.Set("TestPrerequisites_ValueDependentRRSet/net/disco/bar/.PTR", "match.disco.net", 0) + + prereq_foo_a_match, _ := dns.NewRR("foo.disco.net. 0 IN A 1.1.1.1") + prereq_foo_a_miss, _ := dns.NewRR("foo.disco.net. 0 IN A 2.2.2.2") + prereq_bar_a_match, _ := dns.NewRR("bar.disco.net. 0 IN A 1.1.1.1") + prereq_bar_a_miss, _ := dns.NewRR("bar.disco.net. 0 IN A 2.2.2.2") + prereq_bar_ptr_match, _ := dns.NewRR("bar.disco.net. 0 IN PTR match.disco.net") + prereq_bar_ptr_miss, _ := dns.NewRR("bar.disco.net. 0 IN PTR miss.disco.net") + + if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeNXRrset, []dns.RR{prereq_foo_a_miss}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeNXRrset, []dns.RR{prereq_foo_a_miss, prereq_bar_a_miss}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeNXRrset, []dns.RR{prereq_foo_a_miss, prereq_bar_ptr_miss}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeNXRrset, []dns.RR{prereq_foo_a_match, prereq_bar_a_miss}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeNXRrset, []dns.RR{prereq_foo_a_match, prereq_bar_ptr_miss}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeSuccess, []dns.RR{prereq_foo_a_match, prereq_bar_a_match}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeSuccess, []dns.RR{prereq_foo_a_match, prereq_bar_a_match, prereq_bar_ptr_match}) { t.Fatal() } +} From ab7021fd78614348fa75977b59098e8c1efaea33 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 24 Feb 2015 01:34:11 +0000 Subject: [PATCH 43/54] Cover RRset does not exist too --- update_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/update_test.go b/update_test.go index bb88464..98b7874 100644 --- a/update_test.go +++ b/update_test.go @@ -191,13 +191,17 @@ func TestPrerequisites_ValueIndependentRRSet(t *testing.T) { prereq_bar_a := &dns.ANY{ Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}} prereq_bar_ptr := &dns.ANY{ Hdr: dns.RR_Header{Name: "bar.disco.net.", Rrtype: dns.TypePTR, Class: dns.ClassINET}} - // same name, many types, expecting failure - if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_a, prereq_foo_ptr}) { t.Fatal() } - // many names, same type, expecting failure - if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_ptr, prereq_bar_ptr}) { t.Fatal() } - // same name, many types, expecting success + if ! _prereqsTestHelper(t, manager, "RRsetNotUsed", dns.RcodeSuccess, []dns.RR{prereq_foo_ptr}) { t.Fatal() } + + if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_a, prereq_foo_ptr}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "RRsetNotUsed", dns.RcodeYXRrset, []dns.RR{prereq_foo_a, prereq_foo_ptr}) { t.Fatal() } + + if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeNXRrset, []dns.RR{prereq_foo_ptr, prereq_bar_ptr}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "RRsetNotUsed", dns.RcodeYXRrset, []dns.RR{prereq_foo_ptr, prereq_bar_ptr}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_bar_a, prereq_bar_ptr}) { t.Fatal() } - // many names, same type, expecting success + if ! _prereqsTestHelper(t, manager, "RRsetNotUsed", dns.RcodeYXRrset, []dns.RR{prereq_bar_a, prereq_bar_ptr}) { t.Fatal() } + if ! _prereqsTestHelper(t, manager, "RRsetUsed", dns.RcodeSuccess, []dns.RR{prereq_foo_a, prereq_bar_a}) { t.Fatal() } } From ab3aaa9afb4fab479f72f1c84b7ec4c850f0f4e1 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 24 Feb 2015 10:48:22 +0000 Subject: [PATCH 44/54] Internal notes on fixes needed in performUpdate --- update.go | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/update.go b/update.go index 9098b91..360d135 100644 --- a/update.go +++ b/update.go @@ -193,16 +193,34 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in node.Key = prefix + node.Key if header.Class == dns.ClassANY { - debugMsg("Deleting all RRs from key " + node.Key) - _, err := etcd.Delete(node.Key, true) - if err != nil { - debugMsg(err) - panic("Failed to delete RRs from key " + node.Key) + + if header.Rrtype == dns.TypeANY { + // RFC Meaning: Delete all RRsets from a name + debugMsg("Deleting all RRs from key " + node.Key) + _, err := etcd.Delete(node.Key, true) + if err != nil { + debugMsg(err) + panic("Failed to delete RRs from key " + node.Key) + } + } else { + // RFC Meaning: Delete an RRset + panic("Not yet supported: Delete an RRset") } + } else if header.Class == dns.ClassNONE { + // RFC Meaning: Delete an RR from an RRset + panic("Not yet supported: Delete an RR from an RRset") + // TODO(orls): We should have already validated that this has a + // valid Type (not ANY) + } else { + // RFC Meaning: Add to an RRset + + // TODO(orls): We should have already validated that the class is + // the same as the zone (which basically means INET, but it should + // be checked prior) + + // TODO(orls): We should have already validated that this has a + // valid Type (not ANY) - } else if header.Class == dns.ClassNONE { // Delete an RR - debugMsg("Delete specific RR: " + rr.String()) - } else { // Insert RR debugMsg("Inserting " + node.Value + " to " + node.Key) // Insert the record into etcd. Use MD5 of the node value as the From b6c5859e9ba3f7d817f4ff524356f4879b225f72 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 24 Feb 2015 11:48:40 +0000 Subject: [PATCH 45/54] Full validation of update RRS before updating --- update.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 3 deletions(-) diff --git a/update.go b/update.go index 360d135..e2d4415 100644 --- a/update.go +++ b/update.go @@ -26,6 +26,11 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) msg = new(dns.Msg) msg.SetReply(req) + // dns update re-aporopriates DNS message blocks: + // msg.Question: Zone info for whole request + // msg.Answer: prerequisites + // msg.Ns: the actual update RRs + // Verify the updates are within the zone we're modifying, since cross // zone updates are invalid. for _, rrs := range rrsets { @@ -64,10 +69,17 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) // Validate the prerequisites of the update, returning immediately if they // are not satisfied. - validationStatus := validatePrerequisites(req.Answer, u.resolver) - if validationStatus != dns.RcodeSuccess { + prereqValidation := validatePrerequisites(req.Answer, u.resolver) + if prereqValidation != dns.RcodeSuccess { debugMsg("Validation of prerequisites failed") - msg.SetRcode(req, validationStatus) + msg.SetRcode(req, prereqValidation) + return + } + + updateValidation := validateUpdates(req.Ns, req.Question[0]) + if updateValidation != dns.RcodeSuccess { + debugMsg("Validation of update instructions failed") + msg.SetRcode(req, updateValidation) return } @@ -90,6 +102,7 @@ type matchKey struct { // validatePrerequisites will perform all necessary validation checks against // update prerequisites and return the relevant status is validation fails, // otherwise NOERROR(0) will be returned. +// See RFC 2136, section 3.2 func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { rrSetsToMatch := make(map[matchKey][]dns.RR) for _, record := range rr { @@ -172,9 +185,55 @@ func validatePrerequisites(rr []dns.RR, resolver *Resolver) (rcode int) { return dns.RcodeSuccess } +// validateUpdates ensures that the given update instructions conform to the RFC +// and are processable, before we begin mutating state +// See RFC 2136, section 3.4.1 +func validateUpdates(rrs []dns.RR, updateZone dns.Question) (rcode int) { + + // name-in-zone checks have already been performed. + + badTypes := map[uint16]bool{ dns.TypeIXFR : true, + dns.TypeAXFR : true, dns.TypeMAILB : true, dns.TypeMAILA : true, + dns.TypeANY : true} + anyClsBadTypes := map[uint16]bool{ dns.TypeIXFR : true, + dns.TypeAXFR : true, dns.TypeMAILB : true,dns.TypeMAILA : true} + + for _, rr := range rrs { + header := rr.Header() + if header.Class == updateZone.Qclass { + if badTypes[header.Rrtype] { + debugMsg("Bad type for class:", dns.ClassToString[header.Class], + header.Name, dns.TypeToString[header.Rrtype]) + return dns.RcodeFormatError + } + } else if header.Class == dns.ClassANY { + if header.Ttl != 0 || header.Rdlength != 0 || anyClsBadTypes[header.Rrtype] { + debugMsg("Bad ttl/length/type for class:", dns.ClassToString[header.Class], + header.Name, header.Ttl, header.Rdlength, dns.TypeToString[header.Rrtype]) + return dns.RcodeFormatError + } + } else if header.Class == dns.ClassNONE { + if header.Ttl != 0 || badTypes[header.Rrtype] { + debugMsg("Bad ttl/type for class:", dns.ClassToString[header.Class], + header.Name, header.Ttl, dns.TypeToString[header.Rrtype]) + return dns.RcodeFormatError + } + } else { + return dns.RcodeFormatError + } + // separately from the RFC validation, fail for RR types we don't understand yet + if _, ok := convertersFromRR[header.Rrtype]; ok != true { + debugMsg("Record converter doesn't exist for " + dns.TypeToString[header.Rrtype]) + return dns.RcodeServerFailure + } + } + return dns.RcodeSuccess +} + // performUpdate will commit the requested updates to the database // It is assumed by this point all prerequisites have been validated and all // domains are locked. +// See RFC 2136, section 3.4.2 func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode int) { for _, rr := range records { header := rr.Header() From a99838acf708596751d790b93a11ec5f454c1ec5 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Tue, 24 Feb 2015 13:39:04 +0000 Subject: [PATCH 46/54] Don't shadow the etcd package --- update.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/update.go b/update.go index e2d4415..9bf424b 100644 --- a/update.go +++ b/update.go @@ -234,7 +234,7 @@ func validateUpdates(rrs []dns.RR, updateZone dns.Question) (rcode int) { // It is assumed by this point all prerequisites have been validated and all // domains are locked. // See RFC 2136, section 3.4.2 -func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode int) { +func performUpdate(prefix string, etcdClient *etcd.Client, records []dns.RR) (rcode int) { for _, rr := range records { header := rr.Header() if _, ok := convertersFromRR[header.Rrtype]; ok != true { @@ -256,7 +256,7 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in if header.Rrtype == dns.TypeANY { // RFC Meaning: Delete all RRsets from a name debugMsg("Deleting all RRs from key " + node.Key) - _, err := etcd.Delete(node.Key, true) + _, err := etcdClient.Delete(node.Key, true) if err != nil { debugMsg(err) panic("Failed to delete RRs from key " + node.Key) @@ -288,7 +288,7 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in hasher := md5.New() hasher.Write([]byte(node.Value)) subkey := hex.EncodeToString(hasher.Sum(nil)) - response, err := etcd.Set(node.Key + "/" + subkey, node.Value, 0) + response, err := etcdClient.Set(node.Key + "/" + subkey, node.Value, 0) if err != nil { debugMsg(err) panic("Failed to insert record into etcd") @@ -297,7 +297,7 @@ func performUpdate(prefix string, etcd *etcd.Client, records []dns.RR) (rcode in // Insert the TTL record if one has been requested if header.Ttl > 0 { ttl := strconv.FormatInt(int64(header.Ttl), 10) - _, err = etcd.Set(response.Node.Key + ".ttl", ttl, 0) + _, err = etcdClient.Set(response.Node.Key + ".ttl", ttl, 0) if err != nil { debugMsg(err) panic("Failed to insert ttl into etcd") From ffb2ff614c92f32cdb7a1a60e2b27872b8fd7b02 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 25 Feb 2015 09:00:18 +0000 Subject: [PATCH 47/54] Fix delete tests: - Clarify that it's remove-name (i.e. all types) - use own etcd namespace and pre-set values - test for deleting something that doesn't exist --- update_test.go | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/update_test.go b/update_test.go index 98b7874..7a2b070 100644 --- a/update_test.go +++ b/update_test.go @@ -38,15 +38,17 @@ func TestInsertNewRecordNoPrerequsites(t *testing.T) { } } -func TestDeleteRecordNoPrerequsites(t *testing.T) { - manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestRecordNoPrerequsites/", resolver: resolver} +func TestDeleteNameNoPrerequsites(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestDeleteNameNoPrerequsites/", resolver: resolver} resolver.etcdPrefix = manager.etcdPrefix - // record := &dns.A{ - // Hdr: dns.RR_Header{Name: "disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, - // A: net.ParseIP("1.2.3.4")} + client.Delete("TestDeleteNameNoPrerequsites/", true) + client.Set("TestDeleteNameNoPrerequsites/net/disco/foo/.A", "1.1.1.1", 0) + client.Set("TestDeleteNameNoPrerequsites/net/disco/foo/.PTR/a", "a", 0) + client.Set("TestDeleteNameNoPrerequsites/net/disco/foo/.PTR/b", "b", 0) + client.Set("TestDeleteNameNoPrerequsites/net/disco/foo/.PTR/a.ttl", "100", 0) - record := &dns.ANY{Hdr: dns.RR_Header{Name: "disco.net."}} + record := &dns.ANY{Hdr: dns.RR_Header{Name: "foo.disco.net."}} msg := &dns.Msg{} msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) @@ -59,13 +61,26 @@ func TestDeleteRecordNoPrerequsites(t *testing.T) { t.Fatal() } - answers, err := resolver.LookupAnswersForType("disco.net.", dns.TypeA) + answers, err := resolver.LookupAnswersForType("foo.disco.net.", dns.TypeANY) if err != nil { t.Error("Caught error resolving domain") t.Fatal() } if len(answers) > 0 { - t.Error("Expected zero answers for discodns.net.") + t.Error("Expected zero answers for foo.disco.net.") + t.Fatal() + } + + // Delete for something that doesn't already exist: + record = &dns.ANY{Hdr: dns.RR_Header{Name: "bar.disco.net."}} + msg = &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) + msg.RemoveName([]dns.RR{record}) + + result = manager.Update("disco.net.", msg) + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("Got failure from a no-op delete") t.Fatal() } } From cc870c812a01f38475207fb6853fc65efd32ea2a Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 25 Feb 2015 09:11:37 +0000 Subject: [PATCH 48/54] Check TTL on inserted record --- update_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/update_test.go b/update_test.go index 7a2b070..397b3d8 100644 --- a/update_test.go +++ b/update_test.go @@ -12,7 +12,7 @@ func TestInsertNewRecordNoPrerequsites(t *testing.T) { resolver.etcdPrefix = manager.etcdPrefix record := &dns.A{ - Hdr: dns.RR_Header{Name: "disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET}, + Hdr: dns.RR_Header{Name: "disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 1234}, A: net.ParseIP("1.2.3.4")} msg := &dns.Msg{} @@ -36,6 +36,11 @@ func TestInsertNewRecordNoPrerequsites(t *testing.T) { t.Error("Expected exactly one answer for discodns.net.") t.Fatal() } + answerHeader := answers[0].Header() + if answerHeader.Ttl != 1234 { + t.Error("Didn't get expected TTL on new record") + t.Fatal() + } } func TestDeleteNameNoPrerequsites(t *testing.T) { From fc4b785322b23658f5e920a12f1222802fa8a109 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 25 Feb 2015 10:20:52 +0000 Subject: [PATCH 49/54] Add (failing) test for Delete-RRset: This fails because of how we convert RRs to etcd nodes early on: doesn't work for some specialized delete RR forms --- resolver.go | 2 +- update_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/resolver.go b/resolver.go index b9df866..035de97 100644 --- a/resolver.go +++ b/resolver.go @@ -71,7 +71,7 @@ func (r *Resolver) GetFromStorage(key string) (nodes []*EtcdRecord, err error) { return } - // If we don't have a TLL try and find one + // If we don't have a TTL try and find one if tryTtl { ttlKey := node.Key + ".ttl" diff --git a/update_test.go b/update_test.go index 397b3d8..45b88f5 100644 --- a/update_test.go +++ b/update_test.go @@ -84,7 +84,62 @@ func TestDeleteNameNoPrerequsites(t *testing.T) { result = manager.Update("disco.net.", msg) if result.Rcode != dns.RcodeSuccess { - debugMsg(result) + t.Error("Got failure from a no-op delete") + t.Fatal() + } +} + +func TestDeleteRecordsetNoPrerequsites(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestDeleteRecordsetNoPrerequsites/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + client.Delete("TestDeleteRecordsetNoPrerequsites/", true) + client.Set("TestDeleteRecordsetNoPrerequsites/net/disco/foo/.A", "1.1.1.1", 0) + client.Set("TestDeleteRecordsetNoPrerequsites/net/disco/foo/.PTR/a", "a", 0) + client.Set("TestDeleteRecordsetNoPrerequsites/net/disco/foo/.PTR/b", "b", 0) + client.Set("TestDeleteRecordsetNoPrerequsites/net/disco/foo/.PTR/a.ttl", "100", 0) + + record := &dns.PTR{Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypePTR}, Ptr: "whatever"} + + msg := &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) + msg.RemoveRRset([]dns.RR{record}) + + result := manager.Update("disco.net.", msg) + if result.Rcode != dns.RcodeSuccess { + t.Error("Failed to remove DNS records") + t.Fatal() + } + + answers, err := resolver.LookupAnswersForType("foo.disco.net.", dns.TypePTR) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) > 0 { + t.Error("Expected zero answers for foo.disco.net. PTR") + t.Fatal() + } + + // Check the A record was left alone: + answers, err = resolver.LookupAnswersForType("foo.disco.net.", dns.TypeA) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) != 1 { + t.Error("Expected one answer for foo.disco.net. A") + t.Fatal() + } + + // Delete for something that doesn't already exist: + record = &dns.PTR{Hdr: dns.RR_Header{Name: "foo.disco.net.", Rrtype: dns.TypePTR}, Ptr: "whatever"} + msg = &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) + msg.RemoveRRset([]dns.RR{record}) + + result = manager.Update("disco.net.", msg) + if result.Rcode != dns.RcodeSuccess { t.Error("Got failure from a no-op delete") t.Fatal() } From dbe9da9818770b1361d110d7714efece8fa4a672 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 25 Feb 2015 10:24:38 +0000 Subject: [PATCH 50/54] WIP: rewrite performUpdate: - check for existing keys before RR-deletes and inserts - add some extensive testing for insert/update given different etcd layouts - auto-convert old-style keys to directories if needed --- update.go | 193 +++++++++++++++++++++++++++++++++++++------------ update_test.go | 138 +++++++++++++++++++++++++++++++++++ 2 files changed, 285 insertions(+), 46 deletions(-) diff --git a/update.go b/update.go index 9bf424b..f56d11f 100644 --- a/update.go +++ b/update.go @@ -88,7 +88,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) // result in a partially updated zone. // TODO(tarnfeld): Figure out a way of rolling back changes, perhaps make // use of the etcd indexes? - msg.SetRcode(req, performUpdate(u.etcdPrefix, u.etcd, req.Ns)) + msg.SetRcode(req, performUpdate(u.etcdPrefix, u.etcd, u.resolver, req.Ns)) return } @@ -234,73 +234,155 @@ func validateUpdates(rrs []dns.RR, updateZone dns.Question) (rcode int) { // It is assumed by this point all prerequisites have been validated and all // domains are locked. // See RFC 2136, section 3.4.2 -func performUpdate(prefix string, etcdClient *etcd.Client, records []dns.RR) (rcode int) { +func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, records []dns.RR) (rcode int) { for _, rr := range records { header := rr.Header() + debugMsg("update rr: header", header) if _, ok := convertersFromRR[header.Rrtype]; ok != true { panic("Record converter doesn't exist for " + dns.TypeToString[header.Rrtype]) } - node, err := convertRRToNode(rr, *header) + updateNode, err := convertRRToNode(rr, *header) if err != nil { panic("Got error when converting node") - } else if node == nil { + } else if updateNode == nil { panic("Got NIL after successfully converting node") } // Prepend the etcd prefix, if we're given one - node.Key = prefix + node.Key + newRecordDir := prefix + updateNode.Key if header.Class == dns.ClassANY { + // RFC Meaning if type is ANY: Delete all RRsets from a name + // RFC Meaning if type is not ANY: Delete an RRset (all RRs of type) - if header.Rrtype == dns.TypeANY { - // RFC Meaning: Delete all RRsets from a name - debugMsg("Deleting all RRs from key " + node.Key) - _, err := etcdClient.Delete(node.Key, true) - if err != nil { - debugMsg(err) - panic("Failed to delete RRs from key " + node.Key) - } - } else { - // RFC Meaning: Delete an RRset - panic("Not yet supported: Delete an RRset") - } - } else if header.Class == dns.ClassNONE { - // RFC Meaning: Delete an RR from an RRset - panic("Not yet supported: Delete an RR from an RRset") - // TODO(orls): We should have already validated that this has a - // valid Type (not ANY) - } else { - // RFC Meaning: Add to an RRset + // `convertRRToNode` will already have given us the right key + // depending on Type - // TODO(orls): We should have already validated that the class is - // the same as the zone (which basically means INET, but it should - // be checked prior) + // TODO: the key will be wrong for ANY, no? + // TODO: for ANY, need to only delete RRs and not child names. See what the RFC says, + // is it an error if there are child nodes? - // TODO(orls): We should have already validated that this has a - // valid Type (not ANY) + debugMsg("Deleting all RRs from key " + newRecordDir) + _, err := etcdClient.Delete(newRecordDir, true) + if err != nil && !missingKeyErr(err) { + debugMsg(err) + panic("Failed to delete RRs from key " + newRecordDir) + } - debugMsg("Inserting " + node.Value + " to " + node.Key) + _, err = etcdClient.Delete(newRecordDir + ".ttl", true) + if err != nil && !missingKeyErr(err) { + debugMsg(err) + panic("Failed to delete RR TTLs from key " + newRecordDir + ".ttl") + } + } else { - // Insert the record into etcd. Use MD5 of the node value as the - // 'sub-key'. This makes duplicates impossible without sacrificing - // TTL updates or extra pre-update lookup faff - hasher := md5.New() - hasher.Write([]byte(node.Value)) - subkey := hex.EncodeToString(hasher.Sum(nil)) - response, err := etcdClient.Set(node.Key + "/" + subkey, node.Value, 0) - if err != nil { + existingRecords, err := resolver.GetFromStorage(updateNode.Key) + if err != nil && !missingKeyErr(err) { debugMsg(err) - panic("Failed to insert record into etcd") + panic("Failed to fetch existing RRs for key " + newRecordDir) } - // Insert the TTL record if one has been requested - if header.Ttl > 0 { - ttl := strconv.FormatInt(int64(header.Ttl), 10) - _, err = etcdClient.Set(response.Node.Key + ".ttl", ttl, 0) - if err != nil { - debugMsg(err) - panic("Failed to insert ttl into etcd") + if header.Class == dns.ClassNONE { + // RFC Meaning: Delete an RR from an RRset + for _, existing := range existingRecords { + if existing.node.Value == updateNode.Value { + // delete the node itself, and it's matching TTL record (if any) + debugMsg("Deleting RR " + newRecordDir) + _, err := etcdClient.Delete(newRecordDir, true) + if err != nil && !missingKeyErr(err) { + debugMsg(err) + panic("Failed to delete RR " + newRecordDir) + } + + _, err = etcdClient.Delete(newRecordDir + ".ttl", true) + if err != nil && !missingKeyErr(err) { + debugMsg(err) + panic("Failed to delete RR TTL key " + newRecordDir + ".ttl") + } + } + } + } else { + + // TODO: special-cases for CNAMES, according to RFC 2136: + // - if CNAME update is requested, and non-CNAME records exist for the given name, ignore + // - if non-CNAME update is requested, and CNAME records exist for the given name, ignore + + // Further explanation from http://docs.freebsd.org/doc/8.0-RELEASE/usr/share/doc/bind9/arm/man.nsupdate.html : + // "...cannot conflict with the long-standing rule in RFC1034 that a name must not exist as any other + // record type if it exists as a CNAME. (The rule has been updated for DNSSEC in RFC2535 to allow + // CNAMEs to have RRSIG, DNSKEY and NSEC records.)" + + // TODO(orls): add these special cases to the special case for CNAMEs. Yayyyyy standards + + // RFC Meaning: Add to an RRset + + foundExisting := false + var ttlKeys []string + + // Check for existing matching records, in which case just update TTL. + // Otherwise there's risk of duplicates + for _, existing := range existingRecords { + if existing.node.Value == updateNode.Value { + debugMsg("update req matched existing rr with key " + existing.node.Key) + foundExisting = true + ttlKeys = append(ttlKeys, existing.node.Key + ".ttl") + } + } + + if !foundExisting { + // Then we need to add, which means we need a directory. + // Convert any old-style single-keys to directories + if len(existingRecords) == 1 && existingRecords[0].node.Key == "/" + newRecordDir { + originalNode := existingRecords[0].node + logger.Printf("[WARNING] ------") + logger.Printf("[WARNING] Converting existing value to a directory!") + logger.Printf("[WARNING] Existing record is old-style single key: " + originalNode.Key) + logger.Printf("[WARNING] ------") + + convertedKey := originalNode.Key + "/" + recordSubkey(originalNode.Value) + _, convertErr := etcdClient.SetDir(originalNode.Key, 0) + if convertErr != nil { + debugMsg(convertErr) + // panic("Failed to insert record into etcd") + } + _, convertErr = etcdClient.Set(convertedKey, originalNode.Value, 0) + if convertErr != nil { + debugMsg(convertErr) + // panic("Failed to insert record into etcd") + } + if existingRecords[0].ttl != 0 { + convertTTL := strconv.FormatInt(int64(existingRecords[0].ttl), 10) + _, convertErr = etcdClient.Set(convertedKey + ".ttl", convertTTL, 0) + if convertErr != nil { + debugMsg(convertErr) + // panic("Failed to insert record into etcd") + } + } + } + + newKey := newRecordDir + "/" + recordSubkey(updateNode.Value) + ttlKeys = append(ttlKeys, newKey + ".ttl") + + debugMsg("Inserting new record to " + newKey) + _, err := etcdClient.Set(newKey, updateNode.Value, 0) + if err != nil { + debugMsg(err) + // panic("Failed to insert record into etcd") + } + } + + // Insert the TTL record if one has been requested + if header.Ttl > 0 { + ttl := strconv.FormatInt(int64(header.Ttl), 10) + for _, ttlKey := range ttlKeys { + debugMsg("Inserting/updating TTL key " + ttlKey) + _, err = etcdClient.Set(ttlKey, ttl, 0) + if err != nil { + debugMsg(err) + // panic("Failed to insert ttl into etcd") + } + } } } } @@ -308,3 +390,22 @@ func performUpdate(prefix string, etcdClient *etcd.Client, records []dns.RR) (rc return dns.RcodeSuccess } + +// recordSubkey yields the sub-key string to be used for a new RR in a +// directory, based on it's data. The MD5 of the node value is used, making +// duplicates impossible. +func recordSubkey(value string) (subkey string) { + hasher := md5.New() + hasher.Write([]byte(value)) + return hex.EncodeToString(hasher.Sum(nil)) +} + +// internal helper to determine if an error from etcd operations is a 100-code +// error, i.e. that the key is missing. +func missingKeyErr(err error) (ok bool) { + etcdErr, cast := err.(*etcd.EtcdError) + if cast && etcdErr.ErrorCode == 100 { + return true + } + return false +} diff --git a/update_test.go b/update_test.go index 45b88f5..6430c1e 100644 --- a/update_test.go +++ b/update_test.go @@ -304,3 +304,141 @@ func TestPrerequisites_ValueDependentRRSet(t *testing.T) { if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeSuccess, []dns.RR{prereq_foo_a_match, prereq_bar_a_match}) { t.Fatal() } if ! _prereqsTestHelper(t, manager, "Used", dns.RcodeSuccess, []dns.RR{prereq_foo_a_match, prereq_bar_a_match, prereq_bar_ptr_match}) { t.Fatal() } } + +func TestUpsertExisting(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestUpsertExisting/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + client.Delete("TestUpsertExisting/", true) + client.Set("TestUpsertExisting/net/disco/singlekey/.A", "1.1.1.1", 0) + client.Set("TestUpsertExisting/net/disco/singlekey/.A.ttl", "123", 0) + client.Set("TestUpsertExisting/net/disco/directory/.A/6465ec74397c9126916786bbcd6d7601", "1.1.1.1", 0) + client.Set("TestUpsertExisting/net/disco/directory/.A/6465ec74397c9126916786bbcd6d7601.ttl", "123", 0) + client.Set("TestUpsertExisting/net/disco/directory/.A/nonMd5KeyName", "2.2.2.2", 0) + client.Set("TestUpsertExisting/net/disco/directory/.A/nonMd5KeyName.ttl", "123", 0) + + // Update with same value (to a non-directory key): TTL should change + updateSingle := &dns.A{ + Hdr: dns.RR_Header{Name: "singlekey.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 1234}, + A: net.ParseIP("1.1.1.1")} + + msg := &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net.", Qclass: dns.ClassINET}) + msg.Insert([]dns.RR{updateSingle}) + + result := manager.Update("disco.net.", msg) + + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("Failed to update existing DNS record") + t.Fatal() + } + + answers, err := resolver.LookupAnswersForType("singlekey.disco.net.", dns.TypeA) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) != 1 { + t.Error("Expected exactly one answer for discodns.net.") + t.Fatal() + } + answerHeader := answers[0].Header() + if answerHeader.Ttl != 1234 { + t.Error("Didn't get expected TTL on new record") + t.Fatal() + } + + // Insert a new one: should auto-convert single-value to directory? + // TODO: not sure what we should consider correct behaviour here. + addNewToSingle := &dns.A{ + Hdr: dns.RR_Header{Name: "singlekey.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 1234}, + A: net.ParseIP("2.2.2.2")} + + msg = &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net.", Qclass: dns.ClassINET}) + msg.Insert([]dns.RR{addNewToSingle}) + + result = manager.Update("disco.net.", msg) + + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("Failed to insert new DNS record to single-value (non-directory) node") + t.Error(" -- (Permitting test to continue for now...) --") + // t.Fatal() + } + + answers, err = resolver.LookupAnswersForType("singlekey.disco.net.", dns.TypeA) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) != 2 { + t.Error("Expected two answers for singlekey.discodns.net. after update") + t.Error(" -- (Permitting test to continue for now...) --") + // t.Fatal() + } + + // Update with same value (to a directory child key, md5 subkey): TTL should change + updateDirChild := &dns.A{ + Hdr: dns.RR_Header{Name: "directory.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 1234}, + A: net.ParseIP("1.1.1.1")} + + msg = &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net.", Qclass: dns.ClassINET}) + msg.Insert([]dns.RR{updateDirChild}) + + result = manager.Update("disco.net.", msg) + + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("Failed to update existing record (directory child with hashed subkey)") + t.Fatal() + } + + answers, err = resolver.LookupAnswersForType("directory.disco.net.", dns.TypeA) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) != 2 { + t.Error("Expected two answers for directory.discodns.net.") + t.Fatal() + } + + // Update with same value (to a directory child key, non-md5 subkey): TTL should change + updateDirChildMessyName := &dns.A{ + Hdr: dns.RR_Header{Name: "directory.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 1234}, + A: net.ParseIP("2.2.2.2")} + + msg = &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net.", Qclass: dns.ClassINET}) + msg.Insert([]dns.RR{updateDirChildMessyName}) + + result = manager.Update("disco.net.", msg) + + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("Failed to update existing record (directory child with messy unhashed subkey)") + t.Fatal() + } + + answers, err = resolver.LookupAnswersForType("directory.disco.net.", dns.TypeA) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) != 2 { + t.Error("Expected two answers for directory.discodns.net.") + t.Fatal() + } + + for _, answer := range answers { + header := answer.Header() + if header.Ttl != 1234 { + t.Error("Didn't get expected TTL on directory.discodns.net record", header.Name) + t.Fatal() + } + } +} + From 2bb281e7d435ef8e598d18105965daa1ca49f00e Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Wed, 25 Feb 2015 17:35:06 +0000 Subject: [PATCH 51/54] More refactoring of performUpdate: Bam! Full basic support and green tests --- update.go | 111 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 61 insertions(+), 50 deletions(-) diff --git a/update.go b/update.go index f56d11f..88659bc 100644 --- a/update.go +++ b/update.go @@ -88,7 +88,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) // result in a partially updated zone. // TODO(tarnfeld): Figure out a way of rolling back changes, perhaps make // use of the etcd indexes? - msg.SetRcode(req, performUpdate(u.etcdPrefix, u.etcd, u.resolver, req.Ns)) + msg.SetRcode(req, performUpdate(u.etcdPrefix, u.etcd, u.resolver, req.Question[0], req.Ns)) return } @@ -234,7 +234,8 @@ func validateUpdates(rrs []dns.RR, updateZone dns.Question) (rcode int) { // It is assumed by this point all prerequisites have been validated and all // domains are locked. // See RFC 2136, section 3.4.2 -func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, records []dns.RR) (rcode int) { +func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, updateZone dns.Question, records []dns.RR) (rcode int) { + for _, rr := range records { header := rr.Header() debugMsg("update rr: header", header) @@ -242,68 +243,64 @@ func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, r panic("Record converter doesn't exist for " + dns.TypeToString[header.Rrtype]) } - updateNode, err := convertRRToNode(rr, *header) - if err != nil { - panic("Got error when converting node") - } else if updateNode == nil { - panic("Got NIL after successfully converting node") - } - - // Prepend the etcd prefix, if we're given one - newRecordDir := prefix + updateNode.Key + nameDir := nameToKey(header.Name, "") + // Gather up all deletes if header.Class == dns.ClassANY { - // RFC Meaning if type is ANY: Delete all RRsets from a name - // RFC Meaning if type is not ANY: Delete an RRset (all RRs of type) - - // `convertRRToNode` will already have given us the right key - // depending on Type - - // TODO: the key will be wrong for ANY, no? - // TODO: for ANY, need to only delete RRs and not child names. See what the RFC says, - // is it an error if there are child nodes? - - debugMsg("Deleting all RRs from key " + newRecordDir) - _, err := etcdClient.Delete(newRecordDir, true) - if err != nil && !missingKeyErr(err) { - debugMsg(err) - panic("Failed to delete RRs from key " + newRecordDir) + var typesToDelete []uint16 + if header.Rrtype == dns.TypeANY { + // RFC Meaning: Delete all RRsets from a name + + // Look up all RRs for supported types. Do this 'manually' because we + // don't want standard resolver behaviour about CNAMEs + // TODO: this means it's not parallelized like it normally is + for rrType, _ := range convertersToRR { + if header.Name == updateZone.Name && (rrType == dns.TypeNS || rrType == dns.TypeSOA) { + // RFC explicitly forbids deleting NS/SOA for the zone in this way + continue + } + typesToDelete = append(typesToDelete, rrType) + } + } else { + // RFC Meaning: Delete an RRset (all RRs of type) + typesToDelete = append(typesToDelete, header.Rrtype) } - - _, err = etcdClient.Delete(newRecordDir + ".ttl", true) - if err != nil && !missingKeyErr(err) { - debugMsg(err) - panic("Failed to delete RR TTLs from key " + newRecordDir + ".ttl") + for _, rrType := range typesToDelete { + records, err := resolver.GetFromStorage(nameDir + "/." + dns.TypeToString[rrType]) + if err != nil && !missingKeyErr(err) { + debugMsg(err) + panic("Failed to fetch existing records for " + nameDir) + } + for _, toDelete := range records { + deleteKeyAndTtl(etcdClient, toDelete.node.Key) + } } } else { + updateNode, err := convertRRToNode(rr, *header) + if err != nil { + panic("Got error when converting node") + } else if updateNode == nil { + panic("Got NIL after successfully converting node") + } + existingRecords, err := resolver.GetFromStorage(updateNode.Key) if err != nil && !missingKeyErr(err) { debugMsg(err) - panic("Failed to fetch existing RRs for key " + newRecordDir) + panic("Failed to fetch existing RRs for key " + updateNode.Key) } if header.Class == dns.ClassNONE { // RFC Meaning: Delete an RR from an RRset for _, existing := range existingRecords { if existing.node.Value == updateNode.Value { - // delete the node itself, and it's matching TTL record (if any) - debugMsg("Deleting RR " + newRecordDir) - _, err := etcdClient.Delete(newRecordDir, true) - if err != nil && !missingKeyErr(err) { - debugMsg(err) - panic("Failed to delete RR " + newRecordDir) - } - - _, err = etcdClient.Delete(newRecordDir + ".ttl", true) - if err != nil && !missingKeyErr(err) { - debugMsg(err) - panic("Failed to delete RR TTL key " + newRecordDir + ".ttl") - } + deleteKeyAndTtl(etcdClient, existing.node.Key) } } } else { + // RFC Meaning: Add to an RRset + // TODO: special-cases for CNAMES, according to RFC 2136: // - if CNAME update is requested, and non-CNAME records exist for the given name, ignore // - if non-CNAME update is requested, and CNAME records exist for the given name, ignore @@ -312,11 +309,8 @@ func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, r // "...cannot conflict with the long-standing rule in RFC1034 that a name must not exist as any other // record type if it exists as a CNAME. (The rule has been updated for DNSSEC in RFC2535 to allow // CNAMEs to have RRSIG, DNSKEY and NSEC records.)" - // TODO(orls): add these special cases to the special case for CNAMEs. Yayyyyy standards - // RFC Meaning: Add to an RRset - foundExisting := false var ttlKeys []string @@ -333,7 +327,7 @@ func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, r if !foundExisting { // Then we need to add, which means we need a directory. // Convert any old-style single-keys to directories - if len(existingRecords) == 1 && existingRecords[0].node.Key == "/" + newRecordDir { + if len(existingRecords) == 1 && existingRecords[0].node.Key == "/" + prefix + updateNode.Key { originalNode := existingRecords[0].node logger.Printf("[WARNING] ------") logger.Printf("[WARNING] Converting existing value to a directory!") @@ -361,7 +355,7 @@ func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, r } } - newKey := newRecordDir + "/" + recordSubkey(updateNode.Value) + newKey := prefix + updateNode.Key + "/" + recordSubkey(updateNode.Value) ttlKeys = append(ttlKeys, newKey + ".ttl") debugMsg("Inserting new record to " + newKey) @@ -391,6 +385,23 @@ func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, r return dns.RcodeSuccess } +// Internal boileplate-reducer to delete a specific key (non-recursive) and +// handle it's TTL key too +func deleteKeyAndTtl(etcdClient *etcd.Client, delKey string) { + delTTLKey := delKey + ".ttl" + debugMsg("Deleting RR with key " + delKey) + _, err := etcdClient.Delete(delKey, true) + if err != nil && !missingKeyErr(err) { + debugMsg(err) + panic("Failed to delete RRs with key " + delKey) + } + _, err = etcdClient.Delete(delTTLKey, true) + if err != nil && !missingKeyErr(err) { + debugMsg(err) + panic("Failed to delete RR TTL key " + delTTLKey) + } +} + // recordSubkey yields the sub-key string to be used for a new RR in a // directory, based on it's data. The MD5 of the node value is used, making // duplicates impossible. From 6b0f4501fa8dbecf01fd665195b4259a79ee8ec0 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Thu, 26 Feb 2015 00:27:18 +0000 Subject: [PATCH 52/54] Full CNAME support, warts and all --- record.go | 11 +++++++--- update.go | 33 ++++++++++++++++++++++------ update_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 9 deletions(-) diff --git a/record.go b/record.go index cebd687..912ae35 100644 --- a/record.go +++ b/record.go @@ -238,9 +238,14 @@ var convertersFromRR = map[uint16]func (rr dns.RR, header dns.RR_Header) (node * return }, - // dns.TypeCNAME: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { - // panic("Not implemented") - // }, + dns.TypeCNAME: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { + if record, ok := rr.(*dns.CNAME); ok { + node = &etcd.Node{ + Key: nameToKey(header.Name, "/.CNAME"), + Value: record.Target} + } + return + }, // dns.TypeNS: func (rr dns.RR, header dns.RR_Header) (node *etcd.Node, err error) { // panic("Not implemented") diff --git a/update.go b/update.go index 88659bc..ca4ee22 100644 --- a/update.go +++ b/update.go @@ -7,6 +7,7 @@ import ( "github.com/miekg/dns" "fmt" "strconv" + "strings" ) type DynamicUpdateManager struct { @@ -238,7 +239,6 @@ func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, u for _, rr := range records { header := rr.Header() - debugMsg("update rr: header", header) if _, ok := convertersFromRR[header.Rrtype]; ok != true { panic("Record converter doesn't exist for " + dns.TypeToString[header.Rrtype]) } @@ -298,19 +298,40 @@ func performUpdate(prefix string, etcdClient *etcd.Client, resolver *Resolver, u } } } else { - // RFC Meaning: Add to an RRset - // TODO: special-cases for CNAMES, according to RFC 2136: - // - if CNAME update is requested, and non-CNAME records exist for the given name, ignore - // - if non-CNAME update is requested, and CNAME records exist for the given name, ignore - + // Ignore certain inserts in presence of CNAMEs, as per RFC. // Further explanation from http://docs.freebsd.org/doc/8.0-RELEASE/usr/share/doc/bind9/arm/man.nsupdate.html : // "...cannot conflict with the long-standing rule in RFC1034 that a name must not exist as any other // record type if it exists as a CNAME. (The rule has been updated for DNSSEC in RFC2535 to allow // CNAMEs to have RRSIG, DNSKEY and NSEC records.)" // TODO(orls): add these special cases to the special case for CNAMEs. Yayyyyy standards + hasCNAME := false + hasNonCNAME := false + neighbouringTypes, err := etcdClient.Get(prefix + nameDir, false, false) + if err == nil { + for _, n := range neighbouringTypes.Node.Nodes { + splits := strings.Split(n.Key, nameDir + "/.") + if len(splits) != 2 || strings.HasSuffix(splits[1], ".ttl") { + continue + } + if splits[1] == "CNAME" { + hasCNAME = true + } else { + hasNonCNAME = true + } + } + } + + if header.Rrtype == dns.TypeCNAME && hasNonCNAME { + debugMsg("Ignoring insert for CNAME due to existing non-CNAME record(s) for " + header.Name) + continue + } else if header.Rrtype != dns.TypeCNAME && hasCNAME { + debugMsg("Ignoring insert for " + dns.TypeToString[header.Rrtype] + " due to existing CNAME record(s) for " + header.Name) + continue + } + foundExisting := false var ttlKeys []string diff --git a/update_test.go b/update_test.go index 6430c1e..9fb9c3b 100644 --- a/update_test.go +++ b/update_test.go @@ -442,3 +442,61 @@ func TestUpsertExisting(t *testing.T) { } } +func TestInsertCname(t *testing.T) { + manager := &DynamicUpdateManager{etcd: client, etcdPrefix: "TestInsertCname/", resolver: resolver} + resolver.etcdPrefix = manager.etcdPrefix + + client.Delete("TestInsertCname/", true) + client.Set("TestInsertCname/net/disco/target/.A", "1.1.1.1", 0) + client.Set("TestInsertCname/net/disco/alias/.CNAME", "target.disco.net.", 0) + + newCname := &dns.CNAME{ + Hdr: dns.RR_Header{Name: "target.disco.net.", Rrtype: dns.TypeCNAME, Class: dns.ClassINET, Ttl: 1234}, + Target: "foo.disco.net."} + msg := &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) + msg.Insert([]dns.RR{newCname}) + + result := manager.Update("disco.net.", msg) + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("DNS update query failed") + t.Fatal() + } + + // despite success, the cname insert should have been ignored + answers, err := resolver.LookupAnswersForType("target.disco.net.", dns.TypeCNAME) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) != 0 { + t.Error("Expected no answers for target.discodns.net CNAME") + t.Fatal() + } + + newA := &dns.A{ + Hdr: dns.RR_Header{Name: "alias.disco.net.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 1234}, + A: net.ParseIP("1.2.3.4")} + msg = &dns.Msg{} + msg.Question = append(msg.Question, dns.Question{Name: "disco.net."}) + msg.Insert([]dns.RR{newA}) + + result = manager.Update("disco.net.", msg) + if result.Rcode != dns.RcodeSuccess { + debugMsg(result) + t.Error("DNS update query failed") + t.Fatal() + } + + // despite success, the A insert should have been ignored + answers, err = resolver.LookupAnswersForType("alias.disco.net.", dns.TypeA) + if err != nil { + t.Error("Caught error resolving domain") + t.Fatal() + } + if len(answers) != 0 { + t.Error("Expected no answers for alias.discodns.net A") + t.Fatal() + } +} From c750c0e76156b96818ee54611a5c57a5711b9f43 Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Fri, 22 May 2015 15:25:35 +0100 Subject: [PATCH 53/54] Don't call `SetRcode`, set it directly: Because SetRcode internally calls SetReply, which always resets the response's opcode to QUERY, we can't then change the opcode to UPDATE, meaning we send an invalid response --- update.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/update.go b/update.go index ca4ee22..c80b988 100644 --- a/update.go +++ b/update.go @@ -38,7 +38,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) for _, rr := range rrs { if dns.CompareDomainName(rr.Header().Name, zone) != dns.CountLabel(zone) { debugMsg("Domain " + rr.Header().Name + " is not in the " + zone + " zone") - msg.SetRcode(req, dns.RcodeNotZone) + msg.Rcode = dns.RcodeNotZone return } } @@ -48,7 +48,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) defer func() { if r := recover(); r != nil { debugMsg("[PANIC] " + fmt.Sprint(r)) - msg.SetRcode(req, dns.RcodeServerFailure) + msg.Rcode = dns.RcodeServerFailure } }() @@ -64,7 +64,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) _, err := lock.WaitForAcquire(30) if err != nil { debugMsg("Failed to acquire or keep the update lock: ", err) - msg.SetRcode(req, dns.RcodeServerFailure) + msg.Rcode = dns.RcodeServerFailure return } @@ -73,14 +73,14 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) prereqValidation := validatePrerequisites(req.Answer, u.resolver) if prereqValidation != dns.RcodeSuccess { debugMsg("Validation of prerequisites failed") - msg.SetRcode(req, prereqValidation) + msg.Rcode = prereqValidation return } updateValidation := validateUpdates(req.Ns, req.Question[0]) if updateValidation != dns.RcodeSuccess { debugMsg("Validation of update instructions failed") - msg.SetRcode(req, updateValidation) + msg.Rcode = updateValidation return } @@ -89,7 +89,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) // result in a partially updated zone. // TODO(tarnfeld): Figure out a way of rolling back changes, perhaps make // use of the etcd indexes? - msg.SetRcode(req, performUpdate(u.etcdPrefix, u.etcd, u.resolver, req.Question[0], req.Ns)) + msg.Rcode = performUpdate(u.etcdPrefix, u.etcd, u.resolver, req.Question[0], req.Ns) return } From 65cb285bbaab081c047109aeef7918333fa73ffa Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Fri, 22 May 2015 15:25:46 +0100 Subject: [PATCH 54/54] Send correct response opcode --- update.go | 1 + 1 file changed, 1 insertion(+) diff --git a/update.go b/update.go index c80b988..26f7ddb 100644 --- a/update.go +++ b/update.go @@ -26,6 +26,7 @@ func (u *DynamicUpdateManager) Update(zone string, req *dns.Msg) (msg *dns.Msg) rrsets := [][]dns.RR{req.Answer, req.Ns} msg = new(dns.Msg) msg.SetReply(req) + msg.Opcode = dns.OpcodeUpdate // dns update re-aporopriates DNS message blocks: // msg.Question: Zone info for whole request