Replace invalid UTF-8 in error message (letsencrypt#5341)

andygabby · web-flow · commit 81eed0cd075f · 2021-03-16T14:10:16.000-06:00
Add processing to http body when it is passed as an error to be properly marshalled for grpc. Fixes letsencrypt#5317
diff --git a/va/http.go b/va/http.go
@@ -616,7 +616,7 @@ func (va *ValidationAuthorityImpl) processHTTPValidation(
 	// resulting payload is the same size as maxResponseSize fail
 	if len(body) >= maxResponseSize {
 		return nil, records, berrors.UnauthorizedError("Invalid response from %s [%s]: %q",
-			records[len(records)-1].URL, records[len(records)-1].AddressUsed, body)
+			records[len(records)-1].URL, records[len(records)-1].AddressUsed, replaceInvalidUTF8(body))
 	}
 	if httpResponse.StatusCode != 200 {
 		return nil, records, berrors.UnauthorizedError("Invalid response from %s [%s]: %d",
diff --git a/va/http_test.go b/va/http_test.go
@@ -579,6 +579,20 @@ func httpTestSrv(t *testing.T) *httptest.Server {
 		fmt.Fprint(resp, tooLargeBuf)
 	})
 
+	// Create a buffer that starts with invalid UTF8 and is bigger than
+	// maxResponseSize
+	tooLargeInvalidUTF8 := bytes.NewBuffer([]byte{})
+	tooLargeInvalidUTF8.WriteString("f\xffoo")
+	tooLargeInvalidUTF8.Write(tooLargeBuf.Bytes())
+	// invalid-utf8-body Responds with body that is larger than
+	// maxResponseSize and starts with an invalid UTF8 string. This is to
+	// test the codepath where invalid UTF8 is converted to valid UTF8
+	// that can be passed as an error message via grpc.
+	mux.HandleFunc("/invalid-utf8-body", func(resp http.ResponseWriter, req *http.Request) {
+		resp.WriteHeader(http.StatusOK)
+		fmt.Fprint(resp, tooLargeInvalidUTF8)
+	})
+
 	mux.HandleFunc("/redir-path-too-long", func(resp http.ResponseWriter, req *http.Request) {
 		http.Redirect(
 			resp,
@@ -1049,6 +1063,23 @@ func TestFetchHTTP(t *testing.T) {
 	}
 }
 
+func TestFetchHTTPInvalidUTF8(t *testing.T) {
+	testSrv := httpTestSrv(t)
+	defer testSrv.Close()
+	va, _ := setup(testSrv, 0, "", nil)
+	ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500)
+	defer cancel()
+	_, _, prob := va.fetchHTTP(ctx, "example.com", "/invalid-utf8-body")
+	expectedResult := "f\ufffdoo"
+	// If the body of the http response is larger than the maxResponseSize
+	// a truncated body is returned as part of the error detail. If the
+	// body contains invalid UTF-8 the invalid characters must be replaced
+	// before the error is marshalled for grpc. This tests that the
+	// invalid string "f\xffoo" is expected to be converted to
+	// "f\ufffdoo".
+	test.AssertContains(t, string(prob.Detail), expectedResult)
+}
+
 // All paths that get assigned to tokens MUST be valid tokens
 const pathWrongToken = "i6lNAC4lOOLYCl-A08VJt9z_tKYvVk63Dumo8icsBjQ"
 const path404 = "404"

Original file line number	Diff line number	Diff line change
`@@ -616,7 +616,7 @@ func (va *ValidationAuthorityImpl) processHTTPValidation(`
`616`	`616`	`// resulting payload is the same size as maxResponseSize fail`
`617`	`617`	`if len(body) >= maxResponseSize {`
`618`	`618`	`return nil, records, berrors.UnauthorizedError("Invalid response from %s [%s]: %q",`
`619`		`- records[len(records)-1].URL, records[len(records)-1].AddressUsed, body)`
	`619`	`+ records[len(records)-1].URL, records[len(records)-1].AddressUsed, replaceInvalidUTF8(body))`
`620`	`620`	`}`
`621`	`621`	`if httpResponse.StatusCode != 200 {`
`622`	`622`	`return nil, records, berrors.UnauthorizedError("Invalid response from %s [%s]: %d",`