Skip to content

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Feb 24, 2020

Fixes some config related bugs that came up from the fuzzer.

  • Validations on request_type in RateLimit proto.
  • Admin handler mocking issues
  • Linking issue for grpc service auth definitions
  • Invalid header characters fixed by validating input

Fixes:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20851
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20845
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20839

Testing: regression corpus entries added

Signed-off-by: Asra Ali asraa@google.com

Signed-off-by: Asra Ali <asraa@google.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #10144 was opened by asraa.

see: more, trace.

@asraa
Copy link
Contributor Author

asraa commented Feb 24, 2020

I think there is already an existing RequestType enum in the IPTagging filter. Thoughts on adding the definition to a common directory or replicating and deprecating the string request_type?

Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
@dio dio requested review from yanavlasov and removed request for yanavlasov February 24, 2020 21:43
Signed-off-by: Asra Ali <asraa@google.com>
@asraa asraa requested a review from yanavlasov February 28, 2020 14:18
@@ -38,7 +38,8 @@ message RateLimit {
// :ref:`x-envoy-internal<config_http_conn_man_headers_x-envoy-internal>` is not set or false, a
// request is considered external. The filter defaults to *both*, and it will apply to all request
// types.
string request_type = 3;
string request_type = 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird it is not an enum

@repokitteh-read-only repokitteh-read-only bot removed the api label Mar 3, 2020
@mattklein123 mattklein123 merged commit 8ef84ae into envoyproxy:master Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants