Skip to content

fix(security): enforce signature verification for Kafka, MongoDB, and Redis proxy actions - #142

Open
blue4209211 wants to merge 4 commits into
mainfrom
sentinel/enforce-proxy-action-signatures
Open

fix(security): enforce signature verification for Kafka, MongoDB, and Redis proxy actions#142
blue4209211 wants to merge 4 commits into
mainfrom
sentinel/enforce-proxy-action-signatures

Conversation

@blue4209211

Copy link
Copy Markdown
Contributor

Summary

This PR fixes an authorization and signature verification bypass vulnerability in pkg/ws/handler.go where proxy actions across Kafka, MongoDB, and Redis were missing from signedActions, allowing unauthenticated / unsigned messages from the WebSocket relay to be dispatched without cryptographic signature verification.

Key Changes

  1. Registered 21 Missing Proxy Actions in signedActions:
    • Kafka: kafka_consumer_lag, kafka_consumer_groups, kafka_consumer_group_describe, kafka_topics, kafka_topic_describe, kafka_brokers, kafka_topic_offsets
    • MongoDB: mongo_server_status, mongo_repl_status, mongo_collection_stats, mongo_current_ops, mongo_db_stats, mongo_list_databases, mongo_list_collections
    • Redis: redis_info, redis_info_section, redis_slowlog, redis_client_list, redis_memory_stats, redis_cluster_info, redis_keyspace_stats
  2. Fail-Secure Signature Enforcement:
    • Updated HandleMessage in pkg/ws/handler.go to enforce verification whenever signing is enabled (h.verifier.Enabled()), ensuring unknown or unlisted actions cannot fail open.
  3. Comprehensive Unit & Wire Tests:
    • Added unit test suite in pkg/ws/handler_test.go (TestHandler_SignatureEnforcement) verifying rejection (HTTP 403) for unsigned messages and acceptance (HTTP 200) for validly signed messages across Kafka, MongoDB, and Redis.
    • Updated pkg/ws/discovery_wire_test.go (TestActionsThatTouchRemoteSystemsAreSigned) to assert that all actions require signatures.
  4. Security Journal Entry:
    • Recorded vulnerability analysis, root cause, and defense-in-depth prevention in .jules/sentinel.md.

Verification

  • make lint passes with 0 issues.
  • go test -count=1 ./... passes all test suites.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces performance optimizations and security enhancements across the proxy and WebSocket handler. Specifically, it replaces a static map literal with a switch statement in osFamily to achieve zero-allocation lookups, optimizes signature line parsing in ParseAndVerify, and significantly expands the signedActions map to enforce signature verification across 21 previously omitted Kafka, MongoDB, and Redis proxy actions. Additionally, signature verification is now enforced globally when enabled as a defense-in-depth measure. Feedback on the changes focuses on improving test robustness in pkg/ws/handler_test.go by explicitly checking errors returned from secrets.NewCloudPushStore and json.Unmarshal instead of ignoring them.

Comment thread pkg/ws/handler_test.go
Comment thread pkg/ws/handler_test.go
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