Merge pull request cert-manager#1325 from DanielMorsing/caaCheck

Extend ACME self check to check CAA records

pkg/acme/acme.go

@@ -112,8 +112,7 @@ func ClientWithKey(iss cmapi.GenericIssuer, pk *rsa.PrivateKey) (acme.Interface,
 	if acmeSpec == nil {
 		return nil, fmt.Errorf("issuer %q is not an ACME issuer. Ensure the 'acme' stanza is correctly specified on your Issuer resource", iss.GetObjectMeta().Name)
 	}
-	acmeStatus := iss.GetStatus().ACME
-	acmeCl := lookupClient(acmeSpec, acmeStatus, pk)
+	acmeCl := lookupClient(acmeSpec, pk)
 
 	return acmemw.NewLogger(acmeCl), nil
 }
@@ -151,27 +150,21 @@ var (
 )
 
 type repoKey struct {
-	accounturi string
-	skiptls    bool
-	server     string
-	publickey  string
-	exponent   int
+	skiptls   bool
+	server    string
+	publickey string
+	exponent  int
 }
 
-func lookupClient(spec *cmapi.ACMEIssuer, status *cmapi.ACMEIssuerStatus, pk *rsa.PrivateKey) *acmecl.Client {
+func lookupClient(spec *cmapi.ACMEIssuer, pk *rsa.PrivateKey) *acmecl.Client {
 	clientRepoMu.Lock()
 	defer clientRepoMu.Unlock()
 	if clientRepo == nil {
 		clientRepo = make(map[repoKey]*acmecl.Client)
 	}
-	accountURI := ""
-	if status != nil && status.URI != "" {
-		accountURI = status.URI
-	}
 	repokey := repoKey{
-		accounturi: accountURI,
-		skiptls:    spec.SkipTLSVerify,
-		server:     spec.Server,
+		skiptls: spec.SkipTLSVerify,
+		server:  spec.Server,
 	}
 	// Encoding a big.Int cannot fail
 	pkbytes, _ := pk.PublicKey.N.GobEncode()
@@ -188,7 +181,6 @@ func lookupClient(spec *cmapi.ACMEIssuer, status *cmapi.ACMEIssuerStatus, pk *rs
 		DirectoryURL: spec.Server,
 		UserAgent:    util.CertManagerUserAgent,
 	}
-	acmeCl.SetAccountURL(accountURI)
 	clientRepo[repokey] = acmeCl
 	return acmeCl
 }

pkg/acme/client/fake.go

@@ -41,6 +41,7 @@ type FakeACME struct {
 	FakeGetAccount              func(ctx context.Context) (*acme.Account, error)
 	FakeHTTP01ChallengeResponse func(token string) (string, error)
 	FakeDNS01ChallengeRecord    func(token string) (string, error)
+	FakeDiscover                func(ctx context.Context) (acme.Directory, error)
 }
 
 func (f *FakeACME) CreateOrder(ctx context.Context, order *acme.Order) (*acme.Order, error) {
@@ -133,3 +134,12 @@ func (f *FakeACME) DNS01ChallengeRecord(token string) (string, error) {
 	}
 	return "", fmt.Errorf("DNS01ChallengeRecord not implemented")
 }
+
+func (f *FakeACME) Discover(ctx context.Context) (acme.Directory, error) {
+	if f.FakeDiscover != nil {
+		return f.FakeDiscover(ctx)
+	}
+	// We only use Discover to find CAAIdentities, so returning an
+	// empty directory here will be fine
+	return acme.Directory{}, nil
+}

pkg/acme/client/interfaces.go

@@ -36,6 +36,7 @@ type Interface interface {
 	GetAccount(ctx context.Context) (*acme.Account, error)
 	HTTP01ChallengeResponse(token string) (string, error)
 	DNS01ChallengeRecord(token string) (string, error)
+	Discover(ctx context.Context) (acme.Directory, error)
 }
 
 var _ Interface = &acme.Client{}

pkg/acme/client/middleware/logger.go

@@ -98,3 +98,8 @@ func (l *Logger) DNS01ChallengeRecord(token string) (string, error) {
 	glog.Infof("Calling DNS01ChallengeRecord")
 	return l.baseCl.DNS01ChallengeRecord(token)
 }
+
+func (l *Logger) Discover(ctx context.Context) (acme.Directory, error) {
+	glog.Infof("Calling Discover")
+	return l.baseCl.Discover(ctx)
+}

pkg/controller/acmechallenges/BUILD.bazel

@@ -17,6 +17,7 @@ go_library(
         "//pkg/controller:go_default_library",
         "//pkg/controller/acmechallenges/scheduler:go_default_library",
         "//pkg/issuer/acme/dns:go_default_library",
+        "//pkg/issuer/acme/dns/util:go_default_library",
         "//pkg/issuer/acme/http:go_default_library",
         "//pkg/util:go_default_library",
         "//third_party/crypto/acme:go_default_library",

pkg/controller/acmechallenges/sync.go

@@ -31,6 +31,8 @@ import (
 	cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1"
 	controllerpkg "github.com/jetstack/cert-manager/pkg/controller"
 	acmeapi "github.com/jetstack/cert-manager/third_party/crypto/acme"
+
+	dnsutil "github.com/jetstack/cert-manager/pkg/issuer/acme/dns/util"
 )
 
 const (
@@ -131,6 +133,26 @@ func (c *Controller) Sync(ctx context.Context, ch *cmapi.Challenge) (err error)
 		return nil
 	}
 
+	// check for CAA records.
+	// CAA records are static, so we don't have to present anything
+	// before we check for them.
+
+	// Find out which identity the ACME server says it will use.
+	dir, err := cl.Discover(ctx)
+	if err != nil {
+		return err
+	}
+	// TODO(dmo): figure out if missing CAA identity in directory
+	// means no CAA check is performed by ACME server or if any valid
+	// CAA would stop issuance (strongly suspect the former)
+	if len(dir.CAA) != 0 {
+		err := dnsutil.ValidateCAA(ch.Spec.DNSName, dir.CAA, ch.Spec.Wildcard, c.Context.DNS01Nameservers)
+		if err != nil {
+			ch.Status.Reason = fmt.Sprintf("CAA self-check failed: %s", err)
+			return err
+		}
+	}
+
 	solver, err := c.solverFor(ch.Spec.Type)
 	if err != nil {
 		return err

pkg/issuer/acme/dns/util/wait.go

@@ -168,6 +168,87 @@ func dnsQuery(fqdn string, rtype uint16, nameservers []string, recursive bool) (
 	return
 }
 
+func ValidateCAA(domain string, issuerID []string, iswildcard bool, nameservers []string) error {
+	// see https://tools.ietf.org/html/rfc6844#section-4
+	// for more information about how CAA lookup is performed
+	fqdn := ToFqdn(domain)
+
+	issuerSet := make(map[string]bool)
+	for _, s := range issuerID {
+		issuerSet[s] = true
+	}
+
+	var caas []*dns.CAA
+	for {
+		// follow at most 8 cnames per label
+		queryDomain := fqdn
+		var msg *dns.Msg
+		var err error
+		for i := 0; i < 8; i++ {
+			//TODO(dmo): figure out if we need these servers to be configurable as well
+			msg, err = dnsQuery(queryDomain, dns.TypeCAA, nameservers, true)
+			if err != nil {
+				return fmt.Errorf("Could not validate CAA record: %s", err)
+			}
+			// we expect the domain to exist, but it might not have a CAA record
+			// (this is the root domain that we're validating, if it errors, then
+			// there are bigger problems)
+			if msg.Rcode != dns.RcodeSuccess {
+				return fmt.Errorf("Could not validate CAA: domain %q not found", domain)
+			}
+			oldQuery := queryDomain
+			queryDomain = updateDomainWithCName(msg, queryDomain)
+			if queryDomain == oldQuery {
+				break
+			}
+		}
+		// we have a response that's not a CNAME. It might be empty.
+		// if it is, go up a label and ask again
+		for _, rr := range msg.Answer {
+			caa, ok := rr.(*dns.CAA)
+			if !ok {
+				continue
+			}
+			caas = append(caas, caa)
+		}
+		if len(caas) != 0 {
+			break
+		}
+
+		index := strings.Index(fqdn, ".")
+		if index == -1 {
+			panic("should never happen")
+		}
+		fqdn = fqdn[index+1:]
+		if len(fqdn) == 0 {
+			// we reached the root with no CAA, don't bother asking
+			return nil
+		}
+	}
+
+	if !matchCAA(caas, issuerSet, iswildcard) {
+		// TODO(dmo): better error message
+		return fmt.Errorf("CAA record does not match issuer")
+	}
+	return nil
+}
+
+func matchCAA(caas []*dns.CAA, issuerIDs map[string]bool, iswildcard bool) bool {
+	expectedTag := "issue"
+	if iswildcard {
+		expectedTag = "issuewild"
+	}
+	for _, caa := range caas {
+		if caa.Tag != expectedTag {
+			continue
+		}
+		if issuerIDs[caa.Value] {
+			return true
+		}
+	}
+	return false
+}
+
 // lookupNameservers returns the authoritative nameservers for the given fqdn.
 func lookupNameservers(fqdn string, nameservers []string) ([]string, error) {
 	var authoritativeNss []string

pkg/issuer/acme/dns/util/wait_test.go

@@ -177,3 +177,24 @@ func TestResolveConfServers(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateCAA(t *testing.T) {
+	// google installs a CAA record at google.com
+	// ask for the www.google.com record to test that
+	// we recurse up the labels
+	err := ValidateCAA("www.google.com", []string{"letsencrypt", "pki.goog"}, false, RecursiveNameservers)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	// now ask, expecting a CA that wont match
+	err = ValidateCAA("www.google.com", []string{"daniel.homebrew.ca"}, false, RecursiveNameservers)
+	if err == nil {
+		t.Fatalf("expected err, got success")
+	}
+	// ask for a domain you know does not have CAA records.
+	// it should succeed
+	err = ValidateCAA("www.example.org", []string{"daniel.homebrew.ca"}, false, RecursiveNameservers)
+	if err != nil {
+		t.Fatalf("expected err, got %s", err)
+	}
+}

third_party/crypto/acme/acme.go

@@ -90,15 +90,6 @@ type Client struct {
 	accountURL string
 }
 
-// SetAccountURL will set the account URL cached by the client.
-// This should be used with caution, in order to reduce the number of calls
-// made to the 'new-acct' endpoint in 'cacheAccountURL'
-func (c *Client) SetAccountURL(url string) {
-	c.urlMu.Lock()
-	defer c.urlMu.Unlock()
-	c.accountURL = url
-}
-
 // Discover performs ACME server discovery using c.DirectoryURL.
 //
 // It caches successful result. So, subsequent calls will not result in