dynamic_modules: add remote HTTP support for asyncDataSource module binary

[kanurag94]

Commit Message: dynamic_modules: add remote HTTP support for asyncDataSource module binary
Additional Description: Extends the asyncDataSource support (added in #43333 for local.filename) to support fetching dynamic module binaries from remote HTTP sources via module.remote in DynamicModuleConfig. The module is downloaded asynchronously during listener initialization with SHA256 verification, written to a temporary file, and loaded via dlopen. If the remote fetch fails, the filter is not installed and requests pass through (fail-open).
Risk Level: Low
Testing: Added unit tests + Manually tested
Docs Changes: API docs already a part of this PR
Release Notes: Added

[repokitteh-read-only]

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: #43818 was opened by kanurag94.

see: more, trace.

[kanurag94]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces the ability to fetch dynamic modules from remote HTTP sources asynchronously. However, critical security vulnerabilities were identified in the caching and temporary file handling mechanisms. These include the use of the world-writable /tmp directory with predictable filenames and a lack of integrity verification for cached files, which could lead to local privilege escalation and arbitrary code execution. The staging file creation is also vulnerable to symlink attacks and race conditions. Furthermore, a critical use-after-free bug was found in the asynchronous callback implementation.

[gemini-code-assist]

The lambda for the RemoteAsyncDataProvider captures context and scope by reference. These references point to objects that are typically allocated on the stack during the configuration loading process. Since the lambda is executed asynchronously after the remote data fetch completes, the original stack frame will be gone, leading to dangling references and a use-after-free. This is a critical issue that could lead to crashes.

To resolve this, you should avoid capturing stack variables by reference in an asynchronous callback. Instead, you should extract the required long-lived objects (e.g., from context.api(), context.mainThreadDispatcher(), etc.) and capture those. This will likely require refactoring buildFilterFactoryCallback to accept these individual components instead of the entire ServerFactoryContext.

[kanurag94]

• ServerFactoryContext is long-lived and not stack allocated.
• stats::scope is owned by listener and is again long lived.
• RemoteAsyncDataProvider is fired during init so both references are guaranteed valid.

This comment seems wrong.

[kanurag94]

/retest

[kanurag94]

/retest

[repokitteh-read-only]

CC @envoyproxy/coverage-shephards: FYI only for changes made to (test/coverage.yaml).
envoyproxy/coverage-shephards assignee is @RyanTheOptimist
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #43818 was ready_for_review by kanurag94.

see: more, trace.

[agrawroh]

Do we really have to decrease the coverage here? is there no way to have an integration test to cover everything?

[kanurag94]

The coverage drop comes mainly from defensive paths:

• mkstemp() failure -- don't know how clearly but probably a exhaustion of FDs should only trigger this.
• write() failure -- needs interrupt signals or disk permission issues
• std::filesystem::permissions/rename failure -- needs mocking read only permissions.
• weak_ptr expiry -- needs a race condition to trigger.

They'd need exceptional situations to happen. All of them are very tought for UTs so I lowered the coverage instead.

Writing an integration test is quite hard for this one, if the approach looks good -- I can give it a try.

I have tried to increase coverage back with some possible paths that can be tested.

[wbpcode]

Because the following lambda will be called at any threads, I think there may result in unexpected contention? here?

[async_state](Http::FilterChainFactoryCallbacks& callbacks) -> void {
    if (async_state->filter_factory_cb) {
      async_state->filter_factory_cb(callbacks);
    }
  };

As the first version, to simplify the review, we I think we and use a lock and left a TODO here. Then, we can resolve the lock with another PR. cc @agrawroh WDYT?

[wbpcode]

wait, the remote fetching will pause the warming...so, it may be safe