Anchor the identity XPaths to the root Response
Nobody has claimed this yet.
- Dominant language
- Perl
- Stars
- 2
- Forks
- 3
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 3
Description
The identity readers compiled in src/saml.c all start with //:
//samlp:Response/saml:Assertion/saml:AttributeStatement/saml:Attribute
//samlp:Response/saml:Assertion/saml:Subject/saml:NameID
//samlp:Response/saml:Assertion/saml:AuthnStatement/@SessionIndex
//samlp:Response/saml:Assertion/saml:AuthnStatement/@SessionNotOnOrAfter
//samlp:Response matches a Response at any depth, so an assertion that is a direct child of a Response nested under saml:Advice is reader-visible. That is the root cause of the signature-wrapping exposure fixed in #32. That PR closes the hole from the other side, by requiring every reader-visible assertion to be a direct child of the root Response and covered by the verified signature.
Anchoring the readers instead would remove the need for that rule:
/samlp:Response/saml:Assertion[1]/...
Two reasons to do it:
- The invariant currently lives in
saml_verified_identity_is_signed(src/sig.c), a different function from the queries it protects. A future reader added with a//prefix, or an edit to one of the four above, silently reopens the gap.XPATH_ASSERTIONSsits next to them as a comment-level guard, which is weaker than the queries being incapable of reaching a nested assertion in the first place. saml_doc_attrsreads every match rather than the first. Scoping it tosaml:Assertion[1]makes the identity and the attributes come from one assertion, which is what the Web Browser SSO profile expects.
Behavior change to weigh: attributes spread across sibling assertions would stop being aggregated. Worth checking against real IdP output before changing.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/saml.c, inspect the four identity XPath readers and saml_doc_attrs, then read saml_verified_identity_is_signed in src/sig.c for the existing protection. Anchor the readers to the root Response and first Assertion, then check real IdP output to confirm that identity and attributes still come from the expected assertion without sibling aggregation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, xml
- Domain
- authentication, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100