🛡️ Sentinel: Fix signature verification bypass for Kafka, Mongo, and Redis proxy actions

[blue4209211]

Description

🛡️ Sentinel: Fix signature verification bypass for Kafka, Mongo, and Redis proxy actions.

Unsigned control plane WebSocket messages for Kafka (kafka_*), Mongo status/stats (mongo_server_status, mongo_list_databases, etc.), and Redis info (redis_info, redis_client_list, etc.) bypassed signature verification even when message signing was enabled (signing_public_key configured).

Added all missing Kafka, MongoDB, and Redis proxy actions to signedActions map in pkg/ws/handler.go.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Enhancement (non-breaking change which improves existing functionality)
• Refactor (non-breaking change which improves code structure)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation
• CI/CD

How Has This Been Tested?

• Unit tests
• Manual testing

Checklist

• CLA signed (the CLA bot will prompt on your first PR)
• make validate passes (fmt + lint + test)
• Docs updated if the wire shape, config surface, or proxy module behavior changed

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request addresses a security vulnerability by registering missing MongoDB, Redis, and Kafka proxy actions in the signedActions map, ensuring they require signature verification. It also documents this vulnerability in the sentinel log and adds a unit test to verify signature enforcement. The feedback suggests improving the unit test to dynamically iterate over the signedActions map instead of using a hardcoded list, which ensures comprehensive coverage and prevents future regressions.

[gemini-code-assist]

Code Review

This pull request registers missing MongoDB, Redis, and Kafka proxy actions in the signedActions map to ensure they undergo signature verification, and adds a corresponding unit test to enforce this behavior. The reviewer suggested improving the new unit test by dynamically iterating over all keys in the signedActions map instead of hardcoding a subset of actions, which will automatically cover any future additions.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request registers several newly added MongoDB, Redis, and Kafka proxy actions in the signedActions map to ensure they undergo signature verification, and adds a unit test to enforce this behavior. The review feedback highlights a critical security risk with this opt-in design, noting that it is fail-open and prone to future omissions. It is highly recommended to refactor the signature verification to be fail-closed (secure by default) for all actions, which would also simplify the code and allow the unit tests to robustly verify that any unregistered or new actions are protected by default.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the signature verification logic to be fail-closed and secure by default, enforcing signature verification for all incoming control plane messages in HandleMessage instead of relying on an opt-in signedActions allowlist. The associated tests have been updated and expanded to verify this behavior across standard, unknown, and future actions. The review feedback points out a redundant if h.verifier.Enabled() check in the verification path that can be safely removed to simplify the security-critical code.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request transitions the signature verification logic in pkg/ws/handler.go to a fail-closed, secure-by-default architecture by enforcing signature verification uniformly across all incoming messages and removing the manual opt-in signedActions map. While this is a significant security improvement, a critical vulnerability remains: certain actions like test_datasource_config and legacy HTTP proxy requests do not have their security-critical fields registered in SigningFields in pkg/signing/sign.go, allowing attackers to tamper with their payloads without invalidating the signature.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements fail-closed signature verification for all incoming relay messages by removing the explicit opt-in signedActions map and enforcing signature verification uniformly. It also updates signing logic to handle legacy HTTP proxy requests and adds several anti-tamper and signature enforcement tests. The reviewer suggests adding an additional unit test to verify that legacy action requests are protected against tampering.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors signature verification to adopt a fail-closed, secure-by-default approach. It removes the explicit signedActions allowlist in pkg/ws/handler.go and enforces signature verification uniformly on all incoming messages. Additionally, it registers the test_datasource_config action and adds fallback signing fields for legacy HTTP proxy requests. Unit tests are updated and expanded to verify these security improvements. Feedback suggests adding a defensive check in HandleMessage to prevent a potential nil pointer dereference if h.verifier is not initialized.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements fail-closed signature verification for all incoming relay messages, removing the previous opt-in signedActions map to ensure secure-by-default behavior. It also adds signing support for temporary datasource configurations and legacy HTTP proxy requests, along with comprehensive anti-tamper and signature enforcement tests. The review feedback highlights a critical security vulnerability in pkg/signing/sign.go where legacy action requests can bypass signature verification because their payload fields are expected at the top level but reside inside the body object; a code suggestion is provided to sign the entire body field for these requests.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request transitions the signature verification logic from an opt-in map to a fail-closed, secure-by-default model where all incoming messages are verified. It also adds support for signing legacy HTTP proxy requests and introduces corresponding unit tests. A critical security vulnerability was identified in the signing logic for legacy action requests, where the lack of top-level fields results in an empty payload being signed, allowing attackers to tamper with the request. A code suggestion has been provided to resolve this by signing the entire nested body field.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements fail-closed signature verification for all incoming relay messages, replacing the previous opt-in signedActions map with a secure-by-default approach. It also updates the signer to handle legacy action and HTTP proxy request formats and adds corresponding unit tests. The review feedback highlights a critical signature bypass vulnerability in the legacy action request detection logic when an empty top-level action field is present, and recommends a robust detection mechanism along with an accompanying test case to prevent payload tampering.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements fail-closed signature verification for all incoming relay messages by removing the explicit opt-in signedActions map and enforcing verification uniformly in HandleMessage. It also updates signing field configurations for legacy HTTP and action formats, and adds comprehensive anti-tamper tests. Feedback points out a critical bug in HandleMessage where legacy HTTP proxy requests with string bodies will fail to unmarshal because envelope.Body is defined as a struct instead of json.RawMessage. A fix is suggested to conditionally unmarshal the body.

[blue4209211]

/gemini review

[gemini-code-assist]

Code Review

This pull request transitions the message signature verification to a secure-by-default, fail-closed model by enforcing verification on all incoming control plane messages and removing the manual opt-in map. It also updates the signing logic to support legacy action and HTTP proxy formats, accompanied by comprehensive anti-tamper tests. Feedback on the changes suggests expanding the legacy HTTP request tests to include JSON object bodies, which reveals an existing parsing limitation in the handler for non-string bodies.