CAA: Don't fail on critical iodef property tags (#6921)

RFC 8659 (CAA; https://www.rfc-editor.org/rfc/rfc8659) says that "A CA
MUST NOT issue certificates for any FQDN if the Relevant RRset for that
FQDN contains a CAA critical Property for an unknown or unsupported
Property Tag."

Let's Encrypt does technically support the iodef property tag: we
recognize it, but then ignore it and never choose to send notifications
to the given contact address. Historically, we have carried around the
iodef property tags in our internal structures as though we might use
them, but all code referencing them was essentially dead code.

As part of a set of simplifications,
#6886 made it so that we
completely ignore iodef property tags. However, this had the unintended
side-effect of causing iodef property tags with the Critical bit set to
be counted as "unknown critical" tags, which prevent issuance.

This change causes our property tag parsing code to recognize iodef tags
again, so that critical iodef tags don't prevent issuance.

va/caa.go

@@ -104,6 +104,12 @@ func filterCAA(rrs []*dns.CAA) ([]*dns.CAA, []*dns.CAA, bool) {
 			issue = append(issue, caaRecord)
 		case "issuewild":
 			issuewild = append(issuewild, caaRecord)
+		case "iodef":
+			// We support the iodef property tag insofar as we recognize it, but we
+			// never choose to send notifications to the specified addresses. So we
+			// do not store the contents of the property tag, but also avoid setting
+			// the criticalUnknown bit if there are critical iodef tags.
+			continue
 		default:
 			// The critical flag is the bit with significance 128. However, many CAA
 			// record users have misinterpreted the RFC and concluded that the bit

va/caa_test.go

@@ -637,6 +637,77 @@ func TestCAAFailure(t *testing.T) {
 	test.AssertEquals(t, prob.Type, probs.CAAProblem)
 }
 
+func TestFilterCAA(t *testing.T) {
+	testCases := []struct {
+		name              string
+		input             []*dns.CAA
+		expectedIssueVals []string
+		expectedWildVals  []string
+		expectedCU        bool
+	}{
+		{
+			name: "recognized non-critical",
+			input: []*dns.CAA{
+				{Tag: "issue", Value: "a"},
+				{Tag: "issuewild", Value: "b"},
+				{Tag: "iodef", Value: "c"},
+			},
+			expectedIssueVals: []string{"a"},
+			expectedWildVals:  []string{"b"},
+		},
+		{
+			name: "recognized critical",
+			input: []*dns.CAA{
+				{Tag: "issue", Value: "a", Flag: 128},
+				{Tag: "issuewild", Value: "b", Flag: 128},
+				{Tag: "iodef", Value: "c", Flag: 128},
+			},
+			expectedIssueVals: []string{"a"},
+			expectedWildVals:  []string{"b"},
+		},
+		{
+			name: "unrecognized non-critical",
+			input: []*dns.CAA{
+				{Tag: "unknown", Flag: 2},
+			},
+		},
+		{
+			name: "unrecognized critical",
+			input: []*dns.CAA{
+				{Tag: "unknown", Flag: 128},
+			},
+			expectedCU: true,
+		},
+		{
+			name: "unrecognized improper critical",
+			input: []*dns.CAA{
+				{Tag: "unknown", Flag: 1},
+			},
+			expectedCU: true,
+		},
+		{
+			name: "unrecognized very improper critical",
+			input: []*dns.CAA{
+				{Tag: "unknown", Flag: 9},
+			},
+			expectedCU: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			issue, wild, cu := filterCAA(tc.input)
+			for _, tag := range issue {
+				test.AssertSliceContains(t, tc.expectedIssueVals, tag.Value)
+			}
+			for _, tag := range wild {
+				test.AssertSliceContains(t, tc.expectedWildVals, tag.Value)
+			}
+			test.AssertEquals(t, tc.expectedCU, cu)
+		})
+	}
+}
+
 func TestSelectCAA(t *testing.T) {
 	expected := dns.CAA{Tag: "issue", Value: "foo"}