config: strengthen validation for gRPC config sources. #4171
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This addresses oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9335, where a bad config could cause the protobuf library to throw a non-EnvoyException CHECK exception, causing Envoy to abort. As a bonus, made sure we include the ApiConfigSource debug string in respective EnvoyExceptions, this makes pinpointing the specific part of the config easier in large configs. Risk level: Low Testing: Corpus entry and unit test added. Signed-off-by: Harvey Tuch <htuch@google.com>
htuch
added a commit
to htuch/envoy
that referenced
this pull request
Aug 15, 2018
This came up while addressing oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9335 in envoyproxy#4171. Without this PR, the server would shutdown non-gracefully, with TLS posts still possible to deleted workerer thread dispatchers, resulting in heap-user-after-free. Protobuf was throwing a CHECK exception, which was not picked up as EnvoyException. Risk level: Low Testing: Unit tests added, corpus entry is in envoyproxy#4171. Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
ambuc
approved these changes
Aug 16, 2018
|
@ambuc no, these unit tests are very fast anyway. Thanks for the review! |
htuch
added a commit
that referenced
this pull request
Aug 16, 2018
…4173) This came up while addressing oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9335 in #4171. Without this PR, the server would shutdown non-gracefully, with TLS posts still possible to deleted worker thread dispatchers, resulting in heap-user-after-free. Protobuf was throwing a CHECK exception, which was not picked up as EnvoyException. Risk level: Low Testing: Unit tests added, corpus entry is in #4171. Signed-off-by: Harvey Tuch <htuch@google.com>
rshriram
pushed a commit
to rshriram/envoy
that referenced
this pull request
Oct 30, 2018
…#1938) This is far from finished, but it reduces memory usage by ~10%. Pulling the following changes from github.com/envoyproxy/envoy: c1cc68d stats: refactoring MetricImpl without strings (envoyproxy#4190) 36809d8 fuzz: coverage profile generation instructions. (envoyproxy#4193) ba40cc9 upstream: rebuild cluster when health check config is changed (envoyproxy#4075) 05c0d52 build: use clang-6.0. (envoyproxy#4168) 01f403e thrift_proxy: introduce header transport (envoyproxy#4082) 564d256 tcp: allow connection pool callers to store protocol state (envoyproxy#4131) 3e1d643 configs: match latest API changes (envoyproxy#4185) bc6a10c Fix wrong mock function name. (envoyproxy#4187) e994c1c Bump yaml-cpp so it builds with Visual Studio 15.8 (envoyproxy#4182) 3d1325e Converting envoy configs to V2 (envoyproxy#2957) 8d0680f Add timestamp to HealthCheckEvent definition (envoyproxy#4119) 497efb9 server: handle non-EnvoyExceptions safely if thrown in constructor. (envoyproxy#4173) 6d6fafd config: strengthen validation for gRPC config sources. (envoyproxy#4171) 132302c fuzz: reduce log level when running under fuzz engine. (envoyproxy#4161) 7c04ac2 test: fix V6EmptyOptions in coverage with IPv6 enable (envoyproxy#4155) 1b2219b ci: remove deprecated bazel --batch option (envoyproxy#4166) 2db6a4c ci: update clang to version 6.0 in the Ubuntu build image. (envoyproxy#4157) 71152b7 ratelimit: Add ratelimit custom response headers (envoyproxy#4015) 3062874 ssl: make Ssl::Connection const everywhere (envoyproxy#4179) 706e262 Handle validation of non expiring tokens in jwt_authn filter (envoyproxy#4007) f06e958 fuzz: tag trivial fuzzers with no_fuzz. (envoyproxy#4156) 27fb1d3 thrift_proxy: add service name matching to router implementation (envoyproxy#4130) 8c189a5 Make over provisioning factor configurable (envoyproxy#4003) 6c08fb4 Making test less flaky (envoyproxy#4149) 592775b fuzz: bare bones HCM fuzzer. (envoyproxy#4118) For istio/istio#7912. Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses oss-fuzz issue
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9335, where a bad
config could cause the protobuf library to throw a non-EnvoyException
CHECK exception, causing Envoy to abort.
As a bonus, made sure we include the ApiConfigSource debug string in
respective EnvoyExceptions, this makes pinpointing the specific part of
the config easier in large configs.
Risk level: Low
Testing: Corpus entry and unit test added.
Signed-off-by: Harvey Tuch htuch@google.com