From 2d1a0d8e48bc35a0e6684a901e5c01a06a54cb85 Mon Sep 17 00:00:00 2001
From: Daniel McCarney <cpu@letsencrypt.org>
Date: Thu, 27 Jun 2019 13:22:38 -0400
Subject: [PATCH] ra/pa: fix suberrors for single error case. (#4305)

If there is only one overall error then there is no reason to include it
as a sub-error, just return a top level error without any sub-errors.
---
 policy/pa.go      | 12 +++++++-----
 policy/pa_test.go | 12 ++++++++++++
 ra/ra.go          | 18 ++++++++----------
 ra/ra_test.go     | 13 +++++++++++++
 4 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/policy/pa.go b/policy/pa.go
index 256ba5491..3d097365c 100644
--- a/policy/pa.go
+++ b/policy/pa.go
@@ -347,13 +347,15 @@ func (pa *AuthorityImpl) WillingToIssueWildcards(idents []identifier.ACMEIdentif
 	}
 	if len(subErrors) > 0 {
 		var detail string
+		// If there was only one error, then use it as the top level error that is
+		// returned.
 		if len(subErrors) == 1 {
-			detail = subErrors[0].BoulderError.Detail
-		} else {
-			detail = fmt.Sprintf("Policy forbids issuing for %q and %d more identifiers. "+
-				"Refer to sub-problems for more information",
-				firstBadIdent.Value, len(subErrors)-1)
+			return subErrors[0].BoulderError
 		}
+
+		detail = fmt.Sprintf("Policy forbids issuing for %q and %d more identifiers. "+
+			"Refer to sub-problems for more information",
+			firstBadIdent.Value, len(subErrors)-1)
 		return (&berrors.BoulderError{
 			Type:   berrors.RejectedIdentifier,
 			Detail: detail,
diff --git a/policy/pa_test.go b/policy/pa_test.go
index 565e3cb67..42ea1ddda 100644
--- a/policy/pa_test.go
+++ b/policy/pa_test.go
@@ -352,6 +352,18 @@ func TestWillingToIssueWildcards(t *testing.T) {
 
 	test.AssertEquals(t, subErrA.Type, berrors.RejectedIdentifier)
 	test.AssertEquals(t, subErrB.Type, berrors.Malformed)
+
+	// Test willing to issue with only *one* bad identifier.
+	err = pa.WillingToIssueWildcards([]identifier.ACMEIdentifier{
+		identifier.DNSIdentifier("letsdecrypt.org"),
+	})
+	// It should error
+	test.AssertError(t, err, "Expected err from WillingToIssueWildcards")
+
+	berr, ok = err.(*berrors.BoulderError)
+	test.AssertEquals(t, ok, true)
+	// There should be *no* suberrors because there was only one error overall.
+	test.AssertEquals(t, len(berr.SubErrors), 0)
 }
 
 var accountKeyJSON = `{
diff --git a/ra/ra.go b/ra/ra.go
index 312839c63..e53b79e8f 100644
--- a/ra/ra.go
+++ b/ra/ra.go
@@ -922,18 +922,16 @@ func (ra *RegistrationAuthorityImpl) recheckCAA(ctx context.Context, authzs []*c
 	}
 	if len(subErrors) > 0 {
 		var detail string
+		// If there was only one error, then use it as the top level error that is
+		// returned.
 		if len(subErrors) == 1 {
-			detail = fmt.Sprintf(
-				"Rechecking CAA for %q: %s",
-				subErrors[0].Identifier.Value,
-				subErrors[0].BoulderError.Detail)
-		} else {
-			detail = fmt.Sprintf(
-				"Rechecking CAA for %q and %d more identifiers failed. "+
-					"Refer to sub-problems for more information",
-				subErrors[0].Identifier.Value,
-				len(subErrors)-1)
+			return subErrors[0].BoulderError
 		}
+		detail = fmt.Sprintf(
+			"Rechecking CAA for %q and %d more identifiers failed. "+
+				"Refer to sub-problems for more information",
+			subErrors[0].Identifier.Value,
+			len(subErrors)-1)
 		return (&berrors.BoulderError{
 			Type:   berrors.CAA,
 			Detail: detail,
diff --git a/ra/ra_test.go b/ra/ra_test.go
index 31af9a52f..babb77671 100644
--- a/ra/ra_test.go
+++ b/ra/ra_test.go
@@ -2099,6 +2099,19 @@ func TestRecheckCAAFail(t *testing.T) {
 	test.AssertEquals(t, foundB, true)
 	test.AssertEquals(t, subErrA.Type, berrors.CAA)
 	test.AssertEquals(t, subErrB.Type, berrors.CAA)
+
+	// Recheck CAA with just one bad authz
+	authzs = []*core.Authorization{
+		makeHTTP01Authorization("a.com"),
+	}
+	err = ra.recheckCAA(context.Background(), authzs)
+	// It should error
+	test.AssertError(t, err, "expected err from recheckCAA")
+	// It should be a berror
+	berr, ok := err.(*berrors.BoulderError)
+	test.AssertEquals(t, ok, true)
+	// There should be *no* suberrors because there was only one overall error
+	test.AssertEquals(t, len(berr.SubErrors), 0)
 }
 
 func TestRecheckCAAInternalServerError(t *testing.T) {