-
Notifications
You must be signed in to change notification settings - Fork 3
Add ttc sampling and expected value to AttackGraphNode #128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
4b9cf7d
to
33b1332
Compare
Reading the wiki:
To me, this means "disable the step". What you suggest (setting a TTC of infinite) is similar but different. The agents can still attempt the step but never progress past it. However, according to the wiki, the step should not be "performable" at all. |
Removing the attack step completely might be the solution then. But maybe that is a choice to be taken by the simulator when it samples the ttcs and removing those that has a sampled ttc that disables the node. |
maltoolbox/attackgraph/node.py
Outdated
def sample(exponential: float, bernoulli=1.0): | ||
""" | ||
Generate a random sample for the given distributions. | ||
If bernoulli distribution is not given, the sample will | ||
simply be from an exponential. | ||
|
||
If the Bernoulli trial fails (0), return infinity (impossible). | ||
If the Bernoulli trial succeeds (1), return sample from | ||
exponential distribution. | ||
""" | ||
|
||
# If bernoulli is set to 1, the sample is just exponential | ||
if np.random.choice([0, 1], p=[1 - bernoulli, bernoulli]): | ||
return np.random.exponential(scale=1 / exponential) | ||
return math.inf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is too convoluted. Why not use np.random.exponential
directly in all cases in the lines below, except the bernoulli one where you do just random.choice? The way it is now, it seems like you are doing Bernoulli * Exponential, when the TTC in the mal spec does may not use the Bernoulli distribution at all and you have to default to the Bernoulli part returning 1 for such cases. While the result is the same, it is unnecessarily complex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After I wrote this, I realized that most of the distributions here are the codenamed ones, eg hard and uncertain which do use the Bernoulli distribution in their formula. But still others, like the pure exponential, do not and I still think that this could be simplified a bit.
Maybe it merits an online call to discuss some aspects of this.
maltoolbox/attackgraph/node.py
Outdated
|
||
# If bernoulli is set to 1, the sample is just exponential | ||
if np.random.choice([0, 1], p=[1 - bernoulli, bernoulli]): | ||
return np.random.exponential(scale=1 / exponential) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need numpy (a dependency). We can use random.expovariate
that is builtin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this will return a float. The TTC is an integer. There should be some rounding done here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we decide that the TTC is an integer?
So, I think I'd like two things to happen.
|
| expo [Exponential(0.1)] | ||
| berni [Bernoulli(0.5)] | ||
| composite [Exponential(0.1) * Bernoulli(0.5)] | ||
| compositeComplex [Bernoulli(0.5) * (Binomial(10, 0.1) + Exponential(0.2) - Gamma(0.1, 0.2) * LogNormal(5, 0.5) / Pareto(7, 0.25) ^ TruncatedNormal(6, 0.3) + Uniform(0.1, 0.9))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nkakouros just as a heads up, we don't need it right now since it will not be used any time soon, but the compiler does not handle the /
correctly. The expression simply terminates when it encounters it.
maltoolbox/language/languagegraph.py
Outdated
@@ -654,6 +654,7 @@ class LanguageGraph(): | |||
"""Graph representation of a MAL language""" | |||
def __init__(self, lang: Optional[dict] = None): | |||
self.assets: dict = {} | |||
self.load_predefined_ttcs() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs to be a method since all it does is to set an instance variable (predef_ttcs). I would either remove the method or have the method return the predefined ttc values so the instance variable is defined in __init__
.
p = float(probs_dict['arguments'][1]) | ||
# TODO: Someone with basic probabilities competences should | ||
# actually check if this is correct. | ||
return random.binomialvariate(n, p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My linter tells me this method does not exist in module random
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am also not the correct person to say if these sampling methods are correct :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.python.org/3/library/random.html#random.binomialvariate
I think the issue is that it was introduced in python 3.12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh alright, that would make mal-toolbox require 3.12 I guess so maybe it is worth to see if there is a different option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There isn't and something somewhere(maybe in the mal simulator) already requires 3.12. Some people trying to use it ran into it. So, I think we should just specify 3.12 as a requirement.
Looks good to me, but I can not say if the sampling methods are correct |
…ey can be used with any probabilities
I thinl it makes sense having ttc distribution sampling/expected value methods in the AttackGraphNode class since ttcs are part of the AttackGraphNode.
This way the sampling can be used in the simulator (or any other tool that wants to sample ttcs for attack graph nodes).
Problem is I don't understand fully how these distributions should work. I could only look in the traverser and the MAL documentation and try to understand it.
Need some expert knowledge and someone who knows how the distributions should work.
Note: