diff --git a/cmd/controller/app/options/options.go b/cmd/controller/app/options/options.go index ca531e64ff2..a4801a7d563 100644 --- a/cmd/controller/app/options/options.go +++ b/cmd/controller/app/options/options.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "net" + "net/url" "strings" "time" @@ -97,6 +98,11 @@ type ControllerOptions struct { DefaultAutoCertificateAnnotations []string // Allows specifying a list of custom nameservers to perform DNS checks on. + // Each nameserver can be either the IP address and port of a standard + // recursive DNS server, or the endpoint to an RFC 8484 DNS over HTTPS + // endpoint. For example, the following values are valid: + // - "8.8.8.8:53" (Standard DNS) + // - "https://1.1.1.1/dns-query" (DNS over HTTPS) DNS01RecursiveNameservers []string // Allows controlling if recursive nameservers are only used for all checks. // Normally authoritative nameservers are used for checking propagation. @@ -362,10 +368,13 @@ func (s *ControllerOptions) AddFlags(fs *pflag.FlagSet) { "Kind of the Issuer to use when the tls is requested but issuer kind is not specified on the ingress resource.") fs.StringVar(&s.DefaultIssuerGroup, "default-issuer-group", defaultTLSACMEIssuerGroup, ""+ "Group of the Issuer to use when the tls is requested but issuer group is not specified on the ingress resource.") + fs.StringSliceVar(&s.DNS01RecursiveNameservers, "dns01-recursive-nameservers", - []string{}, "A list of comma separated dns server endpoints used for "+ - "DNS01 check requests. This should be a list containing host and "+ - "port, for example 8.8.8.8:53,8.8.4.4:53") + []string{}, "A list of comma separated dns server endpoints used for DNS01 and DNS-over-HTTPS (DoH) check requests. "+ + "This should be a list containing entries of the following formats: `:` or `https://`. "+ + "For example: `8.8.8.8:53,8.8.4.4:53` or `https://1.1.1.1/dns-query,https://8.8.8.8/dns-query`. "+ + "To make sure ALL DNS requests happen through DoH, `dns01-recursive-nameservers-only` should also be set to true.") + fs.BoolVar(&s.DNS01RecursiveNameserversOnly, "dns01-recursive-nameservers-only", defaultDNS01RecursiveNameserversOnly, "When true, cert-manager will only ever query the configured DNS resolvers "+ @@ -434,7 +443,7 @@ func (o *ControllerOptions) Validate() error { return fmt.Errorf("invalid value for kube-api-burst: %v must be higher or equal to kube-api-qps: %v", o.KubernetesAPIQPS, o.KubernetesAPIQPS) } - for _, server := range append(o.DNS01RecursiveNameservers, o.ACMEHTTP01SolverNameservers...) { + for _, server := range o.ACMEHTTP01SolverNameservers { // ensure all servers have a port number _, _, err := net.SplitHostPort(server) if err != nil { @@ -442,6 +451,24 @@ func (o *ControllerOptions) Validate() error { } } + for _, server := range o.DNS01RecursiveNameservers { + // ensure all servers follow one of the following formats: + // - : + // - https:// + + if strings.HasPrefix(server, "https://") { + _, err := url.ParseRequestURI(server) + if err != nil { + return fmt.Errorf("invalid DNS server (%v): %v", err, server) + } + } else { + _, _, err := net.SplitHostPort(server) + if err != nil { + return fmt.Errorf("invalid DNS server (%v): %v", err, server) + } + } + } + errs := []error{} allControllersSet := sets.NewString(allControllers...) for _, controller := range o.controllers { diff --git a/cmd/controller/app/options/options_test.go b/cmd/controller/app/options/options_test.go index 3a0ca143928..f78f58dc082 100644 --- a/cmd/controller/app/options/options_test.go +++ b/cmd/controller/app/options/options_test.go @@ -17,9 +17,11 @@ limitations under the License. package options import ( + "strings" "testing" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/component-base/logs" ) func TestEnabledControllers(t *testing.T) { @@ -63,3 +65,48 @@ func TestEnabledControllers(t *testing.T) { }) } } + +func TestValidate(t *testing.T) { + tests := map[string]struct { + DNS01RecursiveServers []string + expError string + }{ + "if valid dns servers with ip address and port, return no errors": { + DNS01RecursiveServers: []string{"192.168.0.1:53", "10.0.0.1:5353"}, + expError: "", + }, + "if valid DNS servers with DoH server addresses including https prefix, return no errors": { + DNS01RecursiveServers: []string{"https://dns.example.com", "https://doh.server"}, + expError: "", + }, + "if invalid DNS server format due to missing https prefix, return 'invalid DNS server' error": { + DNS01RecursiveServers: []string{"dns.example.com"}, + expError: "invalid DNS server", + }, + "if invalid DNS server format due to invalid IP address length and no port, return 'invalid DNS server' error": { + DNS01RecursiveServers: []string{"192.168.0.1.53"}, + expError: "invalid DNS server", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + o := ControllerOptions{ + DNS01RecursiveNameservers: test.DNS01RecursiveServers, + DefaultIssuerKind: defaultTLSACMEIssuerKind, + KubernetesAPIBurst: defaultKubernetesAPIBurst, + KubernetesAPIQPS: defaultKubernetesAPIQPS, + Logging: logs.NewOptions(), + } + + err := o.Validate() + if test.expError != "" { + if err == nil || !strings.Contains(err.Error(), test.expError) { + t.Errorf("expected error containing '%s', but got: %v", test.expError, err) + } + } else if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} diff --git a/pkg/issuer/acme/dns/util/wait.go b/pkg/issuer/acme/dns/util/wait.go index 562af9fdb45..59a5e21cfcb 100644 --- a/pkg/issuer/acme/dns/util/wait.go +++ b/pkg/issuer/acme/dns/util/wait.go @@ -9,8 +9,12 @@ this directory. package util import ( + "bytes" + "context" "fmt" + "io/ioutil" "net" + "net/http" "strings" "sync" "time" @@ -153,13 +157,20 @@ func checkAuthoritativeNss(fqdn, value string, nameservers []string) (bool, erro return false, nil } } - + logf.V(logf.DebugLevel).Infof("Selfchecking using the DNS Lookup method was successful") return true, nil } // DNSQuery will query a nameserver, iterating through the supplied servers as it retries // The nameserver should include a port, to facilitate testing where we talk to a mock dns server. func DNSQuery(fqdn string, rtype uint16, nameservers []string, recursive bool) (in *dns.Msg, err error) { + switch rtype { + case dns.TypeCAA, dns.TypeCNAME, dns.TypeNS, dns.TypeSOA, dns.TypeTXT: + default: + // We explicitly specified here what types are supported, so we can more confidently create tests for this function. + return nil, fmt.Errorf("unsupported DNS record type %d", rtype) + } + m := new(dns.Msg) m.SetQuestion(fqdn, rtype) m.SetEdns0(4096, false) @@ -168,18 +179,30 @@ func DNSQuery(fqdn string, rtype uint16, nameservers []string, recursive bool) ( m.RecursionDesired = false } + udp := &dns.Client{Net: "udp", Timeout: DNSTimeout} + tcp := &dns.Client{Net: "tcp", Timeout: DNSTimeout} + httpClient := *http.DefaultClient + httpClient.Timeout = DNSTimeout + http := httpDNSClient{ + HTTPClient: &httpClient, + } + // Will retry the request based on the number of servers (n+1) - for i := 1; i <= len(nameservers)+1; i++ { - ns := nameservers[i%len(nameservers)] - udp := &dns.Client{Net: "udp", Timeout: DNSTimeout} - in, _, err = udp.Exchange(m, ns) - - if (in != nil && in.Truncated) || - (err != nil && strings.HasPrefix(err.Error(), "read udp") && strings.HasSuffix(err.Error(), "i/o timeout")) { - logf.V(logf.DebugLevel).Infof("UDP dns lookup failed, retrying with TCP: %v", err) - tcp := &dns.Client{Net: "tcp", Timeout: DNSTimeout} - // If the TCP request succeeds, the err will reset to nil - in, _, err = tcp.Exchange(m, ns) + for _, ns := range nameservers { + // If the TCP request succeeds, the err will reset to nil + if strings.HasPrefix(ns, "https://") { + in, _, err = http.Exchange(context.TODO(), m, ns) + + } else { + in, _, err = udp.Exchange(m, ns) + + // Try TCP if UDP fails + if (in != nil && in.Truncated) || + (err != nil && strings.HasPrefix(err.Error(), "read udp") && strings.HasSuffix(err.Error(), "i/o timeout")) { + logf.V(logf.DebugLevel).Infof("UDP dns lookup failed, retrying with TCP: %v", err) + // If the TCP request succeeds, the err will reset to nil + in, _, err = tcp.Exchange(m, ns) + } } if err == nil { @@ -189,6 +212,64 @@ func DNSQuery(fqdn string, rtype uint16, nameservers []string, recursive bool) ( return } +type httpDNSClient struct { + HTTPClient *http.Client +} + +const dohMimeType = "application/dns-message" + +func (c *httpDNSClient) Exchange(ctx context.Context, m *dns.Msg, a string) (r *dns.Msg, rtt time.Duration, err error) { + p, err := m.Pack() + if err != nil { + return nil, 0, err + } + + req, err := http.NewRequest(http.MethodPost, a, bytes.NewReader(p)) + if err != nil { + return nil, 0, err + } + + req.Header.Set("Content-Type", dohMimeType) + req.Header.Set("Accept", dohMimeType) + + hc := http.DefaultClient + if c.HTTPClient != nil { + hc = c.HTTPClient + } + + req = req.WithContext(ctx) + + t := time.Now() + + resp, err := hc.Do(req) + if err != nil { + return nil, 0, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, 0, fmt.Errorf("dns: server returned HTTP %d error: %q", resp.StatusCode, resp.Status) + } + + if ct := resp.Header.Get("Content-Type"); ct != dohMimeType { + return nil, 0, fmt.Errorf("dns: unexpected Content-Type %q; expected %q", ct, dohMimeType) + } + + p, err = ioutil.ReadAll(resp.Body) + if err != nil { + return nil, 0, err + } + + rtt = time.Since(t) + + r = new(dns.Msg) + if err := r.Unpack(p); err != nil { + return r, 0, err + } + + return r, rtt, nil +} + func ValidateCAA(domain string, issuerID []string, iswildcard bool, nameservers []string) error { // see https://tools.ietf.org/html/rfc6844#section-4 // for more information about how CAA lookup is performed diff --git a/pkg/issuer/acme/dns/util/wait_test.go b/pkg/issuer/acme/dns/util/wait_test.go index 9ae18cf8b75..2a7ae391ee7 100644 --- a/pkg/issuer/acme/dns/util/wait_test.go +++ b/pkg/issuer/acme/dns/util/wait_test.go @@ -165,6 +165,20 @@ func TestMatchCAA(t *testing.T) { } } +func TestPreCheckDNSOverHTTPSNoAuthoritative(t *testing.T) { + ok, err := PreCheckDNS("google.com.", "v=spf1 include:_spf.google.com ~all", []string{"https://1.1.1.1/dns-query"}, false) + if err != nil || !ok { + t.Errorf("preCheckDNS failed for acme-staging.api.letsencrypt.org: %s", err.Error()) + } +} + +func TestPreCheckDNSOverHTTPS(t *testing.T) { + ok, err := PreCheckDNS("google.com.", "v=spf1 include:_spf.google.com ~all", []string{"https://8.8.8.8/dns-query"}, true) + if err != nil || !ok { + t.Errorf("preCheckDNS failed for acme-staging.api.letsencrypt.org: %s", err.Error()) + } +} + func TestPreCheckDNS(t *testing.T) { // TODO: find a better TXT record to use in tests ok, err := PreCheckDNS("google.com.", "v=spf1 include:_spf.google.com ~all", []string{"8.8.8.8:53"}, true) @@ -259,30 +273,34 @@ func TestResolveConfServers(t *testing.T) { // TODO: find a website which uses issuewild? func TestValidateCAA(t *testing.T) { - // google installs a CAA record at google.com - // ask for the www.google.com record to test that - // we recurse up the labels - err := ValidateCAA("www.google.com", []string{"letsencrypt", "pki.goog"}, false, RecursiveNameservers) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - // now ask, expecting a CA that won't match - err = ValidateCAA("www.google.com", []string{"daniel.homebrew.ca"}, false, RecursiveNameservers) - if err == nil { - t.Fatalf("expected err, got success") - } - // if the CAA record allows non-wildcards then it has an `issue` tag, - // and it is known that it has no issuewild tags, then wildcard certificates - // will also be allowed - err = ValidateCAA("www.google.com", []string{"pki.goog"}, true, RecursiveNameservers) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - // ask for a domain you know does not have CAA records. - // it should succeed - err = ValidateCAA("www.example.org", []string{"daniel.homebrew.ca"}, false, RecursiveNameservers) - if err != nil { - t.Fatalf("expected err, got %s", err) + + for _, nameservers := range [][]string{RecursiveNameservers, []string{"https://1.1.1.1/dns-query"}, []string{"https://8.8.8.8/dns-query"}} { + + // google installs a CAA record at google.com + // ask for the www.google.com record to test that + // we recurse up the labels + err := ValidateCAA("www.google.com", []string{"letsencrypt", "pki.goog"}, false, nameservers) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + // now ask, expecting a CA that won't match + err = ValidateCAA("www.google.com", []string{"daniel.homebrew.ca"}, false, nameservers) + if err == nil { + t.Fatalf("expected err, got success") + } + // if the CAA record allows non-wildcards then it has an `issue` tag, + // and it is known that it has no issuewild tags, then wildcard certificates + // will also be allowed + err = ValidateCAA("www.google.com", []string{"pki.goog"}, true, nameservers) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + // ask for a domain you know does not have CAA records. + // it should succeed + err = ValidateCAA("www.example.org", []string{"daniel.homebrew.ca"}, false, nameservers) + if err != nil { + t.Fatalf("expected err, got %s", err) + } } }