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

Modify exploiters to use OTP in commands #3118

Merged
merged 28 commits into from
Mar 28, 2023

Conversation

shreyamalviya
Copy link
Contributor

@shreyamalviya shreyamalviya commented Mar 16, 2023

What does this PR do?

Fixes #3077

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?
  • Was the CHANGELOG.md updated to reflect the changes?
  • Was the documentation framework updated to reflect the changes?
  • Have you checked that you haven't introduced any duplicate code?

Testing Checklist

  • Added relevant unit tests?
  • Do all unit tests pass?
  • Do all end-to-end tests pass?
  • Any other testing performed?

    Tested in the zoo

  • If applicable, add screenshots or log transcripts of the feature working

@shreyamalviya shreyamalviya changed the base branch from develop to 3077-agent-OTPs March 16, 2023 08:16
@shreyamalviya shreyamalviya force-pushed the 3077-modify-exploiters-to-use-otp branch from 1bc19ba to 7b3381f Compare March 16, 2023 08:17
@mssalvatore
Copy link
Collaborator

Consider rebasing this on 3077-get-otp-from-env. We should use feature flags instead of a long-lived branch to do this work.

@shreyamalviya shreyamalviya force-pushed the 3077-modify-exploiters-to-use-otp branch from 9ee3fd2 to b88cd0b Compare March 17, 2023 06:59
@shreyamalviya shreyamalviya changed the base branch from 3077-agent-OTPs to 3077-get-otp-from-env March 17, 2023 06:59
Base automatically changed from 3077-get-otp-from-env to develop March 17, 2023 10:55
@shreyamalviya shreyamalviya force-pushed the 3077-modify-exploiters-to-use-otp branch from d9ff803 to f11b93e Compare March 17, 2023 11:17
Copy link
Contributor

@VakarisZ VakarisZ left a comment

Choose a reason for hiding this comment

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

OTP is an essential part of the agent command line, it's not exploiter specific option. I think we should've modified build_monkey_commandline and build_monkey_commandline_explicitly

@shreyamalviya
Copy link
Contributor Author

OTP is an essential part of the agent command line, it's not exploiter specific option. I think we should've modified build_monkey_commandline and build_monkey_commandline_explicitly

Both of these build and return the arguments that are passed to the Agent binary when it's run. We need to set the OTP as an environment variable on the machine before the binary is run.

@shreyamalviya shreyamalviya force-pushed the 3077-modify-exploiters-to-use-otp branch from 0930389 to f9e27fc Compare March 20, 2023 06:18
@shreyamalviya shreyamalviya force-pushed the 3077-modify-exploiters-to-use-otp branch 4 times, most recently from a262521 to 595eeeb Compare March 23, 2023 14:32
@ilija-lazoroski ilija-lazoroski marked this pull request as ready for review March 24, 2023 06:28
@shreyamalviya shreyamalviya marked this pull request as draft March 24, 2023 08:02
@VakarisZ VakarisZ force-pushed the 3077-modify-exploiters-to-use-otp branch from 1bf6fa7 to f02bd83 Compare March 24, 2023 11:01
@shreyamalviya shreyamalviya force-pushed the 3077-modify-exploiters-to-use-otp branch 3 times, most recently from b0becc3 to b394e1e Compare March 27, 2023 05:57
shreyamalviya and others added 27 commits March 28, 2023 11:35
Using `export` in agent run commands is unnecessary and undesirable. It
will add the environment variable to the parent process's environment,
not just the environment of the agent process. This will expose the OTP
more than necessary, violating the principle of least privilege. It will
also make it difficult (impossible?) for the agent to properly clean up
all traces of the OTP.
Using `export` in agent run commands is unnecessary and undesirable. It
will add the environment variable to the parent process's environment,
not just the environment of the agent process. This will expose the OTP
more than necessary, violating the principle of least privilege. It will
also make it difficult (impossible?) for the agent to properly clean up
all traces of the OTP.
@mssalvatore mssalvatore force-pushed the 3077-modify-exploiters-to-use-otp branch from 7b7cbc3 to 8d98ef8 Compare March 28, 2023 15:36
@mssalvatore mssalvatore merged commit 780b940 into develop Mar 28, 2023
@mssalvatore mssalvatore deleted the 3077-modify-exploiters-to-use-otp branch March 28, 2023 15:37
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.

Implement OTP compliance on the Agent
5 participants