Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Consensus] Common bus may be a potential attack surface and performance bottleneck #415

Open
7 tasks
gokutheengineer opened this issue Dec 21, 2022 · 0 comments
Assignees
Labels
consensus Consensus specific changes discussion It has a related Discussion

Comments

@gokutheengineer
Copy link
Contributor

Objective

Currently, the node bus is used for all types of events, which currently mainly consists of external events (node to node interactions). With the implementation of issue #395, the pacemaker is becoming a submodule of the consensus module, where the node bus will be used for interaction of pacemaker and consensus modules, so essentially it will be used for internal events. This implies that, we are putting external events (e.g. node to node HotStuffMessages) internal node events (e.g. pacemaker to consensus module function execution requests) together in same bus. This can turn out bad for couple of reasons:

  1. Bus might prioritize external events over internal events, when there are excessive amount of external requests, which may cause the node to slow down, and thus degrade node performance.
  2. Similarly, node might get excessive amount of external requests, which may exhaust node and bus, which in turn may cause internal events to be not processed and proceed properly. Since consensus module orchestrates whole node execution, this will seriously effect node's functionality.
  3. An attacker might expose this potential vulnerability to target a set of nodes and exhaust node bus, which can bring network wide affects on protocol and consensus (e.g. attacking the liveliness of the protocol), if attack is applied in a broad range.

Origin Document

The change on how event bus will be used for internal events with #395:

func (node *Node) handleEvent(message *messaging.PocketEnvelope) error {
	contentType := message.GetContentType()
	switch contentType {
        ...
	case consensus.PacemakerAccessContentType:
		return node.GetBus().GetConsensusModule().HandlePacemakerMessage(message.Content)
        ...
	}
	return nil
}

Goals

  • Separate internal events and external handling.
  • Crate node bus that is more resilient.

Deliverable

  • Crate a new bus for node internal use, maybe only specific for pacemaker and consensus module interaction.

Non-goals / Non-deliverables

  • Business logic changes in modules
  • How external events are handled by the bus

General issue deliverables

  • Update the appropriate CHANGELOG(s)
  • Update any relevant local/global README(s)
  • Update relevant source code tree explanations
  • Add or update any relevant or supporting mermaid diagrams

Testing Methodology

  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md

Creator: @gokutheengineer
Co-Owners: @Olshansk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus Consensus specific changes discussion It has a related Discussion
Projects
Status: Backlog
Development

No branches or pull requests

2 participants