Skip to content

test: Adding zerolen headers upstream flood tests#14035

Merged
yanavlasov merged 4 commits intoenvoyproxy:masterfrom
adisuissa:upstream_flood_mitigation_part1
Nov 19, 2020
Merged

test: Adding zerolen headers upstream flood tests#14035
yanavlasov merged 4 commits intoenvoyproxy:masterfrom
adisuissa:upstream_flood_mitigation_part1

Conversation

@adisuissa
Copy link
Copy Markdown
Contributor

Commit Message: Adding zerolen headers upstream flood tests
Additional Description:
Adding tests that send zero-length headers frame from upstream, with and without the override_stream_error_on_invalid_http_message runtime override feature.

Risk Level: Low (tests)
Testing: Tests added.
Docs Changes: None.
Release Notes: None.
Platform Specific Features: None.
Fixes part of #12281

Signed-off-by: Adi Suissa-Peleg adip@google.com

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@adisuissa
Copy link
Copy Markdown
Contributor Author

/assign @yanavlasov
/cc @asraa


// Send an upstream reply.
auto* upstream = fake_upstreams_.front().get();
auto buf = Http2Frame::makeMalformedRequestWithZerolenHeader(Http2Frame::makeClientStreamId(0),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see. There is no counter specific to 0 length headers and both presence of invalid headers in response (i.e. path) and 0 length header increment the same rx_messaging_error counter.
The test is almost correct. It fails for a different reason though. The makeMalformedRequestWithZerolenHeader method will create invalid response header because it does not have status and has request headers like method, etc that are invalid in responses. So the nghttp2 will fail before it gets to the 0 len header. To make this test work you can add new Http2Frame::makeMalformedResponsetWithZerolenHeader method, that adds status header and 0 len header. See the Http2Frame::makeMalformedRequest on how to add status header.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are correct.
I've made the changes you've requested and now see the codec error due to the 0-length header:
invalid http2: Invalid HTTP header field was received: frame type: 1, stream: 1, name: [], value: []
In the log.
Not sure how to verify this in the test though, as you've mentioned there's only a generic counter.

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@yanavlasov
Copy link
Copy Markdown
Contributor

/wait

…tigation_part1

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@adisuissa
Copy link
Copy Markdown
Contributor Author

@yanavlasov Addressed your comment, and manually verified that the zero-length header is the reason for the codec failure. PTAL

@yanavlasov yanavlasov merged commit 37326ee into envoyproxy:master Nov 19, 2020
andreyprezotto pushed a commit to andreyprezotto/envoy that referenced this pull request Nov 24, 2020
Signed-off-by: Adi Suissa-Peleg <adip@google.com>
qqustc pushed a commit to qqustc/envoy that referenced this pull request Nov 24, 2020
Signed-off-by: Adi Suissa-Peleg <adip@google.com>
Signed-off-by: Qin Qin <qqin@google.com>
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.

2 participants