Skip to content

Commit

Permalink
Implement the DNS-over-HTTPS check
Browse files Browse the repository at this point in the history
  • Loading branch information
FlorianLiebhart committed Mar 30, 2022
1 parent 068c5f0 commit 00baf63
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 8 deletions.
1 change: 1 addition & 0 deletions cmd/controller/app/controller.go
Expand Up @@ -280,6 +280,7 @@ func buildControllerContextFactory(ctx context.Context, opts *options.Controller
HTTP01SolverResourceLimitsCPU: http01SolverResourceLimitsCPU,
HTTP01SolverResourceLimitsMemory: http01SolverResourceLimitsMemory,
HTTP01SolverImage: opts.ACMEHTTP01SolverImage,
ACMEDNS01CheckMethod: opts.ACMEDNS01CheckMethod,
// Allows specifying a list of custom nameservers to perform HTTP01 checks on.
HTTP01SolverNameservers: opts.ACMEHTTP01SolverNameservers,

Expand Down
20 changes: 20 additions & 0 deletions cmd/controller/app/options/options.go
Expand Up @@ -29,6 +29,7 @@ import (
cmdutil "github.com/cert-manager/cert-manager/cmd/util"
"github.com/cert-manager/cert-manager/internal/controller/feature"
cm "github.com/cert-manager/cert-manager/pkg/apis/certmanager"

challengescontroller "github.com/cert-manager/cert-manager/pkg/controller/acmechallenges"
orderscontroller "github.com/cert-manager/cert-manager/pkg/controller/acmeorders"
shimgatewaycontroller "github.com/cert-manager/cert-manager/pkg/controller/certificate-shim/gateways"
Expand All @@ -53,6 +54,7 @@ import (
csrvenaficontroller "github.com/cert-manager/cert-manager/pkg/controller/certificatesigningrequests/venafi"
clusterissuerscontroller "github.com/cert-manager/cert-manager/pkg/controller/clusterissuers"
issuerscontroller "github.com/cert-manager/cert-manager/pkg/controller/issuers"
dnsutil "github.com/cert-manager/cert-manager/pkg/issuer/acme/dns/util"
logf "github.com/cert-manager/cert-manager/pkg/logs"
"github.com/cert-manager/cert-manager/pkg/util"
utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature"
Expand Down Expand Up @@ -83,6 +85,8 @@ type ControllerOptions struct {
// Allows specifying a list of custom nameservers to perform HTTP01 checks on.
ACMEHTTP01SolverNameservers []string

ACMEDNS01CheckMethod string

ClusterIssuerAmbientCredentials bool
IssuerAmbientCredentials bool

Expand Down Expand Up @@ -125,6 +129,8 @@ const (
defaultKubernetesAPIQPS float32 = 20
defaultKubernetesAPIBurst = 50

defaultACMEDNS01CheckMethod = dnsutil.ACMEDNS01CheckViaDNSLookup

defaultClusterResourceNamespace = "kube-system"
defaultNamespace = ""

Expand Down Expand Up @@ -236,6 +242,7 @@ func NewControllerOptions() *ControllerOptions {
DefaultIssuerGroup: defaultTLSACMEIssuerGroup,
DefaultAutoCertificateAnnotations: defaultAutoCertificateAnnotations,
ACMEHTTP01SolverNameservers: []string{},
ACMEDNS01CheckMethod: defaultACMEDNS01CheckMethod,
DNS01RecursiveNameservers: []string{},
DNS01RecursiveNameserversOnly: defaultDNS01RecursiveNameserversOnly,
EnableCertificateOwnerRef: defaultEnableCertificateOwnerRef,
Expand Down Expand Up @@ -290,6 +297,12 @@ func (s *ControllerOptions) AddFlags(fs *pflag.FlagSet) {
"The docker image to use to solve ACME HTTP01 challenges. You most likely will not "+
"need to change this parameter unless you are testing a new feature or developing cert-manager.")

fs.StringVar(&s.ACMEDNS01CheckMethod, "acme-dns01-check-method", defaultACMEDNS01CheckMethod, fmt.Sprintf(
"[%s, %s] Method used to check DNS propagation during ACME DNS01 challenges. You may "+
"want to change this parameter if you run cert-manager with a different DNS view "+
"than the rest of the world (aka DNS split horizon).",
dnsutil.ACMEDNS01CheckViaDNSLookup, dnsutil.ACMEDNS01CheckViaHTTPS))

fs.StringVar(&s.ACMEHTTP01SolverResourceRequestCPU, "acme-http01-solver-resource-request-cpu", defaultACMEHTTP01SolverResourceRequestCPU, ""+
"Defines the resource request CPU size when spawning new ACME HTTP01 challenge solver pods.")

Expand Down Expand Up @@ -375,6 +388,13 @@ 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)
}

switch o.ACMEDNS01CheckMethod {
case dnsutil.ACMEDNS01CheckViaDNSLookup:
case dnsutil.ACMEDNS01CheckViaHTTPS:
default:
return fmt.Errorf("Unsupported DNS01 check method: %s", o.ACMEDNS01CheckMethod)
}

for _, server := range append(o.DNS01RecursiveNameservers, o.ACMEHTTP01SolverNameservers...) {
// ensure all servers have a port number
_, _, err := net.SplitHostPort(server)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -19,7 +19,7 @@ require (
github.com/hashicorp/vault/api v1.1.1
github.com/hashicorp/vault/sdk v0.2.1
github.com/kr/pretty v0.3.0
github.com/miekg/dns v1.1.41
github.com/miekg/dns v1.1.47
github.com/mitchellh/go-homedir v1.1.0
github.com/munnerz/crd-schema-fuzz v1.0.0
github.com/onsi/ginkgo v1.16.5
Expand Down
4 changes: 3 additions & 1 deletion go.sum
Expand Up @@ -944,8 +944,9 @@ github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182aff
github.com/maxbrunsfeld/counterfeiter/v6 v6.2.2/go.mod h1:eD9eIE7cdwcMi9rYluz88Jz2VyhSmden33/aXg4oVIY=
github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
github.com/miekg/dns v1.1.26/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso=
github.com/miekg/dns v1.1.34 h1:SgTzfkN+oLoIHF1bgUP+C71mzuDl3AhLApHzCCIAMWM=
github.com/miekg/dns v1.1.34/go.mod h1:KNUDUusw/aVsxyTYZM1oqvCicbwhgbNgztCETuNZ7xM=
github.com/miekg/dns v1.1.47 h1:J9bWiXbqMbnZPcY8Qi2E3EWIBsIm6MZzzJB9VRg5gL8=
github.com/miekg/dns v1.1.47/go.mod h1:e3IlAVfNqAllflbibAZEWOXOQ+Ynzk/dDozDxY7XnME=
github.com/miekg/pkcs11 v1.0.3/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WTFxgs=
github.com/mistifyio/go-zfs v2.1.2-0.20190413222219-f784269be439+incompatible/go.mod h1:8AuVvqP/mXw1px98n46wfvcGfQ4ci2FwoAjKYxuo3Z4=
github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc=
Expand Down Expand Up @@ -1683,6 +1684,7 @@ golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.6-0.20210726203631-07bc1bf47fb2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.6-0.20210820212750-d4cc65f0b2ff h1:VX/uD7MK0AHXGiScH3fsieUQUcpmRERPDYtqZdJnA+Q=
golang.org/x/tools v0.1.6-0.20210820212750-d4cc65f0b2ff/go.mod h1:YD9qOF0M9xpSpdWTBbzEl5e/RnCefISl8E5Noe10jFM=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
Expand Down
3 changes: 3 additions & 0 deletions pkg/controller/context.go
Expand Up @@ -154,6 +154,9 @@ type IssuerOptions struct {
}

type ACMEOptions struct {
// ACMEDNS01CheckMethod specifies how to check for DNS propagation for DNS01 challenges
ACMEDNS01CheckMethod string

// ACMEHTTP01SolverImage is the image to use for solving ACME HTTP01
// challenges
HTTP01SolverImage string
Expand Down
2 changes: 1 addition & 1 deletion pkg/issuer/acme/dns/dns.go
Expand Up @@ -116,7 +116,7 @@ func (s *Solver) Check(ctx context.Context, issuer v1.GenericIssuer, ch *cmacme.
log.V(logf.DebugLevel).Info("checking DNS propagation", "nameservers", s.Context.DNS01Nameservers)

ok, err := util.PreCheckDNS(fqdn, ch.Spec.Key, s.Context.DNS01Nameservers,
s.Context.DNS01CheckAuthoritative)
s.Context.DNS01CheckAuthoritative, s.Context.ACMEDNS01CheckMethod)
if err != nil {
return err
}
Expand Down
60 changes: 57 additions & 3 deletions pkg/issuer/acme/dns/util/wait.go
Expand Up @@ -9,8 +9,10 @@ this directory.
package util

import (
"encoding/json"
"fmt"
"net"
"net/http"
"strings"
"sync"
"time"
Expand All @@ -21,7 +23,7 @@ import (
)

type preCheckDNSFunc func(fqdn, value string, nameservers []string,
useAuthoritative bool) (bool, error)
useAuthoritative bool, acmeDNS01CheckMethod string) (bool, error)
type dnsQueryFunc func(fqdn string, rtype uint16, nameservers []string, recursive bool) (in *dns.Msg, err error)

var (
Expand All @@ -41,6 +43,11 @@ const defaultResolvConf = "/etc/resolv.conf"
const issueTag = "issue"
const issuewildTag = "issuewild"

const (
ACMEDNS01CheckViaDNSLookup = "dnslookup"
ACMEDNS01CheckViaHTTPS = "dns-over-https"
)

var defaultNameservers = []string{
"8.8.8.8:53",
"8.8.4.4:53",
Expand Down Expand Up @@ -101,9 +108,9 @@ func followCNAMEs(fqdn string, nameservers []string, fqdnChain ...string) (strin
}

// checkDNSPropagation checks if the expected TXT record has been propagated to all authoritative nameservers.
func checkDNSPropagation(fqdn, value string, nameservers []string,
useAuthoritative bool) (bool, error) {

func checkDNSPropagationWithDNSLookup(fqdn, value string, nameservers []string,
useAuthoritative bool) (bool, error) {
var err error
fqdn, err = followCNAMEs(fqdn, nameservers)
if err != nil {
Expand All @@ -125,6 +132,53 @@ func checkDNSPropagation(fqdn, value string, nameservers []string,
return checkAuthoritativeNss(fqdn, value, authoritativeNss)
}

func checkDNSPropagationWithHTTPS(fqdn, value string, nameservers []string, useAuthoritative bool) (bool, error) {
logf.V(logf.InfoLevel).Infof("Checking DNS propagation for FQDN %s using Google's API for DNS over HTTPS", fqdn)

req, err := http.NewRequest("GET", "https://8.8.8.8/resolve?name="+fqdn+"&type=TXT", nil)
if err != nil {
return false, err
}
req.Header.Add("Cache-Control", "no-cache")
r, err := http.DefaultClient.Do(req)
if err != nil {
return false, fmt.Errorf("Unable to lookup DNS via HTTPS: %s", err)
}
defer r.Body.Close()

var resp struct {
Status int
Answer []struct{ Data string }
}

if err = json.NewDecoder(r.Body).Decode(&resp); err != nil {
return false, fmt.Errorf("Error parsing response from DNS over HTTPS: %s", err)
}

if resp.Status == 0 && len(resp.Answer) >= 1 {
for _, answer := range resp.Answer {
if txt := strings.Trim(answer.Data, "\""); txt == value {
return true, nil
}
}
}

logf.V(logf.DebugLevel).Infof("No TXT entry found. Expected='%s'", value)
return false, nil
}

func checkDNSPropagation(fqdn, value string, nameservers []string,
useAuthoritative bool, acmeDNS01CheckMethod string) (bool, error) {
switch acmeDNS01CheckMethod {
case ACMEDNS01CheckViaDNSLookup:
return checkDNSPropagationWithDNSLookup(fqdn, value, nameservers, useAuthoritative)
case ACMEDNS01CheckViaHTTPS:
return checkDNSPropagationWithHTTPS(fqdn, value, nameservers, useAuthoritative)
default:
return false, fmt.Errorf("Unknown DNS propagation method")
}
}

// 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 {
Expand Down
11 changes: 9 additions & 2 deletions pkg/issuer/acme/dns/util/wait_test.go
Expand Up @@ -165,17 +165,24 @@ func TestMatchCAA(t *testing.T) {
}
}

func TestPreCheckDNSOverHTTPS(t *testing.T) {
ok, err := PreCheckDNS("google.com.", "v=spf1 include:_spf.google.com ~all", []string{"8.8.8.8:53"}, true, "dns-over-https")
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)
ok, err := PreCheckDNS("google.com.", "v=spf1 include:_spf.google.com ~all", []string{"8.8.8.8:53"}, true, "dnslookup")
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)
ok, err := PreCheckDNS("google.com.", "v=spf1 include:_spf.google.com ~all", []string{"1.1.1.1:53"}, false, "dnslookup")
if err != nil || !ok {
t.Errorf("preCheckDNS failed for acme-staging.api.letsencrypt.org: %s", err.Error())
}
Expand Down

0 comments on commit 00baf63

Please sign in to comment.