pytm icon indicating copy to clipboard operation
pytm copied to clipboard

Replaced AC22 with AC23 and AC24

Open raphaelahrens opened this issue 1 year ago • 7 comments

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 avatar Apr 20 '24 12:04 raphaelahrens

@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.

raphaelahrens avatar Apr 20 '24 12:04 raphaelahrens

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 ?

izar avatar Apr 24 '24 12:04 izar

@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?

raphaelahrens avatar Apr 24 '24 14:04 raphaelahrens

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?

izar avatar Apr 24 '24 14:04 izar

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?

raphaelahrens avatar Apr 24 '24 15:04 raphaelahrens

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.

izar avatar Apr 24 '24 15:04 izar

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

raphaelahrens avatar May 14 '24 04:05 raphaelahrens

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

izar avatar May 28 '24 18:05 izar