search

riboseinc / retrace

retrace is a versatile security vulnerability / bug discovery tool through monitoring and modifying the behavior of compiled binaries on Linux, OpenBSD/FreeBSD/NetBSD (shared object) and macOS (dynamic library).
Other
60 stars 19 forks source link

Load configuration via environment variable 'RETRACE_CONFIG_VAR' #335

Closed jjr840430 closed 6 years ago

jjr840430 commented 6 years ago

Implemented issue #334

jjr840430 commented 6 years ago

@drystone I think it would be better to modify the configuration of retrace at run-time. Then is it possible to use the wrapper script in this case?

drystone commented 6 years ago

@jjr840430 , no it won't be possible to modify config from the script.

I think it would be better to avoid the temporary file creation - you either have to create a temporary file with no way of deleting or use a predefined path (~/.something) that can cause problems if you have more than one instance of retrace.

How about modifying get_config to add the config from the environment to the config list and then load the config file as before, adding to the list?

It's moot anyway as config is about to receive a major overhaul.

ghost commented 6 years ago

Guys, Agree with @drystone . To summarize:

  1. look for the configuration env. variable. if exists parse and setup config, done
  2. look for the file configuration env. variable. if exists parse the file and setup config, done
  3. Use the default config, done
jjr840430 commented 6 years ago

Agreed, guys. I didn't regard the case of multiple instances of retrace. Thanks for your advice.

ribose-ci commented 6 years ago

Can't set status; build failed.

ribose-ci commented 6 years ago

Can't set status; build succeeded.

erikbor commented 6 years ago

@jjr840430 sorry I didn't have time earlier to review this. I agree that a temporary file is not the best way because that currently can be done outside of Retrace. Moving that into Retrace itself does not improve things.

And I also agree with configuration setup flow that @ikolomi described.

ronaldtse commented 6 years ago

@erikbor @jjr840430 are we ready to merge this?

jjr840430 commented 6 years ago

@ronaldtse I need to modify something here. I will let you know after doing.

jjr840430 commented 6 years ago

Currently retrace is parsing the configurations based on file pointer, so it needs to modify the most part of config parsing. If we need to have new config by JSON, then this pull request may be ignored.