Skip to content

main: run sysupgrade backup through rpcd - #6

Open
JuliusBairaktaris wants to merge 2 commits into
openwrt:masterfrom
JuliusBairaktaris:backup-via-rpcd
Open

main: run sysupgrade backup through rpcd#6
JuliusBairaktaris wants to merge 2 commits into
openwrt:masterfrom
JuliusBairaktaris:backup-via-rpcd

Conversation

@JuliusBairaktaris

Copy link
Copy Markdown

The cgi-backup endpoint forked /sbin/sysupgrade --create-backup - and streamed its stdout while discarding the exit status. Two defects:

  1. Silent data loss. sysupgrade exits non-zero when tar cannot read a listed file, but cgi-io returned HTTP 200 anyway and served the truncated archive. Restoring it destroys data with no error anywhere.
  2. It requires root. The fork inherits uhttpd's uid. Once uhttpd runs as a non-root user, the archive drops from 87 to 38 entries, losing /etc/shadow, /etc/config/network, /etc/config/system and the ssh host keys.

This delegates the backup to rpcd's file exec method, which already validates the session ACL and runs as root, and requests the new stream mode: stdout goes to an anonymous memfd, the memfd is returned attached to the completion status, and the command's exit code is returned in the reply. cgi-io now checks the exit code and the presence of the fd before sending any headers, so a failed archive is never served as a good one.

Depends on the companion rpcd change adding stream to the file object's exec method. Without it, stream is ignored and the reply comes back with an inline stdout capped at 256 KB and no fd; cgi-io detects the missing fd and fails loudly instead of serving a truncated archive.

This is a prerequisite for running uhttpd unprivileged (openwrt/openwrt#24558).

The backup CGI forked sysupgrade, which runs it under uhttpd's uid.
Once uhttpd runs as a non-root user, the generated archive silently
shrinks from 87 to 38 entries, dropping /etc/shadow,
/etc/config/network, /etc/config/system and the ssh host keys.

The fork path also discarded the child's exit status, so a sysupgrade
failure (tar cannot read a listed file) still produced HTTP 200 with a
silently incomplete archive.

Delegate to rpcd instead, which already validated the session and runs
as root. The archive is streamed back through a memfd and the exit
status is checked before any headers are sent, so a failed backup now
returns an error instead of a truncated archive.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
Copilot AI lite review requested due to automatic review settings August 15, 2026 07:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@JuliusBairaktaris

Copy link
Copy Markdown
Author

@jow- @ynezz — review ping.

cgi-backup no longer forks sysupgrade --create-backup - in-process with whatever uid uhttpd hands it; it asks rpcd to run it and splices back the returned file descriptor. That decouples the archive from the web server's uid, which is what openwrt/openwrt#24558 needs.

It also fixes a pre-existing bug that is independent of the uid work: the old code printed Status: 200 OK before the child had finished and discarded its exit status, so a failed or partial archive was served to the browser as a successful one.

Depends on openwrt/rpcd#40 (the stream option) and openwrt/luci#8947 (the ACL grant).

cgi-exec forked the requested command in-process, running it as a child
of uhttpd and therefore under uhttpd's uid. Once uhttpd runs as a
non-root user, every fs.exec_direct() call in LuCI - network
diagnostics, rrdtool graphs, package-manager - would lose the root
privilege those commands need, and the in-process fork never surfaced
a non-zero exit status, serving a partial 200 body instead.

Delegate to rpcd's "file exec" like the backup path: the resolved
executable and its arguments are handed to rpcd, which re-checks the
session ACL against the canonical command line and runs the child as
root, streaming stdout back through a memfd. The exit status is
checked before any headers are sent, so a failed command now returns
an error instead of a partial body, and an rpcd too old to support
streaming fails loudly instead of silently truncating at the 256 KB
inline cap.

rpcd returns stderr inline, so when the caller requested it (stderr=1)
it is prepended to the streamed stdout rather than interleaved with it
as the in-process fork did. The backup and exec paths share the same
reply handling.

The cram tests can no longer run the in-process fork cases without a
live rpcd, so the executable-not-found and invalid-filename cases now
cover the argument parsing and failure paths main_exec still owns,
keeping the result independent of whether a ubusd runs on the build
host.

Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
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.

2 participants