C++ URI validation in Envoy #303
Labels
Comments
rmichela
added
C++
htuch
pushed a commit
to envoyproxy/envoy
that referenced
this issue
Jan 2, 2020
Fixes server_fuzz_test fuzz bugs: * QUIC upstream not implemented in prod, so the fuzzer removes the HTTP3 codec types for http health checks * PGV validate throws a not yet implemented error on URIs. I removed this check for now and replaced with a TODO. If I catch the std::exception that is thrown and bypass it with a warning statement in the logs, we skip over other validations, which means that any fuzz test or user using this uri field will be in danger of failing because of other invalid fields @JimmyCYJ. PGV issue tracked: bufbuild/protoc-gen-validate#303 Fixes OSS-Fuzz Issues: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19614 https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19689 Signed-off-by: Asra Ali <asraa@google.com>
mattklein123
pushed a commit
to envoyproxy/data-plane-api
that referenced
this issue
Jan 2, 2020
Fixes server_fuzz_test fuzz bugs: * QUIC upstream not implemented in prod, so the fuzzer removes the HTTP3 codec types for http health checks * PGV validate throws a not yet implemented error on URIs. I removed this check for now and replaced with a TODO. If I catch the std::exception that is thrown and bypass it with a warning statement in the logs, we skip over other validations, which means that any fuzz test or user using this uri field will be in danger of failing because of other invalid fields @JimmyCYJ. PGV issue tracked: bufbuild/protoc-gen-validate#303 Fixes OSS-Fuzz Issues: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19614 https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19689 Signed-off-by: Asra Ali <asraa@google.com> Mirrored from https://github.com/envoyproxy/envoy @ b7dcf083876f5d34d0dde5df535989a81dfa8023
prakhag1
pushed a commit
to prakhag1/envoy
that referenced
this issue
Jan 3, 2020
Fixes server_fuzz_test fuzz bugs: * QUIC upstream not implemented in prod, so the fuzzer removes the HTTP3 codec types for http health checks * PGV validate throws a not yet implemented error on URIs. I removed this check for now and replaced with a TODO. If I catch the std::exception that is thrown and bypass it with a warning statement in the logs, we skip over other validations, which means that any fuzz test or user using this uri field will be in danger of failing because of other invalid fields @JimmyCYJ. PGV issue tracked: bufbuild/protoc-gen-validate#303 Fixes OSS-Fuzz Issues: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19614 https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19689 Signed-off-by: Asra Ali <asraa@google.com> Signed-off-by: Prakhar <prakhar_au@yahoo.com>
|
@asraa Thanks for letting me know. I just hit the same issue when I start to set up integration test today. |
|
Thanks for posting this issue @asraa, we also ran into the same issue. Would love to see this implemented, even if it's a regex check. Would be very helpful for config validation. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Looks like Envoy is using C++ URI validations in StsService:
https://github.com/envoyproxy/envoy/blob/062c895f499382ae61dead16db2a7e78b9146525/api/envoy/api/v2/core/grpc_service.proto#L94
Config validations throw an unimplemented exception when initializing the server (rather than a controlled a
ProtoValidationerror).https://oss-fuzz.com/testcase-detail/5665272556158976
It might be a good time to implement this constraint. I think less new logic in PGV is better, and I wonder if it's best to use a regex with RE2 to implement the constraint, or a small C++ library (but not ideal, as it's a dependency)
@JimmyCYJ
The text was updated successfully, but these errors were encountered: