Prevent input name collision with db name for refs (#6056) #6080

labo-flg · 2024-02-22T10:54:13Z

Several refs attributes have the same input name as their db name, resulting in bugs during stream event publishing.

If the attribute is multiple, the event published in stream contains null in patch.
If it's a single value, the stream event won't be sent and an error will be thrown (store update event error)

Proposed changes

on schema registration, check the names collisions and throw an error
fix all collisions found by prefixing the name
rename API keys that require changes (--> only MalwareAnalysisAddInput.sample to rename MalwareAnalysisAddInput.analysisSample

Related issues

Worker error when importing Network-Traffic object with nested properties #6056

Checklist

I consider the submitted work as finished
I tested the code for its functionality
I wrote test cases for the relevant uses case
I added/update the relevant documentation (either on github or on notion)
Where necessary I refactored code to improve the overall quality

Further comments

e2e tests would be added in #6073 to cover network traffic creation + ref, I won't add it here (or improve on it here once merge and rebase)

To test this PR, test the following:

open the raw stream at http://localhost:3000/stream
Create an observable > network traffic, with a random dst port
Open it, go to knowledge, and add a nested object
check single ref
- select an ipv4 address, and create a "src" relationship (not dst, you will end up in Having Network Traffic observable with a dst ref makes the observable listing crash #6070)
- BEFORE --> store update event, NOW -> no error
- check in stream the update event is correct
check multiple ref
- create 2 other network traffic using just some random DST port values
- select the inital network traffic, and create 2 "encapsulates" relationship with the 2 other network traffic
- BEFORE --> no store update event but NULL in the stream events, NOW -> no error, correct values in the stream events

labo-flg · 2024-02-22T10:57:04Z

opencti-platform/opencti-graphql/src/schema/stixRefRelationship.ts

None of the input names modified here are exposed in the graphql API

The refs can only be created through stixRefRelationShipAdd, using relationship_type key and it does not need to change there.

labo-flg · 2024-02-22T16:52:19Z

.drone.yml

@@ -32,16 +32,24 @@ steps:
    commands:
      - apk add build-base git libffi-dev cargo
      - pip3 install --upgrade setuptools
+      - cd opencti-platform
+      - OPENCTI_COMMIT=$(git rev-parse --short HEAD)
+      - echo [INFO] using opencti@$DRONE_COMMIT_BRANCH:$OPENCTI_COMMIT


just an improvement on the way, to finally have a clear feedback on what is the precise version used.

labo-flg · 2024-02-22T16:53:04Z

opencti-platform/opencti-front/src/schema/relay.schema.graphql

-  sample: String
+  analysisSample: String


This is the only breaking change to the API.

All other renaming are on attribute never exposed in the API for now.

Why not objectSample?

labo-flg · 2024-02-22T16:56:07Z

opencti-platform/opencti-graphql/src/database/middleware.js

-          if (dst === INPUT_FROM && isStixRefRelationship(type)) {
+          if (dst === 'from' && isStixRefRelationship(type)) {


this one was tricky.

"from" is a key used in 2 different context: it's the "from" of a relationship, and the "from" in an email message.

INPUT_FROM was defined for use with email message only, but it was also imported and used in places related to relationship.

Now that we renamed for email to emailFrom it's not working anymore for relationships. But it was never meant to work like that in the first place.

Now that email is now on INPUT_EMAIL_FROM, are you sure is still needed to not used the constant?

Most of the time the constant INPUT_NAME was not used, its' the string "from".
It was an input for email, but as for relationship it's not in any schema def, it's not an input.
We think it's an old confusion (email 'from' being used in place where we actually meant "from" of a relationship).

We removed the constant INPUT_NAME. It's not used in any schema def.

This part was causing the test "should all elements correctly deleted" in middleware-test to fail, because the "from" was not resolved for one of the entity (it was a stix ref relationship). and yes we think "INPUT_FROM" was used by mistake because it was "from" until we changed it to "emailFrom", since it was an input specific to email message. And it was the only place where it was used as a constant, in all other places in middleware we use the string 'from' directly.

richard-julien · 2024-02-22T17:40:42Z

opencti-platform/opencti-graphql/src/schema/stixRefRelationship.ts

-export const INPUT_TO = 'to';
-export const INPUT_CC = 'cc';
-export const INPUT_BCC = 'bcc';
+export const INPUT_SENDER = 'emailSender';


I thought we talk about pushing the objectXXX convention

the convention objectXXX produces the change contains -> objectContains
but objectContains was an old filter key still renamed on the fly to objects, so we can't use objectContains

one thing leading to another, we thought it would be better to prefix with the entity they're used for as per the schema description. It's the case for a lot of other keys.

Kedae

Need Julien eye on that

On it

SouadHadjiat · 2024-02-27T16:16:29Z

Related python client PR open for malwareAnalysis : OpenCTI-Platform/client-python#563

)

labo-flg commented Feb 22, 2024

View reviewed changes

labo-flg requested a review from richard-julien February 22, 2024 10:57

labo-flg mentioned this pull request Feb 22, 2024

[client] update to MalwareAnalysisAddInput api OpenCTI-Platform/client-python#563

Merged

labo-flg commented Feb 22, 2024

View reviewed changes

labo-flg marked this pull request as ready for review February 22, 2024 16:58

richard-julien reviewed Feb 22, 2024

View reviewed changes

labo-flg self-assigned this Feb 23, 2024

labo-flg added the filigran team use to identify PR from the Filigran team label Feb 23, 2024

SamuelHassine force-pushed the master branch 2 times, most recently from cdc32a6 to 6368e31 Compare February 24, 2024 18:13

lndrtrbn self-requested a review February 26, 2024 14:14

lndrtrbn approved these changes Feb 26, 2024

View reviewed changes

labo-flg requested a review from richard-julien February 26, 2024 16:37

Kedae linked an issue Feb 27, 2024 that may be closed by this pull request

Worker error when importing Network-Traffic object with nested properties #6056

Closed

Kedae previously requested changes Feb 27, 2024

View reviewed changes

richard-julien force-pushed the issue/6056 branch 2 times, most recently from f091b07 to 2f44eff Compare February 27, 2024 15:39

labo-flg and others added 11 commits February 27, 2024 17:37

[backend] update refs input names to avoid collision with db name (#6056

98eb4fa

)

[backend] rename MalwareAnalysisAddInput.sample to objectSample

3ea584d

[backend] rename to more specific names

307fa0b

[tool] improve drone logs

c321f70

[backend] fix name consistency

e566294

[tool] improve drone logs

e450c5c

[tool] improve drone logs

bf49319

[backend] fix input names from and to

ea2413c

[backend/frontend] update generated gql types

83fc6b3

[frontend] update after breaking changes to API

80ed335

[opencti] Align input for network traffic

60b029a

richard-julien force-pushed the issue/6056 branch from 2f44eff to 60b029a Compare February 27, 2024 16:37

richard-julien merged commit dd88a22 into master Feb 27, 2024
4 of 6 checks passed

richard-julien deleted the issue/6056 branch February 27, 2024 17:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent input name collision with db name for refs (#6056) #6080

Prevent input name collision with db name for refs (#6056) #6080

labo-flg commented Feb 22, 2024 •

edited

labo-flg Feb 22, 2024

labo-flg Feb 22, 2024

labo-flg Feb 22, 2024

richard-julien Feb 22, 2024

labo-flg Feb 22, 2024

richard-julien Feb 22, 2024

labo-flg Feb 23, 2024

SouadHadjiat Feb 23, 2024

richard-julien Feb 22, 2024

labo-flg Feb 23, 2024

Kedae left a comment

SouadHadjiat commented Feb 27, 2024

		if (dst === INPUT_FROM && isStixRefRelationship(type)) {
		if (dst === 'from' && isStixRefRelationship(type)) {

Prevent input name collision with db name for refs (#6056) #6080

Prevent input name collision with db name for refs (#6056) #6080

Conversation

labo-flg commented Feb 22, 2024 • edited

Proposed changes

Related issues

Checklist

Further comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kedae left a comment

Choose a reason for hiding this comment

SouadHadjiat commented Feb 27, 2024

labo-flg commented Feb 22, 2024 •

edited