Spyderisk / system-modeller

Spyderisk web service and web client
Other
4 stars 4 forks source link

Bug in reverting from asserted TW Levels to default TW levels #214

Open mike1813 opened 3 weeks ago

mike1813 commented 3 weeks ago

There are three types of asset properties, each of which has an associated level:

These properties are created by the validator with default levels specified in the domain model. Users can then override the default levels by:

The new level entered by the user is saved in the system model asserted graph, which is interpreted as the authoritative source of this information. Any default level will be in the inferred graph, and should be ignored if there is an asserted value (but see below).

What this means is that once you have overridden some of these levels, the only way to remove them again is to find the user-entered values and set them back again. Originally, that was very difficult because there was no way to tell if any level was a default or an asserted value. To fix this, a button is now added to 'revert' to the default level, shown as an 'undo' icon (circular backward-facing arrow). Pressing this should remove the asserted level so it can be replaced by the inferred level.

However, this doesn't work correctly. The attached model demostrates this - it is a very simple model designed to work with any v6a domain model.

Issue 214 Test 01 asserted.nq.gz

This model contains a private space accessible from a public space. Validation creates no new assets or relationships, but adds TWA, Control and Misbehaviour properties to the assets, along with some threats. The default assumed TW levels for the OccupantTW attribute of the two spaces are 'Safe' and 'Very Low' respectively. If no controls are selected, then the untrustworthy occupants of the public space can intrude in the private space, increasing the likelihood for PhysicalIntrusion in that space. The default impact level for this is 'Negligible', so the risk level is initially 'Very Low'.

With the impact level in a Misbehaviour, everything works as expected. If you change the impact level for PhysicalIntrusion in the private space to (say) 'High', the level changes in the display and the revert button appears next to it. If you then calculate the risks, you get a far higher risk level for this behaviour. If you revalidate at this point, the impact level is still set to 'High' and the revert button is still visible. If you then press the 'revert' button, the display goes back to the original level and the revert button disappears to show this is now a default level. If you then calculate the risks, the risk level goes back to 'Very Low'.

If you do the same with assumed TW levels or Control coverage levels, things do not work as expected. Changing the OccupantTW level in the public space to 'Medium' (instead of 'Very Low') seems to work OK: the display shows the new level and a revert button appears. Risk calculation produces a much lower likelihood of intrusion into the private space, bringing its calculated OccupantTW level to the same level as the adjacent public space. If you revalidate, the TW level in the public space is still set to 'Medium' and the revert button is still visible.

However, if you now press the 'revert' button, it disappears (indicating the asserted level has gone), but the display does not update. Risk recalculation still gives the lower likelihood of intrusion into the private space, showing that the old default TW level in the public space has not been used. If you revalidate at this point, the default value is restored - so it looks like the override level has been removed from the asserted graph, but for some reason this doesn't affect the risk calculation unless you revalidate (which takes longer, so not something we would want to do).

If you change the assumed TW level before doing any risk calculation, then press the revert button immediately, it works as expected.

The same problem exists with control coverage levels, but the effect is obscured by a separate bug in the risk calculator. If you select the ChipAndPinLock control at the private space, it has a default coverage level 'Safe', and it prevents the intrusion from the public space. If you reduce the coverage level to (say) 'Medium', the new level is shown and a revert button appears. Pressing the revert button causes it to disappear, but doesn't immediately reset the level unless no risk calculation has yet been performed. However, there is a bug in the risk calculator such that the reduced control coverage has no effect on the calculated PhysicalIntrusion likelihood, so it is harder to work out what might be happening.

Based on what happens with the assumed TW levels, I can think of two possible reasons why this issue may have arisen:

  1. The update may be affecting only the asserted graph, leaving data in the inferred graph which is used in the subsequent risk calculation. That would explain why the subsequent risk calculation doesn't use the default assumed TW level, but not why the display fails to update, nor why the update doesn't affect the inferred graph.
  2. The assumed TW levels (and control coverage levels) come in triplets if the asset class is non-singleton, with three levels indicating the TW level for the least trustworthy, average and most trustworthy members of the population. These levels are not independent, so when a user-defined value is removed, some calculation is needed to figure out what should happen to the levels across all three members of the triplet.

On the second point, the problem is simply that the TW level for the least trustworthy member of a population can't be higher than that of the average member, which can't be higher than that of the most trustworthy member. If these constraints are not met, the risk calculator makes an adjustment on the fly, and those adjustments are applied even to the asserted graph when you revalidate. If you change a user-defined level, that could cause the constraints to be violated unless the levels of the other two members of the triplet are adjusted, so there should be some sort of calculation in the API to work out how to update the display for all three members. I suspect that was left out when the revert button was introduced, and we just never got around to fixing it.

Note that the impact levels across a triplet of misbehaviours are independent of each other, so hitting the revert button requires only that a default value from the domain model is reinstated for one misbehaviour, and this value doesn't depend on any other impact levels.

kenmeacham commented 3 weeks ago

This is quite a complex issue - actually it seems several (possibly related) issues in one. Anyway I have started by trying to replicate the scenario using your example model, initially for the TW levels.

If you do the same with assumed TW levels or Control coverage levels, things do not work as expected. Changing the OccupantTW level in the public space to 'Medium' (instead of 'Very Low') seems to work OK: the display shows the new level and a revert button appears. Risk calculation produces a much lower likelihood of intrusion into the private space, bringing its calculated OccupantTW level to the same level as the adjacent public space.

Firstly I'm starting with a validated model, with future risks calculated (saved). No problem at this stage with setting (or resetting) the OccupantTW level. With this set to Medium, I then run the risk calc - the Calculated value goes to Medium (matching the Assumed level)

If you revalidate, the TW level in the public space is still set to 'Medium' and the revert button is still visible.

Yes. And the Calculated value goes to "N/A" as expected.

However, if you now press the 'revert' button, it disappears (indicating the asserted level has gone), but the display does not update.

NO. I see different here. After validation, (see above) I press the revert button and the TW value reverts to the default (Very Low).

Risk recalculation still gives the lower likelihood of intrusion into the private space, showing that the old default TW level in the public space has not been used. If you revalidate at this point, the default value is restored - so it looks like the override level has been removed from the asserted graph, but for some reason this doesn't affect the risk calculation unless you revalidate (which takes longer, so not something we would want to do).

It seems that the behaviour you are observing happens after revalidation AND risk calc. In this case (with both Assumed and Calculated at Medium, if I press the revert, both values remain at Medium, although the revert button disappears (as expected).

If you change the assumed TW level before doing any risk calculation, then press the revert button immediately, it works as expected.

Agreed - and this seems to match what I observed (that validation alone did not break anything).

The same problem exists with control coverage levels, but the effect is obscured by a separate bug in the risk calculator.

If you select the ChipAndPinLock control at the private space, it has a default coverage level 'Safe', and it prevents the intrusion from the public space. If you reduce the coverage level to (say) 'Medium', the new level is shown and a revert button appears. Pressing the revert button causes it to disappear, but doesn't immediately reset the level unless no risk calculation has yet been performed.

Agreed. Only the validation actually resets the coverage level back to the default (Safe).

I'm part-way through some investigations and will report when I have some clues. Just wanted to clarify how to replicate the issue(s) for now..

kenmeacham commented 3 weeks ago

@mike1813 - quick question in the meantime: as I understand it, the validator adds a "hasAssertedLevel" into the inferred graph to represent the default assumed value, right? In the current example, the OccupantTW for the PublicSpace is set to VeryLow (domain#TrustworthinessLevelVeryLow).

Can you confirm that any subsequent risk calc should NOT change the value of the default assumed value (hasAssertedLevel in the inf graph)?

The user asserted value goes into the hasAssertedLevel in the system graph, and the calculated value becomes the hasInferredLevel value in the inf graph.

Did I undestand correctly?

kenmeacham commented 3 weeks ago

I have done some tests and exported the system models at different points; this seems to have located some anomalies. Here are the test files I've generated:

1) validated.nq.gz Imported system model, validated (but not risk calc) 2) tw-asserted.nq.gz As above, with the TW asserted as Medium 3) tw-asserted-rc.nq.gz As above, then risks calculated 4) tw-reverted-norc.nq.gz After reverting the TW in the UI (no further risk calc)

In each case, I looked at the OccupantTW in the PublicSpace, i.e. searching for this TW URI in the exported file(s): system#TWAS-OccupantTW-8807e8f2 In the below results, I've just shown the relevent quads, i.e. those for asserted and calculated TW levels, and have simplified the URIs for clarity N.B. some are in the system graph and some are in the inferred graph.

1) Validated <system#TWAS-OccupantTW-8807e8f2> <core#hasAssertedLevel> <domain#TrustworthinessLevelVeryLow> http://it-innovation.soton.ac.uk/system/67210860e7a33c2a5f3cd4d8/inf .

Here, validation has correctly set the default TW level to VeryLow. This is stored in the inferred graph, as an "asserted" level.

2) TW asserted as Medium <system#TWAS-OccupantTW-8807e8f2> <core#hasAssertedLevel> <domain#TrustworthinessLevelVeryLow> http://it-innovation.soton.ac.uk/system/67210860e7a33c2a5f3cd4d8/inf . <system#TWAS-OccupantTW-8807e8f2> <core#hasAssertedLevel> <domain#TrustworthinessLevelMedium> http://it-innovation.soton.ac.uk/system/67210860e7a33c2a5f3cd4d8 .

As above, but the user asserted value (Medium) has been added to the system graph. OK so far..

3) Risks calculated <system#TWAS-OccupantTW-8807e8f2> <core#hasAssertedLevel> <domain#TrustworthinessLevelMedium> http://it-innovation.soton.ac.uk/system/67210860e7a33c2a5f3cd4d8/inf . <system#TWAS-OccupantTW-8807e8f2> <core#hasInferredLevel> <domain#TrustworthinessLevelMedium> http://it-innovation.soton.ac.uk/system/67210860e7a33c2a5f3cd4d8/inf . <system#TWAS-OccupantTW-8807e8f2> <core#hasAssertedLevel> <domain#TrustworthinessLevelMedium> http://it-innovation.soton.ac.uk/system/67210860e7a33c2a5f3cd4d8 .

After risk calc, we have new inferred (calculated) value in the inf graph, as expected. The user asserted Medium value remains in the system graph. However you can see that the "asserted" value in the inf graph has (wrongly) been changed to Medium. This implies that the default value has changed!

4) Reverted TW <system#TWAS-OccupantTW-8807e8f2> <core#hasAssertedLevel> <domain#TrustworthinessLevelMedium> http://it-innovation.soton.ac.uk/system/67210860e7a33c2a5f3cd4d8/inf . <system#TWAS-OccupantTW-8807e8f2> <core#hasInferredLevel> <domain#TrustworthinessLevelMedium> http://it-innovation.soton.ac.uk/system/67210860e7a33c2a5f3cd4d8/inf .

Finally, we revert the user asserted TW value (i.e. this triple is simply removed). We are left with the current calculated (inferred) value and the (wrong) default/asserted value in the inf graph.

The effect in the GUI makes sense; the user asserted value has been removed, so the undo button disappears. However the value in the dropdown remains at Medium, as the default value has been corrupted!

Conclusion is thet the Risk Calculator modifies the default TW level (I'm assuming this is wrong?). Looking at the code, this appears to happen in RiskCalculator.java, around line 648:

// This is an old domain model, just initialise from the asserted graph if required
if(twasavgInput != null && twasavgInput.getAssertedLevel() != null) {
    twasavg.setAssertedLevel(twasavgInput.getAssertedLevel());
}
twasavg.setInferredLevel(twasavg.getAssertedLevel());

// Store the reinitialised TWAS in the inferred graph
querier.store(twasavg,"system-inf");

Here you can see the asserted level being set to the user asserted level, then the TWAS being stored in the inferred graph.

I hope the analysis is correct - @mike1813 - please have a look to see if you agree. Probably we then need to discuss how to fix this.

kenmeacham commented 3 weeks ago

Some further observations. The "else" clause referenced in the previous comment seems to kick in even for a recent domain model (supporting populations). I presume this code was also intended for a singleton asset?

In the main part of the "if" statement, this also adjusts "asserted" TW values in the inferred graph for population triples, so this overrides original default values too!

mike1813 commented 2 weeks ago

We need a specification for what each component should do with these default levels:

The three specifications should be consistent. It looks like at this point they are not, and hence what happens in the GUI depends on whether there was a revalidation since the last risk calculation, and whether the previously entered overriding non-default level was asserted before or after the last revalidation and/or risk calculation.

We need to review what happens in the validator and risk calculator. Assign this one to me and I'll look at it next week.