Merge pull request #1184 from tlmiller/feature/authnss

Control authoritative dns01 server check.

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{

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
+	// Allows specifying a list of custom nameservers to perform DNS checks on.
-	DNS01Nameservers []string
+	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{}, ""+
+	fs.StringSliceVar(&s.DNS01RecursiveNameservers, "dns01-recursive-nameservers",
-		"A list of comma seperated DNS server endpoints used for DNS01 check requests. "+
+		[]string{}, "A list of comma seperated dns server endpoints used for "+
-		"This should be a list containing IP address and port, for example: 8.8.8.8:53,8.8.4.4:53")
+			"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 {

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

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
 	}

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
 		}

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())
 	}