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

new: [analyst-data] Added initial support of analyst data concept and functions - WiP #1205

Closed
wants to merge 3 commits into from

Conversation

mokaddem
Copy link
Contributor

@mokaddem mokaddem commented Apr 23, 2024

Usage example

eventreport = MISPEventReport()
eventreport.name = 'Test'
eventreport.content = 'Content'
eventreport.add_note('This is a note')
eventreport.add_opinion(40, 'Wow')

attribute = MISPAttribute()
attribute.value = '8.8.8.8'
attribute.type = 'ip-dst'
attribute.add_note('Another note')

# `add_relationship` Accept either the type and UUID of the target as string, or the AbstractMISP object
relationship1 = eventreport.add_relationship('Attribute', str(uuid.uuid4()), 'Fake attribute')
relationship2 = eventreport.add_relationship(attribute, None, 'Existing Attribute')
print(eventreport.to_dict())

object1 = MISPObject(name='Fake object')
object1.add_opinion(65, 'Cool object')
print(object1.to_dict())

event = MISPEvent(info='Nice event')
note1 = event.add_note('Very cool indeed')
event.add_relationship(object1, None, 'This should also be part of that event')
pprint(event.to_dict())

note2 = note1.add_opinion(90, 'This note is awesome!')
print(event.Note[0].Opinion)

@Rafiot
Copy link
Member

Rafiot commented Apr 23, 2024

I started to poe around a bit and it looks pretty good (the mypy issues are mostly trivial to solve). But there is a problem with AnalystDataBehaviorMixin and AbstractMISP and the edited property.

Would it make sense to have AnalystDataBehaviorMixin as a subclass of AbstractMISP?

@chrisr3d
Copy link
Member

chrisr3d commented Apr 26, 2024

Do you mean keeping the inheritance from all the different data structures (MISPAttribute, MISPObject, MISPEvent, and so on…) to both AbstractMISP and AnalystDataBehaviorMixin while making also AnalystDataBehaviorMixin inherit from AbstractMISP? (or even make all classes subclass of AnalystDataBehaviorMixin and AnalystDataBehaviorMixin subclass of AbstractMISP?)

We discussed it in the office, and to me it sounded like one of the reasons for implementing AnalystDataBehaviorMixin this way was to have the add_note, add_relationship, add_opinion methods in one place instead of having them in each class - i.e the classes for the structures that can have a note, relationship or opinion, which are almost all the data structures except a few ones, like Tag

My points were:

  • Right now methods common to different classes are implemented in each class
  • But at the same time it is a pretty new situation as the only methods that are common to multiple classes so far - except for generic methods like to_json, from_dict and so on - are add_tag and add_attribute, which are only common to MISPEvent and MISPAttribute

So in the end, I guess it is more a matter or implementation preference as both would work.
Moving the methods to MISPNote, MISPOpinion and MISPRelationship would comply with the current implementation of the different classes but require to add all the methods in the different classes
Keeping AnalystDataBehaviorMixin is a new implementation where it is an intermediate class between the existing ones and AbstractMISP

@Rafiot
Copy link
Member

Rafiot commented Apr 29, 2024

I'm fine with having the AnalystDataBehaviorMixin being a standalone class we use as an extra parent whenever we need it.

The reason I was talking of AbstractMISP as a potential parents to AnalystDataBehaviorMixin is in case we need its features applied to the members of AnalystDataBehaviorMixin. The main one being edited, which if True will make sure the timestamp key is removed from the json when dumped, allowing MISP to only update the members that have actually been modified instead of everything in the event.

@mokaddem
Copy link
Contributor Author

mokaddem commented May 3, 2024

Closing as we have another PR open here #1212

@mokaddem mokaddem closed this May 3, 2024
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

3 participants