The issuer allow-list is not weighed again when a session resumes
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 30/100
- Issue type
- Bug
- Clarity
- Needs clarification
- Activity status
- Quiet
- Tech stack
- lua
- Domain
- authentication, backend, security
Research direction
Start by reading login at lua/resty/saml.lua:196 and compare its session-resume path with login_callback's issuer allow-list check. Trace how issuer and idp_cert are stored in sessions, including sessions predating the option. Done requires a maintainer-approved policy for resumed sessions and coverage for both existing and newly established sessions.
Written by the indexing model from the issue text.
Description
Raised on #41. The allow-list added there is checked when a login response arrives and never again.
What
login_callback refuses a response whose issuers are not on idp_issuers, but login (lua/resty/saml.lua:196) returns the stored identity for an existing session, issuer = sess:get("issuer") included, without weighing it against the list.
Why it matters
This is the incident the option exists for. An operator learns that an issuer the shared idp_cert signs for is not one they trust, adds it to the allow-list, and reloads. Every session established before that change keeps working until it expires. Sessions are cookies with no server-side store, so there is nothing to evict per session.
There is a blunt remedy today: rotating secret invalidates all of them at once. Sessions established before the option existed are the reason this is not a one-line change.
The part that needs thought
Sessions predating the change carry no stored issuer, and nil cannot be told apart from an issuer that is no longer allowed:
- refusing on nil logs out every existing session on upgrade
- allowing nil through fails open and hands the incident back
Options worth weighing: store a marker alongside the issuer so the two cases are distinguishable, or make the resume check opt-in, or tie it to a session version that a config change bumps.
Whichever way it goes, the same question applies to idp_cert itself, which a resumed session also never rechecks.
- Dominant language
- Perl
- Stars
- 2
- Forks
- 3
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 3
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.
More from api7/lua-resty-saml
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
api7/lua-resty-saml#63 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
api7/lua-resty-saml#62 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
api7/lua-resty-saml#45 · 1 comment · 1 reaction ·
-
Difficulty 3/5 1-2 days Newbie friendliness 68/100
api7/lua-resty-saml#64 ·
-
api7/lua-resty-saml#61 · 1 assignee ·
All issues in api7/lua-resty-saml
Similar issues
-
1.severity: security
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
Difficulty 1/5 Under an hour Newbie friendliness 68/100
-
Difficulty 1/5 Under an hour Newbie friendliness 82/100
-
Difficulty 1/5 Under an hour Newbie friendliness 72/100
-
Difficulty 1/5 Under an hour Newbie friendliness 60/100
conda-forge/perl-test-warn-feedstock#1 · 1 comment · 1 reaction ·