search

OWASP / pytm

A Pythonic framework for threat modeling
Other
907 stars 169 forks source link

Replaced AC22 with AC23 and AC24 #243

Closed raphaelahrens closed 4 months ago

raphaelahrens commented 5 months ago

As mentioned in #239 AC22 Credential Aging review the threat AC22 Credential Aging was not helpful.

This commit replaces AC22 with two new threats AC23 Credential Disclosure and AC24 Hardcoded Credentials.

AC23 checks if the lifetime of the credentials is LONG, MANAUL, or UNKNOWN. Currently there is no way to resolve this threat by changing the model, besides setting the a different lifetime.

AC24 warns against the use of hardcoded credentials.

raphaelahrens commented 5 months ago

@izar and @colesmj I would like your feedback on these two new threats. They are definitely not perfect, but I thought better to have a first draft.

Also I replaced AC22 since it is not clear how to proceed with old threats.

izar commented 5 months ago

These look good to me, thanks! I wonder if just for the sake of completeness we shouldn't have an entry for AC22 saying it has been superseded by 23 and 24, without any rule, mostly for documentation-in-place sake. @colesmj ?

raphaelahrens commented 5 months ago

@izar thanks. what about the fact "Currently there is no way to resolve this threat by changing the model, besides setting the a different lifetime.", meaning a shorter lifetime.

Is it ok to always get this finding, when using a large livetime or should there be a condition checking for a mitigation? Something like has_high_entropy?

izar commented 5 months ago

I am not sure you can't mitigate it by changing the model. If it absolutely needs to have lifetime validity, it can be better protected, you can add "show current sessions" functionality, etc.

Perhaps add values LIFETIME_CANT_CHANGE and use that to point to additional controls, and create a finding for LIFETIME?

raphaelahrens commented 5 months ago

I wonder if just for the sake of completeness we shouldn't have an entry for AC22 saying it has been superseded by 23 and 24, without any rule, mostly for documentation-in-place sake. @colesmj ?

Maybe we add a deprecated attribute to AC22 instead and when loading the JSON filter all threats with the deprecated attribute. The content of the attribute could also be the reason why it was detracted. The advantage would be that the old condition would still be intact.

Another idea would be to create a deprecated.json in add AC22 there.

I am not sure you can't mitigate it by changing the model. If it absolutely needs to have lifetime validity, it can be better protected, you can add "show current sessions" functionality, etc.

Perhaps add values LIFETIME_CANT_CHANGE and use that to point to additional controls, and create a finding for LIFETIME?

Sorry I'm not sure what you mean. I see that you could add and use a custom value by adding

user_to_web.data = Data(
   "password", isCredentials=True, credentialsLife=random_value
)

Or do you mean that in pytm there should be a Lifetime.CANT_CHANGE?

izar commented 5 months ago

Maybe we add a deprecated attribute to AC22 instead and when loading the JSON filter all threats with the deprecated attribute.

Good idea!

Or do you mean that in pytm there should be a Lifetime.CANT_CHANGE?

That.

raphaelahrens commented 4 months ago

@izar Is there something missing, which I can do?

izar commented 4 months ago

Sorry, dropped this one from my radar. Merged. Thanks!!