From 8a2ad13a87aefcc050f237e0e17ed73a43b57d26 Mon Sep 17 00:00:00 2001 From: Roland Bracewell Shoemaker Date: Fri, 15 Sep 2017 16:29:35 -0700 Subject: [PATCH] Don't tree climb for trees we've already climbed (#3096) Prevents repeated lookups in traditional CNAME or tree based CNAME loops --- va/caa.go | 11 ++++++++--- va/caa_test.go | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/va/caa.go b/va/caa.go index c9700379f..0e97914a8 100644 --- a/va/caa.go +++ b/va/caa.go @@ -139,15 +139,20 @@ func (va *ValidationAuthorityImpl) treeClimbingLookupCAA(ctx context.Context, fq // We will do an (arbitrary) maximum of 15 tree-climbing queries to avoid CNAME/CAA // hybrid loops maxAttempts := 15 - return va.treeClimbingLookupCAAWithCount(ctx, fqdn, &maxAttempts) + targets := map[string]bool{} + return va.treeClimbingLookupCAAWithCount(ctx, fqdn, &maxAttempts, &targets) } -func (va *ValidationAuthorityImpl) treeClimbingLookupCAAWithCount(ctx context.Context, fqdn string, attemptsRemaining *int) ([]*dns.CAA, error) { +func (va *ValidationAuthorityImpl) treeClimbingLookupCAAWithCount(ctx context.Context, fqdn string, attemptsRemaining *int, targets *map[string]bool) ([]*dns.CAA, error) { if *attemptsRemaining < 1 { return nil, fmt.Errorf("too many CNAMEs when looking up CAA") } + if _, present := (*targets)[fqdn]; present { + return nil, nil + } *attemptsRemaining-- caas, cnames, err := va.dnsClient.LookupCAA(ctx, fqdn) + (*targets)[fqdn] = true if err != nil { return nil, err } else if len(caas) > 0 { @@ -163,7 +168,7 @@ func (va *ValidationAuthorityImpl) treeClimbingLookupCAAWithCount(ctx context.Co // list. newTargets := parentDomains(cnames[i].Target) for _, newTarget := range newTargets { - caas, err := va.treeClimbingLookupCAAWithCount(ctx, newTarget, attemptsRemaining) + caas, err := va.treeClimbingLookupCAAWithCount(ctx, newTarget, attemptsRemaining, targets) if len(caas) != 0 || err != nil { return caas, err } diff --git a/va/caa_test.go b/va/caa_test.go index 9a99f9d8f..7b28a87b6 100644 --- a/va/caa_test.go +++ b/va/caa_test.go @@ -60,13 +60,13 @@ func TestTreeClimbingLookupCAASimpleSuccess(t *testing.T) { } } -func TestTreeClimbingLookupCAALimitHit(t *testing.T) { +func TestTreeClimbingLookupCAALoop(t *testing.T) { target := "blog.cname-to-subdomain.com" _ = features.Set(map[string]bool{"LegacyCAA": true}) va, _ := setup(nil, 0) prob := va.checkCAA(ctx, core.AcmeIdentifier{Type: core.IdentifierDNS, Value: target}) - if prob == nil { - t.Fatalf("Expected failure for %q, got success", target) + if prob != nil { + t.Fatalf("Expected success for %q, got failure: %s", target, prob) } }