Fix auth zone hosted domain lookups.

Changes:
    1. When there are multiple route53 hosted top zone and delegated
    zones within the same account, cert-manager incorrectly uses
    the top level domain as auth zone for which it doesn't have perms.
    This DOSes AWS's IAM API.
    2. This change adds the best match in determining auth zone while
    looking up hosted zone IDs.
    3. Defines a GetBestMatch util function to perform longest domain
    matches.
    4. Adds test cases
    5. Fixes #3353

Signed-off-by: Supriya Premkumar <supriyapremkumar1@gmail.com>

pkg/issuer/acme/dns/route53/fixtures_test.go

@@ -30,6 +30,16 @@ var ListHostedZonesByNameResponse = `<?xml version="1.0" encoding="UTF-8"?>
          </Config>
          <ResourceRecordSetCount>10</ResourceRecordSetCount>
       </HostedZone>
+      <HostedZone>
+         <Id>/hostedzone/HIJKLMN</Id>
+         <Name>foo.example.com.</Name>
+         <CallerReference>D2224C5B-684A-DB4A-BB9A-E09E3BAFEA7A</CallerReference>
+         <Config>
+            <Comment>Test comment</Comment>
+            <PrivateZone>false</PrivateZone>
+         </Config>
+         <ResourceRecordSetCount>10</ResourceRecordSetCount>
+      </HostedZone>
    </HostedZones>
    <IsTruncated>true</IsTruncated>
    <NextDNSName>example2.com</NextDNSName>

pkg/issuer/acme/dns/route53/route53.go

@@ -245,16 +245,23 @@ func (r *DNSProvider) getHostedZoneID(fqdn string) (string, error) {
 		return "", err
 	}
 
-	var hostedZoneID string
+	zoneToID := make(map[string]string)
+	var hostedZones []string
 	for _, hostedZone := range resp.HostedZones {
 		// .Name has a trailing dot
-		if !*hostedZone.Config.PrivateZone && *hostedZone.Name == authZone {
-			hostedZoneID = *hostedZone.Id
-			break
+		if !*hostedZone.Config.PrivateZone {
+			zoneToID[*hostedZone.Name] = *hostedZone.Id
+			hostedZones = append(hostedZones, *hostedZone.Name)
 		}
 	}
+	authZone, err = util.FindBestMatch(fqdn, hostedZones...)
+	if err != nil {
+		return "", fmt.Errorf("Zone %s not found in Route 53 for domain %s", authZone, fqdn)
+	}
 
-	if len(hostedZoneID) == 0 {
+	hostedZoneID, ok := zoneToID[authZone]
+
+	if len(hostedZoneID) == 0 || !ok {
 		return "", fmt.Errorf("Zone %s not found in Route 53 for domain %s", authZone, fqdn)
 	}
 

pkg/issuer/acme/dns/route53/route53_test.go

@@ -105,6 +105,7 @@ func TestRoute53Present(t *testing.T) {
 	mockResponses := MockResponseMap{
 		"/2013-04-01/hostedzonesbyname":         MockResponse{StatusCode: 200, Body: ListHostedZonesByNameResponse},
 		"/2013-04-01/hostedzone/ABCDEFG/rrset/": MockResponse{StatusCode: 200, Body: ChangeResourceRecordSetsResponse},
+		"/2013-04-01/hostedzone/HIJKLMN/rrset/": MockResponse{StatusCode: 200, Body: ChangeResourceRecordSetsResponse},
 		"/2013-04-01/change/123456":             MockResponse{StatusCode: 200, Body: GetChangeResponse},
 	}
 
@@ -118,6 +119,18 @@ func TestRoute53Present(t *testing.T) {
 
 	err := provider.Present(domain, "_acme-challenge."+domain+".", keyAuth)
 	assert.NoError(t, err, "Expected Present to return no error")
+
+	subDomain := "foo.example.com"
+	err = provider.Present(subDomain, "_acme-challenge."+subDomain+".", keyAuth)
+	assert.NoError(t, err, "Expected Present to return no error")
+
+	nonExistentSubDomain := "bar.foo.example.com"
+	err = provider.Present(nonExistentSubDomain, nonExistentSubDomain+".", keyAuth)
+	assert.NoError(t, err, "Expected Present to return no error")
+
+	nonExistentDomain := "baz.com"
+	err = provider.Present(nonExistentDomain, nonExistentDomain+".", keyAuth)
+	assert.Error(t, err, "Expected Present to return an error")
 }
 
 func TestAssumeRole(t *testing.T) {

pkg/issuer/acme/dns/util/BUILD.bazel

@@ -16,10 +16,16 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["wait_test.go"],
+    srcs = [
+        "dns_test.go",
+        "wait_test.go",
+    ],
     data = glob(["testdata/**"]),
     embed = [":go_default_library"],
-    deps = ["@com_github_miekg_dns//:go_default_library"],
+    deps = [
+        "@com_github_miekg_dns//:go_default_library",
+        "@com_github_stretchr_testify//assert:go_default_library",
+    ],
 )
 
 filegroup(

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

@@ -33,3 +33,27 @@ func DNS01LookupFQDN(domain string, followCNAME bool, nameservers ...string) (st
 
 	return fqdn, nil
 }
+
+// FindBestMatch returns the longest match for a given domain within a list of domains
+func FindBestMatch(query string, domains ...string) (string, error) {
+	var maxSoFar int
+	var longest string
+
+	for _, domain := range domains {
+		if query == domain {
+			// Found exact match
+			return domain, nil
+		}
+
+		maxHere := dns.CompareDomainName(query, domain)
+		if maxHere > maxSoFar && dns.IsSubDomain(domain, query) {
+			maxSoFar = maxHere
+			longest = domain
+		}
+	}
+
+	if len(longest) == 0 {
+		return "", fmt.Errorf("query: %v has no matches", query)
+	}
+	return longest, nil
+}

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

@@ -0,0 +1,98 @@
+// +skip_license_check
+
+package util
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type input struct {
+	query   string
+	domains []string
+}
+
+type test struct {
+	name      string
+	input     input
+	want, got string
+}
+
+var domains = []string{
+	"foo.example.com",
+	"foo.bar.example.com",
+	"example.com",
+	"baz.com",
+}
+
+var tests = []*test{
+	{
+		name: "TestExactMatchTLD",
+		input: input{
+			query:   "example.com",
+			domains: domains,
+		},
+		want: "example.com",
+	},
+	{
+		name: "TestExactMatchSubDomain",
+		input: input{
+			query:   "foo.example.com",
+			domains: domains,
+		},
+		want: "foo.example.com",
+	},
+	{
+		name: "TestExactMatchSubDomainTwoLevels",
+		input: input{
+			query:   "foo.bar.example.com",
+			domains: domains,
+		},
+		want: "foo.bar.example.com",
+	},
+	{
+		name: "TestPartialMatchTLD",
+		input: input{
+			query:   "baz.example.com",
+			domains: domains,
+		},
+		want: "example.com",
+	},
+	{
+		name: "TestPartialMatchSubDomain",
+		input: input{
+			query:   "baz.foo.example.com",
+			domains: domains,
+		},
+		want: "foo.example.com",
+	},
+	{
+		name: "TestNoMatchReversedOrder", // Negative Test Case
+		input: input{
+			query:   "com.example.foo",
+			domains: domains,
+		},
+		want: "",
+	},
+	{
+		name: "TestNoMatches", // Negative Test Case
+		input: input{
+			query:   "bar.com",
+			domains: domains,
+		},
+		want: "",
+	},
+}
+
+func TestLongestMatches(t *testing.T) {
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			tc.got, _ = FindBestMatch(tc.input.query, tc.input.domains...)
+			if tc.got != tc.want {
+				assert.Equal(t, tc.want, tc.got, fmt.Sprintf("Failed: TestCase: %s | Query: %s | Want: %v | Got: %v", tc.name, tc.input.query, tc.want, tc.got))
+			}
+		})
+	}
+}