Add additional reliability metadata

[adfoster-r7]

This pull request adds additional constants to the reliability metadata for modules. This requirement has come up before for @bcoles and @h00die on modules they have worked on - as well as previous internal Rapid7 requirements, i.e. running modules in an automated scanning scenario, and we wish to filter out modules that require a reboot of target machines.

Verification

Verify we are okay adding this metadata to the module reliability information

[adfoster-r7]

Discussion:

This PR adds one generic event:

  ### Reliability

  | Constant         | Description    |
  | -------------- | ------------- |
  | FIRST_ATTEMPT_FAIL | The module may fail for the first attempt |
  | REPEATABLE_SESSION | The module is expected to get a session every time it runs |
  | UNRELIABLE_SESSION | The module isn't expected to get a shell reliably (such as only once) |
+ | EVENT_DEPENDENT    | The module may not open a shell until an external event occurs. For instance, a cron job, machine restart, user interaction within a GUI, etc |

Alternatively, there was the idea from bcoles to track the potential scenarios separately

This might be too granular, but events that were discussed:

• USER_INTERACTION_REQUIRED - i.e. A user performing an action on a GUI - logging in, clicking a particular menu item etc
• REBOOT_REQUIRED - Machine reboot required
• SERVICE_RESTART_REQUIRED - Requires a reboot of the service
• SCHEDULED_TASK_REQUIRED - Depends on a scheduled automated task to complete, i.e. a cron job/windows task

Maybe instead of adding this to the existing reliability section, we could add a new bucket for event dependencies? That would make consuming the metadata easier, as automated tools can just bail early if module.required_events.any?

        'Notes' => {
          'Stability' => [],
          'Reliability' => [REPEATABLE_SESSION],
          'SideEffects' => [IOC_IN_LOGS],
          'Events' => [USER_INTERACTION_REQUIRED]
        }

Although it's unclear if these events should be treated as an "AND" or "OR" semantics, i.e.:

• the module could succeed on computer restart, or service restart.
• the module could succeed after a cron cron runs, and there is user interaction

Edit: Potentially RequiredEvents as a key is better than Events

[smcintyre-r7]

Would you mind also adding the description to the default_template so it shows up in the output of info -d?

[gwillcox-r7]

@adfoster-r7 @smcintyre-r7 Did we want to discuss this further or what is the general consensus on this given this has been up for a few months now and has had time to mull a bit? I'm happy to take some of this work onboard if we need additional changes to bring this up to a mergeable state, such as updating the default template that Spencer mentioned.

[gwillcox-r7]

For what its worth personally I'd be in favor of being more narrow and specific with things however I can understand if this might add extra maintenance overhead given the potentially ever expanding list of items that could come up.

My main concern with grouping things is that we end up back in the same place that lead us here: that the module options were not specific enough to cover our needs.

Grouping like you have mentioned here might be good but then this still places the emphasis on the end user to add in comments and/or appropriate documentation, which users then have to reference (and many of them don't know where our doc files exist in my experience) to figure out what specific event needs to trigger in order for their payload to run. This is particularly relevant in helping them self debug an issue.

Anyway thats my 2 cents on this, I'm happy to go either way so long as we have a firm reason for why.

[bwatters-r7]

@adfoster-r7 is this something we are still pursuing?

[adfoster-r7]

It's still something we want to pursue, there were open questions above on implementation details that would need to be discussed/planned more

[smcintyre-r7]

I think the generic event is fine as it is. I think it'd flag to the module operator to read through the module's description and documentation which should then clarify exactly what kind of event(s) the action is reliant on.

I'd be happy to merge this once default_template.erb has been updated.

Alternatively, since there hasn't been a lot of discussion going on over the last year, perhaps we could attic this.

[smcintyre-r7]

Release Notes

This adds a new EVENT_DEPENDENT value for module reliability metadata.

[h00die]

To me EVENT_DEPENDENT would also encompass xss needing user interaction (browsing to page, clicking a button). Thoughts? Thinking about #19104 and #19102

[smcintyre-r7]

Yeah, I would agree. user interaction within a GUI seems to cover it.