Browse Source

fix(database): keep the SQLite store owner-only (#6390)

* fix(database): keep the SQLite store owner-only

InitDB created the data directory 0755 and let SQLite create x-ui.db
and its -wal/-shm side files under the default umask, so on a stock
install they are world-readable. The store holds client UUIDs, Reality
private keys and the admin password hash, so any local account could
read them.

Create the directory 0700 and chmod the database files to 0600 right
after opening. SQLite gives -wal/-shm the mode of the main file, so
files created later inherit it; existing installs are tightened on the
next start. PostgreSQL deployments are untouched.

Assisted-by: Claude Code:claude-fable-5-1

* fix(database): tolerate chmod failures, keep the dump and install dir owner-only

Review follow-ups. A store the panel cannot chmod (root_squash NFS, a
foreign uid in a container) refused to start, which is worse than the
0644 it had before; log and continue instead, as the backup-directory
cleanup above already does. install.sh reset /etc/x-ui to 0755 right
after the binary created it 0700, so the directory hunk was inert on
real installs; create it 0700 there too. The migrate-db dump in the same
directory is a plaintext copy of the same secrets and was written 0644.

Assisted-by: Claude Code:claude-fable-5-1
ilyusha 8 hours ago
parent
commit
23511108bf
4 changed files with 96 additions and 3 deletions
  1. 1 1
      install.sh
  2. 15 1
      internal/database/db.go
  3. 79 0
      internal/database/db_permissions_test.go
  4. 1 1
      internal/database/dump_sqlite.go

+ 1 - 1
install.sh

@@ -155,7 +155,7 @@ write_install_result() {
     local u="$1" p="$2" port="$3" wbp="$4" scheme="$5" host="$6" token="$7" dbtype="$8"
     local result_file="/etc/x-ui/install-result.env"
     local url_host="${host:-SERVER_IP_UNKNOWN}"
-    install -d -m 755 /etc/x-ui 2> /dev/null
+    install -d -m 700 /etc/x-ui 2> /dev/null
     local prev_umask
     prev_umask=$(umask)
     umask 077

+ 15 - 1
internal/database/db.go

@@ -2094,7 +2094,7 @@ func InitDB(dbPath string) error {
 		}
 	default:
 		dir := path.Dir(dbPath)
-		if err = os.MkdirAll(dir, 0o755); err != nil {
+		if err = os.MkdirAll(dir, 0o700); err != nil {
 			return err
 		}
 		if err = cleanupSQLiteBackupDirs(filepath.Dir(dbPath)); err != nil {
@@ -2108,6 +2108,9 @@ func InitDB(dbPath string) error {
 		if err != nil {
 			return err
 		}
+		if err := restrictSQLiteFilePerms(dbPath); err != nil {
+			log.Printf("restrict SQLite file permissions: %v", err)
+		}
 		sqlDB, err := db.DB()
 		if err != nil {
 			return err
@@ -2213,6 +2216,17 @@ func openPostgresWithRetry(dsn string, c *gorm.Config) (*gorm.DB, error) {
 	return nil, fmt.Errorf("postgres unreachable after %d attempts: %w", len(delays), lastErr)
 }
 
+// The store holds client secrets, so it and its WAL/SHM side files stay
+// owner-only. Best effort: a store the panel cannot chmod still opens.
+func restrictSQLiteFilePerms(dbPath string) error {
+	for _, name := range []string{dbPath, dbPath + "-wal", dbPath + "-shm"} {
+		if err := os.Chmod(name, 0o600); err != nil && !errors.Is(err, os.ErrNotExist) {
+			return err
+		}
+	}
+	return nil
+}
+
 func sqliteJournalMode() string {
 	switch strings.ToUpper(strings.TrimSpace(os.Getenv("XUI_DB_JOURNAL_MODE"))) {
 	case "DELETE":

+ 79 - 0
internal/database/db_permissions_test.go

@@ -0,0 +1,79 @@
+package database
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+)
+
+func TestInitDBRestrictsSQLiteFilePermissions(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("POSIX permission bits are not meaningful on Windows")
+	}
+	t.Setenv("XUI_DB_JOURNAL_MODE", "")
+	dbDir := filepath.Join(t.TempDir(), "x-ui")
+	dbPath := filepath.Join(dbDir, "x-ui.db")
+
+	if err := InitDB(dbPath); err != nil {
+		t.Fatalf("InitDB: %v", err)
+	}
+	t.Cleanup(func() { _ = CloseDB() })
+
+	if info, err := os.Stat(dbDir); err != nil {
+		t.Fatalf("stat db dir: %v", err)
+	} else if perm := info.Mode().Perm(); perm != 0o700 {
+		t.Fatalf("db dir perm = %o, want 700", perm)
+	}
+	for _, name := range []string{dbPath, dbPath + "-wal", dbPath + "-shm"} {
+		info, err := os.Stat(name)
+		if errors.Is(err, os.ErrNotExist) && name != dbPath {
+			continue
+		}
+		if err != nil {
+			t.Fatalf("stat %s: %v", name, err)
+		}
+		if perm := info.Mode().Perm(); perm != 0o600 {
+			t.Fatalf("%s perm = %o, want 600", filepath.Base(name), perm)
+		}
+	}
+}
+
+func TestInitDBTightensExistingSQLiteFilePermissions(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("POSIX permission bits are not meaningful on Windows")
+	}
+	t.Setenv("XUI_DB_JOURNAL_MODE", "")
+	dbPath := filepath.Join(t.TempDir(), "x-ui.db")
+	if err := InitDB(dbPath); err != nil {
+		t.Fatalf("seed InitDB: %v", err)
+	}
+	if err := CloseDB(); err != nil {
+		t.Fatalf("seed CloseDB: %v", err)
+	}
+	// Simulate a store created by an older release under the default umask.
+	for _, name := range []string{dbPath, dbPath + "-wal", dbPath + "-shm"} {
+		if err := os.Chmod(name, 0o644); err != nil && !errors.Is(err, os.ErrNotExist) {
+			t.Fatalf("chmod %s: %v", name, err)
+		}
+	}
+
+	if err := InitDB(dbPath); err != nil {
+		t.Fatalf("InitDB: %v", err)
+	}
+	t.Cleanup(func() { _ = CloseDB() })
+
+	for _, name := range []string{dbPath, dbPath + "-wal", dbPath + "-shm"} {
+		info, err := os.Stat(name)
+		if errors.Is(err, os.ErrNotExist) && name != dbPath {
+			continue
+		}
+		if err != nil {
+			t.Fatalf("stat %s: %v", name, err)
+		}
+		if perm := info.Mode().Perm(); perm != 0o600 {
+			t.Fatalf("%s perm = %o, want 600", filepath.Base(name), perm)
+		}
+	}
+}

+ 1 - 1
internal/database/dump_sqlite.go

@@ -24,7 +24,7 @@ func DumpSQLite(srcPath, outPath string) error {
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(outPath, data, 0o644)
+	return os.WriteFile(outPath, data, 0o600)
 }
 
 // DumpSQLiteToBytes builds the same `sqlite3 .dump`-style SQL text as DumpSQLite