Ensure gRPC suberror metadata is ascii-only (#7282)

When passing detailed error information between services as gRPC
metadata, ensure that the suberrors being sent contain only ascii
characters, because gRPC metadata is sent as HTTP headers which only
allow visible ascii characters.

Also add a regression test.
This commit is contained in:
Aaron Gable 2024-02-06 17:40:45 -08:00 committed by GitHub
parent af2c1a5963
commit 0358bd7bf3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 46 additions and 2 deletions

View File

@ -41,7 +41,8 @@ func wrapError(ctx context.Context, appErr error) error {
return berrors.InternalServerError( return berrors.InternalServerError(
"error marshaling json SubErrors, orig error %q", err) "error marshaling json SubErrors, orig error %q", err)
} }
pairs = append(pairs, "suberrors", string(jsonSubErrs)) headerSafeSubErrs := strconv.QuoteToASCII(string(jsonSubErrs))
pairs = append(pairs, "suberrors", headerSafeSubErrs)
} }
// If there is a RetryAfter value then extend the metadata pairs to // If there is a RetryAfter value then extend the metadata pairs to
@ -110,7 +111,17 @@ func unwrapError(err error, md metadata.MD) error {
) )
} }
unmarshalErr := json.Unmarshal([]byte(subErrorsVal[0]), &outErr.SubErrors) unquotedSubErrors, unquoteErr := strconv.Unquote(subErrorsVal[0])
if unquoteErr != nil {
return fmt.Errorf(
"unquoting 'suberrors' %q, wrapped error %q: %w",
subErrorsVal[0],
inErrMsg,
unquoteErr,
)
}
unmarshalErr := json.Unmarshal([]byte(unquotedSubErrors), &outErr.SubErrors)
if unmarshalErr != nil { if unmarshalErr != nil {
return berrors.InternalServerError( return berrors.InternalServerError(
"JSON unmarshaling 'suberrors' %q, wrapped error %q: %s", "JSON unmarshaling 'suberrors' %q, wrapped error %q: %s",

View File

@ -148,3 +148,36 @@ func TestAccountEmailError(t *testing.T) {
}) })
} }
} }
func TestRejectedIdentifier(t *testing.T) {
t.Parallel()
// When a single malformed name is provided, we correctly reject it.
domains := []string{
"яџХ6яяdь}",
}
_, err := authAndIssue(nil, nil, domains, true)
test.AssertError(t, err, "issuance should fail for one malformed name")
var prob acme.Problem
test.AssertErrorWraps(t, err, &prob)
test.AssertEquals(t, prob.Type, "urn:ietf:params:acme:error:rejectedIdentifier")
test.AssertContains(t, prob.Detail, "Domain name contains an invalid character")
// When multiple malformed names are provided, we correctly reject all of
// them and reflect this in suberrors. This test ensures that the way we
// encode these errors across the gRPC boundary is resilient to non-ascii
// characters.
domains = []string{
"˜o-",
"ш№Ў",
"р±y",
"яџХ6яя",
"яџХ6яя`ь",
}
_, err = authAndIssue(nil, nil, domains, true)
test.AssertError(t, err, "issuance should fail for multiple malformed names")
test.AssertErrorWraps(t, err, &prob)
test.AssertEquals(t, prob.Type, "urn:ietf:params:acme:error:rejectedIdentifier")
test.AssertContains(t, prob.Detail, "Domain name contains an invalid character")
test.AssertContains(t, prob.Detail, "and 4 more problems")
}