-
Notifications
You must be signed in to change notification settings - Fork 125
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
Feature/config refactor #201
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
And introduce a variable to make code easier to read
get_config_path() should work on an already-parsed config object, and not need to parse it again.
It will default to using the main MultiScanner config, but a different config can be passed in. The component parameter must be provided. This is a backwards-incompatible change as the order of the parameters has switched.
They do the same thing, and it makes more sense to handle this someplace other than the main scanning function.
It now just uses the config parameter. Config file handling and making sure missing sections or values has been relegated to main() in ms.py. We shouldn't need to rewrite config files every time we do a scan, just once when start MultiScanner.
Functions like multiscan() and _subscan() now take a list of module names instead of full paths. Renamed MODULESLIST -> MODULE_LIST. Stopped using globals as default parameters in certain functions so the functions will use the most up-to-date version of the global instead of the global's state when the function was defined.
- jQuery wasn't reading it correctly - ElasticSearch would break if SHA256 module wasn't enabled in the UI - Call multiscan() once per sample in non-distributed mode so we can select which modules to run per-sample
- Consolidated _rewrite_config functions from ms.py, storage.py and sql_driver.py into reset_config in config.py. - Renamed _update_DEFAULTCONF to update_paths_in_configs and put it in config.py. - The sql_driver version of the function allowed passing in a configuration to override the default values. However it was only used by api.py, which creates a sql_driver.Database instance and passes in api_config.ini's Database section config. The consolidated reset_config doesn't have a parameter for overriding a DEFAULTCONF, so api.py uses regenconfig=False on the Database constructor so the Database config section will only be regenerated if it doesn't already exist. - We never used the module_list option for config_init() in ms.py, so I removed it to simplify things.
Have to import config globals like CONFIG_FILE as qualified names because importing them by name copies them so we end up not modifying the global. Also moved config_init and _write_missing_module_configs to make it easier to see how they were called, in preparation for consolidating them into config.py.
Namely, _get_main_config, _write_missing_module_config, _rewrite_config.
...instead of dicts for main configs. Needed because write_missing_config() needs a ConfigParser object and it was being called from different places, sometimes as a dict, sometimes ConfigParser.
We can always start with the default config and then update it based on the config passed in.
to convert config values to Python literals (eg. "True" to a Python boolean)
- Removed _load_defalt from storage.py - Changed write_config and read_config to use a dict for the default config instead of a section name and dict of the values in that section. This allows passing in more than one section at once. - Fixed some misnamed parameters. - Fixed bug where Mock couldn't find the Metadefender module.
We were incorrectly replacing sys.argv, omitting the first term. It doesn't matter what we set it to, as argparse won't care. The try/catches are needed because 'init' calls `exit()`.
Also fix a couple config bugs
and use the same config handling code as the other configs
and fix a couple other minor things found in code review
- Similarly we now need to submit each item individually.
In the distributed case, we are obligated to pass a dict, but later we need to convert it back into a MSConfigParser. Essentially, for line 411 fallback="..."
emmanvg
approved these changes
Oct 4, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Re-tested on clean environment and was not able to reproduce past findings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #131, fixes #87.