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

Update Dump Json and jq Implementation #91

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ajread4
Copy link
Contributor

@ajread4 ajread4 commented Jun 29, 2024

New commits address missing data within Event XML so that all necessary information is pulled from log data. Prior commits only pulled EventRecordID from within System section. New upgrades pull things like EventID, TimeCreated, Channel, etc. Also, proper usage with jq added so that output can be piped to jq for ease of analysis. New dataset added to tests folder as well!

@ajread4
Copy link
Contributor Author

ajread4 commented Jun 29, 2024

looks like all checks passed too!

@williballenthin williballenthin self-requested a review June 30, 2024 11:19
@ajread4
Copy link
Contributor Author

ajread4 commented Jul 11, 2024

Checking to see if you can merge! Let me know if there are any other issues

Copy link
Owner

@williballenthin williballenthin left a comment

Choose a reason for hiding this comment

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

program logic looks reasonable. see inline comment about formatting.

would you be up for adding a test case? if not, i can add it after the merge.

thanks!

args = parser.parse_args()

with evtx.Evtx(args.evtx) as log:

# Instantiate the final json object
final_json = []
final_json=[]
Copy link
Owner

Choose a reason for hiding this comment

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

there are a lot of inconsistent formatting changes in this PR. would you please revert the ones that don't influence the program logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made changes to revert back, sorry about that!

@ajread4
Copy link
Contributor Author

ajread4 commented Jul 13, 2024

program logic looks reasonable. see inline comment about formatting.

would you be up for adding a test case? if not, i can add it after the merge.

thanks!

I can definitely try to add a test case, I will admit it is my first time adding one

@williballenthin
Copy link
Owner

Oh, cool! I'm happy to help out, either explaining ideas or answering questions. Thanks for everything you've done so far!

@ajread4
Copy link
Contributor Author

ajread4 commented Jul 13, 2024

Oh, cool! I'm happy to help out, either explaining ideas or answering questions. Thanks for everything you've done so far!

I want to create a test that runs the evtx_dump_json with the evtx data located within the data folder. However, I am having trouble calling the main function. I tried to import scripts but it was never able to import the module even after I added a init.py to the scripts directory. Any thoughts?

@ajread4
Copy link
Contributor Author

ajread4 commented Jul 13, 2024

image

This is how I am trying to set it up. But, I keep getting errors of the below.

image

@ajread4
Copy link
Contributor Author

ajread4 commented Jul 14, 2024

I wasn't able to figure it out, apologies!

@ajread4
Copy link
Contributor Author

ajread4 commented Jul 27, 2024

fixed issues with EventData missing key and added a UserData loop to cover newly discovered evtx data fields

@williballenthin
Copy link
Owner

(sorry I wasn't able to get this merged before I left for a little PTO. i have an explicit TODO item to merge this when i return. i hope that's ok. )

@ajread4
Copy link
Contributor Author

ajread4 commented Jul 28, 2024

No worries! Was just working on some other tasks and found something I needed to address here.

Didn't mean to come across as pressuring!

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