Skip to content

Conversation

@kmroz
Copy link
Contributor

@kmroz kmroz commented Nov 7, 2021

Something along these lines:

$ ./flightsim run imposter
[...]
05:54:16 [imposter] Done (5/5)

All done! Check your SIEM for alerts using the timestamps and details above.
$ echo $?
0


$ ./flightsim run -iface lo0 imposter
[...]
05:54:21 [imposter] Resolving random imposter domains
05:54:22 [imposter] Resolving office365.com-edge2-cdn.net
05:54:22 [imposter] ERROR: office365.com-edge2-cdn.net: lookup office365.com-edge2-cdn.net. on 192.168.1.1:53: write udp 127.0.0.1:54473->192.168.1.1:53: write: can't assign requested address
[...]
05:54:26 [imposter] Done (0/5)

All done, but simulation errors ocurred! Check your SIEM for alerts using the timestamps and details above.
The following simulations experienced errors: imposter
$ echo $?
1


$ ./flightsim run imposter
[...]
06:08:53 [imposter] Resolving random imposter domains
06:08:54 [imposter] Resolving office365.com-edge2-cdn.net
06:08:54 [imposter] ERROR: office365.com-edge2-cdn.net: TEST ERROR
[...]
06:08:58 [imposter] Done (4/5)

All done, but simulation errors ocurred! Check your SIEM for alerts using the timestamps and details above.
The following simulations experienced errors: imposter
$ echo $?
1


$ ./flightsim run -iface lo0 tunnel-icmp
[...]
06:02:40 [tunnel-icmp] FATAL: Couldn't start the module: listen ip4:icmp 127.0.0.1: socket: operation not permitted (make sure you have sufficient network privileges or try to run as root)

All done, but simulation errors ocurred! Check your SIEM for alerts using the timestamps and details above.
The following simulations experienced errors: tunnel-icmp
$ echo $?
1


$ ./flightsim run -iface lo0 -fast
[...]

All done, but simulation errors ocurred! Check your SIEM for alerts using the timestamps and details above.
The following simulations experienced errors: c2, dga, imposter, miner, scan, sink, spambot, ssh-exfil, ssh-transfer, tunnel-dns, tunnel-icmp
$ echo $?
1

@kmroz kmroz requested review from ioj and tg November 7, 2021 05:17
Comment on lines +463 to +469
printGoodbye(failedSimNames != nil)
if failedSimNames != nil {
msg := fmt.Sprintf(
"The following simulations experienced errors: %v",
strings.Join(failedSimNames, ", "))
return errors.New(msg)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

printGoodbye is a function which is called only once, so the only reason for its existence is to make the code cleaner. Splitting half of the goodbye message into printGoodbye and leaving another half inlined in the function doesn't make sense. We should either pass the failed simulations into the printGoodbye or remove printGoodbye and inline everything.

// TODO: some module can return custom messages (e.g. hijack)
// and "ERROR" prefix shouldn't be printed then
printMsg(sim, fmt.Sprintf("ERROR: %s: %s", host, err.Error()))
failedSims[sim.Name()] = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this won't be misleading in some scenarios. What if we run the simulation against 10 hosts and one of them fails? Should we report the whole module as failed?

I don't have an answer here, but we need to define what it means for the module to fail. Maybe simple boolean flag is not enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also most probably we should extract this for loop body into a separate function/structure so we don't repeat the errors handling (printMsg+failedSims). But we before we do so we should address what we really want to report at the end and how to handle partial failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK... I was a little concerned that a single host failure was a bit too coarse. I'll mark this as a draft PR until we discuss further.

Thanks for the review.

@kmroz kmroz marked this pull request as draft November 8, 2021 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants