What
Nothing in the library validates the constraints an IdP attaches to an assertion. grep -riE 'conditions|notonorafter|notbefore|audience|subjectconfirmation' src lua returns only @SessionNotOnOrAfter, which is read for the session expiry and never enforced as a validity bound on the assertion itself.
So a signature that verifies is treated as sufficient. The elements that say when and for whom the assertion is good are parsed by the schema and then ignored.
Missing, in rough order of impact:
<Conditions NotBefore=... NotOnOrAfter=...>, the assertion validity window<AudienceRestriction><Audience>, the SP the assertion was issued for<SubjectConfirmationData> and its Recipient, NotOnOrAfter and InResponseToInResponseTo on the <Response> itself, tying it to a request this SP issuedDestination on the <Response>- any replay cache over assertion
ID
login_callback in lua/resty/saml.lua checks the IdP status code and compares args.RelayState against the saml_state it stored on the session, then goes straight to doc_name_id/doc_attrs and sets authenticated = true. The RelayState comparison binds the callback to a login attempt made from that browser session, which is worth noting because it blunts some of what follows, but it is not a substitute for any of the above: RelayState is opaque application state, it is not signed, and it says nothing about the assertion.
Why it matters
Two consequences are concrete rather than theoretical.
An assertion never expires. With no Conditions/@NotOnOrAfter check, an assertion captured once stays usable indefinitely. The RelayState check does not prevent replay, because the party replaying it controls their own browser session: start a fresh login to get a valid saml_state on the session, then post the old assertion back with the matching RelayState. Nothing compares the assertion's issue time to now.
An assertion issued for a different SP is accepted. With no AudienceRestriction check, any assertion signed by the configured IdP is accepted, whatever SP the IdP minted it for. In a federation where one IdP serves several SPs, an assertion obtained from a lower-value SP in that federation is accepted here as-is. This is the case the audience restriction exists to prevent, and it is the reason the profile makes the check mandatory rather than advisory.
What the profile requires
SAML 2.0 Web Browser SSO, <Response> message processing rules, requires the service provider to verify that:
- the
Recipient in SubjectConfirmationData matches the assertion consumer service URL
- its
NotOnOrAfter has not passed
- its
InResponseTo matches the ID of the AuthnRequest this SP issued, when the response is solicited
- the assertion's
Conditions are satisfied, including an AudienceRestriction naming this SP
SAML Core section 2.5.1 defines the Conditions processing, including that an unrecognised condition type makes the assertion Indeterminate rather than valid.
Suggested scope
Worth splitting rather than landing as one change, because the pieces have different interop risk:
Conditions/@NotBefore and @NotOnOrAfter, with a configurable clock skew allowance. Low risk, highest value.AudienceRestriction against a configured SP entity ID. Needs a new config knob; sp_issuer already exists and is the natural value.SubjectConfirmationData Recipient and NotOnOrAfter.InResponseTo on both the SubjectConfirmationData and the Response, which requires persisting the issued AuthnRequest ID on the session. The ID is already generated by generate_saml_id, it is just not retained.- Assertion ID replay cache, which needs shared storage across workers and is the largest of the five.
Whichever way the readers end up scoped in #34 matters here: these conditions live inside a specific assertion, so they have to be evaluated against the same assertion the identity is read from, not unioned across the document the way doc_attrs currently reads attributes.
Notes
Pre-existing, and independent of #32. Noticed while reviewing that PR, in the same pass as #33 through #36. Filing separately because #32 is about binding the read identity to the verified signature, which is a different question from whether a correctly signed and correctly bound assertion is one this SP should accept right now.
What
Nothing in the library validates the constraints an IdP attaches to an assertion.
grep -riE 'conditions|notonorafter|notbefore|audience|subjectconfirmation' src luareturns only@SessionNotOnOrAfter, which is read for the session expiry and never enforced as a validity bound on the assertion itself.So a signature that verifies is treated as sufficient. The elements that say when and for whom the assertion is good are parsed by the schema and then ignored.
Missing, in rough order of impact:
<Conditions NotBefore=... NotOnOrAfter=...>, the assertion validity window<AudienceRestriction><Audience>, the SP the assertion was issued for<SubjectConfirmationData>and itsRecipient,NotOnOrAfterandInResponseToInResponseToon the<Response>itself, tying it to a request this SP issuedDestinationon the<Response>IDlogin_callbackinlua/resty/saml.luachecks the IdP status code and comparesargs.RelayStateagainst thesaml_stateit stored on the session, then goes straight todoc_name_id/doc_attrsand setsauthenticated = true. TheRelayStatecomparison binds the callback to a login attempt made from that browser session, which is worth noting because it blunts some of what follows, but it is not a substitute for any of the above:RelayStateis opaque application state, it is not signed, and it says nothing about the assertion.Why it matters
Two consequences are concrete rather than theoretical.
An assertion never expires. With no
Conditions/@NotOnOrAftercheck, an assertion captured once stays usable indefinitely. TheRelayStatecheck does not prevent replay, because the party replaying it controls their own browser session: start a fresh login to get a validsaml_stateon the session, then post the old assertion back with the matchingRelayState. Nothing compares the assertion's issue time to now.An assertion issued for a different SP is accepted. With no
AudienceRestrictioncheck, any assertion signed by the configured IdP is accepted, whatever SP the IdP minted it for. In a federation where one IdP serves several SPs, an assertion obtained from a lower-value SP in that federation is accepted here as-is. This is the case the audience restriction exists to prevent, and it is the reason the profile makes the check mandatory rather than advisory.What the profile requires
SAML 2.0 Web Browser SSO,
<Response>message processing rules, requires the service provider to verify that:RecipientinSubjectConfirmationDatamatches the assertion consumer service URLNotOnOrAfterhas not passedInResponseTomatches the ID of theAuthnRequestthis SP issued, when the response is solicitedConditionsare satisfied, including anAudienceRestrictionnaming this SPSAML Core section 2.5.1 defines the
Conditionsprocessing, including that an unrecognised condition type makes the assertionIndeterminaterather than valid.Suggested scope
Worth splitting rather than landing as one change, because the pieces have different interop risk:
Conditions/@NotBeforeand@NotOnOrAfter, with a configurable clock skew allowance. Low risk, highest value.AudienceRestrictionagainst a configured SP entity ID. Needs a new config knob;sp_issueralready exists and is the natural value.SubjectConfirmationDataRecipientandNotOnOrAfter.InResponseToon both theSubjectConfirmationDataand theResponse, which requires persisting the issuedAuthnRequestID on the session. The ID is already generated bygenerate_saml_id, it is just not retained.Whichever way the readers end up scoped in #34 matters here: these conditions live inside a specific assertion, so they have to be evaluated against the same assertion the identity is read from, not unioned across the document the way
doc_attrscurrently reads attributes.Notes
Pre-existing, and independent of #32. Noticed while reviewing that PR, in the same pass as #33 through #36. Filing separately because #32 is about binding the read identity to the verified signature, which is a different question from whether a correctly signed and correctly bound assertion is one this SP should accept right now.