USArmyResearchLab / Dshell

Dshell is a network forensic analysis framework.
Other
5.44k stars 1.14k forks source link

Override line-break delimiter for followstream module #131

Closed amm3 closed 3 years ago

amm3 commented 3 years ago

This may just be personal preference, but the line-break added to the end of each blob in followstream output makes typical flows more difficult to read (for me). Without the line-break, the output more closely approximates that of Wireshark's follow stream output.

dev195 commented 3 years ago

Using the premodule function to change the delimiter can have unexpected issues, but I can see what you are trying to do here. Instead, I would recommend the following changes:

These changes should get you what you want to see, and make colorout a bit more flexible.

amm3 commented 3 years ago

Good thoughts, and yes, I did have some reluctance to directly modifying instantiated object variables from within the decoder... as it can have unexpected issues. The down side to making the change in the __init__ though is that it wouldn't apply to command-line specified output modules (i.e. htmlout). Any ideas on how to cover that condition without using the premodule approach?

dev195 commented 3 years ago

There is the --oarg command line argument. It should allow users to run a command like this to get the output you want:

decode -p followstream example.pcap --oarg delim= -O htmlout

In that line, I'm using --oarg to set the delimiter (delim) to nothing (the blank space after the equal sign).

However, I took a moment to review the code for the output modules you mentioned, and neither would allow this because they hardcode their delimiters.

Give me a bit of time to review and update the output modules to consistently handle the delim argument, then you and other users should be able to use any preferred delimiter.

amm3 commented 3 years ago

Overall, I think that would work. But my preference would still be to have a mechanism for the decoder/plugin to specify its preferred behavior for the output. Maybe this would be better specified in the call to write() rather than defining it in the initialization to the output module itself. Any thoughts on that?

dev195 commented 3 years ago

I think I see what you're saying. The plugin developers should have an option to overwrite the output defaults with defaults of their own. That might take a bit of work to implement consistently, since I see a hierarchy of preference here:

user preference (--oarg) > plugin preference > default

Something (ideally, not the plugin, since that would put more of the work on the plugin writers) would need to perform checks to determine which to use. I can see that happening at initialization or write time.

amm3 commented 3 years ago

Well said. Yes, that's what I'm going for and I think that's the correct hierarchy. I think updating the output modules (as you said before) to consistently handle the delim arg is a good first step and provides a workable short-term fix as well. Definitely interested in a longer term solution to implement the full hierarchy.

amm3 commented 3 years ago

139 was a needed improvement and gets us most of the way there. I'll close this and reevaluate. Thanks.