Conversation
Web UI for the console veracrypt package (packages feed). Not bundled into veracrypt; apk add luci-app-veracrypt depends on +veracrypt. Calls veracrypt --text only, with passwords on stdin. CLI reference: https://www.veracrypt.fr/en/Command%20Line%20Usage.html Signed-off-by: Ville Takio <ville+git@takio.fi>
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
13657fa"luci-app-veracrypt: add optional LuCI UI" — "with passwords on stdin" holds only for the current password;--new-password=,--protection-password=and--token-pin=are built into the argv at lines 897/908/917 ofluci.veracryptand are visible inps. Reword to say the current password goes on stdin, and qualify the same claim inREADME.md:9.
Generated by Claude Code
| "ubus": { | ||
| "file": [ "list", "stat" ], | ||
| "luci.veracrypt": [ "status", "listdir", "listdev", "mkdir", "rm", "job", "job_log", "job_abort", "job_answer", "tools", "pkg_install", "run" ] | ||
| }, |
There was a problem hiding this comment.
mkdir, rm, run, job_abort and pkg_install all mutate state, but granting them in read lets a read-only ACL group create and delete files, run veracrypt and install packages. Keep only the query methods in read; the write block already lists the full set. The file ubus entry is unused too — the view never require fs.
| "ubus": { | |
| "file": [ "list", "stat" ], | |
| "luci.veracrypt": [ "status", "listdir", "listdev", "mkdir", "rm", "job", "job_log", "job_abort", "job_answer", "tools", "pkg_install", "run" ] | |
| }, | |
| "ubus": { | |
| "luci.veracrypt": [ "status", "listdir", "listdev", "job", "job_log", "tools" ] | |
| }, |
Generated by Claude Code
| "file": { | ||
| "*": [ "list", "stat" ], | ||
| "/bin/mkdir": [ "exec" ], | ||
| "/usr/bin/mkdir": [ "exec" ], | ||
| "/bin/rm": [ "exec" ], | ||
| "/usr/bin/rm": [ "exec" ], | ||
| "/bin/rmdir": [ "exec" ], | ||
| "/usr/bin/rmdir": [ "exec" ], | ||
| "/bin/busybox": [ "exec" ] | ||
| }, |
There was a problem hiding this comment.
exec on /bin/busybox combined with "file": [ "exec" ] in the write ubus block (line 23) is arbitrary root command execution for anyone holding this ACL, and "*": [ "list", "stat" ] exposes the entire filesystem. None of it is used: the view declares no require fs and only calls luci.veracrypt.*, and mkdir/rm go through the backend's own methods. Drop this block, the identical one at lines 26-34, and "file": [ "exec" ] at line 23.
| "file": { | |
| "*": [ "list", "stat" ], | |
| "/bin/mkdir": [ "exec" ], | |
| "/usr/bin/mkdir": [ "exec" ], | |
| "/bin/rm": [ "exec" ], | |
| "/usr/bin/rm": [ "exec" ], | |
| "/bin/rmdir": [ "exec" ], | |
| "/usr/bin/rmdir": [ "exec" ], | |
| "/bin/busybox": [ "exec" ] | |
| }, |
Generated by Claude Code
|
|
||
| LUCI_TITLE:=LuCI support for VeraCrypt | ||
| LUCI_DESCRIPTION:=Optional web UI for console VeraCrypt. Uses veracrypt --text only; no VeraCrypt GUI toolkit. | ||
| LUCI_DEPENDS:=+luci-base +veracrypt |
There was a problem hiding this comment.
There is no veracrypt package in openwrt/packages at 0a78135 — nothing matches veracrypt anywhere in that tree — so this dependency cannot resolve today. The checklist ticks "includes what it depends on" but the body names no sister PR; please link the openwrt/packages PR that adds the CLI package.
Generated by Claude Code
| return 0 | ||
| ;; | ||
| esac | ||
| err=/tmp/vc-mkdir.err |
There was a problem hiding this comment.
Fixed, predictable path in world-writable /tmp opened for writing as root — an unprivileged local user can pre-create it as a symlink and the 2>"$err" redirect will truncate and overwrite the target. Same pattern at line 664 (/tmp/vc-rm.err) and line 960 (/tmp/vc-umount.out).
| err=/tmp/vc-mkdir.err | |
| err=$(mktemp /tmp/vc-mkdir.XXXXXX) || err=/dev/null |
Generated by Claude Code
| return 0 ;; | ||
| esac | ||
| case $p in | ||
| /dev/*|/proc/*|/sys/*|/etc/*|/usr/*|/bin/*|/sbin/*|/lib/*|/rom/*|/overlay/*|/tmp/luci-veracrypt-job*) |
There was a problem hiding this comment.
/www, /root and /boot are protected as exact paths in the list above but their subtrees are not covered here, so rm will delete e.g. /root/.ssh or /www/luci-static as root.
| /dev/*|/proc/*|/sys/*|/etc/*|/usr/*|/bin/*|/sbin/*|/lib/*|/rom/*|/overlay/*|/tmp/luci-veracrypt-job*) | |
| /dev/*|/proc/*|/sys/*|/etc/*|/usr/*|/bin/*|/sbin/*|/lib/*|/rom/*|/overlay/*|/www/*|/root/*|/boot/*|/tmp/luci-veracrypt-job*) |
Generated by Claude Code
| vc_invoke "$pw" --text --non-interactive \ | ||
| --pim="$pim_m" --protect-hidden=no \ | ||
| --mount-options=nokernelcrypto \ | ||
| --slot="$nslot" \ | ||
| ${kf:+--keyfiles="$kf"} \ | ||
| "$vol_path" "$mp" >> "$JOB.log" 2>&1 |
There was a problem hiding this comment.
${kf:+...} is unquoted, so a keyfile path containing spaces or glob characters gets word-split and globbed before it reaches veracrypt — valid_path permits both. Use the set -- accumulation this function already uses at line 1184.
| vc_invoke "$pw" --text --non-interactive \ | |
| --pim="$pim_m" --protect-hidden=no \ | |
| --mount-options=nokernelcrypto \ | |
| --slot="$nslot" \ | |
| ${kf:+--keyfiles="$kf"} \ | |
| "$vol_path" "$mp" >> "$JOB.log" 2>&1 | |
| set -- --text --non-interactive \ | |
| --pim="$pim_m" --protect-hidden=no \ | |
| --mount-options=nokernelcrypto \ | |
| --slot="$nslot" | |
| [ -n "$kf" ] && set -- "$@" --keyfiles="$kf" | |
| vc_invoke "$pw" "$@" "$vol_path" "$mp" >> "$JOB.log" 2>&1 |
Generated by Claude Code
2 participants
Web UI for the console veracrypt package (packages feed). Not bundled into veracrypt; apk add luci-app-veracrypt depends on +veracrypt. Calls veracrypt --text only, with passwords on stdin.
CLI reference: https://www.veracrypt.fr/en/Command%20Line%20Usage.html
Pull request details
Description
Screenshot or video of changes (if applicable)
Maintainer (preferred)
@
Tested on
OpenWrt version:
LuCI version:
Web browser(s):
Checklist