api7 / api7/lua-resty-saml

saml.verify_doc is exported without the identity-coverage check

Open
#35 1 comment 1 reaction 0 assignees View on GitHub

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_doc as a low-level primitive that does not bind identity, and point users at binding_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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.