Skip to content

Conversation

@yanjunxiang-google
Copy link
Contributor

Local replies generated from upstream filter chain should be able to sent to the ext_proc server.

@repokitteh-read-only
Copy link

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #41769 was opened by yanjunxiang-google.

see: more, trace.

@yanjunxiang-google
Copy link
Contributor Author

/assign @yanavlasov @tyxia

@yanjunxiang-google
Copy link
Contributor Author

/retest

@yanjunxiang-google
Copy link
Contributor Author

@botengyao

Copy link
Member

@tyxia tyxia left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@yanjunxiang-google could you please add some context in pr description, so that people can track it easily

I think we can re-enable it is because CVE #38818 has been fixed. I don't remember which PR # fix ext_proc+web_socket

@yanjunxiang-google
Copy link
Contributor Author

LGTM, Thanks!

@yanjunxiang-google could you please add some context in pr description, so that people can track it easily

I think we can re-enable it is because CVE #38818 has been fixed. I don't remember which PR # fix ext_proc+web_socket

Yeah, for CVE #38818, we made two changes, one in ext_proc to disable local reply sending to the ext_proc server. The other is in router.h which is to cleanup some dangling pointer. Looking back, the change in ext_proc avoided the crash in some specific case, but not fixing the root cause. The change in router.h fixed the root cause. As there are some ext_proc users are asking for having local replies be sent to the ext_proc server, let's revert the ext_proc changes there.

@tyxia
Copy link
Member

tyxia commented Oct 31, 2025

LGTM, Thanks!
@yanjunxiang-google could you please add some context in pr description, so that people can track it easily
I think we can re-enable it is because CVE #38818 has been fixed. I don't remember which PR # fix ext_proc+web_socket

Yeah, for CVE #38818, we made two changes, one in ext_proc to disable local reply sending to the ext_proc server. The other is in router.h which is to cleanup some dangling pointer. Looking back, the change in ext_proc avoided the crash in some specific case, but not fixing the root cause. The change in router.h fixed the root cause. As there are some ext_proc users are asking for having local replies be sent to the ext_proc server, let's revert the ext_proc changes there.

Yea, i meant could you reference the PR that fix the issue in router.h? Thanks

@tyxia tyxia merged commit 37efb74 into envoyproxy:main Oct 31, 2025
25 checks passed
@yanjunxiang-google
Copy link
Contributor Author

yanjunxiang-google commented Oct 31, 2025

Yea, i meant could you reference the PR that fix the issue in router.h? Thanks

The fix of CVE #38818, including the change in router.h, is committed into another branch first. It was then ported into envoy main branch by #38818.

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.

3 participants