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

Added the_commander to KP server plugins #41

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Gilks
Copy link
Contributor

@Gilks Gilks commented Nov 24, 2019

Added the_commander.py to help operators perform phishing engagements against targets with MFA. This plugin can be used to automatically execute arbitrary commands from the KP server after the target enters credentials.

I've included two example commands in the description to help users get started. One command executes openconnect from the KP server and the second command establishes an SSH tunnel to another resource before executing openconnect.

See the description for more details.

Note: The plugin makes no attempt to verify the package being executed is actually installed. It's up to the user to make sure packages like openconnect are already installed prior to using this plugin.

@zeroSteiner zeroSteiner self-assigned this Nov 24, 2019
@zeroSteiner zeroSteiner added new-plugin server-plugin Relating to the server component labels Nov 24, 2019
server/the_commander.py Outdated Show resolved Hide resolved
server/the_commander.py Outdated Show resolved Hide resolved
server/the_commander.py Outdated Show resolved Hide resolved
server/the_commander.py Show resolved Hide resolved
server/the_commander.py Outdated Show resolved Hide resolved
server/the_commander.py Outdated Show resolved Hide resolved
return
username = username.split('\\')[-1]

illegal_chars = ['/', '\\', '[', ']', ':', ';', '|', '=', ',', '+', '*', '?', '<', '>', ' ', '&', '!', '~', '#', '%', '^', '(', ')', '{', '}' '`']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of escaping, it might be better to use the shlex.quote function to escape the username, password, and mfa-token values. I think the downside to this approach would be that it'd break options --like=this where the value is not exactly one token. It would be wroth testing that though.

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 was using shlex.quote but it ended up not working out well. Just running shlex.quote on the user input led to stack traces in instances where the users password contained a single or double quote.

Reproduce

  1. Try changing out the re.escape with shlex.quote
  2. Put a single quote somewhere in your password.
  3. Let the rage flow through you while troubleshooting the ValueError: No closing quotation exception from shlex

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm unable to reproduce this. StackOverflow might be a questionable source but per this answer it looks like it's the correct way to go.

In [1]: import shlex                                                                                                                                                                                               

In [2]: print(shlex.quote("""it's a nice day..."""))                                                                                                                                                               
'it'"'"'s a nice day...'

In [3]: len(shlex.split(shlex.quote("""it's a nice day...""")))                                                                                                                                                    
Out[3]: 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not quite what's taking place in the code.

import shlex
command = 'echo "{username} {password} {mfa}"'
passw = "super'secret"
passw = shlex.quote(passw)
command = command.format(username='test', password=passw, mfa='123456')
shlex.split(command)

It's all dependent on what the user enters as a command. You can follow the same steps above without double quotes in command and be all set.

@Gilks Gilks requested a review from zeroSteiner November 27, 2019 17:08
@zeroSteiner zeroSteiner added the pinned Do not mark as stale label Dec 12, 2019
@zeroSteiner
Copy link
Collaborator

Alright, I've given this alot of thought and I think the best way to move forward with this is to separate out the command and parsing logic a bit more. As it is now, it's a bit over-fitted for your particular use case which as awesome as it is, I think there's alot more potential here to use this for executing something the user specifies.

To that end, what I'd like to see is this reworked to take more configuration around the command, it should really be an array where the first element is an executable. This would require you to off-load more logic for your particular VPN use case into a bash or python script but would have the added benefit of being less prone to potential command injection vulnerabilities in the username, password and mfa fields.

I'm thinking the command would be like:

command:
  - /path/to/your/script.sh
  - {username}
  - {password}
  - someOtherArg

Then you'd iterate over command, formatting each element in the array to replace {username} etc and pass them together as the args argument to either start_process or run_process. The full command would automatically get logged. Use start_process if you don't want to wait for the subprocess to complete, or run_process if you do. Waiting and using run_process would be necessary to get the output (via the tee argument). Bonus points if you want to make waiting an option and dispatch appropriately.

Finally, to determine success, you should check the status code which is again only available if you wait on the child process. The error message should then be a bit more generic as the script that was executed could have done anything from send an alert, SSH'ed into something or started the VPN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-plugin pinned Do not mark as stale server-plugin Relating to the server component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants