OWASP / pytm

A Pythonic framework for threat modeling
Other
927 stars 176 forks source link

AC22 Credential Aging review #239

Open noloader opened 8 months ago

noloader commented 8 months ago

Hi Everyone,

Threatlib has an item for AC22 Credential Aging:

{
    "SID": "AC22",
    "target": [
      "Dataflow"
    ],
    "description": "Credentials Aging",
    "details": "If no mechanism is in place for managing credentials (passwords and certificates) aging, users will have no incentive to update passwords or rotate certificates in a timely manner. Allowing password aging to occur unchecked or long certificate expiration dates can result in the possibility of diminished password integrity.",
    "Likelihood Of Attack": "Medium",
    "severity": "High",
    "prerequisites": "",
    "condition": "any(d.isCredentials for d in target.data) and target.sink.inScope and any(d.credentialsLife in (Lifetime.UNKNOWN, Lifetime.LONG, Lifetime.MANUAL, Lifetime.HARDCODED) for d in target.data)",
    "mitigations": "All passwords and other credentials should have a relatively short expiration date with a possibility to be revoked immediately under special circumstances.",
    "example": "",
    "references": "https://cwe.mitre.org/data/definitions/262.html, https://cwe.mitre.org/data/definitions/263.html, https://cwe.mitre.org/data/definitions/798.html"
  }

We have learned that continuity is a better security property than rotation. Unexpected changes, like gratuitously changing public keys changing, is bad for security because it breaks pinning controls. And requiring users to rotate their password results in users choosing weaker and weaker passwords over time just to comply with a policy based on reading tea leaves.

The details and mitigation detailed in threatlib are completely wrong nowadays. They run counter to what we have learned from real world incidents and security usability studies. Nowadays we want public keys and passwords written in stone, and only changed if there is suspicion or proof of breach or misuse.

I think AC22 should be either removed from the model or limited in scope.

izar commented 8 months ago

Agreed. But being prepared for key rotation, on the other side, is an important requirement IMHO. Would you like to submit a PR with better details and mitigation?

raphaelahrens commented 8 months ago

What is the policy for changing an SID? Should this get a new SID or can this just be rewritten?

Further I would suggest to extract the Lifetime.HARDCODED condition and make this a new threat, since this is a design failure, which can not be fixed.

Here a first draft to change AC22:

Credential disclosure

If credentials (passwords and certificates) have a long lifetime their disclosure can have severe consequences, if the credentials cannot quickly be revoked and/or rotated.

Mitigations

All credentials need a possibility to be revoked immediately and the credentials have to have high enough entropy and length to be future proof. To detect disclosure of the credentials their use should to be monitored for suspicions activity.

References

raphaelahrens commented 7 months ago

@izar should I write a PR?

izar commented 7 months ago

sure thing!

raphaelahrens commented 7 months ago

@noloader could you also look over my PR and give your opinion?