From dacd0b45cbbe5603674aeccb25a75835458e2a4d Mon Sep 17 00:00:00 2001 From: Thomas Miller Date: Wed, 9 Jan 2019 17:09:39 +1000 Subject: [PATCH] Control authoritative dns01 server check. Adds cmd flag for controlling if authoritative dns servers are used to check RR propagation or just normal resolvers. This change is added so that constrained enviornments can control more aspects of DNS queries performed. - Applying PR feedback Signed-off-by: Thomas Miller --- cmd/controller/app/controller.go | 3 ++- cmd/controller/app/options/options.go | 33 +++++++++++++++++++++------ pkg/controller/context.go | 4 ++++ pkg/issuer/acme/dns/dns.go | 3 ++- pkg/issuer/acme/dns/util/wait.go | 15 +++++++++--- pkg/issuer/acme/dns/util/wait_test.go | 16 +++++++++---- 6 files changed, 58 insertions(+), 16 deletions(-) diff --git a/cmd/controller/app/controller.go b/cmd/controller/app/controller.go index 8710a35a1..48820f243 100644 --- a/cmd/controller/app/controller.go +++ b/cmd/controller/app/controller.go @@ -126,7 +126,7 @@ func buildControllerContext(opts *options.ControllerOptions) (*controller.Contex return nil, nil, fmt.Errorf("error creating kubernetes client: %s", err.Error()) } - nameservers := opts.DNS01Nameservers + nameservers := opts.DNS01RecursiveNameservers if len(nameservers) == 0 { nameservers = dnsutil.RecursiveNameservers } @@ -177,6 +177,7 @@ func buildControllerContext(opts *options.ControllerOptions) (*controller.Contex HTTP01SolverResourceRequestMemory: HTTP01SolverResourceRequestMemory, HTTP01SolverResourceLimitsCPU: HTTP01SolverResourceLimitsCPU, HTTP01SolverResourceLimitsMemory: HTTP01SolverResourceLimitsMemory, + DNS01CheckAuthoritative: !opts.DNS01RecursiveNameserversOnly, DNS01Nameservers: nameservers, }, IssuerOptions: controller.IssuerOptions{ diff --git a/cmd/controller/app/options/options.go b/cmd/controller/app/options/options.go index 9058a116c..33ee28d08 100644 --- a/cmd/controller/app/options/options.go +++ b/cmd/controller/app/options/options.go @@ -62,8 +62,11 @@ type ControllerOptions struct { DefaultACMEIssuerChallengeType string DefaultACMEIssuerDNS01ProviderName string - // DNS01Nameservers allows specifying a list of custom nameservers to perform DNS checks - DNS01Nameservers []string + // Allows specifying a list of custom nameservers to perform DNS checks on. + DNS01RecursiveNameservers []string + // Allows controlling if recursive nameservers are only used for all checks. + // Normally authoritative nameservers are used for checking propagation. + DNS01RecursiveNameserversOnly bool EnableCertificateOwnerRef bool } @@ -87,6 +90,8 @@ const ( defaultACMEIssuerChallengeType = "http01" defaultACMEIssuerDNS01ProviderName = "" defaultEnableCertificateOwnerRef = false + + defaultDNS01RecursiveNameserversOnly = false ) var ( @@ -126,7 +131,8 @@ func NewControllerOptions() *ControllerOptions { DefaultAutoCertificateAnnotations: defaultAutoCertificateAnnotations, DefaultACMEIssuerChallengeType: defaultACMEIssuerChallengeType, DefaultACMEIssuerDNS01ProviderName: defaultACMEIssuerDNS01ProviderName, - DNS01Nameservers: []string{}, + DNS01RecursiveNameservers: []string{}, + DNS01RecursiveNameserversOnly: defaultDNS01RecursiveNameserversOnly, EnableCertificateOwnerRef: defaultEnableCertificateOwnerRef, } } @@ -200,9 +206,22 @@ func (s *ControllerOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.DefaultACMEIssuerDNS01ProviderName, "default-acme-issuer-dns01-provider-name", defaultACMEIssuerDNS01ProviderName, ""+ "Required if --default-acme-issuer-challenge-type is set to dns01. The DNS01 provider to use for ingresses using ACME dns01 "+ "validation that do not explicitly state a dns provider.") - fs.StringSliceVar(&s.DNS01Nameservers, "dns01-self-check-nameservers", []string{}, ""+ - "A list of comma seperated DNS server endpoints used for DNS01 check requests. "+ - "This should be a list containing IP address and port, for example: 8.8.8.8:53,8.8.4.4:53") + fs.StringSliceVar(&s.DNS01RecursiveNameservers, "dns01-recursive-nameservers", + []string{}, "A list of comma seperated dns server endpoints used for "+ + "DNS01 check requests. This should be a list containing IP address and "+ + "port, for example 8.8.8.8:53,8.8.4.4:53") + fs.BoolVar(&s.DNS01RecursiveNameserversOnly, "dns01-recursive-nameservers-only", + defaultDNS01RecursiveNameserversOnly, + "When true, cert-manager will only ever query the configured DNS resolvers "+ + "to perform the ACME DNS01 self check. This is useful in DNS constrained "+ + "environments, where access to authoritative nameservers is restricted. "+ + "Enabling this option could cause the DNS01 self check to take longer "+ + "due to caching performed by the recursive nameservers.") + fs.StringSliceVar(&s.DNS01RecursiveNameservers, "dns01-self-check-nameservers", + []string{}, "A list of comma seperated dns server endpoints used for "+ + "DNS01 check requests. This should be a list containing IP address and "+ + "port, for example 8.8.8.8:53,8.8.4.4:53") + fs.MarkDeprecated("dns01-self-check-nameservers", "Deprecated in favour of dns01-recursive-nameservers") fs.BoolVar(&s.EnableCertificateOwnerRef, "enable-certificate-owner-ref", defaultEnableCertificateOwnerRef, ""+ "Whether to set the certificate resource as an owner of secret where the tls certificate is stored. "+ "When this flag is enabled, the secret will be automatically removed when the certificate resource is deleted.") @@ -216,7 +235,7 @@ func (o *ControllerOptions) Validate() error { return fmt.Errorf("invalid default issuer kind: %v", o.DefaultIssuerKind) } - for _, server := range o.DNS01Nameservers { + for _, server := range o.DNS01RecursiveNameservers { // ensure all servers have a port number host, _, err := net.SplitHostPort(server) if err != nil { diff --git a/pkg/controller/context.go b/pkg/controller/context.go index d5788587b..781495950 100644 --- a/pkg/controller/context.go +++ b/pkg/controller/context.go @@ -94,6 +94,10 @@ type ACMEOptions struct { // HTTP01SolverResourceLimitsMemory defines the ACME pod's resource limits Memory size HTTP01SolverResourceLimitsMemory resource.Quantity + // DNS01CheckAuthoritative is a flag for controlling if auth nss are used + // for checking propogation of an RR. This is the ideal scenario + DNS01CheckAuthoritative bool + // DNS01Nameservers is a list of nameservers to use when performing self-checks // for ACME DNS01 validations. DNS01Nameservers []string diff --git a/pkg/issuer/acme/dns/dns.go b/pkg/issuer/acme/dns/dns.go index 4c48cd5e0..9c4f307af 100644 --- a/pkg/issuer/acme/dns/dns.go +++ b/pkg/issuer/acme/dns/dns.go @@ -109,7 +109,8 @@ func (s *Solver) Check(ctx context.Context, issuer v1alpha1.GenericIssuer, ch *v glog.Infof("Checking DNS propagation for %q using name servers: %v", ch.Spec.DNSName, s.Context.DNS01Nameservers) - ok, err := util.PreCheckDNS(fqdn, value, s.Context.DNS01Nameservers) + ok, err := util.PreCheckDNS(fqdn, value, s.Context.DNS01Nameservers, + s.Context.DNS01CheckAuthoritative) if err != nil { return false, err } diff --git a/pkg/issuer/acme/dns/util/wait.go b/pkg/issuer/acme/dns/util/wait.go index cad1a7cd9..94dc471aa 100644 --- a/pkg/issuer/acme/dns/util/wait.go +++ b/pkg/issuer/acme/dns/util/wait.go @@ -19,7 +19,8 @@ import ( "github.com/miekg/dns" ) -type preCheckDNSFunc func(fqdn, value string, nameservers []string) (bool, error) +type preCheckDNSFunc func(fqdn, value string, nameservers []string, + useAuthoritative bool) (bool, error) var ( // PreCheckDNS checks DNS propagation before notifying ACME that @@ -77,7 +78,8 @@ func updateDomainWithCName(r *dns.Msg, fqdn string) string { } // checkDNSPropagation checks if the expected TXT record has been propagated to all authoritative nameservers. -func checkDNSPropagation(fqdn, value string, nameservers []string) (bool, error) { +func checkDNSPropagation(fqdn, value string, nameservers []string, + useAuthoritative bool) (bool, error) { // Initial attempt to resolve at the recursive NS r, err := dnsQuery(fqdn, dns.TypeTXT, nameservers, true) if err != nil { @@ -87,18 +89,25 @@ func checkDNSPropagation(fqdn, value string, nameservers []string) (bool, error) fqdn = updateDomainWithCName(r, fqdn) } + if !useAuthoritative { + return checkAuthoritativeNss(fqdn, value, nameservers) + } + authoritativeNss, err := lookupNameservers(fqdn, nameservers) if err != nil { return false, err } + for i, ans := range authoritativeNss { + authoritativeNss[i] = net.JoinHostPort(ans, "53") + } return checkAuthoritativeNss(fqdn, value, authoritativeNss) } // 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, []string{net.JoinHostPort(ns, "53")}, false) + r, err := dnsQuery(fqdn, dns.TypeTXT, []string{ns}, true) if err != nil { return false, err } diff --git a/pkg/issuer/acme/dns/util/wait_test.go b/pkg/issuer/acme/dns/util/wait_test.go index c976e5ee9..7e7cee951 100644 --- a/pkg/issuer/acme/dns/util/wait_test.go +++ b/pkg/issuer/acme/dns/util/wait_test.go @@ -53,15 +53,15 @@ var checkAuthoritativeNssTests = []struct { ok bool }{ // TXT RR w/ expected value - {"8.8.8.8.asn.routeviews.org.", "151698.8.8.024", []string{"asnums.routeviews.org."}, + {"8.8.8.8.asn.routeviews.org.", "151698.8.8.024", []string{"asnums.routeviews.org.:53"}, true, }, // No TXT RR - {"ns1.google.com.", "", []string{"ns2.google.com."}, + {"ns1.google.com.", "", []string{"ns2.google.com.:53"}, false, }, // TXT RR /w unexpected value - {"8.8.8.8.asn.routeviews.org.", "fe01=", []string{"asnums.routeviews.org."}, + {"8.8.8.8.asn.routeviews.org.", "fe01=", []string{"asnums.routeviews.org.:53"}, false, }, } @@ -88,7 +88,15 @@ var checkResolvConfServersTests = []struct { 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"}) + ok, err := PreCheckDNS("google.com.", "v=spf1 include:_spf.google.com ~all", []string{"8.8.8.8:53"}, true) + if err != nil || !ok { + t.Errorf("preCheckDNS failed for acme-staging.api.letsencrypt.org: %s", err.Error()) + } +} + +func TestPreCheckDNSNonAuthoritative(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{"1.1.1.1:53"}, false) if err != nil || !ok { t.Errorf("preCheckDNS failed for acme-staging.api.letsencrypt.org: %s", err.Error()) }