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

Add value validation for iocs #130

Merged
merged 1 commit into from
Aug 13, 2022

Conversation

Matthijsy
Copy link
Contributor

Currently the IoC values are allowing everything as value. However, it might be good to check that they match the value of the selected type. This PR implements this by adding a regex field to IocType, which will be checked during validation.

Currently I only added regex for the hashes, and the filename|hash types. But after this method is in place it is easy to add more regex values. If a user decides to add a custom type it also easy to add a regex, since it is expose to the UI as well.

@whikernel whikernel changed the base branch from master to develop August 5, 2022 06:06
@whikernel whikernel self-requested a review August 5, 2022 06:07
@whikernel whikernel self-assigned this Aug 5, 2022
@whikernel whikernel added the enhancement New feature or request label Aug 5, 2022
@whikernel whikernel added this to the v1.5.0 milestone Aug 5, 2022
Copy link
Contributor

@whikernel whikernel left a comment

Choose a reason for hiding this comment

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

That's great, thanks a lot!
In the alembic migration script we need to add the IOC validation regex you set in post_init, otherwise on existing installation the post_init script will duplicate the entries as it will not detect them as existing. (create_safe checks an exact match on all specified fields).

You can either add it in the migration script, or we do. Let us know what you'd prefer.

Thanks again for the PR!
Cheers

@Matthijsy
Copy link
Contributor Author

Thanks for your feedback! I did not know that. I just tried adding them to the alembic migration script, however then it does not set them anymore in case the database is still empty. Does that mean I have to add them to both the alembic script and the create_safe method? Since that would mean quite some work to define everything twice

@whikernel
Copy link
Contributor

Thanks for your feedback! I did not know that. I just tried adding them to the alembic migration script, however then it does not set them anymore in case the database is still empty. Does that mean I have to add them to both the alembic script and the create_safe method? Since that would mean quite some work to define everything twice

Yes unfortunately in the way it's working you need to define it twice. Once for the instances upgrading, and once for the new instances. The issue is that alembic wasn't used from the start, so the alembic migrations are not defining the whole database and thus the post_init was/is needed.

We can add them in the migration script if you want. In that case I'll merge now. Otherwise I'll wait for your update.

@Matthijsy
Copy link
Contributor Author

Matthijsy commented Aug 13, 2022 via email

@whikernel
Copy link
Contributor

Sure thing!

Okay, no problem. If you can add them that would be great! Op za 13 aug. 2022 08:16 schreef whitekernel @.***>:

Copy link
Contributor

@whikernel whikernel left a comment

Choose a reason for hiding this comment

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

@dfir-iris/devs - need to add migration of IOCs for existing IOCs - see #130 (review)

@whikernel whikernel merged commit 03fb5de into dfir-iris:develop Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants