Add migrate timeout option

[OJ]

This commit adds an option that allows the user to specify a larger time to wait for migration to finish. The minimum will always be 60 seconds, but in cases where the latency in the environment is really high, there is a need for a longer migration timeout.

I moved the writable folder for linux so that it's passed with the -p flag.

The migrate function now takes opts instead of parameters, and those opts contain all the things that were required before, along with the new timeout.

The code has a few "tidies" as well, bringing things closer to the current standard.

Verification

• Migration still works as it always did.
• Use of the -t parameter does change the timeout value (hard to simulate, I used debug output).
• Help contains appropriate detail of the flags.

[hdm]

Changing the calling syntax of migrate() may break scripts and post-modules.

[wchen-r7]

The new calling syntax also must have the :pid and :timeout key, it would also be nice to document these too.

# Correct way of calling the new #migrate method
client.core.migrate(pid: target_pid, timeout: 60)

[hdm]

Examples that this would break:

lib/msf/core/post/windows/mssql.rb:              session.core.migrate(pid)

scripts/meterpreter/screenspy.rb:      session.core.migrate(x['pid'].to_i)
scripts/meterpreter/virusscan_bypass.rb:client.core.migrate(target_pid)
scripts/meterpreter/winenum.rb:  @client.core.migrate(newproc.pid.to_i)
scripts/meterpreter/migrate.rb:    client.core.migrate(target_pid)
scripts/meterpreter/keylogrecorder.rb:      session.core.migrate(x['pid'].to_i)
scripts/meterpreter/enum_chrome.rb:    client.core.migrate(target_pid)
scripts/meterpreter/enum_chrome.rb:  client.core.migrate(@old_pid)

[void-in]

@hmoore-r7 Since one PR is only for one action, I will generate another PR for the above scripts. We can then land both PRs simultaneously.

[OJ]

Many thanks to @void-in for helping me out with filling the gaps, and doing documentation. His PR has been merged into mine.

[wchen-r7]

Eye-balling this right quick. Looks like we missed:

modules/post/windows/manage/smart_migrate.rb:      client.core.migrate(target_pid)
modules/post/windows/manage/migrate.rb:      session.core.migrate(target_pid)
modules/post/windows/gather/smart_hashdump.rb:        session.core.migrate(p["pid"])
modules/post/windows/gather/screen_spy.rb:          session.core.migrate(p['pid'].to_i)
modules/post/windows/gather/memory_grep.rb:      session.core.migrate(target_pid)
modules/post/windows/gather/enum_chrome.rb:          session.core.migrate(target_pid)
modules/post/windows/gather/enum_chrome.rb:        session.core.migrate(target_pid)
modules/post/windows/capture/lockout_keylogger.rb:        session.core.migrate(targetpid)
modules/post/windows/capture/keylog_recorder.rb:    session.core.migrate(pid)
modules/post/windows/capture/keylog_recorder.rb:        session.core.migrate(x['pid'].to_i)
modules/exploits/windows/http/hp_nnm_ovbuildpath_textfile.rb:      client.core.migrate(target_pid)

Looks like another patch is needed.

[hdm]

Given the impact of this change, is it worth preserving the old API behavior? Changing the function signature to something like migrate(timeout, opts={})?

[wchen-r7]

It kinds of makes more sense to me to preserve the original behavior. But I'm okay with whatever the decision is :-)

[OJ]

I'm happy to go either way. I went with the opts thing to begin with because that's what I thought was preferred.

[hdm]

Maybe something like:
def migrate(pid, writable_dir = nil, opts={})

Given how many modules/scripts are calling this directly, it would be less risky to stick with the old calling convention and add a new opts parameter on the end that is optional. Even if we get all of the framework references, there may be third-party modules and scripts that this breaks.

[void-in]

@hmoore-r7 I like the opts way as well. Updating the scripts identified by @wchen-r7 is not difficult. It was my bad I updated only the scripts identified by you and didn't look at other places. I can generate another PR to @OJ branch for the remaining modules. However, we can't be sure for third-party modules and scripts. If you are willing to take that risk, let me know. I suppose at certain time we would have to make changes which might break some third party scripts as we did in case of payload URIs.

[hdm]

@bcook-r7 Want to be a deciding vote here? Continue with the changed calling convention, possibly break third-party stuff, or make this backwards compatible?

[bcook-r7]

I gotta go with backward compatible. If pid is not optional, it should not be in opts, but left as the first non-optional parameter.

[void-in]

@bcook-r7 Okay. The name opts also make sense for optional data. I wish we had a better name which might have persuade you guys :). It means @OJ need to update the migrate def to cater for the decision. The don't think any of the post modules need the timeout so they are good to go.

Can OJ selectively merge from my PR? Like merge changes of only lib/rex/post/meterpreter/client_core.rb? All the other changes need to be reverted.

[OJ]

I think the simplest thing to do at this point would be for me to close this PR and open a new one. The number of changes required will be minimal, and dancing with git to make it work through this PR seems like unnecessary effort!

Thanks for the support guys.

[coveralls]

Changes Unknown when pulling e697312 on OJ:migrate-timeout into ** on rapid7:master**.

[busterb]

What is wrong with you @coveralls ? You're drunk.