diff --git a/cmd/controller/app/controller.go b/cmd/controller/app/controller.go index 574930241..0e47a1d70 100644 --- a/cmd/controller/app/controller.go +++ b/cmd/controller/app/controller.go @@ -133,7 +133,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 } @@ -185,6 +185,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 700a3e6c9..fdd6b5c07 100644 --- a/cmd/controller/app/options/options.go +++ b/cmd/controller/app/options/options.go @@ -63,8 +63,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 } @@ -89,6 +92,8 @@ const ( defaultACMEIssuerChallengeType = "http01" defaultACMEIssuerDNS01ProviderName = "" defaultEnableCertificateOwnerRef = false + + defaultDNS01RecursiveNameserversOnly = false ) var ( @@ -129,7 +134,8 @@ func NewControllerOptions() *ControllerOptions { DefaultAutoCertificateAnnotations: defaultAutoCertificateAnnotations, DefaultACMEIssuerChallengeType: defaultACMEIssuerChallengeType, DefaultACMEIssuerDNS01ProviderName: defaultACMEIssuerDNS01ProviderName, - DNS01Nameservers: []string{}, + DNS01RecursiveNameservers: []string{}, + DNS01RecursiveNameserversOnly: defaultDNS01RecursiveNameserversOnly, EnableCertificateOwnerRef: defaultEnableCertificateOwnerRef, } } @@ -206,9 +212,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.") @@ -222,7 +241,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 3c00262f7..88d10cd4b 100644 --- a/pkg/controller/context.go +++ b/pkg/controller/context.go @@ -98,6 +98,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()) }