rapid7 / metasploit-framework

Metasploit Framework
https://www.metasploit.com/
Other
34.28k stars 14k forks source link

Improve NagiosXI authenticated exploit modules to increase resilience and for use with Autocheck disabled #17606

Open k0pak4 opened 1 year ago

k0pak4 commented 1 year ago

Summary

During the course of https://github.com/rapid7/metasploit-framework/pull/17494 there were several concerns with how the NagiosXI login mixin was being used in the check method, which prevents the module from working when Autocheck is disabled. Additionally, other improvements were made including cleaner regexes in version detection, nil checks on objects that were assumed to be not nil, and other improvements. After examining the other NagiosXI modules the following modules should be modified to also take advantage of these improvements:

Improvements

Generally, these modules can also be cleaned up and shortened similarly to how the config wizards RCE module was through PR.

Motivation

Currently, these modules will fail with AutoCheck disabled, so we want to improve that first and foremost. Additionally, the version checking and error codes will provide more support when running the modules against old versions of NagiosXI.

Vulnerable Software

In general, older versions of NagiosXI can be found:

documentation\modules\exploit\linux\http\nagios_xi_configwizards_authenticated_rce.md has detailed installation instructions if help is needed on install

Sidharthareddy99 commented 1 year ago

I want to work on this issue please assign me this issue.

gwillcox-r7 commented 1 year ago

@Sidharthareddy99 Your assigned now, let me know if you need additional info on anything.

k0pak4 commented 1 year ago

Yup feel free to ask questions if you need @Sidharthareddy99 , it's very top of mind at the moment 😅

gwillcox-r7 commented 1 year ago

Unassigning for now, if a PR is put up though I can reassign. We mostly use assignment internally to track who is working on what. Anyone is free to put up a PR to fix an issue at any time, at which point we can look into assigning people at that point

XEHIL commented 1 year ago

Can I get to work on this? I'd like to contribute.

gwillcox-r7 commented 1 year ago

Sure @xehil, feel free to take it on 👍 Do you need help forking the repo? I see you haven't done so yet.

XEHIL commented 1 year ago

@gwillcox-r7 thank you. I was waiting for the issue to be assigned.

gwillcox-r7 commented 1 year ago

https://github.com/rapid7/metasploit-framework/pull/17820 resolves most of these issues however the final two points still stand. Reopening this issue as they will need to be addressed before we can consider this truly fixed.

AlejandroCantu commented 1 year ago

Hi @gwillcox-r7 I am a student at UT Austin in the Ethical Hacking class who would like to contribute on this issue as a final project for the class. I see the final two points stand, but upon reading this comment, it seems there is deeper structural changes and checks to pass if the order of the return values of visit_nagios_dashboard change? I'm not sure I fully understand the issue because the place where visit_nagios_dashboard is called does not use the return values. Could you help me understand where these return values from visit_nagios_dashboard are used?

gwillcox-r7 commented 1 year ago

Hi @gwillcox-r7 I am a student at UT Austin in the Ethical Hacking class who would like to contribute on this issue as a final project for the class. I see the final two points stand, but upon reading this comment, it seems there is deeper structural changes and checks to pass if the order of the return values of visit_nagios_dashboard change? I'm not sure I fully understand the issue because the place where visit_nagios_dashboard is called does not use the return values. Could you help me understand where these return values from visit_nagios_dashboard are used?

So as for the issue I think the main problem is seen at https://github.com/rapid7/metasploit-framework/blob/f6c8181b7f3edc3f29f41a323c2f8eb054619cd5/lib/msf/core/exploit/remote/http/nagios_xi/login.rb#L251-L257 where we can see that for one response the response cookies come first and then the response body and yet in another response the response body comes first and then the cookies.

Ideally we should be using hashes here as well instead of arrays to prevent issues related to positional issues. However we decide to do this though, all calling code will need to be updated to account for the changes that we make.

As for where the return codes from visit_nagios_dashboard are used, you will want to see where it is called from first. Sourcegraph is a great tool to help with this. https://sourcegraph.com/github.com/rapid7/metasploit-framework with context:global repo:^github\.com/rapid7/metasploit-framework$ is a good start and then we can search for visit_nagios_dashboard there.

Looking for this shows one reference at https://sourcegraph.com/github.com/rapid7/metasploit-framework/-/blob/lib/msf/core/exploit/remote/http/nagios_xi/login.rb?L180 which is in the nagios_xi_login function. Click on that function name and then "View References" and you should see about 15 references from where that is called.

Let me know if that is clear or not, I know code navigation took some time for me to wrap my head around so I'm happy to help with any questions you might have 👍

AlejandroCantu commented 1 year ago

Grant, I've got a solution that standardizes the authentications in the NagiosXI Scanner and one of the older exploits to not directly call nagios_xi_login, but use authenticate, like nagios_xi_configwizards_authenticated_rce.rb.

Firstly, I changed the order of the return from visit_nagios_dashboard and changed authenticate accordingly. Also made a small change to extract_auth_cookies for the new format, which doesn't break anything else since this is the only call to extract_auth_cookies.

I then changed around some comments to be more accurate.

Take a look at my PR when you can! I have the code there so you can take an initial look at it but I neeed to test these modules out myself now.

nrathaus commented 7 months ago

@k0pak4 this issue seems to have been resolved - can you comment

k0pak4 commented 7 months ago

@nrathaus it looks like the last two requirements are not resolved as far as I can tell? The PR addressing the last one was closed with an attic tag, and from what I see the response codes still are sent in the same format? Unless I'm missing something I don't think the last requirement is addressed

nrathaus commented 7 months ago

Previous versions of NagiosXI are listed here: https://assets.nagios.com/downloads/nagiosxi/versions.php

k0pak4 commented 7 months ago

@nrathaus yes the previous versions are also listed in the original description, but I don't think this is fully addressed