-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gracefully handle removal of critical headers by filters #13756
Comments
This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions. |
This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions. |
I've started looking into this, and it looks like only the response path is not guarded from misbehaving filters (the request path is guarded by After this is fixed, let's revisit to #15487 and think about what we can do at Wasm level. cc @PiotrSikora cc @asraa |
This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions. |
This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions. |
I reworked the PR #16745 to resolve (this issue has not been resolved yet and still causes crashes). |
… chain. (#16745) This is the rework of the previous PR (#15658). Additional Description: Previously, Envoy hasn't been guarded from the removal of critical response headers like :status. This happens when there's misbehaving filter chan, and especially this protection is important when users run their own extensions through Wasm. Resolves #13756 and ref: #15487. Testing: new integration tests Risk: low Signed-off-by: Takeshi Yoneda <[email protected]>
… chain. (envoyproxy#16745) This is the rework of the previous PR (envoyproxy#15658). Additional Description: Previously, Envoy hasn't been guarded from the removal of critical response headers like :status. This happens when there's misbehaving filter chan, and especially this protection is important when users run their own extensions through Wasm. Resolves envoyproxy#13756 and ref: envoyproxy#15487. Testing: new integration tests Risk: low Signed-off-by: Takeshi Yoneda <[email protected]>
It happened on several occasions that buggy filters modify or remove headers critical to Envoy's internals (or other filters in the chain). Envoy should check if required request or response headers are still present and valid after invoking filter's
decodeHeaders()
orencodeHeaders()
method. If a filter has modified or deleted critical header the request should be aborted with 500 response.Critical request header are:
Critical response headers are:
The text was updated successfully, but these errors were encountered: