Skip to content

[fix](security): stop sending AES secret over unauthenticated Netty - #4263

Draft
yuluo-yx wants to merge 1 commit into
masterfrom
0728-yuluo/feat-1
Draft

[fix](security): stop sending AES secret over unauthenticated Netty#4263
yuluo-yx wants to merge 1 commit into
masterfrom
0728-yuluo/feat-1

Conversation

@yuluo-yx

Copy link
Copy Markdown
Member

What's changed?

fix(security): stop sending AES secret over unauthenticated Netty

Tips: this is a break change

Checklist

  • I have read the Contributing Guide
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Add or update API

  • I have added the necessary e2e tests and all cases have passed.

Signed-off-by: yuluo-yx <yuluo08290126@gmail.com>
@yuluo-yx
yuluo-yx marked this pull request as draft July 28, 2026 14:59
@yuluo-yx

yuluo-yx commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

/cc @Duansg @Aias00 @tomsun28 @zqr10159

@zqr10159

Copy link
Copy Markdown
Member

The implementation direction addresses the secret-distribution path, but this is a breaking manager/collector contract change and the PR is still draft. Before release approval, please document mixed-version behavior and the upgrade order, prove the required common.secret failure mode is actionable, and run the manager/collector compatibility and packaging checks. Once the PR is ready and those upgrade guarantees are recorded, I can re-review it.

@zqr10159

Copy link
Copy Markdown
Member

I rechecked the current head (14382ff). This change protects the key-distribution path, but it does not authenticate the Netty peer or any ClusterMsg.Message: unsigned GO_ONLINE, heartbeat, and collection-result messages still reach their processors. In particular, GO_ONLINE can still replace the identity-to-channel mapping, and cyclic results still enter the data queue without a message-integrity check.

I opened the dependent follow-up #4294. It signs the complete message envelope, enforces timestamp freshness and replay rejection, and closes invalid connections before processor dispatch. For this transport boundary, I would only treat #4263 plus #4294 (or an equivalent combined implementation) as complete. The rollout remains intentionally breaking: manager and standalone Collector must be upgraded together, share common.secret, and keep clocks synchronized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants