Skip to content

session: make rpc_session_get() a pure lookup and add "notouch" to session/access - #39

Open
micpf wants to merge 1 commit into
openwrt:masterfrom
micpf:idle-timeout-notouch
Open

session: make rpc_session_get() a pure lookup and add "notouch" to session/access#39
micpf wants to merge 1 commit into
openwrt:masterfrom
micpf:idle-timeout-notouch

Conversation

@micpf

@micpf micpf commented Aug 7, 2026

Copy link
Copy Markdown

Problem

The option sessiontime idle timeout in /etc/config/rpcd never fires when LuCI is open. Every LuCI page runs periodic Poll callbacks that issue ubus RPCs; every RPC goes through session/access, and every plugin (uci, luci-rpc, …) that calls the exported rpc_session_access() for its own ACL checks also touches the session via rpc_session_get(). The idle timer is refreshed continuously, so it behaves as an absolute session lifetime instead.

Reproduce (unpatched): uci set rpcd.@rpcd[0].sessiontime='30'; /etc/init.d/rpcd restart, log in to LuCI, leave the Overview page open. The session never expires.

Fix

Split lookup from touch:

  • rpc_session_get() becomes a pure AVL lookup with no side effects. Internal callers (uci/luci-rpc permission checks, session data accessors) no longer keep the session alive on their own.
  • The top-level session/access ubus method remains the single natural "keep alive" signal – it is invoked by uhttpd on every request. It refreshes the idle timer unless the caller passes the new optional notouch=true argument.

The companion uhttpd change forwards this hint when LuCI marks the request as a background poll (see openwrt/uhttpd#39), and LuCI marks its own polls (see openwrt/luci#8916).

notouch defaults to false, so existing callers see no behaviour change.

Threat model note

The notouch hint does not weaken the idle timer's security properties. Any code running with a valid session cookie can already keep the session alive by omitting the hint, or by synthesising fake user activity to trigger real XHRs. The idle timer defends against the "walked-away-from-the-terminal" case (unattended authenticated browser), not against code executing inside the authenticated origin. Today that defence is silently disabled by LuCI's polling; this PR restores it.

Related PRs

Land in order rpcd → uhttpd → luci. Each is independently ABI-compatible with older peers (notouch defaults to false; unknown query params ignored; unknown blob keys ignored).

…ssion/access

The rpcd session idle timer (sessiontime) is meant to expire a session
after N seconds of inactivity, but it is currently refreshed by every
"session get" and "session access" ubus call, and by every RPC in every
loadable plugin (uci, luci-rpc, ...) that calls rpc_session_access() to
verify permissions. Because LuCI polls its status pages every few
seconds, the timer is refreshed continuously and never expires: the
"idle timeout" behaves as an absolute session lifetime instead.

Split the two concerns:

  * rpc_session_get() becomes a pure AVL lookup with no side effects.
    All internal lookups (uci/luci-rpc permission checks, session data
    accessors, etc.) no longer keep the session alive on their own.

  * The top-level session/access ubus method is the only path that
    still touches the session, and it does so only when the new
    optional "notouch" boolean argument is absent or false. Callers
    that want to distinguish real user activity from background
    polling (uhttpd's /ubus/ handler when it sees LuCI's _luci_bg=1
    marker) pass notouch=1 and the timer keeps counting down.

The rpcd HTTP entry point (uhttpd) already invokes session/access on
every request to authorise the call, so it stays the single natural
"keep alive" signal for the session -- exactly what an idle timeout
needs -- without duplicating touches from every downstream plugin.

The "notouch" argument is optional and defaults to false, so existing
callers see no behaviour change.

Signed-off-by: Michael Pfeifroth <micpf@westermo.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