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.

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

gRPC: Implement max_receive_message_length in envoy gRPC #32711

Merged
merged 21 commits into from
Apr 4, 2024

Conversation

tyxia
Copy link
Member

@tyxia tyxia commented Mar 5, 2024

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

Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
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.

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.

Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
…ecv_length

Signed-off-by: tyxia <tyxia@google.com>
@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
Signed-off-by: tyxia <tyxia@google.com>
…ecv_length

Signed-off-by: tyxia <tyxia@google.com>
@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
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
Signed-off-by: tyxia <tyxia@google.com>
@yanavlasov
Copy link
Contributor

/wait-any

Signed-off-by: tyxia <tyxia@google.com>
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

Signed-off-by: tyxia <tyxia@google.com>
@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
…ecv_length

Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: tyxia <tyxia@google.com>
@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
alyssawilk pushed a commit to alyssawilk/envoy that referenced this pull request Apr 29, 2024
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.

5 participants