ASVS icon indicating copy to clipboard operation
ASVS copied to clipboard

Lack of Coverage for SAML Attacks in ASVS

Open ImanSharaf opened this issue 1 year ago • 19 comments

While reviewing the current version of the ASVS, I've noticed a potential gap in coverage related to SAML attacks. Considering the adoption of SAML for identity federation and single sign-on in enterprise environments, it's important to provide adequate coverage for potential SAML vulnerabilities and attacks.

Specifically, some of the SAML-based attacks that appear to be missing or not thoroughly covered include:

  • XML Signature Wrapping (XSW) Attacks: This involves manipulating the SAML response to insert or wrap additional assertions or elements to trick the service provider (SP) into thinking the signature is still valid.
  • Replay Attacks: An attacker captures and reuses a valid SAML assertion to gain unauthorized access.
  • Clock Skew: Attackers exploit time differences between the IdP and SP to use assertions that should have expired.
  • IdP Confusion/Mix-Up: This involves an attacker tricking the SP into consuming assertions from a malicious IdP.
  • ...

It would be beneficial for the community if the ASVS could provide more detailed guidance on testing, mitigating, and verifying the security of SAML implementations.

ImanSharaf avatar Sep 28 '23 19:09 ImanSharaf

SAML is so old school I politely suggest we drop it and focus on OIDC instead.

jmanico avatar Sep 28 '23 21:09 jmanico

Thank you for sharing your perspective. I concur with your viewpoint on this matter. While I acknowledge the evolving landscape of Single Sign-On (SSO) technologies, it's evident that SAML remains a significant player in this domain and its relevance is unlikely to diminish in the immediate future. Given this context, might I propose we consider introducing a directive within ASVS that advises against the indiscriminate use of SAML?

ImanSharaf avatar Sep 28 '23 21:09 ImanSharaf

@jmanico - if you want to get rid of SAML in the world, you need to have requirement for that in ASVS.

If SAML is not considered as old and vulnerable component, applications are allowed to use it. Even if it so-called "old-school" technology, big companies are really slow to take new technologies in to use and those SAML solutions may live there for long time.

But I have not checked or analyzed the content here yet, just feedback on the "this is so old so it is not valid" topic.

elarlang avatar Sep 28 '23 21:09 elarlang

Fair enough, let's do it.

jmanico avatar Sep 28 '23 23:09 jmanico

Here is ChatGTP's suggestion for SAML requirements.

If you're a developer working on implementing SAML, you've got to be on your toes because there are a lot of security nuances to consider. Let's dig into the nitty-gritty details:

  1. XML Signature Validation Why: SAML assertions and responses are XML-based, and they often come signed. You need to validate these signatures to ensure the integrity and origin of the message. How: Use libraries that are compliant with XML Signature standards and make sure to validate against the correct public key of the Identity Provider (IdP).
  2. Audience Restriction Why: This is to ensure that the SAML assertion is intended for your service and not someone else. How: Validate the Audience element in the SAML assertion against your Service Provider (SP) entity ID.
  3. Time-based Checks Why: SAML assertions are time-bound. You don't want to accept an assertion that's expired or not yet valid. How: Validate NotBefore and NotOnOrAfter attributes in the Conditions element.
  4. Attribute Trust Why: You're going to be making authorization decisions based on these attributes. Make sure they're trustworthy. How: Only accept attributes that you expect and understand, and that come from trusted IdPs.
  5. Replay Attacks Why: An attacker could capture a SAML assertion and try to replay it. How: Implement one-time use checks, often facilitated by using the InResponseTo attribute.
  6. Canonicalization Attacks Why: XML is notoriously complex, and different XML documents can appear identical after canonicalization but have different semantics before. How: Use XML libraries that are aware of and can mitigate against these types of attacks.
  7. Encryption Why: To protect sensitive data within the SAML assertion. How: Use strong encryption algorithms and proper key management. Make sure to decrypt the assertion securely.
  8. Error Handling Why: Improper error handling can leak information that could be useful to an attacker. How: Implement generic error messages and log the details for internal review.
  9. Binding and Endpoint Security Why: The way the SAML assertion is transported is crucial. How: Prefer POST binding over GET to avoid leaking sensitive information in URLs. Make sure your endpoints are secured with HTTPS.
  10. User Session Management Why: After the SAML process, you still need to manage the user's session securely. How: Implement secure session management practices like session timeout, secure cookies, etc.
  11. Logging and Monitoring Why: For auditing and to detect any malicious activities. How: Log SAML transactions and set up monitoring and alerting based on anomalies.
  12. Compliance and Testing Why: To ensure that your SAML implementation is secure and compliant with standards. How: Regularly test your implementation for vulnerabilities and ensure it's compliant with the SAML 2.0 standard.

jmanico avatar Sep 28 '23 23:09 jmanico

Fair enough, let's do it.

We can start with these:

  • XML Signature Wrapping (XSW) Attacks: This involves manipulating the SAML response to insert or wrap additional assertions or elements to trick the service provider (SP) into thinking the signature is still valid.
  • Replay Attacks: An attacker captures and reuses a valid SAML assertion to gain unauthorized access.
  • Clock Skew: Attackers exploit time differences between the IdP and SP to use assertions that should have expired.
  • IdP Confusion/Mix-Up: This involves an attacker tricking the SP into consuming assertions from a malicious IdP.

ImanSharaf avatar Sep 29 '23 16:09 ImanSharaf

Much like with OAuth/OIDC, I think we need to come up with a set of positive requirements that cover the primary attacks.

@ImanSharaf can you work on that?

tghosth avatar Oct 09 '23 09:10 tghosth

Let's start with three:

  • Verify and enforce the presence and integrity of digital signatures on SAML assertions, rejecting any assertions that are unsigned or have invalid signatures.
  • Verify that the application sanitizes SAML input fields to prevent comment injection, ensuring data is correctly parsed and potential comment injections are neutralized or rejected.
  • Verify that each SAML assertion is uniquely processed and used only once within its validity period to prevent replay attacks.

Are they good enough? @tghosth

ImanSharaf avatar Oct 31 '23 21:10 ImanSharaf

@jmanico what do you think about these? I am concerned they overlap with regular requirements...

tghosth avatar Nov 08 '23 08:11 tghosth

If we do wish to consider SAML base requirements, and I think these are well done.

However ASVS is getting kind of bulky, and I tend to want to remove legacy-ish requirements. And I consider SAML legacy tech.

jmanico avatar Nov 08 '23 14:11 jmanico

  • Verify and enforce the presence and integrity of digital signatures on SAML assertions, rejecting any assertions that are unsigned or have invalid signatures.

I think this makes sense specifically in a SAML context.

  • Verify that the application sanitizes SAML input fields to prevent comment injection, ensuring data is correctly parsed and potential comment injections are neutralized or rejected.

Is there a SAML specific attack here or is this a general XML issue? Do you have an example?

  • Verify that each SAML assertion is uniquely processed and used only once within its validity period to prevent replay attacks.

I agree with this but I think it would need to be L3 as it is quite complicated.

Thoughts @ImanSharaf ?

tghosth avatar Nov 08 '23 16:11 tghosth

@ImanSharaf waiting for your responses?

tghosth avatar Jan 25 '24 11:01 tghosth