Add timeout command to binwalk call. Related to FACT_core #820 #94
Conversation
Timeout is set to 580 as the docker image has a 600 seconds timeout, so
580 seconds should be a safe number so partial results could be
extracted.
| @@ -22,7 +22,7 @@ def unpack_function(file_path, tmp_dir): | |||
| ''' | |||
|
|
|||
| logging.debug('File Type unknown: execute binwalk on {}'.format(file_path)) | |||
| output = execute_shell_command(f'binwalk --extract --carve --signature --directory {tmp_dir} {file_path}') | |||
| output = execute_shell_command(f'timeout 580 binwalk --extract --carve --signature --directory {tmp_dir} {file_path}') | |||
There was a problem hiding this comment.
what about turning this a config parameter?
There was a problem hiding this comment.
There was a problem hiding this comment.
Hi @rhelmke, where should the configuration option should be located? I can do it if it is only a change to fact_extractor. If a change to fact_core is required I think it is best if your team do it, as it needs more coordination.
There was a problem hiding this comment.
How did you get to the conclusion that the timeout for the docker image is 600? The call in FACT (FACT_core/src/unpacker/unpack_base.py:28) doesn't set a timeout and the default value (in FACT_core/src/helperFunctions/docker.py:41) is 300 seconds. So maybe it would make sense to also increase the timeout in FACT.
What is more, the "generic carver" is also called as a fallback if another unpacking method fails so it could make sense to increase the time buffer a bit.
As far as placement for the configuration is concerned, there currently is no place for putting configuration of unpacking plugins. In "FACT_core" you can add configuration for each plugin in main.cfg but the development of FACT_core and the extractor diverged quite a bit since the latter moved to its own project. We could rework how extraction plugins are structured and use a similar class-based approach as in FACT_core but this would definitely be out-of-scope for this PR. Therefore, IMHO it would probably be best to set it as a constant inside the plugin (fact_extractor/plugins/unpacking/generic_carver/code/generic_carver.py).
There was a problem hiding this comment.
What might be interesting for you: We identified a "bug" in the helper library common_helper_unpacking_classifier which led to unpacking taking far longer than necessary for large files because of an overly complicated stats generation (see fkie-cad/common_helper_unpacking_classifier#4). With that fix, the chance of timeouts happening should also be a lot smaller.
There was a problem hiding this comment.
Thanks @jstucke, I will look at that fix. You are probably correct, I got the 600 seconds value from how long FACT ran in my test before showing the exception, the difference was probably due to uncompressing other files before the one that causes the error.
I changed the timeout to 280, and added the constant to the plugin.
There was a problem hiding this comment.
Converting to draft as the changes are going do cause unneeded timeouts when using fact_extractor in standalone mode. I think the correct way to implement this should be by getting the timeout value from fact_core as a parameter.
There was a problem hiding this comment.
Thanks @jstucke, I will look at that fix.
After the merge of #95 the docker image of the extractor was rebuilt and pushed to DockerHub so you could simply pull the updated image to try it out. I would be very much interested in knowing how well that change in itself alleviates your timeout problems.
Changing timeout to 280 as the docker image has a 300 seconds timeout.
Timeout is set to 580 as the docker image has a 600 seconds timeout, so 580 seconds should be a safe number so partial results could be extracted.