WIP: Customizable transfer files

[ethack]

General approach:

• Config file that determines the files that are sent. Located on AC-Hunter system.
• Script pulls config file from AC-Hunter system first.
• Script pushes logs according to config file to AC-Hunter system.

Closes: #3

[ethack]

@SamuelCarroll
Took a first pass at a review.
There looks to be some intend difference, maybe tabs vs. spaces? Can you fix those up as well please?

[ethack]

I don't think the backticks are necessary here.

Please quote the variables.

[ethack]

I'm not sure if the spaces around ${zeek_logs[*]} are intended. I don't think the *"all"* needs to be quoted. In general, things in [[ ]] don't need to be quoted. But strings with spaces or variables in [ ] or elsewhere do need to be quoted.

[ethack]

Can you reformat these to be one per line and sorted alphabetically? That'll be easier to tell at a glance if we're missing a file or not.

[ethack]

Same comments here. I'm not sure if the spaces around ${zeek_logs[*]} are intended. I don't think the *"${log}"* needs to be quoted.

[ethack]

I think this will evaluate to a string, and I think it should be quoted: "$(eval "$query")"

[SamuelCarroll]

Testing this works with quotes around the whole command "$(eval $query)" or around query like so $(eval "$query") but I can't have quotes around both like "$(eval $"query")"
I will push up a change for the quotes around the whole command

[ethack]

You can have quotes like "$(eval "$query")" (but not like in your comment with $"query"). From other languages, you'd think that the nested quotes would conflict with each other but $() is a subshell and my understanding is any quotes inside are not matched with anything outside it.

Here's an example:

$ cat test.sh
#!/bin/bash

print_num_args() {
    echo $#
}

args="one two"

# this treats $args as two separate arguments
how_many="$(print_num_args $args)"
echo "without inner quotes: $how_many"

# this treats $args as a single argument
how_many="$(print_num_args "$args")"
echo "with inner quotes: $how_many"

$ bash test.sh
without inner quotes: 2
with inner quotes: 1

So the difference is how bash treats the argument when it has whitespace in it. Bash will split unquoted arguments on whitespace and quoted arguments will not be split.

That being said, I think you're ok in this case. I read the help page and eval takes multiple arguments and just concatenates them together with a space before running.

And I also just learned something. The quotes outside the subshell appear to be unnecessary even with whitespace. Take a look at this example:

$ cat test.sh       
#!/bin/bash

test1=one two three
echo "$test1"

test2=$(echo one two three)
echo "$test2"

The only difference in setting these two variables is that one is in a subshell and one is not. They are both unquoted. But the one in the subshell works and the other doesn't.

$ bash test.sh      
test.sh: line 3: two: command not found

one two three

So with the behavior of eval and subshells combined, I think quotes are completely unnecessary for this particular case. All of these should work and have slightly different nuances. And now I prefer your original line of code lol.

• $(eval $query)
• $(eval "$query") - This will likely be my pick in my code going forward since it reduces the quote soup. Until I learn about some new nuance of bash that bites me in the future.
• "$(eval $query)"
• "$(eval "$query")"

Bash is weird 🤦‍♂️

[ethack]

@SamuelCarroll Round two :)

[ethack]

Instead of pwd we should copy the file to a place we know is writable by the current user. Here's some sample code that might be useful.

recv_dir="$(mktemp)"
recv_loc="$recv_dir/zeek-log-transport.yaml"
# clean up the temporary files on exit
trap "rm -rf '$recv_dir'" EXIT

[ethack]

There are cases where we will want different retention and logs from different sensors. If the sensor is small enough we can transfer all the logs and keep them for longer, but another sensor's logs might be too big where we need to limit what is sent and only keep it for a limited time.

I can think of a couple ways to do this:

• We could pull from a sensor-specific location first (e.g. $remote_top_dir/zeek-log-transport.yaml) and fall back to /etc/AI-Hunter/zeek-log-transport.yaml if that fails. This seems the most straightforward to me.
• We could put sensor-specific config sections in the same /etc/AI-Hunter/zeek-log-transport.yaml file. This would be easier if we were actually parsing structured YAML somehow, but it might be too difficult with the current method of using grep.

[ethack]

Can you change the default here to be 3 just to match the current behavior?

[ethack]

It's not often this would happen, but adding a new log file would mean having to update this list and thus the installed script (which is what we're trying to avoid having to do). For instance, AC-Hunter recently added a new log file open_conn.log. If a user installs a plugin that generates a new log file or Zeek happens to start producing a new log we wouldn't be able to get that log without updating this script.

What if we just tested the string against a regex and only allowed ^[a-zA-Z0-9-_]+$? I'm not aware of anything that could be abused path traversal-wise with only those characters and I'm willing to take the tradeoff of allowing more character combinations in exchange for being more future-proof.

[ethack]

Since the headings don't do anything, I think it would be less confusing to make all the individual log files nested directly under Logs: (so de-indent one level) and make the Default:, Troubleshooting:, and All: comments instead of YAML keys.