Wired usage of continueDecoding in BandwidthLimiter

[spacewander]

If you are reporting any crash or any potential security issue, do not open an issue in this repo. Please report the issue via emailing envoy-security@googlegroups.com where the issue will be triaged appropriately.

Title: One line description

Wired usage of continueDecoding in BandwidthLimiter

Description:

Describe the issue.

The bandwidthLimiter will call continueDecoding when it wants to continue processing the trailers. See https://github.com/envoyproxy/envoy/blob/bb28dda78d462851c1810047a4b1488a5a69279a/source/extensions/filters/http/bandwidth_limit/bandwidth_limit.cc#L85-L98

https://github.com/envoyproxy/envoy/blob/bb28dda78d462851c1810047a4b1488a5a69279a/source/extensions/filters/http/common/stream_rate_limiter.h#L43

And the continueDecoding requires the decodeHeaders() to return StopIteration: https://github.com/envoyproxy/envoy/blob/bb28dda78d462851c1810047a4b1488a5a69279a/envoy/http/filter.h#L535-L545

However, the decodeHeaders() of bandwidthLimiter only returns Http::FilterHeadersStatus::Continue: https://github.com/envoyproxy/envoy/blob/bb28dda78d462851c1810047a4b1488a5a69279a/source/extensions/filters/http/bandwidth_limit/bandwidth_limit.cc#L111

Is the usage of continueDecoding safe in BandwidthLimiter?

[tyxia]

Add @nezdolik is driving the effort of fixing misuse of continue{De/En}coding. We are still discussing the API check that is under review.

But, I don't think bandwidth filter is tagged as misused filter even in current API check

[nezdolik]

The api check currently only verifies the contineEncoding/Decoding()->commonContinue() path.

[nezdolik]

It is not 100% safe but one needs to come up an example of filterchain config that results into filterchain/data processing weirdness due to api misuse (like double processing of data or filters being skipped etc). One similar example here: https://github.com/envoyproxy/envoy/issues/22064