Skip to content

Fix unauthenticated xparl control deserialization - #1176

Merged
ZeyuChen merged 2 commits into
developfrom
fix/xparl-control-security-20260930
Sep 30, 2026
Merged

ZeyuChen merged 2 commits into
developfrom
fix/xparl-control-security-20260930

Conversation

@ZeyuChen

@ZeyuChen ZeyuChen commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

xparl's master accepted network-supplied cloudpickle metadata without authentication. This change removes executable deserialization from control/status messages and requires CURVE authentication and encryption on every xparl ZeroMQ connection, including worker/job execution sockets. The server verifies the client key against the cluster secret; no unauthenticated fallback is provided.

Control metadata now uses a data-only JSON codec with explicit record types, field/type checks, size/depth limits, and malformed-message rejection that preserves the master's REP request/reply cycle. HTTP monitor/log endpoints require credentials, and the master/HTTP listeners default to loopback. The security guide documents private-network operation and coordinated upgrades.

Validation:

  • 14 focused regression tests pass locally: plaintext and incorrect credentials, unauthorized CURVE keys, malicious pickle payloads in all four master branches, malformed messages, valid status/monitor JSON, HTTP credentials, and configuration defaults.
  • An actual authenticated master/worker/job/client smoke test executes a remote actor and verifies add(2, 3) == 5.
  • Changed Python files compile, use the repository's YAPF 0.24.0 style, and pass git diff --check.
  • GitHub Actions passed the 14 security regression tests on both Python 3.9 and 3.10 with the existing pyzmq 22.3.0 pin. The same workflow also passed after the merge on develop: https://github.com/PaddlePaddle/PARL/actions/runs/36689234322.

Migration: configure a fresh shared XPARL_AUTH_TOKEN of at least 32 bytes on every node/client and restart them together. The new JSON/CURVE protocol is incompatible with older processes. Published historical packages require an explicit upgrade; merging source does not update existing installations. Trusted cluster members retain the intended ability to execute Python code, and heartbeat/HTTP access still requires network isolation (SSH tunnel or HTTPS for remote HTTP).

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ZeyuChen
ZeyuChen merged commit 252deeb into develop Sep 30, 2026
2 of 3 checks passed
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.

2 participants