Add feature to define custom properties (name/value pairs) to the TM and elements

[nozmore]

Add custom_properties dict for custom data to be inserted into the model which does not currently exist in pytm's data model.

This can be used for internal values that may be unique to a user/team/company. These values can be used in threatlib via target.getProperty('key') method and can be directly queried in report template using tm.props[key]. If the key does not exist a KeyError will be thrown. Once #155 is merged I will add a method to report_util.py to support querying without throwing a KeyError and also to support looping over all keys in the props dict. Once this is done I can update the template files to include this logic. This can be done first if #155 is able to be reviewed and approved or this can be merged and I will follow up in separate PR.

[ghost]

CodeSee Review Map:

Review in an interactive map

View more CodeSee Maps

Legend

[nineinchnick]

How is this better than extending pytm classes to add more attributes?

[nozmore]

Extending is ok if you are a python developer. Some developers/architects will view this as a tool, yes you write some code but it is minimal. And once we get proper json import/export working you won't need to write any code.

If a central security team where to want to extend it now you have to pass around copies of the extended classes or a forked version of pytm. I think this is another option to extend pytm functionality in a lightweight manor.

If we find that this is getting used for something common then we can incorporate it natively into the model.

[nineinchnick]

Extending is ok if you are a python developer. Some developers/architects will view this as a tool, yes you write some code but it is minimal. And once we get proper json import/export working you won't need to write any code.

If a central security team where to want to extend it now you have to pass around copies of the extended classes or a forked version of pytm. I think this is another option to extend pytm functionality in a lightweight manor.

You can define classes extending base pytm classes in your model file and make it self-contained. Extending pytm should be encouraged. Maybe we need some examples. @izar wdyt?

What's wrong with the current json import/export?

If we find that this is getting used for something common then we can incorporate it natively into the model.

How do we know it's used for something common?

[nineinchnick]

Also, you could just assign properties to an object, without defining them first.

>>> class A:
...     pass
...
>>> a = A()
>>> a.b = 'c'
>>> print(vars(a))
{'b': 'c'}
>>> print(a.b)
c

[colesmj]

For what it is worth, I like this change, and think it doesn't go far enough (but maybe saves the need to fork the project or spin out an incubation branch specifically).

Why not make all properties go into the dictionary? This would let us move in the future perhaps toward generic objects and allow properties to be set rather dynamically (or changed on a global scale through a config/JSON description of each object). Yes it requires some thought on rules, but the proposal here for how threatlib would access properties makes sense.

Would also allow creation of new object "types", or redefine existing ones without having to extend pytm specifically. There is a use case here when we look at non-coding ways to create models (i.e. using a front end processor to build models that pytm can consume).

[nineinchnick]

For what it is worth, I like this change, and think it doesn't go far enough (but maybe saves the need to fork the project or spin out an incubation branch specifically).

Why not make all properties go into the dictionary? This would let us move in the future perhaps toward generic objects and allow properties to be set rather dynamically (or changed on a global scale through a config/JSON description of each object). Yes it requires some thought on rules, but the proposal here for how threatlib would access properties makes sense.

Keeping properties as class attributes allows us to:

• document them
• have default values
• keep them immutable
• keep expressions in threat conditions simple element.attribute vs element.get("attribute")

You lose all of this when switching to a dictionary and you gain nothing, because you can already assign custom values as attributes, without defining them first.

Would also allow creation of new object "types", or redefine existing ones without having to extend pytm specifically. There is a use case here when we look at non-coding ways to create models (i.e. using a front end processor to build models that pytm can consume).

This is already possible, either with the json loader or by using kwargs to populate attributes when creating objects.

[nozmore]

Just to tease the comments apart, while the last reply applies to this PR I think it was more in response to the "use dictionaries for everything" comment.

For this PR, I think this provides a consistent way for anyone to add custom data to their TM and elements. I was originally mainly thinking of reporting, I have some internal metadata that I would add to elements that I do not believe would warrant inclusion in pytm however could be used for threat logic as well.

My point earlier was not all users are python developers and wouldn't know how or go thru the effort of extending objects, granted I missed an easier way to assign elements ... not being a native python developer and that I didn't know this so only proves that point.

I think the advantage here is this provides a consistent way to accomplish this goal. A security team can tell their developers to add custom details here, and once the template PR is merged I'll follow up with a PR to loop over these without having to know the key names. This way they can be included in the report and catch typos or misc custom data the dev team wants to add for their own purposes.

There have been some comments from several of us about decoupling the input model from the processing model, I think when we move in that direction this would be needed. We would have a strict input model based on a given version and only parse expected data when creating the processing objects, all custom data would be handled this way.

[nineinchnick]

For this PR, I think this provides a consistent way for anyone to add custom data to their TM and elements

Consistent with what?

Can you respond to my argument about being able to just assigning custom attributes without defining them?

I thought pytm is about thread modeling as code, not as JSON or other generic formats.

[nozmore]

The entire last comment was in response to your earlier comment.

I am thinking of this from my perspective as a member of a Product Security Team where I would parse and analyze a lot of models. Your technique is viable and for a single team adding a custom fact and creating the template this works fine. But for my case where I want to enable teams but have a consistent way to parse and report on models this doesn't work for me.

I know Izar has has TM as code in some presentations but its not on the readme anywhere, well indirectly because the sample model is code but the mantra is not. IMO Python is how the tool is built and using a.py file is one delivery method but doesn't have to be the only way. Even if we focus only on using Python and removed import I still find this PR valuable. And to your point you can do this other ways, adding this PR doesn't detract from those other method if that works for others, this is just another option.

[izar]

Tell you the truth, Matt and I have half a book around threat modeling with/from/as code and how pytm realizes that vision. I would say that while JSON is a nice addition, focusing on it is not exactly where I see pytm going. We do have the JSON export capability but I always saw that as a convenience rather than a central part of the project.

…

On Tue, Oct 12, 2021 at 9:49 PM nozmore ***@***.***> wrote: The entire last comment was in response to your earlier comment. I am thinking of this from my perspective as a member of a Product Security Team where I would parse and analyze a lot of models. Your technique is viable and for a single team adding a custom fact and creating the template this works fine. But for my case where I want to enable teams but have a consistent way to parse and report on models this doesn't work for me. I know Izar has has TM as code in some presentations but its not on the readme anywhere, well indirectly because the sample model is code but the mantra is not. IMO Python is how the tool is built and using a.py file is one delivery method but doesn't have to be the only way. Even if we focus only on using Python and removed import I still find this PR valuable. And to your point you can do this other ways, adding this PR doesn't detract from those other method if that works for others, this is just another option. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#183 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC2BAJ47TSICRAQ67UCZ2TUGTQTXANCNFSM5FMH2WFA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[nozmore]

haha well I forgot about the book. Either way that discussion is not really related to this PR, only in response to a comment I made about this being even more important with JSON import. Even using .py files I feel this is important for my usage and I think it could be valuable to include and document it for others.

I'll open an issue and we can talk about "tm as code" in a separate place.

[raphaelahrens]

@nozmore May I ask how you use pytm? What is the process for your team and the devs? I'm asking because to me it looks like what you care most about is how you can get the data out of the model the devs created and that t might not matter how it was created.
Second, how do the devs create the model? Do they use JSON or python?

I think it would helpful in understanding the problems you want to solve and how they could be integrated by nineinchnick and izar.

When you mostly care about the output, then the discussion can be shifted to the question, how can the output be generated, while not messing with the idea or pytm.
For example in python you can use the __setattr__ method to check if a member variable is a custom property or not.

The python model could then look like this.

tm = TM("my test tm")
tm.isOrdered = True
tm.mergeResponses = True
tm.repo_url =  "https://github.com/izar/pytm"
tm.lucky_number = 7

And in the output could be

    "isOrdered": true,
    "mergeResponses": true,
    "name": "my test tm",
    "custom_props": {
           "repo_url": "https://github.com/izar/pytm",
           "lucky_number": 7
     }
}

Python is like wet clay and you can form it into any shapes and it can result in a well structured document for reports. Similar behavior could be achieved with the JSON import.