Skip to content
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.

Sign up for GitHub

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

Jump to bottom

gRPC: Implement max_receive_message_length in envoy gRPC #32711

Merged
merged 21 commits into from
Apr 4, 2024
Merged

gRPC: Implement max_receive_message_length in envoy gRPC #32711

merged 21 commits into from
Apr 4, 2024

Conversation

tyxia
Copy link
Member

@tyxia tyxia commented Mar 5, 2024 •
edited
Loading

Over-frame-limit error can now be surfaced from gRPC codec level after PR #32511

Envoy gRPC client can leverage this to implement max_recevie_meassage_length with config field. This field is defaulted to 0, which means no limits.

  • When the message is over limit, it can be rejected before frame data is fully decoded (i.e. expanded).
  • This can prevent malicious attack , for example, unbounded and huge message is sent over channel and is injected and buffered in Envoy over Envoy-gRPC.

error status will be improved by #32676

tyxia added 9 commits February 26, 2024 22:11
@tyxia
api: async mode in ext_proc
ab556e4 
Signed-off-by: tyxia <[email protected]>
@tyxia
update
62b0a35 
Signed-off-by: tyxia <[email protected]>
@tyxia
update
0ccd355 
Signed-off-by: tyxia <[email protected]>
@tyxia
update
95b3897 
Signed-off-by: tyxia <[email protected]>
@tyxia
format
9ba98a0 
Signed-off-by: tyxia <[email protected]>
@tyxia
format
15ee4bd 
Signed-off-by: tyxia <[email protected]>
@tyxia
format
e7466d4 
Signed-off-by: tyxia <[email protected]>
@tyxia
fix typo
fb58d0e 
Signed-off-by: tyxia <[email protected]>
@tyxia
gprc: Implement max_receive_message_length in envoy gRPC
91f4e82 
Signed-off-by: tyxia <[email protected]>
@repokitteh-read-only RepoKitteh - Read Only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #32711 was opened by tyxia.

see: more, trace.

@repokitteh-read-only RepoKitteh - Read Only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @lizan
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #32711 was opened by tyxia.

see: more, trace.

@tyxia
Copy link
Member Author

tyxia commented Mar 6, 2024

/retest

@tyxia tyxia marked this pull request as ready for review March 6, 2024 18:46
@tyxia
Copy link
Member Author

tyxia commented Mar 6, 2024

/assign @yanavlasov @htuch

@tyxia tyxia changed the title gprc: Implement max_receive_message_length in envoy gRPC gRPC: Implement max_receive_message_length in envoy gRPC Mar 6, 2024
@tyxia tyxia marked this pull request as draft March 8, 2024 02:57
@tyxia tyxia marked this pull request as ready for review March 8, 2024 19:32
@tyxia
Copy link
Member Author

tyxia commented Mar 8, 2024

/retest

htuch
htuch previously approved these changes Mar 11, 2024
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM

source/common/grpc/codec.cc Outdated Show resolved Hide resolved
@tyxia
format
5b1bfb6 
Signed-off-by: tyxia <[email protected]>
@yanavlasov
Copy link
Contributor

/wait-any

@tyxia
upate
8d468b1 
Signed-off-by: tyxia <[email protected]>
lizan
lizan reviewed Mar 14, 2024
View reviewed changes
Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

Can you add a release note?

@yanavlasov
Copy link
Contributor

Will wait for Lizan's comment to be addressed.

/wait

@tyxia
add rel note
b40a4c2 
Signed-off-by: tyxia <[email protected]>
@tyxia
Copy link
Member Author

tyxia commented Mar 27, 2024

Sorry for delay. Release note has been added.

PTAL @htuch @yanavlasov @lizan

@tyxia tyxia requested review from yanavlasov and lizan March 28, 2024 23:38
@tyxia
fix
ebfc99c 
Signed-off-by: tyxia <[email protected]>
@tyxia tyxia mentioned this pull request Mar 29, 2024
Closed
@soulxu
Copy link
Member

soulxu commented Apr 2, 2024

gentle ping @htuch @yanavlasov @lizan

@repokitteh-read-only repokitteh-read-only bot removed the api label Apr 3, 2024
@yanavlasov yanavlasov merged commit c3f7225 into envoyproxy:main Apr 4, 2024
54 checks passed
@tyxia tyxia deleted the max_recv_length branch April 4, 2024 19:21
@botengyao botengyao mentioned this pull request Apr 5, 2024
Merged
alyssawilk pushed a commit to alyssawilk/envoy that referenced this pull request Apr 29, 2024
@tyxia @alyssawilk
gRPC: Implement max_receive_message_length in envoy gRPC (envoyproxy#…
ffe9a17 
…32711)


---------

Signed-off-by: tyxia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@htuch htuch htuch approved these changes

@yanavlasov yanavlasov yanavlasov approved these changes

@lizan lizan lizan approved these changes

Assignees

@lizan lizan

@yanavlasov yanavlasov

@htuch htuch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tyxia @yanavlasov @soulxu @lizan @htuch