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.

Sign up for GitHub

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

Jump to bottom

Replace SystemSingleton #2817

Closed
3 of 4 tasks
mssalvatore opened this issue Jan 11, 2023 · 0 comments · Fixed by #3068
Closed
3 of 4 tasks

Replace SystemSingleton #2817

mssalvatore opened this issue Jan 11, 2023 · 0 comments · Fixed by #3068
Assignees
@cakekoa
Labels
Complexity: Medium Impact: High Refactor Security sp/5
Milestone
v2.1.0

Comments

@mssalvatore
Copy link
Collaborator

mssalvatore commented Jan 11, 2023 •
edited by cakekoa
Loading

Refactor

Component(s) to be refactored

  • Agent
  • SystemSingleton

Explanation

The Agent tries to avoid having multiple instances of itself running simultaneously on a single machine. To achieve this, it opens an abstract unix socket with a constant name. When another agent attempts to run, it will be unable to open the socket (it's already open) and the agent will exit gracefully.

Unfortunately, this constitutes a low-severity DoS vulnerability. Anyone with access to a target machine can open up an abstract socket with the name that the agent expects. This will prevent any agents from running on the machine.

To resolve this issue, the Island could enforce the rule that only one agent at a time may run on a machine. When the agent starts, it can query the Island to see if any terminate signal has been sent to the agent. The Island can determine if the agent is a duplicate and send it a terminal using the following criteria:

  1. Are there any other agents currently running on the machine?
  2. Do the running agents have an earlier registration time than this agent?

This solution has a very small risk of race conditions. These race conditions are likely to resolve themselves with minimal (if any) impact to the running agents. When the SystemSingleton was introduced, two agents running on the system would interfere with each other. Significant refactoring and redesign has taken place that could allow multiple agents to run simultaneously. Whereas before it was impossible for agents to run simultaneously, now it is simply undesirable.

Tasks

  • Add Agent.registration_time (0d) @cakekoa
  • Modify the agent registration event handler to set Agent.registration_time (0d) @cakekoa
  • Modify the Island's AgentSignalsService to determine whether or not an agent is a duplicate. (0d) @cakekoa
  • Modify the Agent to check for a terminate signal instead of using the SystemSingleton and remove the SystemSingleton (0.25d) @cakekoa
mssalvatore added a commit that referenced this issue Jan 11, 2023
@mssalvatore
Agent: Spawn the manager process before acquiring the SystemSingleton
2e168f6 
The SystemSingleton uses an abstract unix socket as a "lock" to ensure
only one agent at a time runs on a given machine. It seems that if a
manager process is spawned after this unix socket is created, the
manager process inherits this file handle, which leads to the socket
never being properly closed.

Spawning the manager before the socket is opened is a quick solution to
this problem. A better solution (see
#2817) is to use a different
method than a unix socket to achieve this goal, but, baby steps for now.
mssalvatore added a commit that referenced this issue Jan 12, 2023
@mssalvatore
Agent: Spawn the manager process before acquiring the SystemSingleton
dcf1e72 
The SystemSingleton uses an abstract unix socket as a "lock" to ensure
only one agent at a time runs on a given machine. It seems that if a
manager process is spawned after this unix socket is created, the
manager process inherits this file handle, which leads to the socket
never being properly closed.

Spawning the manager before the socket is opened is a quick solution to
this problem. A better solution (see
#2817) is to use a different
method than a unix socket to achieve this goal, but, baby steps for now.
mssalvatore added a commit that referenced this issue Jan 12, 2023
@mssalvatore
Agent: Spawn the manager process before acquiring the SystemSingleton
4494875 
The SystemSingleton uses an abstract unix socket as a "lock" to ensure
only one agent at a time runs on a given machine. It seems that if a
manager process is spawned after this unix socket is created, the
manager process inherits this file handle, which leads to the socket
never being properly closed.

Spawning the manager before the socket is opened is a quick solution to
this problem. A better solution (see
#2817) is to use a different
method than a unix socket to achieve this goal, but, baby steps for now.
mssalvatore added a commit that referenced this issue Jan 12, 2023
@mssalvatore
Agent: Spawn the manager process before acquiring the SystemSingleton
02e5ee6 
The SystemSingleton uses an abstract unix socket as a "lock" to ensure
only one agent at a time runs on a given machine. It seems that if a
manager process is spawned after this unix socket is created, the
manager process inherits this file handle, which leads to the socket
never being properly closed.

Spawning the manager before the socket is opened is a quick solution to
this problem. A better solution (see
#2817) is to use a different
method than a unix socket to achieve this goal, but, baby steps for now.
mssalvatore added a commit that referenced this issue Jan 13, 2023
@mssalvatore
Agent: Spawn the manager process before acquiring the SystemSingleton
ea33d75 
The SystemSingleton uses an abstract unix socket as a "lock" to ensure
only one agent at a time runs on a given machine. It seems that if a
manager process is spawned after this unix socket is created, the
manager process inherits this file handle, which leads to the socket
never being properly closed.

Spawning the manager before the socket is opened is a quick solution to
this problem. A better solution (see
#2817) is to use a different
method than a unix socket to achieve this goal, but, baby steps for now.
cakekoa pushed a commit that referenced this issue Jan 13, 2023
@mssalvatore @cakekoa
Agent: Spawn the manager process before acquiring the SystemSingleton
b0d0d1b 
The SystemSingleton uses an abstract unix socket as a "lock" to ensure
only one agent at a time runs on a given machine. It seems that if a
manager process is spawned after this unix socket is created, the
manager process inherits this file handle, which leads to the socket
never being properly closed.

Spawning the manager before the socket is opened is a quick solution to
this problem. A better solution (see
#2817) is to use a different
method than a unix socket to achieve this goal, but, baby steps for now.
@cakekoa cakekoa self-assigned this Mar 6, 2023
@cakekoa cakekoa mentioned this issue Mar 6, 2023
Merged
8 tasks
mssalvatore added a commit that referenced this issue Mar 6, 2023
@mssalvatore
Merge branch '2817-add-agent-registration-time-field' into develop
72affe2 
Issue #2817
PR #3057
@cakekoa cakekoa mentioned this issue Mar 6, 2023
Merged
8 tasks
mssalvatore added a commit that referenced this issue Mar 7, 2023
@mssalvatore
Merge branch '2817-set-terminate-signal-for-duplicate-agents' into de…
fc8d9ba 
…velop

Issue #2817
PR #3058
mssalvatore added a commit that referenced this issue Mar 7, 2023
@mssalvatore
Island: Fix logic error in AgentSignalsService
3a96504 
Issue #2817
PR #3065
mssalvatore added a commit that referenced this issue Mar 7, 2023
@mssalvatore
Island: Fix logic error in AgentSignalsService
453eeb7 
Issue #2817
PR #3065
mssalvatore added a commit that referenced this issue Mar 7, 2023
@mssalvatore
Island: Fix logic error in AgentSignalsService
5425279 
Issue #2817
PR #3065
@cakekoa cakekoa mentioned this issue Mar 7, 2023
Merged
8 tasks
mssalvatore added a commit that referenced this issue Mar 7, 2023
@mssalvatore
Changelog: Add an entry for #2817
7a856d6
cakekoa pushed a commit that referenced this issue Mar 8, 2023
@mssalvatore @cakekoa
Changelog: Add an entry for #2817
fe6b06a
mssalvatore added a commit that referenced this issue Mar 8, 2023
@mssalvatore
Changelog: Add an entry for #2817
94342fd
mssalvatore added a commit that referenced this issue Mar 8, 2023
@mssalvatore
Changelog: Add an entry for #2817
98f836f
mssalvatore added a commit that referenced this issue Mar 8, 2023
@mssalvatore
Merge branch '2817-replace-agent-singleton' into develop
ee39637 
Issue #2817
PR #3068
@mssalvatore mssalvatore added this to the v2.1.0 milestone Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cakekoa cakekoa

Labels
Complexity: Medium Impact: High Refactor Security sp/5
Projects
None yet
Milestone
v2.1.0
Development

Successfully merging a pull request may close this issue.

2817 replace agent singleton guardicore/monkey
2 participants
@cakekoa @mssalvatore