Preserve controller ACL across firmware upgrades and recover missing access

docs/guide/firmware.md

@@ -115,6 +115,42 @@ firmware flash**. See the
 [official upgrade overview](https://openwrt.org/docs/guide-user/installation/generic.sysupgrade)
 and [LuCI verification procedure](https://openwrt.org/docs/guide-quick-start/sysupgrade.luci).
 
+## Preserve and recover controller access
+
+In development builds after v0.1.8, adoption and **Review or refresh controller
+access payload** register `/usr/share/rpcd/acl.d/oonfeewrt.json` in
+`/etc/sysupgrade.conf`. This keeps the ACL alongside the controller login when
+an OpenWrt upgrade preserves settings, including upgrades through owut or
+Attended Sysupgrade. Monitor-only devices preserve their read-only ACL using
+**Review or refresh read-only observation ACL**. Un-adoption and failed-adoption
+cleanup remove the ACL's backup entry without removing other backup paths.
+
+For devices adopted by an earlier controller, run the appropriate refresh action
+once before the next firmware upgrade. Upgrading the controller alone cannot
+change an existing router's backup list. Verify inclusion over administrator SSH
+without flashing anything:
+
+```sh
+sysupgrade -l | grep -Fx '/usr/share/rpcd/acl.d/oonfeewrt.json'
+```
+
+If an upgrade already removed the ACL, the device can still accept its controller
+login while all observations fail with `access denied`. Open the device's detail
+panel and use the same refresh action with its administrator password or SSH key.
+Recovery requires the SSH host key saved at adoption to match before any
+administrator credential is sent. The controller then restores the appropriate
+ACL, registers it for future upgrades, and verifies a fresh controller session,
+device MAC, and capability probe before reporting success. It keeps the existing
+controller login, management mode, inventory, and configuration ownership.
+
+If there is no saved SSH host key, verify the router through a trusted
+administrator SSH connection and restore the ACL matching its managed or
+monitor-only mode. Refresh can then verify its MAC and record the SSH identity.
+If a saved key changed, investigate and resolve the stored identity mismatch
+before retrying; recovery never accepts a replacement key silently. Upgrades
+that discard settings also discard the controller login and require adoption
+again.
+
 ## Optional read-only router helper
 
 The source tree includes an OpenWrt SDK package at `deploy/openwrt-agent`. This

internal/adoption/acl_preservation_test.go

@@ -0,0 +1,167 @@
+package adoption
+
+import (
+	"context"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/aiden0rchad/oonfeewrt/internal/testutil"
+)
+
+func aclFilesystem(t *testing.T) (*SSHBootstrap, string) {
+	t.Helper()
+	root := t.TempDir()
+	for _, dir := range []string{"etc", "usr/share/rpcd/acl.d", "bin"} {
+		if err := os.MkdirAll(filepath.Join(root, dir), 0755); err != nil {
+			t.Fatal(err)
+		}
+	}
+	// Only the login's absence matters to these footprint-removal tests.
+	if err := os.WriteFile(filepath.Join(root, "bin/uci"), []byte("#!/bin/sh\n[ \"$2\" != get ]\n"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	server := testutil.StartSSH(t, "", func(command string, stdin io.Reader, stdout, stderr io.Writer) uint32 {
+		if !strings.Contains(command, "conf=/etc/sysupgrade.conf") {
+			command = strings.ReplaceAll(command, "/usr/share/rpcd/acl.d/", root+"/usr/share/rpcd/acl.d/")
+		}
+		command = strings.ReplaceAll(command, "/etc/sysupgrade.conf", root+"/etc/sysupgrade.conf")
+		cmd := exec.Command("sh", "-c", command)
+		cmd.Env = append(os.Environ(), "PATH="+root+"/bin:"+os.Getenv("PATH"))
+		cmd.Stdin, cmd.Stdout, cmd.Stderr = stdin, stdout, stderr
+		if err := cmd.Run(); err != nil {
+			return 1
+		}
+		return 0
+	})
+	boot, err := DialSSH(context.Background(), SSHOptions{Host: server.Addr, HostKeyFP: server.Fingerprint})
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { boot.Close() })
+	return boot, root
+}
+
+func TestACLBackupEntryLifecycle(t *testing.T) {
+	const other = "# custom backup files\n# " + DefaultACLPath + "\n/etc/custom.conf\n/usr/share/rpcd/acl.d/oonfeewrtXjson\n"
+	for _, tc := range []struct {
+		name, before, remaining string
+		missing                 bool
+	}{
+		{name: "missing file", missing: true},
+		{name: "empty file"},
+		{name: "operator entries", before: other, remaining: other},
+		{name: "no trailing newline", before: "/etc/custom.conf", remaining: "/etc/custom.conf\n"},
+		{name: "existing entry", before: other + DefaultACLPath + "\n", remaining: other},
+		{name: "duplicate entries", before: DefaultACLPath + "\n" + other + DefaultACLPath + "\n", remaining: other},
+	} {
+		for _, footprint := range []bool{false, true} {
+			t.Run(tc.name+map[bool]string{false: "/ACL rollback", true: "/unadopt"}[footprint], func(t *testing.T) {
+				boot, root := aclFilesystem(t)
+				conf := filepath.Join(root, "etc/sysupgrade.conf")
+				if !tc.missing {
+					if err := os.WriteFile(conf, []byte(tc.before), 0640); err != nil {
+						t.Fatal(err)
+					}
+				}
+				for range 2 {
+					if err := boot.InstallACL(context.Background(), DefaultACLPath, []byte(`{"acl":"test"}`)); err != nil {
+						t.Fatal(err)
+					}
+					got, err := os.ReadFile(conf)
+					if err != nil || string(got) != tc.remaining+DefaultACLPath+"\n" {
+						t.Fatalf("backup list = %q, err=%v", got, err)
+					}
+				}
+				info, err := os.Stat(conf)
+				if err != nil {
+					t.Fatal(err)
+				}
+				wantMode := os.FileMode(0640)
+				if tc.missing {
+					wantMode = 0644
+				}
+				if info.Mode().Perm() != wantMode {
+					t.Fatalf("backup list permissions = %o, want %o", info.Mode().Perm(), wantMode)
+				}
+				for range 2 {
+					var err error
+					if footprint {
+						err = boot.RemoveFootprint(context.Background(), DefaultACLPath, DefaultUser)
+					} else {
+						err = boot.RemoveACL(context.Background(), DefaultACLPath)
+					}
+					if err != nil {
+						t.Fatal(err)
+					}
+				}
+				got, err := os.ReadFile(conf)
+				if err != nil || string(got) != tc.remaining {
+					t.Fatalf("cleanup changed unrelated entries: %q, err=%v", got, err)
+				}
+				if _, err := os.Stat(root + DefaultACLPath); !os.IsNotExist(err) {
+					t.Fatalf("ACL still exists after cleanup: %v", err)
+				}
+			})
+		}
+	}
+}
+
+func TestACLRemovalWithoutBackupListDoesNotCreateOne(t *testing.T) {
+	boot, root := aclFilesystem(t)
+	if err := boot.RemoveACL(context.Background(), DefaultACLPath); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(filepath.Join(root, "etc/sysupgrade.conf")); !os.IsNotExist(err) {
+		t.Fatalf("cleanup created a backup list: %v", err)
+	}
+}
+
+func TestACLBackupWriteFailurePreservesOperatorListAndReportsFailure(t *testing.T) {
+	boot, root := aclFilesystem(t)
+	conf := filepath.Join(root, "etc/sysupgrade.conf")
+	const original = "# keep this exactly\n/etc/custom.conf"
+	if err := os.WriteFile(conf, []byte(original), 0640); err != nil {
+		t.Fatal(err)
+	}
+	// Fail only the backup-list rename, after the ACL itself was installed.
+	script := "#!/bin/sh\nfor dest do :; done\ncase \"$dest\" in */etc/sysupgrade.conf) exit 1;; esac\nexec /bin/mv \"$@\"\n"
+	if err := os.WriteFile(filepath.Join(root, "bin/mv"), []byte(script), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := boot.InstallACL(context.Background(), DefaultACLPath, []byte("acl")); err == nil || !strings.Contains(err.Error(), "preserve controller ACL") {
+		t.Fatalf("backup registration failure was hidden: %v", err)
+	}
+	got, err := os.ReadFile(conf)
+	if err != nil || string(got) != original {
+		t.Fatalf("failed write damaged the original list: %q, err=%v", got, err)
+	}
+	if err := boot.RemoveACL(context.Background(), DefaultACLPath); err == nil || !strings.Contains(err.Error(), "backup entry") {
+		t.Fatalf("backup cleanup failure was hidden: %v", err)
+	}
+	leftovers, err := filepath.Glob(conf + ".oonfee.*")
+	if err != nil || len(leftovers) != 0 {
+		t.Fatalf("temporary backup lists remain: %v, err=%v", leftovers, err)
+	}
+}
+
+func TestACLBackupListSymlinkIsNotReplaced(t *testing.T) {
+	boot, root := aclFilesystem(t)
+	target := filepath.Join(root, "etc/operator-list")
+	if err := os.WriteFile(target, []byte("/etc/private\n"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(target, filepath.Join(root, "etc/sysupgrade.conf")); err != nil {
+		t.Fatal(err)
+	}
+	if err := boot.InstallACL(context.Background(), DefaultACLPath, []byte("acl")); err == nil {
+		t.Fatal("backup registration replaced an operator's symlink")
+	}
+	got, err := os.ReadFile(target)
+	if err != nil || string(got) != "/etc/private\n" {
+		t.Fatalf("operator list changed: %q, err=%v", got, err)
+	}
+}

internal/adoption/adopt.go

@@ -172,7 +172,8 @@ func (a *Adopter) Adopt(ctx context.Context, operator *ubus.Client, boot Bootstr
 	// 3. Install the ACL file BEFORE the login, so the login never exists with
 	//    its access-groups undefined.
 	if err := boot.InstallACL(ctx, aclPath, a.ACL); err != nil {
-		return nil, fmt.Errorf("adoption: write %s: %w", aclPath, err)
+		primary := fmt.Errorf("adoption: write %s: %w", aclPath, err)
+		return nil, rollbackAdoption(ctx, boot, aclPath, user, false, primary)
 	}
 
 	// 4. Create the login.
@@ -392,7 +393,7 @@ func (r *UnadoptReport) Residue() []string {
 		out = append(out, fmt.Sprintf("config section %s.%s", s.Config, s.Section))
 	}
 	if !r.ACLRemoved {
-		out = append(out, r.ACLPath)
+		out = append(out, r.ACLPath, "backup entry for "+r.ACLPath+" in /etc/sysupgrade.conf")
 	}
 	if !r.LoginRemoved {
 		out = append(out, fmt.Sprintf("config login '%s' in /etc/config/rpcd", r.User))
@@ -440,6 +441,7 @@ func (r *UnadoptReport) CleanupCommands() []string {
 		out = append(out,
 			"rm -f "+quoted,
 			"[ ! -e "+quoted+" ] && echo 'ACL gone' || echo 'ERROR: ACL still present'",
+			"[ ! -e "+quoted+" ] && "+sysupgradeEntryCommand(r.ACLPath, false),
 		)
 	}
 	return out

internal/adoption/adopt_test.go

@@ -126,12 +126,9 @@ func newBootFor(c *ubus.Client) *fakeBoot {
 }
 
 func (b *fakeBoot) InstallACL(_ context.Context, path string, content []byte) error {
-	if b.failACL != nil {
-		return b.failACL
-	}
 	b.acl[path] = append([]byte(nil), content...)
 	b.installed = true
-	return nil
+	return b.failACL
 }
 
 func (b *fakeBoot) CreateLogin(_ context.Context, user, passHash string, groups []string) error {
@@ -420,6 +417,7 @@ func TestAdoptRollbackFailureReportsSafeResidueAndRemedy(t *testing.T) {
 	}
 	wantResidue := []string{
 		DefaultACLPath,
+		"backup entry for " + DefaultACLPath + " in /etc/sysupgrade.conf",
 		"config login '" + DefaultUser + "' in /etc/config/rpcd",
 	}
 	if fmt.Sprint(rollback.Residue) != fmt.Sprint(wantResidue) {
@@ -458,7 +456,7 @@ func TestAdoptACLOnlyRollbackFailureDoesNotPrescribeLoginRemoval(t *testing.T) {
 	if !errors.As(err, &rollback) || !errors.Is(err, primary) {
 		t.Fatalf("rollback failure lost its primary cause: %v", err)
 	}
-	if fmt.Sprint(rollback.Residue) != fmt.Sprint([]string{DefaultACLPath}) {
+	if fmt.Sprint(rollback.Residue) != fmt.Sprint([]string{DefaultACLPath, "backup entry for " + DefaultACLPath + " in /etc/sysupgrade.conf"}) {
 		t.Fatalf("ACL-only residue = %v", rollback.Residue)
 	}
 	commands := strings.Join(rollback.CleanupCommands, "\n")
@@ -510,21 +508,22 @@ func TestUnadoptWithoutOperatorStopsAndReportsResidue(t *testing.T) {
 		t.Error("the footprint must be reported as remaining")
 	}
 	residue := rep.Residue()
-	if len(residue) != 2 {
-		t.Fatalf("residue should name both the ACL file and the login, got %v", residue)
+	if len(residue) != 3 {
+		t.Fatalf("residue should name the ACL, backup entry, and login, got %v", residue)
 	}
 	// The fallback screen shows these, so they must be the real paths.
 	if !strings.Contains(residue[0], DefaultACLPath) {
 		t.Errorf("residue should name the ACL path, got %q", residue[0])
 	}
-	if !strings.Contains(residue[1], "/etc/config/rpcd") {
-		t.Errorf("residue should name the rpcd login, got %q", residue[1])
+	if !strings.Contains(residue[2], "/etc/config/rpcd") {
+		t.Errorf("residue should name the rpcd login, got %q", residue[2])
 	}
 	commands := rep.CleanupCommands()
 	want := "uci -q delete rpcd.oonfeewrt\nuci commit rpcd\n" +
 		"uci -q get rpcd.oonfeewrt >/dev/null 2>&1 && echo 'ERROR: login still present' || echo 'login gone'\n" +
 		"rm -f '/usr/share/rpcd/acl.d/oonfeewrt.json'\n" +
-		"[ ! -e '/usr/share/rpcd/acl.d/oonfeewrt.json' ] && echo 'ACL gone' || echo 'ERROR: ACL still present'"
+		"[ ! -e '/usr/share/rpcd/acl.d/oonfeewrt.json' ] && echo 'ACL gone' || echo 'ERROR: ACL still present'\n" +
+		"[ ! -e '/usr/share/rpcd/acl.d/oonfeewrt.json' ] && " + sysupgradeEntryCommand(DefaultACLPath, false)
 	if got := strings.Join(commands, "\n"); got != want {
 		t.Errorf("manual cleanup recipe = %q, want %q", got, want)
 	}
@@ -619,6 +618,9 @@ func TestAdoptDoesNotCreateALoginIfTheACLFails(t *testing.T) {
 	if boot.login != "" {
 		t.Fatalf("a login %q was created after the ACL write failed", boot.login)
 	}
+	if len(boot.acl) != 0 || boot.removeACLCalls != 1 || boot.removeFootprintCalls != 0 {
+		t.Fatalf("partial ACL installation was not rolled back safely: %+v", boot)
+	}
 }
 
 // The reverse ordering check: a login failure happens after the ACL landed,

internal/adoption/bootstrap.go

@@ -28,16 +28,16 @@ import (
 // afterwards is ubus, and §2's argument for it is untouched: this is the
 // bootstrap, not the transport.
 type Bootstrap interface {
-	// InstallACL writes the access-control file and verifies it landed intact.
+	// InstallACL writes and verifies the ACL, and registers it in sysupgrade.conf.
 	InstallACL(ctx context.Context, path string, content []byte) error
 
 	// CreateLogin adds the rpcd login and commits it. passHash must already be
 	// a crypt hash — rpcd rejects a plaintext password outright.
 	CreateLogin(ctx context.Context, user, passHash string, groups []string) error
 
-	// RemoveACL removes only the ACL file. Adoption uses this when login
-	// creation fails: removing a login whose creation was not proved could
-	// delete pre-existing state that does not belong to the controller.
+	// RemoveACL removes the ACL and its sysupgrade.conf entry. Adoption uses
+	// this when login creation fails: removing an unproved login could delete
+	// pre-existing state that does not belong to the controller.
 	RemoveACL(ctx context.Context, aclPath string) error
 
 	// RemoveFootprint deletes the login and the ACL file. Used by un-adopt.

internal/adoption/ssh.go

@@ -257,7 +257,7 @@ func redactSSHError(msg string, values []string) string {
 	return msg
 }
 
-// InstallACL writes the file and proves it landed.
+// InstallACL writes the file, proves it landed, and preserves it across upgrades.
 //
 // The content goes over stdin rather than inside the command, so no amount of
 // JSON quoting can become shell syntax. It is written to a temporary file and
@@ -284,7 +284,37 @@ func (b *SSHBootstrap) InstallACL(ctx context.Context, path string, content []by
 		"mv "+shellQuote(tmp)+" "+shellQuote(path)+" && chmod 0644 "+shellQuote(path)); err != nil {
 		return err
 	}
-	return nil
+	_, err = b.run(ctx, nil, "preserve controller ACL across firmware upgrades", sysupgradeEntryCommand(path, true))
+	return err
+}
+
+// Rewrite atomically: a full overlay must not truncate the operator's backup
+// list. Fixed-string matching leaves comments and unrelated paths untouched.
+func sysupgradeEntryCommand(aclPath string, keep bool) string {
+	missing, appendEntry := "exit 0", ""
+	if keep {
+		missing = "source=/dev/null"
+		appendEntry = "printf '%s\\n' " + shellQuote(aclPath) + " >> \"$tmp\" || exit 1\n"
+	}
+	return `(conf=/etc/sysupgrade.conf
+source="$conf"
+if [ -e "$conf" ] || [ -L "$conf" ]; then
+  [ -f "$conf" ] && [ ! -L "$conf" ] || exit 1
+else
+  ` + missing + `
+fi
+tmp=$(mktemp "$conf.oonfee.XXXXXX") || exit 1
+trap 'rm -f "$tmp"' EXIT
+if [ "$source" = /dev/null ]; then
+  chmod 0644 "$tmp" || exit 1
+else
+  cp -p "$source" "$tmp" || exit 1
+fi
+grep -Fvx -- ` + shellQuote(aclPath) + ` "$source" > "$tmp"
+result=$?
+[ "$result" -le 1 ] || exit "$result"
+` + appendEntry + `mv -f "$tmp" "$conf"
+)`
 }
 
 func verifyACLHash(content []byte, output string) error {
@@ -355,7 +385,8 @@ func (b *SSHBootstrap) RemoveACL(ctx context.Context, aclPath string) error {
 	if strings.Contains(out, "acl=present") {
 		return fmt.Errorf("adoption: the ACL file %s is still on the device after removal", aclPath)
 	}
-	return nil
+	_, err = b.run(ctx, nil, "remove controller ACL backup entry", sysupgradeEntryCommand(aclPath, false))
+	return err
 }
 
 // RemoveFootprint deletes the login and the ACL file.
@@ -404,7 +435,8 @@ func (b *SSHBootstrap) RemoveFootprint(ctx context.Context, aclPath, user string
 		return fmt.Errorf("adoption: the ACL file %s is still on the device "+
 			"after removal", aclPath)
 	}
-	return nil
+	_, err = b.run(ctx, nil, "remove controller ACL backup entry", sysupgradeEntryCommand(aclPath, false))
+	return err
 }
 
 // PackageState reads only the fixed package/service facts needed by the LLDP