mal-lang / mal-toolbox

Apache License 2.0
4 stars 2 forks source link

Fix coloring of attack steps in neo4j #40

Closed nkakouros closed 3 months ago

nkakouros commented 3 months ago

This makes it so that attack steps follow the colors of the asset they belong to in neo4j.

andrewbwm commented 3 months ago

I'm a bit confused about this one. What was the issue and what was the intended solution? I just tested this again on the latest version and the colouring(node type) is the asset name for each attack step. Is this a problem, or was this fixed in one of the other branches?

nkakouros commented 3 months ago

For me, when I import a graph to neo4j the instance model has unique colors for each asset but the attack graph is all gray. With this PR, the steps in the attack graph are colored using the color of the asset they belong to making it easier to pinpoint attack steps.

andrewbwm commented 3 months ago

For me, when I import a graph to neo4j the instance model has unique colors for each asset but the attack graph is all gray. With this PR, the steps in the attack graph are colored using the color of the asset they belong to making it easier to pinpoint attack steps.

I still need some clarification, we did have a situation where the ids were off, but I think in the current iteration the asset name is what is used to create the nodes in which case the colouring should work just fine. So, maybe this was already resolved. Can you please check with the latest release version on main? I think this PR also changes it to the asset type rather than individual assets, which is probably not what we want, or was that the intention?

nkakouros commented 3 months ago

I see. Yes, the current main branch colors the attack graph too. My intention in this PR was to color the attack steps using the same color used to color the asset they belong to in the instance model graph. In pictures, this is what the current main branch does:

graph(2)

And here is what this branch does:

graph(3)

In the first case, the attack steps are easier to group visualize per asset but there is a disconnect with the color of the asset type in the bottom right instance model graph.

In the second case, the attack steps follow the same color as the assets in the instance model graph which is based on the asset type. It is easier to see which asset type an attack step belongs to but it is harder to distinguish between individual assets of the same type.

I don't know which approach is better. I think I like the current behavior (main branch) better but the disconnect between the colors of the two graphs puts me off. What do others think?

andrewbwm commented 3 months ago

In the first case, the attack steps are easier to group visualize per asset but there is a disconnect with the color of the asset type in the bottom right instance model graph.

In the second case, the attack steps follow the same color as the assets in the instance model graph which is based on the asset type. It is easier to see which asset type an attack step belongs to but it is harder to distinguish between individual assets of the same type.

I don't know which approach is better. I think I like the current behavior (main branch) better but the disconnect between the colors of the two graphs puts me off. What do others think?

The problem is that you cannot colour nodes based on anything except for their type. So, if we define their type on the asset type, which makes sense, we can never colour different assets of the same asset type differently. So, I think I will drop this pull request for now.