From 68961f2f5b0edabfdaa099cfb0125964c44818d5 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 11:24:01 +0200 Subject: [PATCH 01/11] Make the IsTsig and IsEdn0 more usefull by returning the record --- client.go | 8 ++++---- defaults.go | 19 +++++++++++-------- server.go | 8 ++++---- tsig.go | 2 +- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/client.go b/client.go index d982de7f..1a1370c0 100644 --- a/client.go +++ b/client.go @@ -164,8 +164,8 @@ func (w *reply) receive() (*Msg, error) { } w.rtt = time.Since(w.t) m.Size = n - if m.IsTsig() { - secret := m.Extra[len(m.Extra)-1].(*RR_TSIG).Hdr.Name + if t := m.IsTsig(); t != nil { + secret := t.Hdr.Name if _, ok := w.client.TsigSecret[secret]; !ok { w.tsigStatus = ErrSecret return m, nil @@ -249,9 +249,9 @@ func (w *reply) readClient(p []byte) (n int, err error) { // signature is calculated. func (w *reply) send(m *Msg) (err error) { var out []byte - if m.IsTsig() { + if t := m.IsTsig(); t != nil { mac := "" - name := m.Extra[len(m.Extra)-1].(*RR_TSIG).Hdr.Name + name := t.Hdr.Name if _, ok := w.client.TsigSecret[name]; !ok { return ErrSecret } diff --git a/defaults.go b/defaults.go index 4a1a4bb8..36cfc766 100644 --- a/defaults.go +++ b/defaults.go @@ -196,23 +196,26 @@ func (dns *Msg) IsIxfr() (ok bool) { } // IsTsig checks if the message has a TSIG record as the last record -// in the additional section. -func (dns *Msg) IsTsig() (ok bool) { +// in the additional section. It returns the TSIG record found or nil. +func (dns *Msg) IsTsig() *RR_TSIG { if len(dns.Extra) > 0 { - return dns.Extra[len(dns.Extra)-1].Header().Rrtype == TypeTSIG + if dns.Extra[len(dns.Extra)-1].Header().Rrtype == TypeTSIG { + return dns.Extra[len(dns.Extra)-1].(*RR_TSIG) + } } - return + return nil } // IsEdns0 checks if the message has a EDNS0 (OPT) record, any EDNS0 -// record in the additional section will do. -func (dns *Msg) IsEdns0() (ok bool) { +// record in the additional section will do. It returns the OPT record +// found or nil. +func (dns *Msg) IsEdns0() *RR_OPT { for _, r := range dns.Extra { if r.Header().Rrtype == TypeOPT { - return true + return r.(*RR_OPT) } } - return + return nil } // IsDomainName checks if s is a valid domainname, it returns diff --git a/server.go b/server.go index a8bbd1c0..7dc02cab 100644 --- a/server.go +++ b/server.go @@ -330,8 +330,8 @@ func (c *conn) serve() { } w.tsigStatus = nil - if req.IsTsig() { - secret := req.Extra[len(req.Extra)-1].(*RR_TSIG).Hdr.Name + if t := req.IsTsig(); t != nil { + secret := t.Hdr.Name if _, ok := w.conn.tsigSecret[secret]; !ok { w.tsigStatus = ErrKeyAlg } @@ -360,8 +360,8 @@ func (w *response) Write(m *Msg) (err error) { if m == nil { return &Error{Err: "nil message"} } - if m.IsTsig() { - data, w.tsigRequestMAC, err = TsigGenerate(m, w.conn.tsigSecret[m.Extra[len(m.Extra)-1].(*RR_TSIG).Hdr.Name], w.tsigRequestMAC, w.tsigTimersOnly) + if t := m.IsTsig(); t != nil { + data, w.tsigRequestMAC, err = TsigGenerate(m, w.conn.tsigSecret[t.Hdr.Name], w.tsigRequestMAC, w.tsigTimersOnly) if err != nil { return err } diff --git a/tsig.go b/tsig.go index 6ce2a1b6..43ed9fb5 100644 --- a/tsig.go +++ b/tsig.go @@ -154,7 +154,7 @@ type timerWireFmt struct { // timersOnly is false. // If something goes wrong an error is returned, otherwise it is nil. func TsigGenerate(m *Msg, secret, requestMAC string, timersOnly bool) ([]byte, string, error) { - if !m.IsTsig() { + if m.IsTsig() == nil { panic("TSIG not last RR in additional") } // If we barf here, the caller is to blame From 2a391d079b1626b5c43fafbfe3000594d7602443 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 11:32:11 +0200 Subject: [PATCH 02/11] fmt --- msg.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/msg.go b/msg.go index c8ced0da..e01eecd2 100644 --- a/msg.go +++ b/msg.go @@ -79,9 +79,8 @@ type MsgHdr struct { // The layout of a DNS message. type Msg struct { MsgHdr - Compress bool // If true, the message will be compressed when converted to wire format. - Size int // Number of octects in the message received from the wire. - // Remote addr? back + Compress bool // If true, the message will be compressed when converted to wire format. + Size int // Number of octects in the message received from the wire. Question []Question // Holds the RR(s) of the question section. Answer []RR // Holds the RR(s) of the answer section. Ns []RR // Holds the RR(s) of the authority section. From 21a11bb188e03e7a55b831cee2136c33d349378b Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 12:59:39 +0200 Subject: [PATCH 03/11] need somewhat different structure for efficient deletes --- zone.go | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/zone.go b/zone.go index b78e9b57..022763b6 100644 --- a/zone.go +++ b/zone.go @@ -51,8 +51,8 @@ func NewZone(origin string) *Zone { return z } -// Insert inserts an RR into the zone. Duplicate data overwrites the old data without -// warning. +// Insert inserts an RR into the zone. There is no check for duplicate data, allthough +// Remove will remove all duplicates. func (z *Zone) Insert(r RR) error { if !IsSubDomain(z.Origin, r.Header().Name) { return &Error{Err: "out of zone data", Name: r.Header().Name} @@ -102,9 +102,36 @@ func (z *Zone) Insert(r RR) error { } // Remove removes the RR r from the zone. If there RR can not be found, -// this is a no-op. TODO(mg): not implemented. +// this is a no-op. func (z *Zone) Remove(r RR) error { - // Wildcards + /* + key := toRadixName(r.Header().Name) + zd := z.Radix.Find(key) + if zd == nil { + return nil + } + switch t := r.Header().Rrtype; t { + case TypeRRSIG: + sigtype := r.(*RR_RRSIG).TypeCovered + default: + for zr := range zd.Value.(*ZoneData).RR[t] { + // if there is a match, there can only be one + if r == zr { + + } + + } + zd.Value.(*ZoneData).RR[t] = append(zd.Value.(*ZoneData).RR[t], r) + } + return nil + + if len(r.Header().Name) > 1 && r.Header().Name[0] == '*' && r.Header().Name[1] == '.' { + z.Wildcard-- + if z.Wildcard < 0 { + z.Wildcard = 0 + } + } + */ return nil } From 9de9ce4424a01be5336dcf883824f9a590ffc70e Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 14:44:51 +0200 Subject: [PATCH 04/11] Implement Zone.Remove() --- zone.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/zone.go b/zone.go index 022763b6..14623695 100644 --- a/zone.go +++ b/zone.go @@ -19,8 +19,7 @@ type ZoneData struct { Name string // Domain name for this node RR map[uint16][]RR // Map of the RR type to the RR Signatures map[uint16][]*RR_RRSIG // DNSSEC signatures for the RRs, stored under type covered - // Always false, except for NSsets that differ from z.Origin - NonAuth bool + NonAuth bool // Always false, except for NSsets that differ from z.Origin } // toRadixName reverses a domainname so that when we store it in the radix tree @@ -104,34 +103,36 @@ func (z *Zone) Insert(r RR) error { // Remove removes the RR r from the zone. If there RR can not be found, // this is a no-op. func (z *Zone) Remove(r RR) error { - /* key := toRadixName(r.Header().Name) zd := z.Radix.Find(key) if zd == nil { return nil } + remove := false switch t := r.Header().Rrtype; t { case TypeRRSIG: sigtype := r.(*RR_RRSIG).TypeCovered - default: - for zr := range zd.Value.(*ZoneData).RR[t] { - // if there is a match, there can only be one + for i, zr := range zd.Value.(*ZoneData).RR[sigtype] { if r == zr { - + zd.Value.(*ZoneData).RR[sigtype] = append(zd.Value.(*ZoneData).RR[sigtype][:i], zd.Value.(*ZoneData).RR[sigtype][i+1:]...) + remove = true + } + } + default: + for i, zr := range zd.Value.(*ZoneData).RR[t] { + if r == zr { + zd.Value.(*ZoneData).RR[t] = append(zd.Value.(*ZoneData).RR[t][:i], zd.Value.(*ZoneData).RR[t][i+1:]...) + remove = true } - } - zd.Value.(*ZoneData).RR[t] = append(zd.Value.(*ZoneData).RR[t], r) } - return nil - - if len(r.Header().Name) > 1 && r.Header().Name[0] == '*' && r.Header().Name[1] == '.' { + if remove && len(r.Header().Name) > 1 && r.Header().Name[0] == '*' && r.Header().Name[1] == '.' { z.Wildcard-- if z.Wildcard < 0 { z.Wildcard = 0 } } - */ + // TODO(mg): what to do if the whole structure is empty? Set it to nil? return nil } From 0d148c2369845b73a53643f7c14b7287c09072fa Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 14:59:05 +0200 Subject: [PATCH 05/11] add empty testfile --- zone_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 zone_test.go diff --git a/zone_test.go b/zone_test.go new file mode 100644 index 00000000..341c0f0a --- /dev/null +++ b/zone_test.go @@ -0,0 +1,10 @@ +package dns + +import ( + "testing" +) + +func TestInsert(t *testing.T) { +} +func TestRemove(t *testing.T) { +} From c146770f82ebaeb0407cfe1038326179b8a11d5a Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 19:03:49 +0200 Subject: [PATCH 06/11] locking here...? --- zone.go | 1 + 1 file changed, 1 insertion(+) diff --git a/zone.go b/zone.go index 14623695..8d20ba52 100644 --- a/zone.go +++ b/zone.go @@ -20,6 +20,7 @@ type ZoneData struct { RR map[uint16][]RR // Map of the RR type to the RR Signatures map[uint16][]*RR_RRSIG // DNSSEC signatures for the RRs, stored under type covered NonAuth bool // Always false, except for NSsets that differ from z.Origin + mutex *sync.RWMutex // lock for reading/writing } // toRadixName reverses a domainname so that when we store it in the radix tree From 8ee22a4494ec9cade4a29f07757bee34b4250761 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 19:38:52 +0200 Subject: [PATCH 07/11] typo --- zone.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/zone.go b/zone.go index 14623695..adc63757 100644 --- a/zone.go +++ b/zone.go @@ -14,7 +14,7 @@ type Zone struct { *radix.Radix // Zone data } -// ZoneData holds all the RRs having their ownername equal to Name. +// ZoneData holds all the RRs having their owner name equal to Name. type ZoneData struct { Name string // Domain name for this node RR map[uint16][]RR // Map of the RR type to the RR @@ -22,7 +22,7 @@ type ZoneData struct { NonAuth bool // Always false, except for NSsets that differ from z.Origin } -// toRadixName reverses a domainname so that when we store it in the radix tree +// toRadixName reverses a domain name so that when we store it in the radix tree // we preserve the nsec ordering of the zone (this idea was stolen from NSD). // each label is also lowercased. func toRadixName(d string) string { @@ -50,7 +50,7 @@ func NewZone(origin string) *Zone { return z } -// Insert inserts an RR into the zone. There is no check for duplicate data, allthough +// Insert inserts an RR into the zone. There is no check for duplicate data, although // Remove will remove all duplicates. func (z *Zone) Insert(r RR) error { if !IsSubDomain(z.Origin, r.Header().Name) { From 23d4972158bd61db6b4955026a100b7a5325b38a Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 19:57:25 +0200 Subject: [PATCH 08/11] locking --- zone.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/zone.go b/zone.go index 8d20ba52..abc41872 100644 --- a/zone.go +++ b/zone.go @@ -5,6 +5,7 @@ package dns import ( "github.com/miekg/radix" "strings" + "sync" ) // Zone represents a DNS zone. Currently there is no locking implemented. @@ -12,6 +13,7 @@ type Zone struct { Origin string // Origin of the zone Wildcard int // Whenever we see a wildcard name, this is incremented *radix.Radix // Zone data + mutex *sync.RWMutex } // ZoneData holds all the RRs having their ownername equal to Name. @@ -20,7 +22,7 @@ type ZoneData struct { RR map[uint16][]RR // Map of the RR type to the RR Signatures map[uint16][]*RR_RRSIG // DNSSEC signatures for the RRs, stored under type covered NonAuth bool // Always false, except for NSsets that differ from z.Origin - mutex *sync.RWMutex // lock for reading/writing + mutex *sync.RWMutex } // toRadixName reverses a domainname so that when we store it in the radix tree @@ -59,8 +61,10 @@ func (z *Zone) Insert(r RR) error { } key := toRadixName(r.Header().Name) + z.mutex.Lock() zd := z.Radix.Find(key) if zd == nil { + defer z.mutex.Unlock() // Check if its a wildcard name if len(r.Header().Name) > 1 && r.Header().Name[0] == '*' && r.Header().Name[1] == '.' { z.Wildcard++ @@ -85,6 +89,9 @@ func (z *Zone) Insert(r RR) error { z.Radix.Insert(key, zd) return nil } + z.mutex.Unlock() + zd.Value.(*ZoneData).mutex.Lock() + defer zd.Value.(*ZoneData).mutex.Unlock() // Name already there switch t := r.Header().Rrtype; t { case TypeRRSIG: @@ -105,10 +112,15 @@ func (z *Zone) Insert(r RR) error { // this is a no-op. func (z *Zone) Remove(r RR) error { key := toRadixName(r.Header().Name) + z.mutex.Lock() zd := z.Radix.Find(key) if zd == nil { + defer z.mutex.Unlock() return nil } + z.mutex.Unlock() + zd.Value.(*ZoneData).mutex.Lock() + defer zd.Value.(*ZoneData).mutex.Unlock() remove := false switch t := r.Header().Rrtype; t { case TypeRRSIG: From 251af89dcc82a3835d42c26981013c69eade207a Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 20:19:13 +0200 Subject: [PATCH 09/11] Add locking for the zone structure --- zone.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/zone.go b/zone.go index abc41872..efd9ea37 100644 --- a/zone.go +++ b/zone.go @@ -8,7 +8,7 @@ import ( "sync" ) -// Zone represents a DNS zone. Currently there is no locking implemented. +// Zone represents a DNS zone. type Zone struct { Origin string // Origin of the zone Wildcard int // Whenever we see a wildcard name, this is incremented @@ -48,6 +48,7 @@ func NewZone(origin string) *Zone { return nil } z := new(Zone) + z.mutex = new(sync.RWMutex) z.Origin = Fqdn(origin) z.Radix = radix.New() return z @@ -73,6 +74,7 @@ func (z *Zone) Insert(r RR) error { zd.Name = r.Header().Name zd.RR = make(map[uint16][]RR) zd.Signatures = make(map[uint16][]*RR_RRSIG) + zd.mutex = new(sync.RWMutex) switch t := r.Header().Rrtype; t { case TypeRRSIG: sigtype := r.(*RR_RRSIG).TypeCovered From b22e2787fae83e969625bcab09991a606ae8882d Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 20:57:26 +0200 Subject: [PATCH 10/11] gofmt --- zone.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zone.go b/zone.go index 08ecc4a7..077c77a8 100644 --- a/zone.go +++ b/zone.go @@ -8,12 +8,12 @@ import ( "sync" ) -// Zone represents a DNS zone. +// Zone represents a DNS zone. The structure is safe for concurrent access. type Zone struct { Origin string // Origin of the zone Wildcard int // Whenever we see a wildcard name, this is incremented *radix.Radix // Zone data - mutex *sync.RWMutex + mutex *sync.RWMutex } // ZoneData holds all the RRs having their owner name equal to Name. From 49eebd3be4b62ac4c9a9a360a45e454cd83a3e33 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sat, 25 Aug 2012 21:30:16 +0200 Subject: [PATCH 11/11] fix benchmarking for zonedata --- parse_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/parse_test.go b/parse_test.go index c66c8eed..5bb186ff 100644 --- a/parse_test.go +++ b/parse_test.go @@ -380,14 +380,17 @@ func TestParseFailure(t *testing.T) { // A bit useless, how to use b.N? func BenchmarkZoneParsing(b *testing.B) { + b.StopTimer() f, err := os.Open("t/miek.nl.signed_test") if err != nil { return } defer f.Close() - to := ParseZone(f, "", "t/miek.nl.signed_test") - for x := range to { - x = x + b.StartTimer() + for i := 0; i < b.N; i++ { + to := ParseZone(f, "", "t/miek.nl.signed_test") + for _ = range to { + } } }