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

Add -i option to Msf::Ui::Console::CommandDispatcher::Session mixin's sessions command #18885

Merged
merged 4 commits into from
Mar 20, 2024

Conversation

errorxyz
Copy link
Contributor

@errorxyz errorxyz commented Feb 25, 2024

Fixes #18811

This PR adds a -i option to Msf::Ui::Console::CommandDispatcher::Session mixin's sessions command. This change would affect meterpreter, smb, sql consoles.

I've also updated the spec tests for the same

Verification

  • Get 2 or more meterpreter sessions
  • use the -i option to interact with different sessions
msf6 exploit(multi/handler) > sessions

Active sessions
===============

  Id  Name  Type                   Information            Connection
  --  ----  ----                   -----------            ----------
  1         meterpreter x64/linux  errorxyz @ linome.xyz  127.0.0.1:4444 -> 127.0.0.1:52652 (127.0.0.1)
  2         meterpreter x64/linux  errorxyz @ linome.xyz  127.0.0.1:4444 -> 127.0.0.1:33242 (127.0.0.1)

msf6 exploit(multi/handler) > sessions -i 1
[*] Starting interaction with 1...

meterpreter > sessions -i 2
[*] Backgrounding session 1...
[*] Starting interaction with 2...

meterpreter > sessions -i 1
[*] Backgrounding session 2...
[*] Starting interaction with 1...

meterpreter > sessions -i -1
[*] Backgrounding session 1...
[*] Starting interaction with 2...

meterpreter > sessions -i 2
[*] Session 2 is already interactive.

- Fixes spec test for sessions command where session id should be
  sent as a string and not as an integer
@zeroSteiner
Copy link
Contributor

I like that this makes the usage syntax consistent.

print_line
print_line('Interact with a different session Id.')
print_line('This works the same as calling this from the MSF shell: sessions -i <session id>')
print_line
end

def cmd_sessions(*args)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to add in a Rex Parser here?:

@@session_opts = Rex::Parser::Arguments.new(
            ['-h', '--help] => [ false, 'Show this message' ],
            ['-i', '--interact'] => [ true, 'Interact with a provided session ID' ]
          )

@@session_opts.parse(args) do |opt, _idx, val|
              case opt
              when '-h'
                ...
              when '-i'
                ...
              else
                 ...
              end
end

Copy link
Contributor Author

@errorxyz errorxyz Mar 14, 2024

Choose a reason for hiding this comment

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

Since there's only one option needed here, for simplicity's sake I feel we don't need this. If we use this, we'll also have to separately handle sessions <id> as done here. If there's plans of adding new options here, it'd make sense to use the rex parser. I'll happily add the rex parser if you still feel we should use it.

Yup will add that👍


it 'backgrounds the session and switches to the new session' do
subject.cmd_sessions(new_session_id)
subject.cmd_sessions('-i', new_session_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why this test was changed, rather than a new one added for sessions -i <id>? It seems like now we only test the -i functionality, with the previous functionality of sessions <id> not being tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially I thought since both use the same back-end, a single test would suffice. But now in hindsight, I feel we'd need two separate tests. Will add it👍

@errorxyz
Copy link
Contributor Author

Sorry for the late response @sjanusz-r7 had some personal work, I've now made the reviewed changes.

@errorxyz errorxyz requested a review from sjanusz-r7 March 16, 2024 05:34
@adfoster-r7
Copy link
Contributor

This is very nice, thank you!

msf6 auxiliary(scanner/mysql/mysql_login) > sessions -i -1
[*] Starting interaction with 2...

sesMySQL @ 127.0.0.1:3306 () > sessions -1
[*] Backgrounding session 2...
[*] Starting interaction with 2...

MySQL @ 127.0.0.1:3306 () > sessions -2
[*] Backgrounding session 2...
[*] Starting interaction with 1...

PostgreSQL @ 127.0.0.1:9000 (template1) > sessions -i -1
[*] Backgrounding session 1...
se[*] Starting interaction with 2...

ssiMySQL @ 127.0.0.1:3306 () > sessions -i -2
[*] Backgrounding session 2...
[*] Starting interaction with 1...

PostgreSQL @ 127.0.0.1:9000 (template1) > 

@adfoster-r7
Copy link
Contributor

This is a bit weird that the validation doesn't happen before attempting to swap sessions, but I believe that's existing behavior so not an issue!

PostgreSQL @ 127.0.0.1:9000 (template1) > sessions -i 3
[*] Backgrounding session 1...
[-] Invalid session identifier: 3
msf6 auxiliary(scanner/mysql/mysql_login) > 

@adfoster-r7 adfoster-r7 merged commit 298e03b into rapid7:master Mar 20, 2024
34 checks passed
@adfoster-r7 adfoster-r7 added the rn-enhancement release notes enhancement label Mar 20, 2024
@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Mar 20, 2024

Release Notes

Enhances the sessions command so that both Meterpreter and the top level Metasploit prompt support sessions -i -1.

@errorxyz
Copy link
Contributor Author

@adfoster-r7 Another (small) issue I found while testing this was that when we switch sessions immediately after getting a meterpreter shell, we aren't prompted about the session we are switching into.

msf6 exploit(multi/handler) > run

[!] You are binding to a loopback address by setting LHOST to 127.0.0.1. Did you want ReverseListenerBindAddress?
[*] Started reverse TCP handler on 127.0.0.1:4444 
[*] Sending stage (3045380 bytes) to 127.0.0.1
[*] Meterpreter session 1 opened (127.0.0.1:4444 -> 127.0.0.1:54846) at 2024-03-21 02:40:13 +0530

meterpreter > bg
[*] Backgrounding session 1...
msf6 exploit(multi/handler) > run

[!] You are binding to a loopback address by setting LHOST to 127.0.0.1. Did you want ReverseListenerBindAddress?
[*] Started reverse TCP handler on 127.0.0.1:4444 
[*] Sending stage (3045380 bytes) to 127.0.0.1
[*] Meterpreter session 2 opened (127.0.0.1:4444 -> 127.0.0.1:43242) at 2024-03-21 02:40:24 +0530

meterpreter > sessions 1
[*] Backgrounding session 2...       # Notice that we aren't prompted about the session we are switching to
meterpreter > bg
[*] Backgrounding session 1...
msf6 exploit(multi/handler) > sessions -i 1
[*] Starting interaction with 1...

meterpreter > sessions 2
[*] Backgrounding session 1...
[*] Starting interaction with 2...   # We are prompted about the session we are switching to

meterpreter > 

It's a minor inconsistency in output, I thought I should mention it somewhere

@adfoster-r7
Copy link
Contributor

Interesting! I wonder why that's happening 🤔

Agreed that it's a minor issue 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rn-enhancement release notes enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Align Meterpreter sessions command with msfconsole sessions command
4 participants