fix(pam-access): bind /pam/* to a PAM relying party and refuse self-declared bastions - #92
Conversation
guimard
left a comment
There was a problem hiding this comment.
Found a blocker while reviewing: setting pamAccessAllowedRps breaks /pam/heartbeat for every enrolled device.
Trace: _checkCallerRp reads $session->data->{rp}, but refresh-token sessions from the device flow never carry rp — newRefreshToken receives {scope, client_id, _session_uid, auth_time, grant_type, %$session_data} (OIDCDeviceAuthorization _generateTokens) and the synthetic swap only stamps _clientConfKey (OIDCDeviceOrganization.pm:81). Only newAccessToken stamps rp (core Lib/OpenIDConnect.pm:1307). /pam/heartbeat passes session => $rtSession into _checkCaller, so rp is empty and the gate answers 403 PAM_CALLER_RP_REFUSED even for the listed RP. Devices can never refresh; the fleet falls over at access-token expiry (~1 h). t/16 only exercises the gate with access tokens, which is why CI is green.
Confirmed empirically with a throwaway test: heartbeat with pamAccessAllowedRps = 'pam-access' → 403 {"error":"Token is not a PAM token"}.
Suggested fix:
my $rp = $session->data->{rp} // $session->data->{_clientConfKey} // '';— the same vocabulary heartbeat step 5 already uses. For non-org deployments (neither key), extract the client_id → conf-key scan from heartbeat step 5 into a helper and reuse it here, or document that non-org tokens are refused. Please also add a heartbeat + allowlist case to t/16.
|
Confirmed and fixed (commit d3360e8). Your trace was exact — I reproduced it by writing the test first: with Took the second half of your suggestion rather than the one-liner: heartbeat step 5 already did the full resolution inline, so I extracted it as
README updated with how the RP is resolved and why the three steps exist. Also retargeted this PR onto |
721387b to
356f20c
Compare
…eclared bastions
The caller gate tested the grant type and the scope, and nothing else. But
`grant_type => device_code` is stamped for every RP using the device flow, and
the core's getAccessToken performs no audience or RP check, so any device-grant
token carrying a pam scope reached /pam/*. The cheap attacker is not an
unrelated application: it is a compromise of any ordinary enrolled host in the
same project, which already holds exactly such a token.
Worse, in the default configuration server_group came straight from the request
body, so that host could POST {"server_group":"bastion"} to /pam/authorize and
collect (bastion_id, user) vouchers for users it had never seen. The voucher
binding is sound, and that is precisely the problem: it binds to the ATTACKER's
device id, which is what then gets the hop certificates.
pamAccessAllowedRps lists the RP configuration keys allowed to call PAM. It is
empty by default, so an upgrade changes nothing; a once-per-worker warning says
the endpoints are open. When set:
* a token from an unlisted RP is refused on all six endpoints, 403 +
PAM_CALLER_RP_REFUSED;
* a bastion group may no longer be self-declared in the body — it must come
from pamAccessServerGroups. Ordinary groups still come from the body.
The second half is gated on the same setting on purpose: a deployment that has
named its PAM relying parties can name its bastions too. In practice a bastion
then needs a client_id the map can key on — a host trusted to vouch for
arbitrary users has to be distinguishable at enrollment.
Closes #50
Claude-Session: https://claude.ai/code/session_01GfBG36HfzjGy8W9rJQbxBL
…dy did
_checkCallerRp read `$session->data->{rp}`, which the core stamps in
newAccessToken and nowhere else. The refresh-token session /pam/heartbeat
passes into the gate never carries it — the device flow's newRefreshToken
gets {scope, client_id, _session_uid, auth_time, grant_type, ...} and the
oidc-device-organization swap stamps `_clientConfKey`. So setting
pamAccessAllowedRps answered 403 PAM_CALLER_RP_REFUSED to every heartbeat,
including the listed RP's: no enrolled device could refresh, and the fleet
would have fallen over at access-token expiry. t/16 only exercised access
tokens, which is why CI was green.
Extract heartbeat step 5's own resolution into _resolveRp (rp, then
_clientConfKey, then a client_id scan of the RP list) and use it in both
places, so the gate and the endpoint agree on what "the caller's RP" means.
t/16 now covers the heartbeat path: listed RP served, unlisted refused.
pam-lib grows enroll_server_tokens for the offline enrollment it needs.
Reported in review of #92.
Claude-Session: https://claude.ai/code/session_01GfBG36HfzjGy8W9rJQbxBL
0988d98 to
d4a5a71
Compare
The fallback scan returns the first configuration key whose client_id matches the token's. That is only well-defined while client_ids are unique across RPs -- which the bastion setup requires anyway, a host trusted to vouch for arbitrary users having to be distinguishable at enrollment. Say so, and say what happens when the prerequisite is broken: one key comes back, and an allowlist naming the other refuses a caller it meant to admit. Iterate in sorted order while here. It does not make the answer right, only stable: Perl randomises hash order per process, so without it two portal nodes -- or one node after a restart -- could disagree about the same token, turning a static misconfiguration into an intermittent 403. Claude-Session: https://claude.ai/code/session_01GfBG36HfzjGy8W9rJQbxBL
Closes #50. Stacked on #91 → #90 → #89 → #88 → #87.
Implements the arbitration from the last round: RP allowlist AND forbid a
self-declared bastion
server_group, both halves, with an empty allowlistmeaning "no change" so the upgrade is not a rupture.
The hole
The caller gate tested the grant type and the scope, and nothing else.
grant_type = device_codeis stamped for every RP using the device flow,and
Lib/OpenIDConnect.pm'sgetAccessTokenperforms no audience or RP check.So any device-grant token carrying a
pamscope reached/pam/*.The cheap attacker is not an unrelated application: it is a compromise of any
ordinary enrolled host in the same project, which already holds exactly such
a token and needs nothing extra.
And in the default configuration
server_groupcame straight from the requestbody, so that host could POST
{"server_group":"bastion"}to/pam/authorizeand collect
(bastion_id, user)vouchers for users it had never seen. Thevoucher binding is sound, and that is the problem: it binds to the attacker's
device id, so the hop certificates go to the attacker.
pamAccessAllowedRpsLists the RP configuration keys — the same vocabulary as
pamAccessRp— thatmay call PAM. Accepts a comma/space separated string or a hashref.
Empty by default, keeping the historical behaviour; a once-per-worker
warning states that the endpoints accept any pam-scoped device-grant token.
When set:
PAM_CALLER_RP_REFUSED;server_groupmay no longer be self-declared in the body: itmust come from
pamAccessServerGroups, else 403 +self_declared_bastion.Ordinary groups still come from the body exactly as before.
Why the second half is gated on the same setting
Because it is the honest reading of "a deployment that has named its PAM
relying parties can name its bastions too". Turning it on unconditionally would
break every bastion deployment running the legacy path.
It does have a consequence worth stating plainly, and the README does: a bastion
then needs a
client_idthe map can key on. The issue notes that open-bastion'sarchitecture doc says the map is inadequate when one
client_idcovers amulti-group project — the answer there is to give bastions their own
client_id. A host you trust to vouch for arbitrary users has to bedistinguishable at enrollment; there is no way around that.
Tests
New
t/16-PamAccess-CallerRp.t, 29 assertions. It first demonstrates thebug in the default configuration (an unrelated pam-scoped RP declares itself a
bastion and receives a voucher), then pins every branch of the fix: the listed
RP served, the unlisted one refused, the gate on
/pam/verifyand/pam/userinfotoo, the self-declared bastion refused, an ordinary group stillaccepted, a mapped bastion still working and still getting its voucher, and
both accepted config shapes. pam-access 798 total, green.
https://claude.ai/code/session_01GfBG36HfzjGy8W9rJQbxBL