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

Added support for hashed directories #8

Merged
merged 3 commits into from
Sep 18, 2018

Conversation

xme
Copy link
Contributor

@xme xme commented Sep 18, 2018

If you use CertStreamMonitor to track many websites like me, the number of JSON files created in the alerts directory may quickly become unmanageable.
I added support for hashed directories to create sub-directories based on the server current time.
You can define an alerts directory in the config like:

./alerts/%%Y/%%m/%%d

And sub-directories will be created to store alerts at each new scanhost.py execution.

What do you think about an option to create hashed directories based on the FQDN?
Ex:
./alerts/be/rootshell/blog/

A bad indentation caused a wrong processing of the command line arguments.
@xme
Copy link
Contributor Author

xme commented Sep 18, 2018

Small bug fix (indentation)

@cbrocas
Copy link
Contributor

cbrocas commented Sep 18, 2018

Hi Xavier!
Xavier, thank you very much for this useful PR!

We see two modifications that can be useful before merging :

  • conf/example.conf: do not modify the Alerts_dir parameter value in the configuration file (ie not adding /%%Y/%%m/%%d)
  • def generate_alert_dir(path): append (an not replace so) /%Y/%m/%d values to the directory name generated by this new function

The main benefits would be :

  • we keep your feature (adding timestamp hashed alerts sub-directories)
  • the feature is strictly compatible with current installations and implies no modification at all of the configuration file for existing users.
  • it requires no migration code to handle previous version of the Alerts_dir keyword (ie without any reference to time/date)

Agree with this proposal ?

@xme
Copy link
Contributor Author

xme commented Sep 18, 2018

Agree but this means that the user won't be able to select the directory hierarchy fitting best for him.
Ex:
/YYYY/mm
/YYYY/mm/dd
/HH/MM

My change is compatible with existing config files because nothing is replaced if no "%" is found :)

I've almost implemented the other suggestion to make sub-dirs based on the FQDN... PR on its way :)

@cbrocas
Copy link
Contributor

cbrocas commented Sep 18, 2018

Retro compatibility : and ... you are right ! OK for me.

@cbrocas cbrocas merged commit d5f9a8b into AssuranceMaladieSec:master Sep 18, 2018
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