-
Notifications
You must be signed in to change notification settings - Fork 14.6k
Add Linux Container Enumeration Module #13844
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 Linux Container Enumeration Module #13844
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stealthcopter Very nice first PR, congrats! A few minor suggestions, the only real issue I found was an issue with the author field which may confuse some of our backend systems, and a case of you using the print command instead of something like print_status. Rest of it is suggestions for ways to improve your code so that it makes a bit more sense to users in some cases.
|
Also one more comment @stealthcopter but you may want to rename this module to enum_running_containers.rb if its only going to be enumerating running containers. Otherwise as of right now, given the module name, I would expect the module to also enumerate non-running containers and list those as well (could always add this in though, shouldn't be too hard to do). |
|
This would be good info to either store as a host note, or as loot. May be nice to sync the output styles as well instead of having one in a framed table and another not. (I prefer the first table personally) |
|
@gwillcox-r7 @h00die @adfoster-r7 thanks for all the feedback! I'll make some changes and update :) |
|
Also, Also, the loops could be cleaned up a lot. The temporary state variables aren't required. Also, you can probably use Here's some psuedo code. # Run Method for when run command is issued
def run
platforms = %w[docker lxc rkt].map{|p| runnable(p)}
if platforms.empty?
print_error('No container software appears to be installed or runnable by the current user')
return
end
platforms.each do |platform|
active_containers = count_containers(platform)
print_status("#{platform}: #{active_containers} active containers")
next unless active_containers
containers = list_containers(platform)
print_line(containers.to_s)
end
end |
|
@bcoles thanks for the tips, I've not really used ruby before so that's very helpful. Appreciate it! |
Here's an example of how you can use |
|
Made some changes following the feedback above:
Issues not solved yet:
Hope I haven't made it too complicate adding the |
|
Thanks for your pull request! Before this pull request can be merged, it must pass the checks of our automated linting tools. We use Rubocop and msftidy to ensure the quality of our code. This can be ran from the root directory of Metasploit: You can automate most of these changes with the Please update your branch after these have been made, and reach out if you have any problems. |
@stealthcopter I don't think that just using
Alright so after some more discussions with the team on this, I think the general feeling is that the tables themselves should have consistent rough styling. What do I mean by this? Well I mean that in your example you had one table with lines within it, and one without. I would lean towards the one without and not try to table everything. As for their content, when adding it to For an example of how You can find the definition for this function here: metasploit-framework/lib/msf/core/auxiliary/report.rb Lines 371 to 388 in d631448
|
|
@gwillcox-r7 Thanks again. Yup, it's pretty annoying how LXC have decided to make their own fancy tables. I've fixed this using the following code. Please let me know if it could be improved: result = cmd_exec('lxc list').each_line.reject { |st| st =~ /^\+--/ }.map.with_index.map do |s, i|
if i == 0
s.split('| ').map { |t| t.strip.ljust(t.size, ' ').gsub(/\|/, '') }.join + "\n"
else
s.gsub(/\| /, '').gsub(/\|/, '')
end
end.join.stripThis creates the following output on my test setup: And the loot looks like below: |
|
Very nice work @stealthcopter, that looks a lot better! |
|
@stealthcopter Sorry for delay on this. Initial testing seems to be good, showing that no containers on running on the target host (as is the case): Unfortunately though if your not running as an admin user sometimes its not possible to run the After installing Docker and setting up a nginx container: It would be good if you could detect this for your various platforms to allow us to say "hey container software appears to be running on this host, but we can't check whats running due to a lack of permissions" |
|
@gwillcox-r7 No worries. Good spot, I'll get this fixed shortly! |
…are is installed on the host even if the user isn't the 'root' user.
…or, and generally make code changes to ensure that we print out if a container system exists on a target, but if we don't have permissions to list what its running that we alert the user of this and print a properly highlighted message that informs them of this, without storing information into any loot files
|
Okay made some more updates to fix some order of some checks output now informs the user when we run into errors due to a lack of permissions: |
…nario 2 a bit better
|
@stealthcopter Applied some more updates to the documentation to introduce a new scenario and better explain scenario 2, however the documentation will need to be updated to better reflect the output from the module now that a few lines have been updated. |
…ers so we properly print out the actual number of running containers and the number of total containers (logic was correct but order was backwards))
|
@stealthcopter Applying another round of updates as after further review of the code you had some logic backwards and were printing the total number of containers as the number of running containers and the number of running containers as the total number of containers 🥴 |
|
Hmm looks like even as the |
Seems like this was due to this command So I've changed this command to |
|
Some other points: you used Otherwise your going to end up calling string functions on a Boolean, which will end up causing stack traces. And no one likes stack traces 😁 |
…st_running_containers_id, and list_containers might fail due to an invalid container type
…mand is not supplied, there is no need to supply a default command.
|
Okay one more thing: you have a section of your code which does |
|
One last thing before I upload these changes: you have the option in your code to execute an arbitrary command and print it to the screen but for some reason you never decided to store this info in |
…s for the host in the loot.
|
Next commit should also update the documentation so it is all up to scratch with the new output |
Dismissing review as I applied these changes myself
|
@gwillcox-r7 Tested here and it's all working well, thanks for all the changes, it looks great now! RuboCop spotted a few more issues for me, so updated. |
Looks good to me :) I'll go ahead and land this into the framework so long. Thanks for the contribution! |
Release NotesNew module |
|
@gwillcox-r7 awesome thanks for all your help! |
|
@stealthcopter No problem, and thanks for that last RuboCop fix; somehow it slipped past me when I ran RuboCop last night 👍 |
Adds a new module
post/linux/gather/enum_containersthat will detect if there are any container platforms (runnable by the current user) on the target machine and list any actively running containers on any it finds. Currently supports Docker, LXC and RKTVerification
List the steps needed to make sure this thing works
msfconsoleuse post/linux/gather/enum_containersset session 1runScenarios
Scenario 1: Docker is installed and there are 4 running containers
Scenario 2: Docker, LXC and RKT are installed
Scenario 3: No container software is runnable