From e772779cafafbfb96620798e6eac4893d1e02ef0 Mon Sep 17 00:00:00 2001 From: Pauline Middelink Date: Sat, 27 Feb 2016 23:50:42 +0100 Subject: [PATCH 1/3] Fix for issue/140: - Removal of RFC2136_ZONE from help text - Query nameserver directly to find zone we have to update - During insert, make sure the new record is the ONLY challence. (I had a few panics, hence 3 challences left. Not good.) --- README.md | 2 +- acme/dns_challenge_rfc2136.go | 78 +++++++++++++++++++++++++++++------ cli.go | 2 +- cli_handlers.go | 3 +- 4 files changed, 69 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 9d1807aa..b03d776c 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ GLOBAL OPTIONS: digitalocean: DO_AUTH_TOKEN dnsimple: DNSIMPLE_EMAIL, DNSIMPLE_API_KEY route53: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION - rfc2136: RFC2136_TSIG_KEY, RFC2136_TSIG_SECRET, RFC2136_NAMESERVER, RFC2136_ZONE + rfc2136: RFC2136_TSIG_KEY, RFC2136_TSIG_SECRET, RFC2136_NAMESERVER manual: none --help, -h show help --version, -v print the version diff --git a/acme/dns_challenge_rfc2136.go b/acme/dns_challenge_rfc2136.go index e2954d25..cab76817 100644 --- a/acme/dns_challenge_rfc2136.go +++ b/acme/dns_challenge_rfc2136.go @@ -12,26 +12,23 @@ import ( // uses dynamic DNS updates (RFC 2136) to create TXT records on a nameserver. type DNSProviderRFC2136 struct { nameserver string - zone string tsigAlgorithm string tsigKey string tsigSecret string - records map[string]string + domain2zone map[string]string } // NewDNSProviderRFC2136 returns a new DNSProviderRFC2136 instance. // To disable TSIG authentication 'tsigAlgorithm, 'tsigKey' and 'tsigSecret' must be set to the empty string. -// 'nameserver' must be a network address in the the form "host" or "host:port". 'zone' must be the fully -// qualified name of the zone. -func NewDNSProviderRFC2136(nameserver, zone, tsigAlgorithm, tsigKey, tsigSecret string) (*DNSProviderRFC2136, error) { +// '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" } d := &DNSProviderRFC2136{ - nameserver: nameserver, - zone: zone, - records: make(map[string]string), + nameserver: nameserver, + domain2zone: make(map[string]string), } if tsigAlgorithm == "" { tsigAlgorithm = dns.HmacMD5 @@ -48,18 +45,21 @@ func NewDNSProviderRFC2136(nameserver, zone, tsigAlgorithm, tsigKey, tsigSecret // Present creates a TXT record using the specified parameters func (r *DNSProviderRFC2136) Present(domain, token, keyAuth string) error { fqdn, value, ttl := DNS01Record(domain, keyAuth) - r.records[fqdn] = value return r.changeRecord("INSERT", fqdn, value, ttl) } // CleanUp removes the TXT record matching the specified parameters func (r *DNSProviderRFC2136) CleanUp(domain, token, keyAuth string) error { - fqdn, _, ttl := DNS01Record(domain, keyAuth) - value := r.records[fqdn] + fqdn, value, ttl := DNS01Record(domain, keyAuth) return r.changeRecord("REMOVE", fqdn, value, ttl) } func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) error { + zone, err := r.findZone(fqdn) + if err != nil { + return err + } + // Create RR rr := new(dns.TXT) rr.Hdr = dns.RR_Header{Name: fqdn, Rrtype: dns.TypeTXT, Class: dns.ClassINET, Ttl: uint32(ttl)} @@ -69,9 +69,11 @@ func (r *DNSProviderRFC2136) changeRecord(action, fqdn, value string, ttl int) e // Create dynamic update packet m := new(dns.Msg) - m.SetUpdate(dns.Fqdn(r.zone)) + m.SetUpdate(zone) switch action { case "INSERT": + // Always remove old challenge left over from who knows what. + m.RemoveRRset(rrs) m.Insert(rrs) case "REMOVE": m.Remove(rrs) @@ -99,3 +101,55 @@ 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/cli.go b/cli.go index 893d1cda..91bef0cd 100644 --- a/cli.go +++ b/cli.go @@ -124,7 +124,7 @@ func main() { "\n\tdigitalocean: DO_AUTH_TOKEN" + "\n\tdnsimple: DNSIMPLE_EMAIL, DNSIMPLE_API_KEY" + "\n\troute53: AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_REGION" + - "\n\trfc2136: RFC2136_TSIG_KEY, RFC2136_TSIG_SECRET, RFC2136_NAMESERVER, RFC2136_ZONE" + + "\n\trfc2136: RFC2136_TSIG_KEY, RFC2136_TSIG_SECRET, RFC2136_NAMESERVER" + "\n\tmanual: none", }, } diff --git a/cli_handlers.go b/cli_handlers.go index cdf4dca8..3a8d3d6b 100644 --- a/cli_handlers.go +++ b/cli_handlers.go @@ -79,12 +79,11 @@ func setup(c *cli.Context) (*Configuration, *Account, *acme.Client) { provider, err = acme.NewDNSProviderRoute53("", "", awsRegion) case "rfc2136": nameserver := os.Getenv("RFC2136_NAMESERVER") - zone := os.Getenv("RFC2136_ZONE") tsigAlgorithm := os.Getenv("RFC2136_TSIG_ALGORITHM") tsigKey := os.Getenv("RFC2136_TSIG_KEY") tsigSecret := os.Getenv("RFC2136_TSIG_SECRET") - provider, err = acme.NewDNSProviderRFC2136(nameserver, zone, tsigAlgorithm, tsigKey, tsigSecret) + provider, err = acme.NewDNSProviderRFC2136(nameserver, tsigAlgorithm, tsigKey, tsigSecret) case "manual": provider, err = acme.NewDNSProviderManual() } From 4945919c69977bda1ae9c1f2181a6102e11a6b74 Mon Sep 17 00:00:00 2001 From: Pauline Middelink Date: Sun, 28 Feb 2016 21:09:05 +0100 Subject: [PATCH 2/3] - 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", }, } From 8b90b1a3801fb7fd4a621fe9782c4441082467b1 Mon Sep 17 00:00:00 2001 From: Pauline Middelink Date: Mon, 29 Feb 2016 08:46:15 +0100 Subject: [PATCH 3/3] Added testcase for in-valid.co.uk Camelcased: fqdn2zone to fqdnToZone Grammatical fix in externally visible error message --- acme/dns_challenge.go | 17 +++++++++++------ acme/dns_challenge_rfc2136_test.go | 10 +++++----- acme/dns_challenge_test.go | 8 ++++++-- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/acme/dns_challenge.go b/acme/dns_challenge.go index bf837760..533431ca 100644 --- a/acme/dns_challenge.go +++ b/acme/dns_challenge.go @@ -18,7 +18,7 @@ type preCheckDNSFunc func(fqdn, value string) (bool, error) var ( preCheckDNS preCheckDNSFunc = checkDNSPropagation - fqdn2zone = map[string]string{} + fqdnToZone = map[string]string{} ) var recursiveNameserver = "google-public-dns-a.google.com:53" @@ -185,7 +185,7 @@ func lookupNameservers(fqdn string) ([]string, error) { // 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 { + if zone, ok := fqdnToZone[fqdn]; ok { return zone, nil } @@ -210,9 +210,9 @@ func findZoneByFqdn(fqdn, nameserver string) (string, error) { // 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") + return "", fmt.Errorf("Could not determine zone authoritatively") } - fqdn2zone[fqdn] = zone + fqdnToZone[fqdn] = zone return zone, nil } } @@ -225,9 +225,9 @@ func findZoneByFqdn(fqdn, nameserver string) (string, error) { // 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") + return "", fmt.Errorf("Could not determine zone authoritatively") } - fqdn2zone[fqdn] = zone + fqdnToZone[fqdn] = zone return zone, nil } } @@ -252,6 +252,11 @@ func unFqdn(name string) string { return name } +// clearFqdnCache clears the cache of fqdn to zone mappings. Primarily used in testing. +func clearFqdnCache() { + fqdnToZone = map[string]string{} +} + // waitFor polls the given function 'f', once every 'interval' seconds, up to 'timeout' seconds. func waitFor(timeout, interval int, f func() (bool, error)) error { var lastErr string diff --git a/acme/dns_challenge_rfc2136_test.go b/acme/dns_challenge_rfc2136_test.go index b5905556..1a0ed5cc 100644 --- a/acme/dns_challenge_rfc2136_test.go +++ b/acme/dns_challenge_rfc2136_test.go @@ -26,7 +26,7 @@ var ( var reqChan = make(chan *dns.Msg, 10) func TestRFC2136CanaryLocalTestServer(t *testing.T) { - fqdn2zone = map[string]string{} + clearFqdnCache() dns.HandleFunc("example.com.", serverHandlerHello) defer dns.HandleRemove("example.com.") @@ -50,7 +50,7 @@ func TestRFC2136CanaryLocalTestServer(t *testing.T) { } func TestRFC2136ServerSuccess(t *testing.T) { - fqdn2zone = map[string]string{} + clearFqdnCache() dns.HandleFunc(rfc2136TestZone, serverHandlerReturnSuccess) defer dns.HandleRemove(rfc2136TestZone) @@ -70,7 +70,7 @@ func TestRFC2136ServerSuccess(t *testing.T) { } func TestRFC2136ServerError(t *testing.T) { - fqdn2zone = map[string]string{} + clearFqdnCache() dns.HandleFunc(rfc2136TestZone, serverHandlerReturnErr) defer dns.HandleRemove(rfc2136TestZone) @@ -92,7 +92,7 @@ func TestRFC2136ServerError(t *testing.T) { } func TestRFC2136TsigClient(t *testing.T) { - fqdn2zone = map[string]string{} + clearFqdnCache() dns.HandleFunc(rfc2136TestZone, serverHandlerReturnSuccess) defer dns.HandleRemove(rfc2136TestZone) @@ -112,7 +112,7 @@ func TestRFC2136TsigClient(t *testing.T) { } func TestRFC2136ValidUpdatePacket(t *testing.T) { - fqdn2zone = map[string]string{} + clearFqdnCache() dns.HandleFunc(rfc2136TestZone, serverHandlerPassBackRequest) defer dns.HandleRemove(rfc2136TestZone) diff --git a/acme/dns_challenge_test.go b/acme/dns_challenge_test.go index 84b805f4..54d5e095 100644 --- a/acme/dns_challenge_test.go +++ b/acme/dns_challenge_test.go @@ -35,11 +35,15 @@ var lookupNameserversTestsErr = []struct { }{ // invalid tld {"_null.n0n0.", - "Could not determine zone authoritively", + "Could not determine zone authoritatively", }, // invalid domain {"_null.com.", - "Could not determine zone authoritively", + "Could not determine zone authoritatively", + }, + // invalid domain + {"in-valid.co.uk.", + "Could not determine zone authoritatively", }, }