Adding uniqueId to get static references for findings

[per-oestergaard]

In my use case, I want to synchronize the findings with an external risk system. To be able to do so, I added these features -

1. Each element can be provided with a uniqueId. The idea is that these are specified when writing the model and thus static.
2. Findings will get a uniqueId which is a combination of the uniqueId and the findings's ID. In this way, these are also static and can be used as the primary key to the external risk system.
3. To make communication easier, the Element's uniqueId can be added to the name and thus become visible on the diagrams. This enables smoother dialog when talking about the output as talking about ABC becomes very precise.

I apologize upfront for these things -

• I only changed the pytm.py file to now. Any changes to README.md etc, I'll await your first feedback.
• I spend considerably time trying to get the value of the TM varStrings in the Element class. I could not find a way, so I added a global variable hack.
• I did not add any tests. Any advice on what tests I should add would be welcome.

[ghost]

CodeSee Review Map:

Review in an interactive map

View more CodeSee Maps

Legend

[izar]

Hey, thanks for the addition!

A suggestion on tests would be that the uniqueId stays constant between runs. You could create an external file with a set of uniqueIds, run a TM and see that the newly generated ones match with the ones in the file.

About the global variable, we are trying to stay away from those as much as possible. What was it you weren't able to access on Elements? Let's see if we can fix that.

[per-oestergaard]

A suggestion on tests would be that the uniqueId stays constant between runs. You could create an external file with a set of uniqueIds, run a TM and see that the newly generated ones match with the ones in the file.

I added test test_uniqueid_two_runs

[per-oestergaard]

About the global variable, we are trying to stay away from those as much as possible. What was it you weren't able to access on Elements? Let's see if we can fix that.

I definitely also want to avoid global contexts ;) The problem seems to stem from the "weird" (to be at least) way variables in TM are handled. Settings and getters seems to be used and the getter does not work globally.

If I break in init of Class Element -

    def __init__(self, name, **kwargs):
        for key, value in kwargs.items():
            setattr(self, key, value)
        self.name = nameUniqueIdFormat_shadow.format(name, self.uniqueId)
        self.controls = Controls()
        self.uuid = uuid.UUID(int=random.getrandbits(128))
        self._is_drawn = False
        TM._elements.append(self)

at the self.name line. My debugger evaluates nameUniqueIdFormat_shadow as expected to a string but TM.nameUniqueIdFormat what looks like class that will not reveal its value -

> /home/vscode/.local/lib/python3.9/site-packages/pytm/pytm.py(1337)__init__()
-> self.name = nameUniqueIdFormat_shadow.format(name, self.uniqueId)
(Pdb) p nameUniqueIdFormat_shadow
'{1}:{0}'
(Pdb) p TM.nameUniqueIdFormat
<pytm.pytm.varString object at 0x7fb60697cd30>
(Pdb) 

I cannot make it return the value

 p TM.uniqueFindingIdFormat.data
<WeakKeyDictionary at 0x7fb60697cca0>
(Pdb) p TM.uniqueFindingIdFormat.data('value')
*** TypeError: 'WeakKeyDictionary' object is not callable
(Pdb) p TM.uniqueFindingIdFormat.data['value']
*** TypeError: cannot create weak reference to 'str' object

If I instead break in say check of class TM, I get the same error for the TM (static) reference; however, it works using self -

> /home/vscode/.local/lib/python3.9/site-packages/pytm/pytm.py(839)check()
-> if self.description is None:
(Pdb) p TM.nameUniqueIdFormat
<pytm.pytm.varString object at 0x7f64acb45d90>
(Pdb) p self.nameUniqueIdFormat
'{1}:{0}'

Any ideas?

[izar]

If there's any confusion or error in the way varStrings works, it is probably due to my own mistake. It seems to be working everywhere else, what makes it unique to your case? I mean, not to be all "it works on my machine", which I am not, I just want to try and understand better the uniqueness of this case. I will try to run it later today (or this week) and see if I can figure something out. Perhaps varStrings needs an explicit get ?

[per-oestergaard]

The "only" special I see is that I try to access the value from another class. I do not see that anywhere else in the code.

[per-oestergaard]

If I subclass Element of TM:

class Element(TM):

I can access the values. However that have side-effects (like including too much in json dumps), so tests fails. I cannot create a new instance of TM tm = TM() as it has required values and I cannot provide them as I cannot get TM.name (a loop). I was also trying

TM.nameUniqueIdFormat.__get__(TM.nameUniqueIdFormat, TM.nameUniqueIdFormat.default)

But that seems only to get the default value.

Last, I tried adding

self.value = value

to the class var __set__. That works when I run the project, but tests fails with

======================================================================
ERROR: test_uniqueid_two_runs (tests.test_pytmfunc.TestTM)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/c/Users/redacted/pytm/tests/test_pytmfunc.py", line 299, in test_uniqueid_two_runs
    internet = Boundary("Internet",uniqueId="B1")
  File "/mnt/c/Users/redacted/pytm/pytm/pytm.py", line 1792, in __init__
    super().__init__(name, **kwargs)
  File "/mnt/c/Users/redacted/pytm/pytm/pytm.py", line 1325, in __init__
    self.name = TM.nameUniqueIdFormat.value.format(name, self.uniqueId)
AttributeError: 'varString' object has no attribute 'value'

----------------------------------------------------------------------

Any ideas?

[izar]

Well that goes beyond my Python-fu. @nineinchnick some insight perhaps?

[nineinchnick]

There's an order field, which name is unfortunate because that's actually an ID. It can be assigned automatically, as an incrementing number, or manually.

Can you check if that would solve your issue?

[per-oestergaard]

There's an order field, which name is unfortunate because that's actually an ID. It can be assigned automatically, as an incrementing number, or manually.

Can you check if that would solve your issue?

Hi @nineinchnick, pleased to meet your.

Do understand you question, you suggest that I use the order instead of adding the unique id? Perhaps that could be a solution to the uniqueness issue. However, I would still like to add the number to the drawings etc. so I would end up with the python issue anyway :'(

[nineinchnick]

There's an order field, which name is unfortunate because that's actually an ID. It can be assigned automatically, as an incrementing number, or manually. Can you check if that would solve your issue?

Hi @nineinchnick, pleased to meet your.

Do understand you question, you suggest that I use the order instead of adding the unique id? Perhaps that could be a solution to the uniqueness issue. However, I would still like to add the number to the drawings etc. so I would end up with the python issue anyway :'(

The finding holds a reference to both the threat and the element it was matched against, so you can access the threat id and the element id (order). You should be able to extend pytm to change the way it renders things. I'd be happy to help you with that, ping me on our Slack.

[per-oestergaard]

A way (around the Python issue, but perhaps also generic) could be to update display_name. I can see that order is added for Dataflow. To be backwards compatible, that would require a new boolean property say showOrder can could be set on where wanted. Not sure this can be set to TM, but I will look at it.

[per-oestergaard]

I have PR'ed another method #190 and thus closing this