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

Golang module support [WIP] #10758

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@busterb
Contributor

busterb commented Oct 6, 2018

Welcome to a Derbycon special addition. With so many laptops around, I couldn't resist fiddling with mine!

This adds initial support for Go as a language for Metasploit modules. It implements just-enough of the API to support the 'description' and 'run' actions. Since Go is a compiled language, this implementation uses a trick in most shells where the lack of a #! indicates that /bin/sh should be run, and uses that // is treated as /. Since // is also a comment in the Go language, it is subsequently ignored by the compiler, and it remains fully compatible go syntax.

The specific API for the 'metasploit' module is subject to change. Since Golang supports compile-time type checking, this means that metadata and options passed to report methods can support the same, so I am in the process of converting them to use method-specific structs for parameter passing instead of a map[string]string.

Verification

Feel free to play with the test.go module, this shouldn't use anything too fancy or outside of the standard go library. I do not believe this causes any side-effects of trying to write to the metasploit-framework source tree during the compile phase (go run in modern versions should always compile to a temp or cache directory outside of the source tree).

Let me know if there are any comments or suggestions; looking forward to completing the first golang port of a scanner module after this.

busterb added some commits Oct 6, 2018

}
func rpc_send(res interface{}) {
res_str, _ := json.Marshal(res)

This comment has been minimized.

@FireFart

FireFart Oct 7, 2018

Contributor

shouldn't we handle the error here and return it so the run function can check it?

@FireFart

FireFart Oct 7, 2018

Contributor

shouldn't we handle the error here and return it so the run function can check it?

This comment has been minimized.

@busterb

busterb Oct 8, 2018

Contributor

yes, definitely

@busterb

busterb Oct 8, 2018

Contributor

yes, definitely

func rpc_send(res interface{}) {
res_str, _ := json.Marshal(res)
f := bufio.NewWriter(os.Stdout)
defer f.Flush()

This comment has been minimized.

@FireFart

FireFart Oct 8, 2018

Contributor

the problem with defer is it does not check for errors. Flush() can also return an error (https://golang.org/pkg/bufio/#Writer.Flush) so maybe we should convert the defer statement in a normal call and return an error if it happens

@FireFart

FireFart Oct 8, 2018

Contributor

the problem with defer is it does not check for errors. Flush() can also return an error (https://golang.org/pkg/bufio/#Writer.Flush) so maybe we should convert the defer statement in a normal call and return an error if it happens

This comment has been minimized.

@busterb

busterb Oct 10, 2018

Contributor

What would we do with the error? If we cannot send, we can only die, since we cannot bubble the error up (it would mean MSF has died). Dying seems like the best approach.

@busterb

busterb Oct 10, 2018

Contributor

What would we do with the error? If we cannot send, we can only die, since we cannot bubble the error up (it would mean MSF has died). Dying seems like the best approach.

This comment has been minimized.

@FireFart

FireFart Oct 10, 2018

Contributor

oh ok missed that

@FireFart

FireFart Oct 10, 2018

Contributor

oh ok missed that

@@ -4,7 +4,7 @@
class Msf::Modules::External::Shim
def self.generate(module_path, framework)
mod = Msf::Modules::External.new(module_path, framework: framework)
return '' unless mod.meta
return nil unless mod.meta

This comment has been minimized.

@acammack-r7

acammack-r7 Oct 10, 2018

Contributor

So this will raise an error for every module that the current environment cannot run, like if you don't have a version of Python. This can get kinda noisy, which is why an empty module used to be returned. Maybe we should do some logging here instead of returning nil?

@acammack-r7

acammack-r7 Oct 10, 2018

Contributor

So this will raise an error for every module that the current environment cannot run, like if you don't have a version of Python. This can get kinda noisy, which is why an empty module used to be returned. Maybe we should do some logging here instead of returning nil?

This comment has been minimized.

@busterb

busterb Oct 10, 2018

Contributor

That was the point. Silently failing to load a module is worse for someone trying to debug a module than logging something. It logs to the .msf4/ logs/framework.log file, so it's not right in front of the user, but there's enough info that a developer has a fighting chance of debugging in the field.

@busterb

busterb Oct 10, 2018

Contributor

That was the point. Silently failing to load a module is worse for someone trying to debug a module than logging something. It logs to the .msf4/ logs/framework.log file, so it's not right in front of the user, but there's enough info that a developer has a fighting chance of debugging in the field.

This comment has been minimized.

@busterb

busterb Oct 10, 2018

Contributor

Ideally, all of our users eventually get all of the dependencies. That's our plan anyway, or they're going to miss out on more modules over time. We should tolerate half-configured environments, but not make it impossible to tell if they are.

@busterb

busterb Oct 10, 2018

Contributor

Ideally, all of our users eventually get all of the dependencies. That's our plan anyway, or they're going to miss out on more modules over time. We should tolerate half-configured environments, but not make it impossible to tell if they are.

This comment has been minimized.

@busterb

busterb Oct 10, 2018

Contributor

Maybe as a followup, I could look at stderr for each failure, and only emit unique failures once. That makes the lack of python a single message, rather than dozens.

@busterb

busterb Oct 10, 2018

Contributor

Maybe as a followup, I could look at stderr for each failure, and only emit unique failures once. That makes the lack of python a single message, rather than dozens.

This comment has been minimized.

@acammack-r7

acammack-r7 Oct 10, 2018

Contributor

Or maybe just one log line per module instead of a whole stacktrace unless debug is enabled.

@acammack-r7

acammack-r7 Oct 10, 2018

Contributor

Or maybe just one log line per module instead of a whole stacktrace unless debug is enabled.

This comment has been minimized.

@busterb

busterb Oct 11, 2018

Contributor

Yeah, agree we don't need a full stracktrace.

@busterb

busterb Oct 11, 2018

Contributor

Yeah, agree we don't need a full stracktrace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment