Fix spawn source subcommand #954

tobim · 2020-06-26T15:36:11Z

No description provided.

libvast/src/system/node.cpp

libvast/src/detail/line_range.cpp

mavam · 2020-06-29T11:59:54Z

I haven't seen any unit or integration tests yet. What was your strategy in this regard?

mavam · 2020-06-29T12:14:04Z

When I test this locally, I get the following warning in vast start:

2020-06-29T14:09:19.767 spawn_command got an error from spawn_component: none

mavam · 2020-06-29T12:17:27Z

I tried vast spawn source pcap -i em0 and get a segfault:

Target 0: (vast) stopped.
(lldb) bt
* thread #2, name = 'caf.multiplexer', stop reason = EXC_BAD_ACCESS (code=1, address=0x368)
  * frame #0: 0x00007fff6e6b3148 libpcap.A.dylib`pcap_stats + 4
    frame #1: 0x0000000100597d7d libvast.2020.dylib`::status() at pcap.cpp:127:18 [opt]
    frame #2: 0x00000001003755b8 libvast.2020.dylib`::send_report() at source.hpp:181:30 [opt]
    frame #3: 0x0000000100374b0e libvast.2020.dylib`::operator()() [inlined] operator() at source.hpp:261:12 [opt]

tobim · 2020-06-29T12:55:33Z

When I test this locally, I get the following warning in vast start:
2020-06-29T14:09:19.767 spawn_command got an error from spawn_component: none

I missed that. I think this is because of the eraser. spawn_node tries to create one but spawn_eraser returns early with ec::no_error if no aging query is configured. I believe this should be handled by explicitly not printing a warning in case of no_error.

tobim · 2020-06-29T13:04:35Z

I tried vast spawn source pcap -i em0 and get a segfault:

I don't know what's causing this, can you vast import pcap -i em0?

tobim · 2020-06-29T14:15:45Z

I haven't seen any unit or integration tests yet. What was your strategy in this regard?

Commands should be covered by integration tests in general. I'm still thinking about how this can be worked into the dsl.

mavam · 2020-06-29T18:23:25Z

I'm already getting a segfault when I run vast spawn source pcap, without any params.

Backtrace:

    frame #0: 0x000000010042b7b9 libvast.2020.dylib`::spawn_command() [inlined] __tree at __tree:1726:42 [opt]
    frame #1: 0x000000010042b784 libvast.2020.dylib`::spawn_command() [inlined] __tree at __tree:1721 [opt]
    frame #2: 0x000000010042b784 libvast.2020.dylib`::spawn_command() [inlined] map at map:1031 [opt]
    frame #3: 0x000000010042b784 libvast.2020.dylib`::spawn_command() [inlined] map at map:1032 [opt]
  * frame #4: 0x000000010042b784 libvast.2020.dylib`::spawn_command() [inlined] dictionary at dictionary.hpp:89 [opt]
    frame #5: 0x000000010042b784 libvast.2020.dylib`::spawn_command() [inlined] dictionary at dictionary.hpp:89 [opt]
    frame #6: 0x000000010042b784 libvast.2020.dylib`::spawn_command() [inlined] get<caf::dictionary<caf::config_value> > at settings.hpp:61 [opt]
    frame #7: 0x000000010042b765 libvast.2020.dylib`::spawn_command() at node.cpp:341 [opt]
    frame #8: 0x0000000100131cff libvast.2020.dylib`::run() [inlined] __invoke<caf::message (*const &)(const vast::invocation &, caf::actor_system &), vast::invocation &, caf::actor_system &> at type_traits:4425:1 [opt]
    frame #9: 0x0000000100131cf7 libvast.2020.dylib`::run() [inlined] invoke<caf::message (*const &)(const vast::invocation &, caf::actor_system &), vast::invocation &, caf::actor_system &> at functional:2848 [opt]
    frame #10: 0x0000000100131cf7 libvast.2020.dylib`::run() at command.cpp:433 [opt]

mavam · 2020-06-29T18:25:56Z

I don't know what's causing this [...]

How did you test invalid parameters on the command line? This always causes segfaults for me, like this one ☝️.

dominiklohmann · 2020-06-30T09:58:26Z

I'll give this a spin to try reproducing the issues locally.

dominiklohmann · 2020-06-30T11:39:00Z

I can reproduce the changes mentioned by @tobim locally on macOS.

The PCAP source spawned by the node hangs after forwarding to the importer once. Sadly we don't have good enough trace logs for the PCAP source, which is what we should start with.

I cannot, however, reproduce the issues mentioned by @mavam. I think something may be messed up on your end.

dominiklohmann · 2020-06-30T11:43:24Z

Upon further investigation, the PCAP source inside the node seems to be down after a while. Sending a status atom to the source returns with request_receiver_down after the configured timeout.

libvast/src/system/spawn_source.cpp

libvast/vast/system/source.hpp

When a line_range was created on stdin in an actor context, the actor would wait until data was available, and would not be able to handle other messages in the meantime. This happened with the `spawn source` command, which was an easy way to "dos" the node.

Co-authored-by: Matthias Vallentin <matthias@tenzir.com>

dominiklohmann · 2020-07-02T12:58:06Z

integration/default_set.yaml

+    fixture: ServerTester
+    steps:
+      - command: spawn source suricata -r @./data/suricata/eve.json
+      - command: import -b zeek


Can you add a comment explaining why this is necessary?

dominiklohmann · 2020-07-02T14:04:16Z

We should add changelog entries for the other fixes we made as well @tobim.

GCC warns about unused variables in conjunction with the comma operator inside of decltype expressions. This change unwraps the __VA_ARGS__ pack outside and silences the warning for each parameter individually.

tobim

I consider the changes by @dominiklohmann in this PR ready to merge.

dominiklohmann

Approving @tobim's side of things here.

Addressed.

tobim added the bug Incorrect behavior label Jun 26, 2020

tobim force-pushed the story/ch17783/fix-spawn-source branch from 406781c to 561a969 Compare June 28, 2020 21:14

tobim marked this pull request as ready for review June 29, 2020 09:44

tobim requested a review from a team June 29, 2020 11:28

mavam previously requested changes Jun 29, 2020

View reviewed changes

libvast/src/system/node.cpp Show resolved Hide resolved

libvast/src/system/node.cpp Outdated Show resolved Hide resolved

libvast/src/detail/line_range.cpp Show resolved Hide resolved

tobim force-pushed the story/ch17783/fix-spawn-source branch from 21a2df9 to da11c90 Compare June 29, 2020 14:13

mavam reviewed Jul 1, 2020

View reviewed changes

libvast/src/system/spawn_source.cpp Outdated Show resolved Hide resolved

mavam reviewed Jul 1, 2020

View reviewed changes

libvast/vast/system/source.hpp Outdated Show resolved Hide resolved

mavam reviewed Jul 1, 2020

View reviewed changes

libvast/vast/system/source.hpp Outdated Show resolved Hide resolved

tobim and others added 10 commits July 2, 2020 10:13

Fix spawn source subcommand

f7b72b9

Move merge_settings helper into a dedicated tu

b6cd096

Merge spawn source options into import section

e791656

Use idiomatic API to update settings object

1b746fd

Co-authored-by: Matthias Vallentin <matthias@tenzir.com>

Remove unused includes

ed4f1a3

Don't warn if a component is not needed

23edfa8

Fix produced event counter in PCAP reader

e540a9a

Print sender of duplicate exit message

0f6ebf9

Fix sink setup in source actor

b887299

dominiklohmann force-pushed the story/ch17783/fix-spawn-source branch from da47787 to 8bc4946 Compare July 2, 2020 08:18

dominiklohmann added 4 commits July 2, 2020 10:25

Improve formatting of export command invocations

711b3ea

Add missing spawn source subcommand invocations

d8fe83f

Enable spawn source subcommands for all formats

5a3d4ed

Remove unnecessary down message handler

0361ef9

dominiklohmann reviewed Jul 2, 2020

View reviewed changes

dominiklohmann added 6 commits July 3, 2020 08:11

Align test-reader with other readers

fbd8590

Improve exit message for remotely spawned source

8c022ac

Remove doubled up sink setup in source

fba64d4

Remove short option for import.test.seed

cfdb5f2

Simplify error logic in source/sink pairing

2ed3c6a

Fix segfault for an invalid import.pcap.interface

5212fdd

dominiklohmann force-pushed the story/ch17783/fix-spawn-source branch from 86a48e9 to e5ec970 Compare July 3, 2020 06:11

tobim force-pushed the story/ch17783/fix-spawn-source branch 2 times, most recently from dd97b28 to cb640ae Compare July 3, 2020 09:04

tobim and others added 8 commits July 3, 2020 11:16

Add an empty stats type to the suricata schema

6d40848

Add an integration test for spawn source

7dff0f8

Add a changelog entry for the suricata stats type

228a5b2

Disable spawning source/sink with --node enabled

d970e6d

Add changelog entry for pcap-reader change

6eadc50

Fix crash during merge of spawn options

58d0a94

Tell that the test source has no timeout support

d790d04

Fix unused variable warnings from log statements

d301f6f

GCC warns about unused variables in conjunction with the comma operator inside of decltype expressions. This change unwraps the __VA_ARGS__ pack outside and silences the warning for each parameter individually.

tobim force-pushed the story/ch17783/fix-spawn-source branch from cb640ae to d301f6f Compare July 3, 2020 10:33

tobim commented Jul 3, 2020

View reviewed changes

dominiklohmann approved these changes Jul 3, 2020

View reviewed changes

tobim merged commit aeb3f46 into master Jul 3, 2020

tobim deleted the story/ch17783/fix-spawn-source branch July 3, 2020 13:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix spawn source subcommand #954

Fix spawn source subcommand #954

tobim commented Jun 26, 2020

mavam commented Jun 29, 2020

mavam commented Jun 29, 2020

mavam commented Jun 29, 2020

tobim commented Jun 29, 2020

tobim commented Jun 29, 2020

tobim commented Jun 29, 2020

mavam commented Jun 29, 2020

mavam commented Jun 29, 2020

dominiklohmann commented Jun 30, 2020

dominiklohmann commented Jun 30, 2020

dominiklohmann commented Jun 30, 2020

dominiklohmann Jul 2, 2020

dominiklohmann commented Jul 2, 2020

tobim left a comment

dominiklohmann left a comment

Fix spawn source subcommand #954

Fix spawn source subcommand #954

Conversation

tobim commented Jun 26, 2020

mavam commented Jun 29, 2020

mavam commented Jun 29, 2020

mavam commented Jun 29, 2020

tobim commented Jun 29, 2020

tobim commented Jun 29, 2020

tobim commented Jun 29, 2020

mavam commented Jun 29, 2020

mavam commented Jun 29, 2020

dominiklohmann commented Jun 30, 2020

dominiklohmann commented Jun 30, 2020

dominiklohmann commented Jun 30, 2020

dominiklohmann Jul 2, 2020

Choose a reason for hiding this comment

dominiklohmann commented Jul 2, 2020

tobim left a comment

Choose a reason for hiding this comment

dominiklohmann left a comment

Choose a reason for hiding this comment