Skip to content
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

Replaced AC22 with AC23 and AC24 #243

Merged
merged 2 commits into from
May 28, 2024
Merged

Conversation

raphaelahrens
Copy link
Contributor

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
Copy link
Contributor Author

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

As mentioned in izar#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.
@izar
Copy link
Owner

izar commented Apr 24, 2024

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
Copy link
Contributor Author

@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
Copy link
Owner

izar commented Apr 24, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

izar commented Apr 24, 2024

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.

When a threat in `threats.json` has a `DEPRECATED` attribute the threat
will be ignored.
The value of `DEPRECATED` is irrelevant for pytm, but it can describe
the reason for the deprecation.
@raphaelahrens
Copy link
Contributor Author

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

@izar izar merged commit b4ac2e0 into izar:master May 28, 2024
4 checks passed
@izar
Copy link
Owner

izar commented May 28, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants