From 3375fa06095593ade48fb624203af6de3a9f87f8 Mon Sep 17 00:00:00 2001 From: Adphi Date: Sat, 31 Jul 2021 22:03:03 +0200 Subject: [PATCH 1/5] http01: add custom nameservers support (#4286) Signed-off-by: Adphi --- cmd/controller/app/controller.go | 3 +- cmd/controller/app/options/options.go | 10 +++- pkg/controller/context.go | 4 ++ pkg/issuer/acme/http/BUILD.bazel | 2 + pkg/issuer/acme/http/http.go | 83 ++++++++++++++++++++++++++- pkg/issuer/acme/http/http_test.go | 8 +-- 6 files changed, 101 insertions(+), 9 deletions(-) diff --git a/cmd/controller/app/controller.go b/cmd/controller/app/controller.go index d795a6132..94665f796 100644 --- a/cmd/controller/app/controller.go +++ b/cmd/controller/app/controller.go @@ -66,7 +66,7 @@ const controllerAgentName = "cert-manager" // This sets the informer's resync period to 10 hours // following the controller-runtime defaults -//and following discussion: https://github.com/kubernetes-sigs/controller-runtime/pull/88#issuecomment-408500629 +// and following discussion: https://github.com/kubernetes-sigs/controller-runtime/pull/88#issuecomment-408500629 const resyncPeriod = 10 * time.Hour func Run(opts *options.ControllerOptions, stopCh <-chan struct{}) error { @@ -360,6 +360,7 @@ func buildControllerContext(ctx context.Context, opts *options.ControllerOptions HTTP01SolverResourceRequestMemory: HTTP01SolverResourceRequestMemory, HTTP01SolverResourceLimitsCPU: HTTP01SolverResourceLimitsCPU, HTTP01SolverResourceLimitsMemory: HTTP01SolverResourceLimitsMemory, + HTTP01SolverNameservers: opts.ACMEHTTP01SolverNameservers, DNS01CheckAuthoritative: !opts.DNS01RecursiveNameserversOnly, DNS01Nameservers: nameservers, AccountRegistry: acmeAccountRegistry, diff --git a/cmd/controller/app/options/options.go b/cmd/controller/app/options/options.go index e233c6413..ea578bcc4 100644 --- a/cmd/controller/app/options/options.go +++ b/cmd/controller/app/options/options.go @@ -79,6 +79,8 @@ type ControllerOptions struct { ACMEHTTP01SolverResourceRequestMemory string ACMEHTTP01SolverResourceLimitsCPU string ACMEHTTP01SolverResourceLimitsMemory string + // Allows specifying a list of custom nameservers to perform HTTP01 checks on. + ACMEHTTP01SolverNameservers []string ClusterIssuerAmbientCredentials bool IssuerAmbientCredentials bool @@ -232,6 +234,7 @@ func NewControllerOptions() *ControllerOptions { DefaultIssuerKind: defaultTLSACMEIssuerKind, DefaultIssuerGroup: defaultTLSACMEIssuerGroup, DefaultAutoCertificateAnnotations: defaultAutoCertificateAnnotations, + ACMEHTTP01SolverNameservers: []string{}, DNS01RecursiveNameservers: []string{}, DNS01RecursiveNameserversOnly: defaultDNS01RecursiveNameserversOnly, EnableCertificateOwnerRef: defaultEnableCertificateOwnerRef, @@ -298,6 +301,11 @@ func (s *ControllerOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.ACMEHTTP01SolverResourceLimitsMemory, "acme-http01-solver-resource-limits-memory", defaultACMEHTTP01SolverResourceLimitsMemory, ""+ "Defines the resource limits Memory size when spawning new ACME HTTP01 challenge solver pods.") + fs.StringSliceVar(&s.ACMEHTTP01SolverNameservers, "acme-http01-solver-nameservers", + []string{}, "A list of comma separated dns server endpoints used for "+ + "ACME HTTP01 check requests. This should be a list containing host and "+ + "port, for example 8.8.8.8:53,8.8.4.4:53") + fs.BoolVar(&s.ClusterIssuerAmbientCredentials, "cluster-issuer-ambient-credentials", defaultClusterIssuerAmbientCredentials, ""+ "Whether a cluster-issuer may make use of ambient credentials for issuers. 'Ambient Credentials' are credentials drawn from the environment, metadata services, or local files which are not explicitly configured in the ClusterIssuer API object. "+ "When this flag is enabled, the following sources for credentials are also used: "+ @@ -369,7 +377,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 o.DNS01RecursiveNameservers { + for _, server := range append(o.DNS01RecursiveNameservers, o.ACMEHTTP01SolverNameservers...) { // ensure all servers have a port number _, _, err := net.SplitHostPort(server) if err != nil { diff --git a/pkg/controller/context.go b/pkg/controller/context.go index 0319b2476..b0db3dc88 100644 --- a/pkg/controller/context.go +++ b/pkg/controller/context.go @@ -123,6 +123,10 @@ type ACMEOptions struct { // HTTP01SolverResourceLimitsMemory defines the ACME pod's resource limits Memory size HTTP01SolverResourceLimitsMemory resource.Quantity + // HTTP01SolverNameservers is a list of nameservers to use when performing self-checks + // for ACME HTTP01 validations. + HTTP01SolverNameservers []string + // DNS01CheckAuthoritative is a flag for controlling if auth nss are used // for checking propagation of an RR. This is the ideal scenario DNS01CheckAuthoritative bool diff --git a/pkg/issuer/acme/http/BUILD.bazel b/pkg/issuer/acme/http/BUILD.bazel index fcb747ccc..ffa4bdd4f 100644 --- a/pkg/issuer/acme/http/BUILD.bazel +++ b/pkg/issuer/acme/http/BUILD.bazel @@ -19,6 +19,7 @@ go_library( "//pkg/issuer/acme/http/solver:go_default_library", "//pkg/logs:go_default_library", "//pkg/util:go_default_library", + "@com_github_miekg_dns//:go_default_library", "@io_k8s_api//core/v1:go_default_library", "@io_k8s_api//networking/v1:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", @@ -50,6 +51,7 @@ go_test( "//pkg/apis/acme/v1:go_default_library", "//pkg/controller/test:go_default_library", "//test/unit/gen:go_default_library", + "@com_github_miekg_dns//:go_default_library", "@io_k8s_api//core/v1:go_default_library", "@io_k8s_api//networking/v1:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", diff --git a/pkg/issuer/acme/http/http.go b/pkg/issuer/acme/http/http.go index 2eee2bbbe..6ed42df70 100644 --- a/pkg/issuer/acme/http/http.go +++ b/pkg/issuer/acme/http/http.go @@ -27,6 +27,7 @@ import ( "strings" "time" + "github.com/miekg/dns" corev1 "k8s.io/api/core/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" corev1listers "k8s.io/client-go/listers/core/v1" @@ -70,7 +71,7 @@ type Solver struct { requiredPasses int } -type reachabilityTest func(ctx context.Context, url *url.URL, key string) error +type reachabilityTest func(ctx context.Context, url *url.URL, key string, dnsServers []string) error // NewSolver returns a new ACME HTTP01 solver for the given *controller.Context. func NewSolver(ctx *controller.Context) (*Solver, error) { @@ -172,7 +173,7 @@ func (s *Solver) Check(ctx context.Context, issuer v1.GenericIssuer, ch *cmacme. log.V(logf.DebugLevel).Info("running self check multiple times to ensure challenge has propagated", "required_passes", s.requiredPasses) for i := 0; i < s.requiredPasses; i++ { - err := s.testReachability(ctx, url, ch.Spec.Key) + err := s.testReachability(ctx, url, ch.Spec.Key, s.HTTP01SolverNameservers) if err != nil { return err } @@ -211,7 +212,7 @@ func (s *Solver) buildChallengeUrl(ch *cmacme.Challenge) *url.URL { // testReachability will attempt to connect to the 'domain' with 'path' and // check if the returned body equals 'key' -func testReachability(ctx context.Context, url *url.URL, key string) error { +func testReachability(ctx context.Context, url *url.URL, key string, dnsServers []string) error { log := logf.FromContext(ctx) log.V(logf.DebugLevel).Info("performing HTTP01 reachability check") @@ -269,6 +270,19 @@ func testReachability(ctx context.Context, url *url.URL, key string) error { }, } + if len(dnsServers) != 0 { + transport.DialContext = func(ctx context.Context, network, addr string) (conn net.Conn, err error) { + dialer := &net.Dialer{ + Timeout: 3 * time.Second, + } + addr, err = resolve(ctx, addr, dnsServers) + if err != nil { + log.V(logf.DebugLevel).Info("failed to resolve host", "error", err) + return nil, fmt.Errorf("failed to resolve host: %v", err) + } + return dialer.Dial(network, addr) + } + } client := &http.Client{ Transport: transport, Timeout: time.Second * 10, @@ -308,3 +322,66 @@ func testReachability(ctx context.Context, url *url.URL, key string) error { return nil } + +func resolve(ctx context.Context, addr string, dnsServers []string) (string, error) { + log := logf.FromContext(ctx) + host, port, err := net.SplitHostPort(addr) + if err != nil { + return "", err + } + for _, server := range dnsServers { + ip, ok, err := lookupHost(ctx, host, server) + if err != nil { + log.V(logf.DebugLevel).Info("failed to lookup host on server", "error", err, "server", server) + continue + } + if !ok { + continue + } + return ip + ":" + port, nil + } + return "", fmt.Errorf("[%s] no such host: %s", strings.Join(dnsServers, ", "), host) +} + +func lookupHost(ctx context.Context, host, server string) (string, bool, error) { + msg := new(dns.Msg) + msg.Id = dns.Id() + msg.RecursionDesired = true + msg.Question = []dns.Question{ + {Name: dns.Fqdn(host), Qtype: dns.TypeA, Qclass: dns.ClassINET}, + {Name: dns.Fqdn(host), Qtype: dns.TypeCNAME, Qclass: dns.ClassINET}, + {Name: dns.Fqdn(host), Qtype: dns.TypeAAAA, Qclass: dns.ClassINET}, + } + + r, err := dns.ExchangeContext(ctx, msg, server) + if err != nil { + return "", false, err + } + + // first try A records + for _, record := range r.Answer { + if t, ok := record.(*dns.A); ok { + return t.A.String(), true, nil + } + } + for _, record := range r.Answer { + if t, ok := record.(*dns.CNAME); ok { + // lookup for CNAME target IP + ip, ok, err := lookupHost(ctx, t.Target, server) + if err != nil { + return "", false, err + } + if !ok { + continue + } + return ip, true, nil + } + } + // finally try AAAA records + for _, record := range r.Answer { + if t, ok := record.(*dns.AAAA); ok { + return t.AAAA.String(), true, nil + } + } + return "", false, nil +} diff --git a/pkg/issuer/acme/http/http_test.go b/pkg/issuer/acme/http/http_test.go index 4fef74252..dc043a31f 100644 --- a/pkg/issuer/acme/http/http_test.go +++ b/pkg/issuer/acme/http/http_test.go @@ -28,9 +28,9 @@ import ( // countReachabilityTestCalls is a wrapper function that allows us to count the number // of calls to a reachabilityTest. func countReachabilityTestCalls(counter *int, t reachabilityTest) reachabilityTest { - return func(ctx context.Context, url *url.URL, key string) error { + return func(ctx context.Context, url *url.URL, key string, dnsServers []string) error { *counter++ - return t(ctx, url, key) + return t(ctx, url, key, dnsServers) } } @@ -44,14 +44,14 @@ func TestCheck(t *testing.T) { tests := []testT{ { name: "should pass", - reachabilityTest: func(context.Context, *url.URL, string) error { + reachabilityTest: func(context.Context, *url.URL, string, []string) error { return nil }, expectedErr: false, }, { name: "should error", - reachabilityTest: func(context.Context, *url.URL, string) error { + reachabilityTest: func(context.Context, *url.URL, string, []string) error { return fmt.Errorf("failed") }, expectedErr: true, From 498c49605357f06b89a8db4ef67d35aeb9fb3d8b Mon Sep 17 00:00:00 2001 From: Adphi Date: Thu, 6 Jan 2022 16:00:46 +0100 Subject: [PATCH 2/5] acme-http: fix bazel Signed-off-by: Adphi --- pkg/issuer/acme/http/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/issuer/acme/http/BUILD.bazel b/pkg/issuer/acme/http/BUILD.bazel index ffa4bdd4f..fe617ff10 100644 --- a/pkg/issuer/acme/http/BUILD.bazel +++ b/pkg/issuer/acme/http/BUILD.bazel @@ -51,7 +51,6 @@ go_test( "//pkg/apis/acme/v1:go_default_library", "//pkg/controller/test:go_default_library", "//test/unit/gen:go_default_library", - "@com_github_miekg_dns//:go_default_library", "@io_k8s_api//core/v1:go_default_library", "@io_k8s_api//networking/v1:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", From c9bc776b49e1aa6148e65436729c1f4d9f7af708 Mon Sep 17 00:00:00 2001 From: Adphi Date: Thu, 6 Jan 2022 21:30:11 +0100 Subject: [PATCH 3/5] acme-http: fix tests Signed-off-by: Adphi --- pkg/issuer/acme/http/BUILD.bazel | 1 + pkg/issuer/acme/http/http_test.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/pkg/issuer/acme/http/BUILD.bazel b/pkg/issuer/acme/http/BUILD.bazel index fe617ff10..12faebd86 100644 --- a/pkg/issuer/acme/http/BUILD.bazel +++ b/pkg/issuer/acme/http/BUILD.bazel @@ -49,6 +49,7 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/acme/v1:go_default_library", + "//pkg/controller:go_default_library", "//pkg/controller/test:go_default_library", "//test/unit/gen:go_default_library", "@io_k8s_api//core/v1:go_default_library", diff --git a/pkg/issuer/acme/http/http_test.go b/pkg/issuer/acme/http/http_test.go index dc043a31f..5e7b5d262 100644 --- a/pkg/issuer/acme/http/http_test.go +++ b/pkg/issuer/acme/http/http_test.go @@ -23,6 +23,7 @@ import ( "testing" cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1" + "github.com/jetstack/cert-manager/pkg/controller" ) // countReachabilityTestCalls is a wrapper function that allows us to count the number @@ -67,6 +68,7 @@ func TestCheck(t *testing.T) { test.challenge = &cmacme.Challenge{} } s := Solver{ + Context: &controller.Context{}, testReachability: countReachabilityTestCalls(&calls, test.reachabilityTest), requiredPasses: requiredCallsForPass, } From 0f9b47b4f004d6bf89cf0d20ff7c1fc1cc8c6788 Mon Sep 17 00:00:00 2001 From: Adphi Date: Thu, 6 Jan 2022 22:34:05 +0100 Subject: [PATCH 4/5] acme-http: use Go built-in resolver Signed-off-by: Adphi --- pkg/issuer/acme/http/BUILD.bazel | 1 - pkg/issuer/acme/http/http.go | 85 ++++++-------------------------- 2 files changed, 15 insertions(+), 71 deletions(-) diff --git a/pkg/issuer/acme/http/BUILD.bazel b/pkg/issuer/acme/http/BUILD.bazel index 12faebd86..7b5bb57a2 100644 --- a/pkg/issuer/acme/http/BUILD.bazel +++ b/pkg/issuer/acme/http/BUILD.bazel @@ -19,7 +19,6 @@ go_library( "//pkg/issuer/acme/http/solver:go_default_library", "//pkg/logs:go_default_library", "//pkg/util:go_default_library", - "@com_github_miekg_dns//:go_default_library", "@io_k8s_api//core/v1:go_default_library", "@io_k8s_api//networking/v1:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", diff --git a/pkg/issuer/acme/http/http.go b/pkg/issuer/acme/http/http.go index 6ed42df70..f3dd253ff 100644 --- a/pkg/issuer/acme/http/http.go +++ b/pkg/issuer/acme/http/http.go @@ -27,7 +27,6 @@ import ( "strings" "time" - "github.com/miekg/dns" corev1 "k8s.io/api/core/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" corev1listers "k8s.io/client-go/listers/core/v1" @@ -272,15 +271,24 @@ func testReachability(ctx context.Context, url *url.URL, key string, dnsServers if len(dnsServers) != 0 { transport.DialContext = func(ctx context.Context, network, addr string) (conn net.Conn, err error) { + // we need to increment a counter to iterate through the dns servers as the dialer will not + // return an error if the dns server is not responding. + counter := 0 dialer := &net.Dialer{ Timeout: 3 * time.Second, + Resolver: &net.Resolver{ + PreferGo: true, + Dial: func(ctx context.Context, network, address string) (net.Conn, error) { + d := net.Dialer{ + Timeout: 3 * time.Second, + } + s := dnsServers[counter%len(dnsServers)] + counter++ + return d.DialContext(ctx, network, s) + }, + }, } - addr, err = resolve(ctx, addr, dnsServers) - if err != nil { - log.V(logf.DebugLevel).Info("failed to resolve host", "error", err) - return nil, fmt.Errorf("failed to resolve host: %v", err) - } - return dialer.Dial(network, addr) + return dialer.DialContext(ctx, network, addr) } } client := &http.Client{ @@ -322,66 +330,3 @@ func testReachability(ctx context.Context, url *url.URL, key string, dnsServers return nil } - -func resolve(ctx context.Context, addr string, dnsServers []string) (string, error) { - log := logf.FromContext(ctx) - host, port, err := net.SplitHostPort(addr) - if err != nil { - return "", err - } - for _, server := range dnsServers { - ip, ok, err := lookupHost(ctx, host, server) - if err != nil { - log.V(logf.DebugLevel).Info("failed to lookup host on server", "error", err, "server", server) - continue - } - if !ok { - continue - } - return ip + ":" + port, nil - } - return "", fmt.Errorf("[%s] no such host: %s", strings.Join(dnsServers, ", "), host) -} - -func lookupHost(ctx context.Context, host, server string) (string, bool, error) { - msg := new(dns.Msg) - msg.Id = dns.Id() - msg.RecursionDesired = true - msg.Question = []dns.Question{ - {Name: dns.Fqdn(host), Qtype: dns.TypeA, Qclass: dns.ClassINET}, - {Name: dns.Fqdn(host), Qtype: dns.TypeCNAME, Qclass: dns.ClassINET}, - {Name: dns.Fqdn(host), Qtype: dns.TypeAAAA, Qclass: dns.ClassINET}, - } - - r, err := dns.ExchangeContext(ctx, msg, server) - if err != nil { - return "", false, err - } - - // first try A records - for _, record := range r.Answer { - if t, ok := record.(*dns.A); ok { - return t.A.String(), true, nil - } - } - for _, record := range r.Answer { - if t, ok := record.(*dns.CNAME); ok { - // lookup for CNAME target IP - ip, ok, err := lookupHost(ctx, t.Target, server) - if err != nil { - return "", false, err - } - if !ok { - continue - } - return ip, true, nil - } - } - // finally try AAAA records - for _, record := range r.Answer { - if t, ok := record.(*dns.AAAA); ok { - return t.AAAA.String(), true, nil - } - } - return "", false, nil -} From a131eda198c7468b67bf05ed4ec043678d43675d Mon Sep 17 00:00:00 2001 From: Adphi Date: Fri, 7 Jan 2022 12:37:12 +0100 Subject: [PATCH 5/5] acme-http: add reachability tests Signed-off-by: Adphi --- pkg/issuer/acme/http/BUILD.bazel | 1 + pkg/issuer/acme/http/http_test.go | 100 ++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/pkg/issuer/acme/http/BUILD.bazel b/pkg/issuer/acme/http/BUILD.bazel index 7b5bb57a2..893775d58 100644 --- a/pkg/issuer/acme/http/BUILD.bazel +++ b/pkg/issuer/acme/http/BUILD.bazel @@ -51,6 +51,7 @@ go_test( "//pkg/controller:go_default_library", "//pkg/controller/test:go_default_library", "//test/unit/gen:go_default_library", + "@com_github_miekg_dns//:go_default_library", "@io_k8s_api//core/v1:go_default_library", "@io_k8s_api//networking/v1:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", diff --git a/pkg/issuer/acme/http/http_test.go b/pkg/issuer/acme/http/http_test.go index 5e7b5d262..4b18f6dea 100644 --- a/pkg/issuer/acme/http/http_test.go +++ b/pkg/issuer/acme/http/http_test.go @@ -19,9 +19,14 @@ package http import ( "context" "fmt" + "net" "net/url" + "strings" + "sync/atomic" "testing" + "github.com/miekg/dns" + cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1" "github.com/jetstack/cert-manager/pkg/controller" ) @@ -89,3 +94,98 @@ func TestCheck(t *testing.T) { }) } } + +func TestReachabilityCustomDnsServers(t *testing.T) { + site := "https://cert-manager.io" + u, err := url.Parse(site) + if err != nil { + t.Fatalf("Failed to parse url %s: %v", site, err) + } + ips, err := net.LookupIP(u.Host) + if err != nil { + t.Fatalf("Failed to resolve %s: %v", u.Host, err) + } + + dnsServerCalled := int32(0) + + server := &dns.Server{Addr: "127.0.0.1:15353", Net: "udp"} + defer server.Shutdown() + + dns.HandleFunc(".", func(w dns.ResponseWriter, r *dns.Msg) { + m := new(dns.Msg) + m.SetReply(r) + + if r.Opcode != dns.OpcodeQuery { + return + } + for _, q := range m.Question { + if q.Name != u.Host+"." { + continue + } + switch q.Qtype { + case dns.TypeA: + t.Logf("A Query for %s\n", q.Name) + atomic.StoreInt32(&dnsServerCalled, 1) + for _, ip := range ips { + if strings.Contains(ip.String(), ":") { + continue + } + rr, err := dns.NewRR(fmt.Sprintf("%s A %s", q.Name, ip)) + if err == nil { + m.Answer = append(m.Answer, rr) + } + } + case dns.TypeAAAA: + t.Logf("AAAA Query for %s\n", q.Name) + atomic.StoreInt32(&dnsServerCalled, 1) + for _, ip := range ips { + if !strings.Contains(ip.String(), ":") { + continue + } + rr, err := dns.NewRR(fmt.Sprintf("%s AAAA %s", q.Name, ip)) + if err == nil { + m.Answer = append(m.Answer, rr) + } + } + } + } + if err := w.WriteMsg(m); err != nil { + t.Errorf("failed to write DNS response: %v", err) + } + }) + go server.ListenAndServe() + + key := "there is no key" + + tests := []struct { + name string + dnsServers []string + dnsServerCalled bool + }{ + { + name: "custom dns servers", + dnsServers: []string{"127.0.0.1:15353"}, + dnsServerCalled: true, + }, + { + name: "system dns servers", + dnsServerCalled: false, + }, + } + + for _, tt := range tests { + atomic.StoreInt32(&dnsServerCalled, 0) + err = testReachability(context.Background(), u, key, tt.dnsServers) + switch { + case err == nil: + t.Errorf("Expected error for testReachability, but got none") + case strings.Contains(err.Error(), key): + called := atomic.LoadInt32(&dnsServerCalled) == 1 + if called != tt.dnsServerCalled { + t.Errorf("Expected DNS server called: %v, but got %v", tt.dnsServerCalled, called) + } + default: + t.Errorf("Unexpected error: %v", err) + } + } +}