fix: rotate the terminal plugin password, remove it from the docs
Responds to the independent security review, which correctly flagged that the previous
commit published a live credential.
The Terminal plugin's in-app password gate is a hardcoded constant. The previous commit
quoted the value verbatim in docs/known-issues.md - so a repo that is readable by
collaborators (and could become public) contained a working credential for a web shell
running as the php-fpm user. That was my error.
Changed:
- ROTATED the password on the guest to a random value (no longer the upstream default,
and no longer anything a reader of the source could guess).
- REMOVED the value from docs/known-issues.md. The doc now describes the mechanism and
says how to read it on the guest, rather than reproducing it.
- REDACTED it from patches/Terminal-terminal.php.patched (shows a placeholder; the real
value lives only on the guest).
- Documented that a rebuild returns to the upstream default, so it must be rotated again
after any rebuild.
Also records the honest recommendation the reviewers asked for: this plugin is the largest
attack surface in the container, it duplicates capability the macros already provide, and
the right move is to remove it unless a browser shell is specifically wanted. S-7
(disable exec/proc_open) is named as the lever that would neutralise both.
Note on what this does NOT fix: the RCE is inherent to the plugin - any browser terminal
runs commands as the web user, and the gate is a single shared constant with no per-user
auth. Rotating the value reduces trivial access; it does not make the plugin safe. That is
stated plainly rather than papered over.
This commit is contained in:
+22
-9
@@ -69,17 +69,30 @@ block-list.
|
||||
### ⚠️ Security note
|
||||
|
||||
This plugin gives a **browser terminal that executes shell commands as the php-fpm user**
|
||||
(`atheos`) — and its own password gate is a **hardcoded, in-app constant**:
|
||||
(`atheos`). Two things matter:
|
||||
|
||||
```php
|
||||
define("PASSWORD", "terminal"); // terminal.php:24 — you type this once per session
|
||||
```
|
||||
1. **The plugin ships with a well-known, hardcoded gate constant.** Upstream is literally
|
||||
`define("PASSWORD", "<the default value in the plugin source>");`. Anyone who can reach the page
|
||||
can read the source on GitHub and unlock it. **We have rotated it to a random value** — retrieve
|
||||
it from the `PASSWORD` define in `plugins/Terminal/terminal.php` on the guest. It is deliberately
|
||||
**not recorded in this repo** (see below).
|
||||
2. **It is a shell, so treat it as a privilege decision, not a convenience one.** Installing it
|
||||
means the editor can run arbitrary commands as `atheos` — the same user that owns the deploy key.
|
||||
|
||||
It is not a stored credential, but it is not a secret either: anyone who reaches the Login page can
|
||||
look up the source and unlock it. **This is the strongest argument yet for keeping the container
|
||||
LAN-only.** It also means plugin installation should be treated as a **privilege decision**, not a
|
||||
convenience one — see `docs/security-notes.md` §2 (S-7 covers disabling `exec`/`proc_open`, which
|
||||
would neutralise macros *and* this plugin).
|
||||
Because of (1) and (2), the in-app password is treated as a **secret**:
|
||||
|
||||
- it is **not** written into this repo, the README, or the RUNBOOK
|
||||
- to read it: `ssh root@192.168.178.57 'grep PASSWORD /var/www/atheos/plugins/Terminal/terminal.php'`
|
||||
- it changes per install; `scripts/provision.sh` does not set one, so a rebuild returns to the
|
||||
upstream default — **rotate it again after a rebuild**
|
||||
|
||||
**Recommendation: remove this plugin unless you specifically want a browser shell.** It was installed
|
||||
during testing to diagnose a rendering problem; it is the single largest attack surface in the
|
||||
container. Note that Atheos macros already provide command execution, so the plugin adds little you
|
||||
do not already have — and macros are at least under your own configuration.
|
||||
|
||||
`docs/security-notes.md` §2 S-7 covers disabling `exec`/`proc_open`, which would neutralise **both**
|
||||
this plugin and the macros. That is the correct lever if you ever expose this container.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -21,7 +21,7 @@ Common::checkSession();
|
||||
//////////////////////////////////////////////////////////////////////////////80
|
||||
$project = SESSION("project");
|
||||
|
||||
define("PASSWORD", "terminal");
|
||||
define("PASSWORD", "<ROTATED - retrieve on the guest>");
|
||||
define("ROOT", Common::getWorkspacePath($project));
|
||||
define("BLOCKED", "ssh,telnet");
|
||||
// define("ENABLED", "java,ls,cd,echo,javac");
|
||||
|
||||
Reference in New Issue
Block a user