saml.verify_doc is exported without the identity-coverage check
Nobody has claimed this yet.
- Dominant language
- Perl
- Stars
- 2
- Forks
- 3
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 3
Description
PR #32 added saml_verified_identity_is_signed and wired it into saml_binding_post_verify (src/binding.c). saml_verify_doc is exported to Lua on its own as verify_doc (src/lua_saml.c), and it verifies the first <ds:Signature> in the document without checking that the reference covers the assertion the identity is later read from.
A caller doing saml.verify_doc(...) followed by saml.doc_name_id(...) is therefore still open to signature wrapping. The bundled lua/resty/saml.lua goes through binding_post_parse, so it is unaffected.
Options:
- run the coverage check inside
saml_verify_doc, so every caller inherits it, - or document
verify_docas a low-level primitive that does not bind identity, and point users atbinding_post_parse.
The first is safer. It needs a look at whether any caller verifies documents that are not protocol messages, since the check returns early for a non-Response root.
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/lua_saml.c and src/binding.c to compare the exported verify_doc path with saml_binding_post_verify, then inspect bundled lua/resty/saml.lua and any callers of saml_verify_doc. Check how the coverage check handles a non-Response root. Done means either all verify_doc callers receive the identity-coverage protection or the low-level behavior and safer entry point are clearly documented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, lua
- Domain
- authentication, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100