Skip to content
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

Separate 'analyzer' module #873

Merged
merged 14 commits into from
May 23, 2024
Merged

Separate 'analyzer' module #873

merged 14 commits into from
May 23, 2024

Conversation

psrok1
Copy link
Member

@psrok1 psrok1 commented Feb 16, 2024

To make things less hardcoded in the Karton service itself, let's move analysis process to the separate module called "analyzer"

Analyzer is parametrized by AnalysisOptions object that is composed from configuration file values and task parameters. analyzer.analyze_sample and AnalysisOptions interface is used both by DrakrunKarton and new CLI tool called drakstart that allows to run raw analysis process directly from CLI. It will be useful tool for setup and further development and debugging.

List of things implemented a bit different than in the original code:

  • Sample is downloaded to /tmp/drakrun/vm-<N>/sample instead of /tmp/drakrun/vm-<N>/<target_name>. The target file name (which is usually malwar.exe) is used only when writing file to the target VM.
  • metadata.json includes dumps_metadata
  • Network setup is done always at the beginning of the analysis instead of at the start of Karton. Network is also wiped at the end of the analysis, see analyzer.run_vm. It sounds a bit cleaner to me as user may change out_interface/dns_server/net_enable settings and we need to setup network accordingly.
  • Analysis is not retried only when "retryable" error occurs
  • Logging setup is changed, I do it in setup_logger and bind handlers to the root logger. It should fix the "double logging" problem.

@psrok1 psrok1 force-pushed the refactor/separate-analyzer branch 2 times, most recently from 5905ef9 to 77f836a Compare February 22, 2024 15:34
@psrok1 psrok1 force-pushed the refactor/separate-analyzer branch 6 times, most recently from bb3fbf7 to 78c9a0e Compare May 20, 2024 19:27
@psrok1 psrok1 marked this pull request as ready for review May 20, 2024 20:16
@psrok1 psrok1 requested a review from msm-cert May 20, 2024 20:16
Copy link
Contributor

@msm-cert msm-cert left a comment

Choose a reason for hiding this comment

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

Looks good! Left some comments for now

drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
Copy link
Contributor

@msm-cert msm-cert left a comment

Choose a reason for hiding this comment

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

I didn't read everything today, but I've reviewed it before and it looks good

output_dir: pathlib.Path
plugins: List[str]
timeout: int = 600
hooks_path: pathlib.Path = pathlib.Path(ETC_DIR) / "hooks.txt"
Copy link
Contributor

Choose a reason for hiding this comment

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

this default is used by analyzer at one place, but maybe it shouldn't have a default in the dataclass at all?

drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
drakrun/drakrun/analyzer.py Outdated Show resolved Hide resolved
@psrok1 psrok1 force-pushed the refactor/separate-analyzer branch from d676fef to fc8bc18 Compare May 23, 2024 10:32
@psrok1 psrok1 force-pushed the refactor/separate-analyzer branch from dc66501 to 67a8109 Compare May 23, 2024 13:04
@psrok1 psrok1 merged commit 2c04b44 into master May 23, 2024
8 checks passed
@psrok1 psrok1 deleted the refactor/separate-analyzer branch May 23, 2024 13:19
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.

None yet

2 participants