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

Add automated acceptance tests for cmd_exec API #19413

Merged
merged 5 commits into from
Sep 20, 2024
Merged

Add automated acceptance tests for cmd_exec API #19413

merged 5 commits into from
Sep 20, 2024

Conversation

cgranleese-r7
Copy link
Contributor

@cgranleese-r7 cgranleese-r7 commented Aug 23, 2024 •
edited
Loading

This PR adds acceptance tests for the new cmd_exec API changes. More information on these changes can be found in the following PRs:

Verification

  • Code changes are sane
  • CI passes

@cgranleese-r7 cgranleese-r7 added the blocked Blocked by one or more additional tasks label Aug 23, 2024
@cgranleese-r7 cgranleese-r7 mentioned this pull request Aug 27, 2024
Merged
1 task
@cgranleese-r7 cgranleese-r7 force-pushed the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch 5 times, most recently from b280935 to e448a25 Compare August 28, 2024 09:24
@cgranleese-r7 cgranleese-r7 force-pushed the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch 9 times, most recently from e1142fb to 0e6a5a8 Compare August 28, 2024 13:06
@cgranleese-r7 cgranleese-r7 mentioned this pull request Aug 28, 2024
Merged
2 tasks
smcintyre-r7
smcintyre-r7 reviewed Aug 29, 2024
View reviewed changes
.github/workflows/meterpreter_acceptance.yml Outdated
@@ -132,12 +132,11 @@ jobs:
run: |
REM pcap dependencies
powershell -Command "[System.Net.ServicePointManager]::ServerCertificateValidationCallback = {$true} ; [Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12; (New-Object System.Net.WebClient).DownloadFile('https://www.winpcap.org/install/bin/WpdPack_4_1_2.zip', 'C:\Windows\Temp\WpdPack_4_1_2.zip')"
Copy link
Contributor

Choose a reason for hiding this comment

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

@dwelch-r7 You mentioned we're adjusting our WpdPack dependency handling.

smcintyre-r7
smcintyre-r7 reviewed Aug 29, 2024
View reviewed changes
spec/acceptance/command_shell_spec.rb Outdated Show resolved Hide resolved
spec/support/acceptance/command_shell/linux.rb Outdated Show resolved Hide resolved
spec/support/acceptance/command_shell/linux.rb Outdated Show resolved Hide resolved
@cgranleese-r7 cgranleese-r7 force-pushed the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch 2 times, most recently from 1645f9d to f7fe38d Compare August 30, 2024 10:24
@cgranleese-r7 cgranleese-r7 force-pushed the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch from f7fe38d to 5ef3dfd Compare September 19, 2024 10:16
@cgranleese-r7 cgranleese-r7 removed the blocked Blocked by one or more additional tasks label Sep 19, 2024
@cgranleese-r7
Copy link
Contributor Author

I rewrote the original tests we had without having Smashery's changes to test them. Just going to leave the test that covered all the different nuances but needed improved for readability in case something was missed.

  # TODO: example in case `show_args_binary` needs some more massaging once we are able to test the API changes again
  #
  #  if session.platform.eql? 'windows'
  #    output = create_process('./show_args.exe', args: [test_string, '', test_string, '', test_string])
  #    if session.type.eql? 'powershell'
  #      output.rstrip == "#{pwd}\\show_args.exe\r\n#{test_string}\r\n\r\n#{test_string}\r\n\r\n#{test_string}"
  #    elsif session.type.eql? 'shell'
  #      output = create_process('show_args.exe', args: [test_string, '', test_string, '', test_string])
  #      output.rstrip == "show_args.exe\r\n#{test_string}\r\n\r\n#{test_string}\r\n\r\n#{test_string}"
  #    elsif (session.type.eql?('meterpreter') && session.arch.eql?('java'))
  #      output.rstrip == ".\\show_args.exe\r\n#{test_string}\r\n\r\n#{test_string}\r\n\r\n#{test_string}"
  #    elsif session.arch.eql?("php")
  #      # output = create_process('.\\show_args.exe', args: [test_string, '', test_string, '', test_string])
  #      # $stderr.puts output.rstrip.inspect
  #      # output.rstrip == ".\\show_args.exe\r\n#{test_string}\r\n\r\n#{test_string}\r\n\r\n#{test_string}"
  #      # TODO: Fix this functionality
  #      vprint_status("test skipped for PHP - functionality not correct")
  #      true
  #    else
  #      output.rstrip == "./show_args.exe\r\n#{test_string}\r\n\r\n#{test_string}\r\n\r\n#{test_string}"
  #    end
  #  else
  #    output = create_process('./show_args', args: [test_string, '', test_string, '', test_string])
  #    output.rstrip == "./show_args\n#{test_string}\n\n#{test_string}\n\n#{test_string}"
  #  end

@cgranleese-r7 cgranleese-r7 force-pushed the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch 4 times, most recently from c99ee49 to e2d4501 Compare September 19, 2024 11:03
@cgranleese-r7 cgranleese-r7 force-pushed the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch from e2d4501 to 28cdfd6 Compare September 19, 2024 11:28
@cgranleese-r7 cgranleese-r7 marked this pull request as ready for review September 19, 2024 11:31
@cgranleese-r7 cgranleese-r7 changed the title <DRAFT> Add automated acceptance tests for cmd_exec API Add automated acceptance tests for cmd_exec API Sep 19, 2024
adfoster-r7
adfoster-r7 reviewed Sep 20, 2024
View reviewed changes
lib/msf/core/session/provider/single_command_shell.rb
@@ -93,6 +93,14 @@ def shell_command_token(cmd, timeout=10)
output
end

def to_cmd(cmd, args)
if platform == 'windows'
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: I"m not sure if this is the right fix, but if it's passing more tests - then it's definitely putting us more in the right direction 👍

adfoster-r7
adfoster-r7 reviewed Sep 20, 2024
View reviewed changes
test/modules/post/test/cmd_exec.rb
else
output = cmd_exec("echo #{test_string} 1>&2")
output == test_string
end
end
end

# TODO: These tests are in preparation for Smashery's create process API
Copy link
Contributor

Choose a reason for hiding this comment

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

#19108

adfoster-r7
adfoster-r7 reviewed Sep 20, 2024
View reviewed changes
test/modules/post/test/cmd_exec.rb Outdated
@@ -21,21 +21,28 @@ def initialize(info = {})
)
end

def upload_precompiled_binaries
def upload_create_process_precompiled_binaries
Copy link
Contributor

Choose a reason for hiding this comment

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

not a blocker: def upload_show_args_binary

@cgranleese-r7 cgranleese-r7 force-pushed the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch 4 times, most recently from 4c30282 to 0d7166a Compare September 20, 2024 13:51
@cgranleese-r7 cgranleese-r7 force-pushed the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch from 0d7166a to cbd763f Compare September 20, 2024 13:54
@adfoster-r7 adfoster-r7 merged commit 43db34c into rapid7:master Sep 20, 2024
77 checks passed
@cgranleese-r7 cgranleese-r7 deleted the adds-cmd-exec-acceptance-tests-meterpreter-command-shell branch September 23, 2024 08:44
@cgranleese-r7 cgranleese-r7 mentioned this pull request Sep 25, 2024
Merged
2 tasks
@adfoster-r7 adfoster-r7 added the rn-no-release-notes no release notes label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smcintyre-r7 smcintyre-r7 smcintyre-r7 left review comments

@adfoster-r7 adfoster-r7 adfoster-r7 approved these changes

Assignees
No one assigned
Labels
rn-no-release-notes no release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cgranleese-r7 @smcintyre-r7 @adfoster-r7