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 conditional option validation depending on SESSION/RHOST connection #18875

Conversation

dwelch-r7
Copy link
Contributor

@dwelch-r7 dwelch-r7 commented Feb 22, 2024

The new session types allows certain modules to connect over either RHOST or an existing SESSION but this meant when normally the RHOST option was required the be set that could no longer be the case leading to a compromised user experience since we were no longer communicating which options needed to be set

This PR resolves that by adding conditional validation of options depending on the chosen connection type so for example if you want to connect via RHOST we also check (where applicable) that RPORT or the USERNAME is set. Then when a connection is made over an existing SESSION we can still allow the user to only set SESSION and not worry about the missing values only required for a new RHOST connection

Validation Steps

  • Use a module that supports the new session types
  • Check that when SESSION is set no option related to RHOST are forced to be set
  • Check that SESSION doesn't need to be set when connecting via RHOST
  • When attempting to connect over RHOST and RPORT/USERNAME/etc aren't set the user is notified of all missing required options
  • When neither SESSION nor RHOST is set there is a warning telling the user this

Note: Some options can be an empty string so if you unset OPTION and don't get the expected outcome you may need to do set --clear OPTION

@dwelch-r7 dwelch-r7 marked this pull request as ready for review February 23, 2024 17:12
@cgranleese-r7 cgranleese-r7 self-assigned this Mar 4, 2024
@cgranleese-r7 cgranleese-r7 added the rn-enhancement release notes enhancement label Mar 4, 2024
@cgranleese-r7
Copy link
Contributor

Testing against Windows 10 via new SMB session type and everything seems to be working as expected 👍

Check that when SESSION is set no option related to RHOST are forced to be set

image

Check that SESSION doesn't need to be set when connecting via RHOST

image

When attempting to connect over RHOST and RPORT/USERNAME/etc aren't set the user is notified of all missing required options

image

When neither SESSION nor RHOST is set there is a warning telling the user this

image

@cgranleese-r7 cgranleese-r7 merged commit 9b2b042 into rapid7:master Mar 4, 2024
47 of 48 checks passed
@cgranleese-r7
Copy link
Contributor

Release Notes

This PR adds conditional validation of options depending on the chosen connection type, so for example if you want to connect via RHOST we also check (where applicable) that RPORT or the USERNAME is set. When a connection is made over an existing SESSION we can still allow the user to only set SESSION and not worry about the missing values only required for a new RHOST connection.

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.

None yet

2 participants