From 4945919c69977bda1ae9c1f2181a6102e11a6b74 Mon Sep 17 00:00:00 2001 From: Pauline Middelink Date: Sun, 28 Feb 2016 21:09:05 +0100 Subject: [PATCH] - Moved findZone from rfc2136 to dns_challenge.go and renamed to findZoneByFqdn - Reworked the code in dns_challenge.go to not assume nameserver is port-less or defaults to 53. (messes up testing) - Updated nameserver test to clear the fqdn2zone cache and return a dummy SOA RR to make initial findZoneByFqdn call happy. - Used publicsuffix library to determine if the "authorative" zone we found is a public registry, in that case error out. (Also used by boulder btw) --- acme/dns_challenge.go | 76 +++++++++++++++++++++++++----- acme/dns_challenge_rfc2136.go | 71 +++++----------------------- acme/dns_challenge_rfc2136_test.go | 44 ++++++++++------- acme/dns_challenge_test.go | 4 +- 4 files changed, 106 insertions(+), 89 deletions(-) diff --git a/acme/dns_challenge.go b/acme/dns_challenge.go index d187f63b..bf837760 100644 --- a/acme/dns_challenge.go +++ b/acme/dns_challenge.go @@ -11,13 +11,17 @@ import ( "time" "github.com/miekg/dns" + "golang.org/x/net/publicsuffix" ) type preCheckDNSFunc func(fqdn, value string) (bool, error) -var preCheckDNS preCheckDNSFunc = checkDNSPropagation +var ( + preCheckDNS preCheckDNSFunc = checkDNSPropagation + fqdn2zone = map[string]string{} +) -var recursiveNameserver = "google-public-dns-a.google.com" +var recursiveNameserver = "google-public-dns-a.google.com:53" // DNS01Record returns a DNS record which will fulfill the `dns-01` challenge func DNS01Record(domain, keyAuth string) (fqdn string, value string, ttl int) { @@ -105,7 +109,7 @@ func checkDNSPropagation(fqdn, value string) (bool, error) { // checkAuthoritativeNss queries each of the given nameservers for the expected TXT record. func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, error) { for _, ns := range nameservers { - r, err := dnsQuery(fqdn, dns.TypeTXT, ns, false) + r, err := dnsQuery(fqdn, dns.TypeTXT, net.JoinHostPort(ns, "53"), false) if err != nil { return false, err } @@ -133,6 +137,7 @@ func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, erro } // dnsQuery sends a DNS query to the given nameserver. +// The nameserver should include a port, to facilitate testing where we talk to a mock dns server. func dnsQuery(fqdn string, rtype uint16, nameserver string, recursive bool) (in *dns.Msg, err error) { m := new(dns.Msg) m.SetQuestion(fqdn, rtype) @@ -142,7 +147,7 @@ func dnsQuery(fqdn string, rtype uint16, nameserver string, recursive bool) (in m.RecursionDesired = false } - in, err = dns.Exchange(m, net.JoinHostPort(nameserver, "53")) + in, err = dns.Exchange(m, nameserver) if err == dns.ErrTruncated { tcp := &dns.Client{Net: "tcp"} in, _, err = tcp.Exchange(m, nameserver) @@ -155,7 +160,12 @@ func dnsQuery(fqdn string, rtype uint16, nameserver string, recursive bool) (in func lookupNameservers(fqdn string) ([]string, error) { var authoritativeNss []string - r, err := dnsQuery(fqdn, dns.TypeNS, recursiveNameserver, true) + zone, err := findZoneByFqdn(fqdn, recursiveNameserver) + if err != nil { + return nil, err + } + + r, err := dnsQuery(zone, dns.TypeNS, recursiveNameserver, true) if err != nil { return nil, err } @@ -169,15 +179,59 @@ func lookupNameservers(fqdn string) ([]string, error) { if len(authoritativeNss) > 0 { return authoritativeNss, nil } + return nil, fmt.Errorf("Could not determine authoritative nameservers") +} - // Strip of the left most label to get the parent domain. - offset, _ := dns.NextLabel(fqdn, 0) - next := fqdn[offset:] - if dns.CountLabel(next) < 2 { - return nil, fmt.Errorf("Could not determine authoritative nameservers") +// findZoneByFqdn determines the zone of the given fqdn +func findZoneByFqdn(fqdn, nameserver string) (string, error) { + // Do we have it cached? + if zone, ok := fqdn2zone[fqdn]; ok { + return zone, nil } - return lookupNameservers(next) + // Query the authorative nameserver for a hopefully non-existing SOA record, + // in the authority section of the reply it will have the SOA of the + // containing zone. rfc2308 has this to say on the subject: + // Name servers authoritative for a zone MUST include the SOA record of + // the zone in the authority section of the response when reporting an + // NXDOMAIN or indicating that no data (NODATA) of the requested type exists + in, err := dnsQuery(fqdn, dns.TypeSOA, nameserver, true) + if err != nil { + return "", err + } + if in.Rcode != dns.RcodeNameError { + if in.Rcode != dns.RcodeSuccess { + return "", fmt.Errorf("NS %s returned %s for %s", nameserver, dns.RcodeToString[in.Rcode], fqdn) + } + // We have a success, so one of the answers has to be a SOA RR + for _, ans := range in.Answer { + if soa, ok := ans.(*dns.SOA); ok { + zone := soa.Hdr.Name + // If we ended up on one of the TLDs, it means the domain did not exist. + publicsuffix, _ := publicsuffix.PublicSuffix(unFqdn(zone)) + if publicsuffix == unFqdn(zone) { + return "", fmt.Errorf("Could not determine zone authoritively") + } + fqdn2zone[fqdn] = zone + return zone, nil + } + } + // Or it is NODATA, fall through to NXDOMAIN + } + // Search the authority section for our precious SOA RR + for _, ns := range in.Ns { + if soa, ok := ns.(*dns.SOA); ok { + zone := soa.Hdr.Name + // If we ended up on one of the TLDs, it means the domain did not exist. + publicsuffix, _ := publicsuffix.PublicSuffix(unFqdn(zone)) + if publicsuffix == unFqdn(zone) { + return "", fmt.Errorf("Could not determine zone authoritively") + } + fqdn2zone[fqdn] = zone + return zone, nil + } + } + return "", fmt.Errorf("NS %s did not return the expected SOA record in the authority section", nameserver) } // toFqdn converts the name into a fqdn appending a trailing dot. diff --git a/acme/dns_challenge_rfc2136.go b/acme/dns_challenge_rfc2136.go index cab76817..7c6900de 100644 --- a/acme/dns_challenge_rfc2136.go +++ b/acme/dns_challenge_rfc2136.go @@ -2,6 +2,7 @@ package acme import ( "fmt" + "net" "strings" "time" @@ -15,7 +16,6 @@ type DNSProviderRFC2136 struct { tsigAlgorithm string tsigKey string tsigSecret string - domain2zone map[string]string } // NewDNSProviderRFC2136 returns a new DNSProviderRFC2136 instance. @@ -23,12 +23,15 @@ type DNSProviderRFC2136 struct { // 'nameserver' must be a network address in the the form "host" or "host:port". func NewDNSProviderRFC2136(nameserver, tsigAlgorithm, tsigKey, tsigSecret string) (*DNSProviderRFC2136, error) { // Append the default DNS port if none is specified. - if !strings.Contains(nameserver, ":") { - nameserver += ":53" + if _, _, err := net.SplitHostPort(nameserver); err != nil { + if strings.Contains(err.Error(), "missing port") { + nameserver = net.JoinHostPort(nameserver, "53") + } else { + return nil, err + } } d := &DNSProviderRFC2136{ - nameserver: nameserver, - domain2zone: make(map[string]string), + nameserver: nameserver, } if tsigAlgorithm == "" { tsigAlgorithm = dns.HmacMD5 @@ -55,7 +58,8 @@ func (r *DNSProviderRFC2136) CleanUp(domain, token, keyAuth string) error { } func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) error { - zone, err := r.findZone(fqdn) + // Find the zone for the given fqdn + zone, err := findZoneByFqdn(fqdn, r.nameserver) if err != nil { return err } @@ -64,8 +68,7 @@ func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) e rr := new(dns.TXT) rr.Hdr = dns.RR_Header{Name: fqdn, Rrtype: dns.TypeTXT, Class: dns.ClassINET, Ttl: uint32(ttl)} rr.Txt = []string{value} - rrs := make([]dns.RR, 1) - rrs[0] = rr + rrs := []dns.RR{rr} // Create dynamic update packet m := new(dns.Msg) @@ -101,55 +104,3 @@ func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) e return nil } - -// findZone determines the zone of a qualifying cert DNSname -func (r *DNSProviderRFC2136) findZone(fqdn string) (string, error) { - // Do we have it cached? - if val, ok := r.domain2zone[fqdn]; ok { - return val, nil - } - - // Query the authorative nameserver for a hopefully non-existing SOA record, - // in the authority section of the reply it will have the SOA of the - // containing zone. rfc2308 has this to say on the subject: - // Name servers authoritative for a zone MUST include the SOA record of - // the zone in the authority section of the response when reporting an - // NXDOMAIN or indicating that no data (NODATA) of the requested type exists - m := new(dns.Msg) - m.SetQuestion(fqdn, dns.TypeSOA) - m.SetEdns0(4096, false) - m.RecursionDesired = true - m.Authoritative = true - - in, err := dns.Exchange(m, r.nameserver) - if err == dns.ErrTruncated { - tcp := &dns.Client{Net: "tcp"} - in, _, err = tcp.Exchange(m, r.nameserver) - } - if err != nil { - return "", err - } - if in.Rcode != dns.RcodeNameError { - if in.Rcode != dns.RcodeSuccess { - return "", fmt.Errorf("DNS Query for zone %q failed", fqdn) - } - // We have a success, so one of the answers has to be a SOA RR - for _, ans := range in.Answer { - if ans.Header().Rrtype == dns.TypeSOA { - zone := ans.Header().Name - r.domain2zone[fqdn] = zone - return zone, nil - } - } - // Or it is NODATA, fall through to NXDOMAIN - } - // Search the authority section for our precious SOA RR - for _, ns := range in.Ns { - if ns.Header().Rrtype == dns.TypeSOA { - zone := ns.Header().Name - r.domain2zone[fqdn] = zone - return zone, nil - } - } - return "", fmt.Errorf("Expected a SOA record in the authority section") -} diff --git a/acme/dns_challenge_rfc2136_test.go b/acme/dns_challenge_rfc2136_test.go index 9af8f491..b5905556 100644 --- a/acme/dns_challenge_rfc2136_test.go +++ b/acme/dns_challenge_rfc2136_test.go @@ -2,6 +2,7 @@ package acme import ( "bytes" + "fmt" "net" "strings" "sync" @@ -25,6 +26,7 @@ var ( var reqChan = make(chan *dns.Msg, 10) func TestRFC2136CanaryLocalTestServer(t *testing.T) { + fqdn2zone = map[string]string{} dns.HandleFunc("example.com.", serverHandlerHello) defer dns.HandleRemove("example.com.") @@ -48,6 +50,7 @@ func TestRFC2136CanaryLocalTestServer(t *testing.T) { } func TestRFC2136ServerSuccess(t *testing.T) { + fqdn2zone = map[string]string{} dns.HandleFunc(rfc2136TestZone, serverHandlerReturnSuccess) defer dns.HandleRemove(rfc2136TestZone) @@ -57,7 +60,7 @@ func TestRFC2136ServerSuccess(t *testing.T) { } defer server.Shutdown() - provider, err := NewDNSProviderRFC2136(addrstr, rfc2136TestZone, "", "", "") + provider, err := NewDNSProviderRFC2136(addrstr, "", "", "") if err != nil { t.Fatalf("Expected NewDNSProviderRFC2136() to return no error but the error was -> %v", err) } @@ -67,6 +70,7 @@ func TestRFC2136ServerSuccess(t *testing.T) { } func TestRFC2136ServerError(t *testing.T) { + fqdn2zone = map[string]string{} dns.HandleFunc(rfc2136TestZone, serverHandlerReturnErr) defer dns.HandleRemove(rfc2136TestZone) @@ -76,7 +80,7 @@ func TestRFC2136ServerError(t *testing.T) { } defer server.Shutdown() - provider, err := NewDNSProviderRFC2136(addrstr, rfc2136TestZone, "", "", "") + provider, err := NewDNSProviderRFC2136(addrstr, "", "", "") if err != nil { t.Fatalf("Expected NewDNSProviderRFC2136() to return no error but the error was -> %v", err) } @@ -88,6 +92,7 @@ func TestRFC2136ServerError(t *testing.T) { } func TestRFC2136TsigClient(t *testing.T) { + fqdn2zone = map[string]string{} dns.HandleFunc(rfc2136TestZone, serverHandlerReturnSuccess) defer dns.HandleRemove(rfc2136TestZone) @@ -97,7 +102,7 @@ func TestRFC2136TsigClient(t *testing.T) { } defer server.Shutdown() - provider, err := NewDNSProviderRFC2136(addrstr, rfc2136TestZone, "", rfc2136TestTsigKey, rfc2136TestTsigSecret) + provider, err := NewDNSProviderRFC2136(addrstr, "", rfc2136TestTsigKey, rfc2136TestTsigSecret) if err != nil { t.Fatalf("Expected NewDNSProviderRFC2136() to return no error but the error was -> %v", err) } @@ -107,6 +112,7 @@ func TestRFC2136TsigClient(t *testing.T) { } func TestRFC2136ValidUpdatePacket(t *testing.T) { + fqdn2zone = map[string]string{} dns.HandleFunc(rfc2136TestZone, serverHandlerPassBackRequest) defer dns.HandleRemove(rfc2136TestZone) @@ -116,18 +122,11 @@ func TestRFC2136ValidUpdatePacket(t *testing.T) { } defer server.Shutdown() - rr := new(dns.TXT) - rr.Hdr = dns.RR_Header{ - Name: rfc2136TestFqdn, - Rrtype: dns.TypeTXT, - Class: dns.ClassINET, - Ttl: uint32(rfc2136TestTTL), - } - rr.Txt = []string{rfc2136TestValue} - rrs := make([]dns.RR, 1) - rrs[0] = rr + txtRR, _ := dns.NewRR(fmt.Sprintf("%s %d IN TXT %s", rfc2136TestFqdn, rfc2136TestTTL, rfc2136TestValue)) + rrs := []dns.RR{txtRR} m := new(dns.Msg) - m.SetUpdate(dns.Fqdn(rfc2136TestZone)) + m.SetUpdate(rfc2136TestZone) + m.RemoveRRset(rrs) m.Insert(rrs) expectstr := m.String() expect, err := m.Pack() @@ -135,7 +134,7 @@ func TestRFC2136ValidUpdatePacket(t *testing.T) { t.Fatalf("Error packing expect msg: %v", err) } - provider, err := NewDNSProviderRFC2136(addrstr, rfc2136TestZone, "", "", "") + provider, err := NewDNSProviderRFC2136(addrstr, "", "", "") if err != nil { t.Fatalf("Expected NewDNSProviderRFC2136() to return no error but the error was -> %v", err) } @@ -198,6 +197,11 @@ func serverHandlerHello(w dns.ResponseWriter, req *dns.Msg) { func serverHandlerReturnSuccess(w dns.ResponseWriter, req *dns.Msg) { m := new(dns.Msg) m.SetReply(req) + if req.Opcode == dns.OpcodeQuery && req.Question[0].Qtype == dns.TypeSOA && req.Question[0].Qclass == dns.ClassINET { + // Return SOA to appease findZoneByFqdn() + soaRR, _ := dns.NewRR(fmt.Sprintf("%s %d IN SOA ns1.%s admin.%s 2016022801 28800 7200 2419200 1200", rfc2136TestZone, rfc2136TestTTL, rfc2136TestZone, rfc2136TestZone)) + m.Answer = []dns.RR{soaRR} + } if t := req.IsTsig(); t != nil { if w.TsigStatus() == nil { @@ -218,6 +222,11 @@ func serverHandlerReturnErr(w dns.ResponseWriter, req *dns.Msg) { func serverHandlerPassBackRequest(w dns.ResponseWriter, req *dns.Msg) { m := new(dns.Msg) m.SetReply(req) + if req.Opcode == dns.OpcodeQuery && req.Question[0].Qtype == dns.TypeSOA && req.Question[0].Qclass == dns.ClassINET { + // Return SOA to appease findZoneByFqdn() + soaRR, _ := dns.NewRR(fmt.Sprintf("%s %d IN SOA ns1.%s admin.%s 2016022801 28800 7200 2419200 1200", rfc2136TestZone, rfc2136TestTTL, rfc2136TestZone, rfc2136TestZone)) + m.Answer = []dns.RR{soaRR} + } if t := req.IsTsig(); t != nil { if w.TsigStatus() == nil { @@ -227,5 +236,8 @@ func serverHandlerPassBackRequest(w dns.ResponseWriter, req *dns.Msg) { } w.WriteMsg(m) - reqChan <- req + if req.Opcode != dns.OpcodeQuery || req.Question[0].Qtype != dns.TypeSOA || req.Question[0].Qclass != dns.ClassINET { + // Only talk back when it is not the SOA RR. + reqChan <- req + } } diff --git a/acme/dns_challenge_test.go b/acme/dns_challenge_test.go index 850a0f59..84b805f4 100644 --- a/acme/dns_challenge_test.go +++ b/acme/dns_challenge_test.go @@ -35,11 +35,11 @@ var lookupNameserversTestsErr = []struct { }{ // invalid tld {"_null.n0n0.", - "Could not determine authoritative nameservers", + "Could not determine zone authoritively", }, // invalid domain {"_null.com.", - "Could not determine authoritative nameservers", + "Could not determine zone authoritively", }, }