fix: read identity only from the signed assertion

[jarvis9443]

What

saml_verify_doc verifies one <Signature> and reports success. The identity readers (doc_name_id, doc_attrs, doc_session_index, doc_session_expires) then run //samlp:Response/saml:Assertion/... over the whole document, so the assertion an identity is read from need not be the one the signature covered.

That is signature wrapping: present a Response carrying one genuinely IdP-signed assertion beside a forged one, and verification passes while the readers pick the forged assertion.

Approach

Make identity come only from signed content, at the point it is read, instead of adding a separate check that re-derives what the readers can reach:

1. Anchor the four identity readers to /samlp:Response/saml:Assertion (single leading slash). Identity can now only be read from an assertion that is a direct child of the verified root message — never from one nested in Extensions, Advice, or a wrapper such as ArtifactResponse.
2. After verification, confine_identity_to_signature drops every top-level assertion the verified signature does not cover. A whole-message signature (Reference resolves to the root) covers all of them; otherwise only the single assertion its Reference points to may remain, and any other top-level assertion is unsigned and removed. The removed nodes are siblings, so freeing one never dangles another.

The identity a reader returns is then always exactly what the IdP signed, whatever else the document contains.

Why this shape

• The wrapping vector is closed where identity is read, so a reader cannot drift from a separate check — there is no parallel selector to keep in step with the readers by hand.
• It holds for any root element, not just Response. A forged assertion smuggled through a non-Response wrapper (e.g. ArtifactResponse whose own status is Success) yields no identity, because the readers are anchored to a Response root (TEST 14).
• Legitimate interop is preserved rather than rejected: a whole-response signature over several assertions is read in full (TEST 9); a signed assertion carrying Advice is trusted (TEST 11); a response padded with unrelated forged assertions still reads its genuinely signed identity instead of failing the login (TEST 8).

Changes

• Root-anchored identity XPaths in src/saml.c.
• signed_reference_target and confine_identity_to_signature in src/sig.c, wired into saml_binding_post_verify.
• t/signed-response.t, 14 cases: assertion-level and response-level signatures, the forged-sibling / nested-Response / Advice / Extensions / non-Response-root wrapping shapes, multi-assertion coverage, and a LogoutResponse status read.

saml_doc_status_code is unchanged and no new binding status is introduced.

Testing

prove t/signed-response.t (self-contained, no IdP needed), plus the existing Keycloak end-to-end suite in CI. The tree mutation was checked for memory safety with valgrind (0 errors across the removal scenarios) and a repeated-request stress that serializes the whole tree after each confine_identity_to_signature.

Relationship to #32

This is an alternative to #32 for the same vulnerability. #32 keeps the document-wide readers and adds saml_verified_identity_is_signed, a check that mirrors the reader XPaths and only runs when the root local-name is Response; this PR scopes the readers themselves, which is what #34 asked for and which also covers the non-Response-root case. Closes #34.

Out of scope

Reference transform restrictions (SAML Core 5.4), doc_issuer under an assertion-level signature (#33), LogoutRequest identity (#36), and assertion Conditions (#37) — the same boundaries as #32.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d6b4bc2e-ef7e-41d9-ac27-2613de46de12

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}