Skip to content

Access control layer, PR 2: extension access and dispatch checks - #334

Open
ariel3259 wants to merge 5 commits into
joske:masterfrom
ariel3259:feat/access-control-pr2
Open

ariel3259 wants to merge 5 commits into
joske:masterfrom
ariel3259:feat/access-control-pr2

Conversation

@ariel3259

Copy link
Copy Markdown
Contributor

PR 2 of the access control series designed in #320, after #328. It implements the plan in docs/superpowers/plans/2026-11-04-access-control-pr2.md. The plan commits are at the bottom of this branch.

With no policy installed, nothing a client can observe changes. Both new checks are behind policies_installed(), so the empty list costs one branch and builds no context.

What it adds

  • Two hooks on AccessPolicy, both default allow:
    • extension_access(ctx, client, extension) for discovery;
    • extension_dispatch(ctx, client, extension) for requests. The request's major and minor come from ctx.request().
  • One evaluator. check_server, check_extension_access and check_extension_dispatch share one first-denial loop. It keeps installation order and stops at the first Deny.
  • Discovery. A denied extension answers QueryExtension exactly like an unknown name. ListExtensions leaves it out and keeps the order of the rest; if every name is denied, it answers an empty list. Policies are asked only about extensions that are available. The availability match that was duplicated in misc.rs is now one function, extension_available.
  • Dispatch. Every request to a registered extension major is checked, whether or not the extension is advertised. RENDER, XKEYBOARD, GLX and DRI3 run when addressed directly, even when discovery hides them, so the check cannot depend on discovery. Core opcodes and unassigned majors are not asked. The check runs after the transport gate and after the maximum-length BadLength, as Xorg (dix/dispatch.c:546-549).
  • Dispatch denial answers BadRequest: value 0, the request's minor and major. It does so whatever error the policy returns, as Xorg's XaceHookDispatch (Xext/xace.c).
  • BIG-REQUESTS. After forwarding Enable, the client reader waits for the core's answer. A denied Enable sends IgnoreBigRequests so the connection keeps working. This uses the same helper as the existing malformed-minor path.
  • Independence. The discovery and dispatch checks are separate: denying discovery does not deny dispatch, and the reverse.
  • Spec. The table of denials by site gains the dispatch row, and the spec now states the check's position.

Testing

  • cargo +nightly fmt -- --check.
  • cargo clippy --all-targets -- -D warnings, plus with --features tcp-transport and with --features xdmcp.
  • FreeBSD clippy.
  • cargo check -p yserver-core --target x86_64-pc-windows-gnu.
  • cargo test --workspace: 4470 passed. With --features xdmcp: 4583 passed.
  • Each crates/*/tests file.
  • --features xdmcp -- --ignored (lavapipe): 753 passed.
  • The extension tests, plus the two run_core tests below, 5 runs under nproc busy loops: 0 failures.
  • just man.

process_request/tests/extensions.rs has 13 tests, all through process_request. run/tests.rs has two with the real reader and run_core:

  • a denied Enable, then GetInputFocus answers;
  • a request held behind another client's server grab is checked when it is dispatched. Revoking while it is held makes it fail.

Each test names the mutation that makes it fail. One case uses a nonzero minor, so a minor forced to 0 in the check or in the error is caught.

Hardware, on AMD (Raphael) and NVIDIA (5060 Ti): the same harness as #328 (host access probe, glxgears, shutdown), run with the #328 binary and with this branch. On each GPU the xdpyinfo -queryExtensions lists are identical (23 extensions), 12/12 checks passed, and yserver exited cleanly with the same log lines.

🤖 Generated with Claude Code

ariel3259 and others added 5 commits November 4, 2026 18:24
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r release, dispatch by registered major)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed at dispatch)

Sol round 2: 0 blocking, 0 major, 1 minor. m-1: the Review Focus claimed the
revocation test covered requests held during a server grab, but that test only
dispatches sequentially. Task 2 gains a run_core test that parks an XFIXES
request behind another client's grab, revokes, ungrabs and expects BadRequest;
its mutation is a decision cached at intake.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Policies gain extension_access and extension_dispatch hooks (default allow).
QueryExtension answers a denied extension like an unknown name and
ListExtensions leaves denied names out, asked only for available extensions
and only when a policy is installed. check_server, check_extension_access and
check_extension_dispatch share one first-denial loop; misc::extension_available
replaces the duplicated availability match.

Implemented-By: codex (model gpt-6-luna, reasoning effort xhigh)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every request to a registered extension major is checked with
check_extension_dispatch when a policy is installed, whether or not the
extension is advertised. It runs after the transport gate and the
maximum-length BadLength. A denial answers BadRequest with value 0, the
request's minor and major, as Xorg. A denied BIG-REQUESTS Enable releases the
parked client reader through release_big_requests_reader, shared with the
malformed-minor path. The spec records the dispatch row and the position.

Includes review fix round 1: the test policies share one recording and
decision path per hook, a nonzero-minor denial case, the core length check
stays in place, and the stale dead_code allow is gone.

Implemented-By: codex (model gpt-6-luna, reasoning effort xhigh; fix round max)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant