From a86ed0f7534d0601d14858551280193f49d9489b Mon Sep 17 00:00:00 2001
From: Daniel McCarney <cpu@letsencrypt.org>
Date: Thu, 21 Nov 2019 13:54:49 -0500
Subject: [PATCH] RA: fix error returned through WFE2 for too big NewOrders.
 (#4572)

We need the RA's `NewOrder` RPC to return a `berrors.Malformed` instance
when there are too many identifiers. A bare error will be turned into
a server internal problem by the WFE2's `web.ProblemDetailsForError`
call while a `berrors.Malformed` will produce the expected malformed
problem.

This commit fixes the err, updates the unit test, and adds an end-to-end
integration test so we don't mess this up again.
---
 ra/ra.go                        |  3 ++-
 ra/ra_test.go                   |  1 +
 test/integration/common_test.go |  2 +-
 test/integration/errors_test.go | 35 +++++++++++++++++++++++++++++++++
 4 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 test/integration/errors_test.go

diff --git a/ra/ra.go b/ra/ra.go
index 78ed9791d..a2f2c5b1b 100644
--- a/ra/ra.go
+++ b/ra/ra.go
@@ -1796,7 +1796,8 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
 	}
 
 	if len(order.Names) > ra.maxNames {
-		return nil, fmt.Errorf("Order cannot contain more than %d DNS names", ra.maxNames)
+		return nil, berrors.MalformedError(
+			"Order cannot contain more than %d DNS names", ra.maxNames)
 	}
 
 	// Validate that our policy allows issuing for each of the names in the order
diff --git a/ra/ra_test.go b/ra/ra_test.go
index d4f637611..3ca1fab2f 100644
--- a/ra/ra_test.go
+++ b/ra/ra_test.go
@@ -3781,6 +3781,7 @@ func TestNewOrderMaxNames(t *testing.T) {
 	})
 	test.AssertError(t, err, "NewOrder didn't fail with too many names in request")
 	test.AssertEquals(t, err.Error(), "Order cannot contain more than 2 DNS names")
+	test.AssertEquals(t, berrors.Is(err, berrors.Malformed), true)
 }
 
 var CAkeyPEM = `
diff --git a/test/integration/common_test.go b/test/integration/common_test.go
index 88ea9699d..4a4f1b4f7 100644
--- a/test/integration/common_test.go
+++ b/test/integration/common_test.go
@@ -108,7 +108,7 @@ func authAndIssue(c *client, csrKey *ecdsa.PrivateKey, domains []string) (*issua
 	}
 	order, err := c.Client.NewOrder(c.Account, ids)
 	if err != nil {
-		return nil, fmt.Errorf("making order: %s", err)
+		return nil, err
 	}
 
 	for _, authUrl := range order.Authorizations {
diff --git a/test/integration/errors_test.go b/test/integration/errors_test.go
new file mode 100644
index 000000000..3517e0d7a
--- /dev/null
+++ b/test/integration/errors_test.go
@@ -0,0 +1,35 @@
+// +build integration
+
+package integration
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/eggsampler/acme/v3"
+
+	"github.com/letsencrypt/boulder/test"
+)
+
+// TestTooBigOrderError tests that submitting an order with more than 100 names
+// produces the expected problem result.
+func TestTooBigOrderError(t *testing.T) {
+	t.Parallel()
+	os.Setenv("DIRECTORY", "http://boulder:4001/directory")
+
+	var domains []string
+	for i := 0; i < 101; i++ {
+		domains = append(domains, fmt.Sprintf("%d.example.com", i))
+	}
+
+	_, err := authAndIssue(nil, nil, domains)
+	test.AssertError(t, err, "authAndIssue failed")
+
+	if prob, ok := err.(acme.Problem); !ok {
+		t.Fatalf("expected problem result, got %#v\n", err)
+	} else {
+		test.AssertEquals(t, prob.Type, "urn:ietf:params:acme:error:malformed")
+		test.AssertEquals(t, prob.Detail, "Error creating new order :: Order cannot contain more than 100 DNS names")
+	}
+}