fix(security): API import no longer replaces user accounts
Cluster-secret holder could POST a backup with their own admin key to /api/backup/import, replacing all users — privilege escalation from cluster-auth to admin. Also, Kuma imports produced zero users but ImportWipe unconditionally deleted the users table — locking out all accounts until restart reseeded UPTOP_ADMIN_KEY. - Server handlers strip data.Users (set nil) before calling ImportData - ImportData only wipes+replaces users when data.Users != nil - New ImportWipeUsers dialect method separates user wipe from data wipe - CLI restore (main.go) unchanged — full import still replaces users
This commit is contained in:
@@ -205,12 +205,15 @@ func (s *Server) handleImport(w http.ResponseWriter, r *http.Request) {
|
|||||||
http.Error(w, "Invalid JSON", http.StatusBadRequest)
|
http.Error(w, "Invalid JSON", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
// API import never modifies users — cluster-secret holder shouldn't be
|
||||||
|
// able to replace admin accounts. CLI restore still does full import.
|
||||||
|
data.Users = nil
|
||||||
if err := s.store.ImportData(r.Context(), data); err != nil {
|
if err := s.store.ImportData(r.Context(), data); err != nil {
|
||||||
slog.Error("import failed", "err", err)
|
slog.Error("import failed", "err", err)
|
||||||
http.Error(w, "Import failed", http.StatusInternalServerError)
|
http.Error(w, "Import failed", http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
_, _ = w.Write([]byte("Import Successful"))
|
_, _ = w.Write([]byte("Import Successful (users excluded — manage via CLI or UPTOP_KEYS)"))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Server) handleKumaImport(w http.ResponseWriter, r *http.Request) {
|
func (s *Server) handleKumaImport(w http.ResponseWriter, r *http.Request) {
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ type Dialect interface {
|
|||||||
BoolFalse() string
|
BoolFalse() string
|
||||||
ResetSequenceOnEmpty(db *sql.DB, table string)
|
ResetSequenceOnEmpty(db *sql.DB, table string)
|
||||||
ImportWipe(tx *sql.Tx)
|
ImportWipe(tx *sql.Tx)
|
||||||
|
ImportWipeUsers(tx *sql.Tx)
|
||||||
ImportResetSequences(tx *sql.Tx)
|
ImportResetSequences(tx *sql.Tx)
|
||||||
UpsertNodeSQL() string
|
UpsertNodeSQL() string
|
||||||
UpsertAlertHealthSQL() string
|
UpsertAlertHealthSQL() string
|
||||||
|
|||||||
@@ -138,9 +138,6 @@ func (d *PostgresDialect) ImportWipe(tx *sql.Tx) {
|
|||||||
if _, err := tx.Exec("TRUNCATE TABLE alerts RESTART IDENTITY CASCADE"); err != nil {
|
if _, err := tx.Exec("TRUNCATE TABLE alerts RESTART IDENTITY CASCADE"); err != nil {
|
||||||
slog.Debug("import wipe failed", "table", "alerts", "err", err)
|
slog.Debug("import wipe failed", "table", "alerts", "err", err)
|
||||||
}
|
}
|
||||||
if _, err := tx.Exec("TRUNCATE TABLE users RESTART IDENTITY CASCADE"); err != nil {
|
|
||||||
slog.Debug("import wipe failed", "table", "users", "err", err)
|
|
||||||
}
|
|
||||||
if _, err := tx.Exec("TRUNCATE TABLE maintenance_windows RESTART IDENTITY CASCADE"); err != nil {
|
if _, err := tx.Exec("TRUNCATE TABLE maintenance_windows RESTART IDENTITY CASCADE"); err != nil {
|
||||||
slog.Debug("import wipe failed", "table", "maintenance_windows", "err", err)
|
slog.Debug("import wipe failed", "table", "maintenance_windows", "err", err)
|
||||||
}
|
}
|
||||||
@@ -155,6 +152,12 @@ func (d *PostgresDialect) ImportWipe(tx *sql.Tx) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (d *PostgresDialect) ImportWipeUsers(tx *sql.Tx) {
|
||||||
|
if _, err := tx.Exec("TRUNCATE TABLE users RESTART IDENTITY CASCADE"); err != nil {
|
||||||
|
slog.Debug("import wipe failed", "table", "users", "err", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func (d *PostgresDialect) ImportResetSequences(tx *sql.Tx) {
|
func (d *PostgresDialect) ImportResetSequences(tx *sql.Tx) {
|
||||||
if _, err := tx.Exec("SELECT setval('sites_id_seq', (SELECT COALESCE(MAX(id), 1) FROM sites))"); err != nil {
|
if _, err := tx.Exec("SELECT setval('sites_id_seq', (SELECT COALESCE(MAX(id), 1) FROM sites))"); err != nil {
|
||||||
slog.Debug("sequence reset failed", "table", "sites", "err", err)
|
slog.Debug("sequence reset failed", "table", "sites", "err", err)
|
||||||
|
|||||||
@@ -167,12 +167,6 @@ func (d *SQLiteDialect) ImportWipe(tx *sql.Tx) {
|
|||||||
if _, err := tx.Exec("DELETE FROM sqlite_sequence WHERE name='alerts'"); err != nil {
|
if _, err := tx.Exec("DELETE FROM sqlite_sequence WHERE name='alerts'"); err != nil {
|
||||||
slog.Debug("import wipe failed", "table", "sqlite_sequence(alerts)", "err", err)
|
slog.Debug("import wipe failed", "table", "sqlite_sequence(alerts)", "err", err)
|
||||||
}
|
}
|
||||||
if _, err := tx.Exec("DELETE FROM users"); err != nil {
|
|
||||||
slog.Debug("import wipe failed", "table", "users", "err", err)
|
|
||||||
}
|
|
||||||
if _, err := tx.Exec("DELETE FROM sqlite_sequence WHERE name='users'"); err != nil {
|
|
||||||
slog.Debug("import wipe failed", "table", "sqlite_sequence(users)", "err", err)
|
|
||||||
}
|
|
||||||
if _, err := tx.Exec("DELETE FROM maintenance_windows"); err != nil {
|
if _, err := tx.Exec("DELETE FROM maintenance_windows"); err != nil {
|
||||||
slog.Debug("import wipe failed", "table", "maintenance_windows", "err", err)
|
slog.Debug("import wipe failed", "table", "maintenance_windows", "err", err)
|
||||||
}
|
}
|
||||||
@@ -190,4 +184,13 @@ func (d *SQLiteDialect) ImportWipe(tx *sql.Tx) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (d *SQLiteDialect) ImportWipeUsers(tx *sql.Tx) {
|
||||||
|
if _, err := tx.Exec("DELETE FROM users"); err != nil {
|
||||||
|
slog.Debug("import wipe failed", "table", "users", "err", err)
|
||||||
|
}
|
||||||
|
if _, err := tx.Exec("DELETE FROM sqlite_sequence WHERE name='users'"); err != nil {
|
||||||
|
slog.Debug("import wipe failed", "table", "sqlite_sequence(users)", "err", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func (d *SQLiteDialect) ImportResetSequences(tx *sql.Tx) {}
|
func (d *SQLiteDialect) ImportResetSequences(tx *sql.Tx) {}
|
||||||
|
|||||||
@@ -742,9 +742,14 @@ func (s *SQLStore) ImportData(ctx context.Context, data models.Backup) error {
|
|||||||
|
|
||||||
s.dialect.ImportWipe(tx)
|
s.dialect.ImportWipe(tx)
|
||||||
|
|
||||||
for _, u := range data.Users {
|
// Only wipe+replace users when callers explicitly provide them (CLI
|
||||||
if _, err := tx.ExecContext(ctx, s.q("INSERT INTO users (username, public_key, role) VALUES (?, ?, ?)"), u.Username, u.PublicKey, u.Role); err != nil {
|
// full restore). API/Kuma imports pass nil — existing users preserved.
|
||||||
return err
|
if data.Users != nil {
|
||||||
|
s.dialect.ImportWipeUsers(tx)
|
||||||
|
for _, u := range data.Users {
|
||||||
|
if _, err := tx.ExecContext(ctx, s.q("INSERT INTO users (username, public_key, role) VALUES (?, ?, ?)"), u.Username, u.PublicKey, u.Role); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
for _, a := range data.Alerts {
|
for _, a := range data.Alerts {
|
||||||
|
|||||||
@@ -276,6 +276,31 @@ func TestImportData_WipesHistory(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestImportData_NilUsersPreservesExisting(t *testing.T) {
|
||||||
|
s := newTestStore(t)
|
||||||
|
|
||||||
|
if err := s.AddUser(context.Background(), "admin", "ssh-ed25519 ADMINKEY", "admin"); err != nil {
|
||||||
|
t.Fatalf("AddUser: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
backup := models.Backup{
|
||||||
|
Sites: []models.SiteConfig{{ID: 1, Name: "New", URL: "https://new.com", Type: "http", Interval: 30}},
|
||||||
|
Alerts: []models.AlertConfig{{ID: 1, Name: "a", Type: "webhook", Settings: map[string]string{"url": "https://h.com"}}},
|
||||||
|
Users: nil,
|
||||||
|
}
|
||||||
|
if err := s.ImportData(context.Background(), backup); err != nil {
|
||||||
|
t.Fatalf("ImportData: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
users, err := s.GetAllUsers(context.Background())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetAllUsers: %v", err)
|
||||||
|
}
|
||||||
|
if len(users) != 1 || users[0].Username != "admin" {
|
||||||
|
t.Errorf("expected existing admin user preserved, got %d users", len(users))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestCheckHistory(t *testing.T) {
|
func TestCheckHistory(t *testing.T) {
|
||||||
s := newTestStore(t)
|
s := newTestStore(t)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user