Restrict DAPI callable resolution to exposed resources only#34889
Restrict DAPI callable resolution to exposed resources only#34889
Conversation
vikman90
requested review from
Copilot,
jnasselle and
nico-stefani
and removed request for
jnasselle
There was a problem hiding this comment.
Pull request overview
This PR hardens Cluster DAPI callable deserialization by restricting remote callable resolution to only those functions intended to be remotely executable (i.e., explicitly exposed via @expose_resources), reducing the internal callable surface area.
Changes:
- Add exposure validation to
as_wazuh_object()so only explicitly exposed callables are resolvable. - Reject non-exposed callables during DAPI deserialization with a
WazuhInternalError. - Update
test_as_wazuh_object_okto cover newly blocked callables and one allowed exposed callable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
framework/wazuh/core/cluster/common.py |
Adds callable exposure checks during JSON deserialization of callables. |
framework/wazuh/core/cluster/tests/test_common.py |
Updates unit test expectations for blocked vs allowed callables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func = getattr(wazuh, funcname) | ||
| # Verify that the method has the @expose_resources decorator | ||
| if not hasattr(func, '__wrapped__'): | ||
| raise exception.WazuhInternalError(1000, | ||
| extra_message=f"Method '{funcname}' is not exposed with @expose_resources decorator", | ||
| cmd_error=True) |
There was a problem hiding this comment.
The exposure check uses hasattr(func, '__wrapped__') as a proxy for @expose_resources, but __wrapped__ is set by any decorator that uses functools.wraps (e.g. @temporary_cache on wazuh.core.cluster.utils.get_manager_status). This means non-exposed internal functions can still be resolved and executed remotely, defeating the hardening goal. Consider adding an explicit marker in expose_resources (e.g. __wazuh_exposed__ = True or similar) and checking that marker here instead of __wrapped__ (and for methods, check the underlying function via getattr(func, '__func__', func)).
There was a problem hiding this comment.
This. Any wrapped function will be allowed, and there's no way to get the decorator's name. Also, in terms of injecting a DAPI call, we can modify the serialized data to avoid this check
| # Verify that the function has the @expose_resources decorator | ||
| # The decorator uses functools.wraps which sets the __wrapped__ attribute | ||
| if not hasattr(func, '__wrapped__'): | ||
| raise exception.WazuhInternalError(1000, | ||
| extra_message=f"Function '{funcname}' from module '{module_path}' is not exposed with @expose_resources decorator", | ||
| cmd_error=True) |
There was a problem hiding this comment.
Same concern in the function/static-method path: using __wrapped__ does not prove the callable was exposed via @expose_resources (any @wraps-based decorator will pass), so non-exposed callables from allowed packages can still be deserialized. Switch to an explicit expose_resources marker attribute (and optionally verify callable(func) as well) to ensure only intentionally exposed endpoints are resolvable.
| # Test the first condition - non-exposed callables must be blocked (no @expose_resources) | ||
| with pytest.raises(exception.WazuhInternalError) as err: | ||
| cluster_common.as_wazuh_object({"__callable__": {"__name__": "check_user_master", | ||
| "__module__": "api.authentication", | ||
| "__qualname__": "check_user_master", | ||
| "__type__": "function"}}) | ||
| assert "is not exposed with @expose_resources decorator" in str(err.value) |
There was a problem hiding this comment.
The updated test covers an undecorated function being rejected, but it doesn’t cover the bypass where a non-@expose_resources function still has __wrapped__ due to another @wraps-based decorator (e.g. wazuh.core.cluster.utils.get_manager_status is decorated with @temporary_cache). Adding a regression case for that scenario would help ensure the deserializer only allows @expose_resources callables.
jnasselle
left a comment
jnasselle
left a comment
There was a problem hiding this comment.
GJ @vikman90 , but unfortunately, any wrapped function will be allowed, and also the serialized call can be modified to bypass it by simply modifying the __wrapped__ attribute.
On the other hand, there are (at least one found) methods that are not decorated with expose_resource but still need to be distributed i.e api.authentication.get_security_conf is distributed as part of generate_token during login_user and run_as_login API controllers' functions
IMHO, an exhaustive list of allowed methods is the painful way to solve this. The challenge is to provide a maintainable way to achieve this, but we can start with a hand-made list and keep this tech debt for the moment
nico-stefani
force-pushed
the
fix/4169-cluster-dapi-filter-callables
branch
from
84e624e to
62697af
Compare
Add validation to ensure only functions with @expose_resources decorator can be deserialized and executed from network requests. Functions without the decorator are rejected with WazuhInternalError.
Update test_as_wazuh_object_ok to reflect new behavior where functions without @expose_resources decorator are properly rejected. Add test case for get_users which has the required decorator.
vikman90
force-pushed
the
fix/4169-cluster-dapi-filter-callables
branch
from
62697af to
93023ba
Compare
Description
This change hardens DAPI callable resolution by allowing remote execution only for functions explicitly exposed through
@expose_resources.The update reduces the internal callable surface available through DAPI and ensures that low-level helper functions that are not intended for remote execution are rejected during deserialization.
Credit to @alimezar and @maru for reporting and helping identify this issue.
Proposed Changes
as_wazuh_object()and accept only functions explicitly exposed with@expose_resourcestest_as_wazuh_object_okto cover both exposed and non-exposed functionsResults and Evidence
Unit test passing:
Artifacts Affected
wazuh-clusterd(Manager)Configuration Changes
Documentation Updates
Tests Introduced
framework/wazuh/core/cluster/tests/test_common.py::test_as_wazuh_object_okReview Checklist