From 438f43156f7329006d33bff9e056dcb61985d56c Mon Sep 17 00:00:00 2001 From: Daniel Date: Mon, 7 Apr 2025 15:26:35 +0200 Subject: [PATCH] Add upgrade locking mechanism to core ui serving module --- cmds/cmdbase/update.go | 2 + service/instance.go | 9 +++ service/ui/api.go | 22 ++----- service/ui/module.go | 127 +++++++++++++++++++++++++------------ service/ui/serve.go | 42 +++++------- service/updates/module.go | 2 + service/updates/upgrade.go | 4 ++ 7 files changed, 122 insertions(+), 86 deletions(-) diff --git a/cmds/cmdbase/update.go b/cmds/cmdbase/update.go index 65c20a83..6a04c654 100644 --- a/cmds/cmdbase/update.go +++ b/cmds/cmdbase/update.go @@ -9,6 +9,7 @@ import ( "github.com/safing/portmaster/base/log" "github.com/safing/portmaster/base/notifications" "github.com/safing/portmaster/service" + "github.com/safing/portmaster/service/ui" "github.com/safing/portmaster/service/updates" ) @@ -71,3 +72,4 @@ type updateDummyInstance struct{} func (udi *updateDummyInstance) Restart() {} func (udi *updateDummyInstance) Shutdown() {} func (udi *updateDummyInstance) Notifications() *notifications.Notifications { return nil } +func (udi *updateDummyInstance) UI() *ui.UI { return nil } diff --git a/service/instance.go b/service/instance.go index c71f6388..5b148507 100644 --- a/service/instance.go +++ b/service/instance.go @@ -438,6 +438,15 @@ func (i *Instance) BinaryUpdates() *updates.Updater { return i.binaryUpdates } +// GetBinaryUpdateFile returns the file path of a binary update file. +func (i *Instance) GetBinaryUpdateFile(name string) (path string, err error) { + file, err := i.binaryUpdates.GetFile(name) + if err != nil { + return "", err + } + return file.Path(), nil +} + // IntelUpdates returns the updates module. func (i *Instance) IntelUpdates() *updates.Updater { return i.intelUpdates diff --git a/service/ui/api.go b/service/ui/api.go index de7e7758..d74d814b 100644 --- a/service/ui/api.go +++ b/service/ui/api.go @@ -2,35 +2,21 @@ package ui import ( "github.com/safing/portmaster/base/api" - "github.com/safing/portmaster/base/log" ) -func registerAPIEndpoints() error { +func (ui *UI) registerAPIEndpoints() error { return api.RegisterEndpoint(api.Endpoint{ Path: "ui/reload", Write: api.PermitUser, - ActionFunc: reloadUI, + ActionFunc: ui.reloadUI, Name: "Reload UI Assets", Description: "Removes all assets from the cache and reloads the current (possibly updated) version from disk when requested.", }) } -func reloadUI(_ *api.Request) (msg string, err error) { - appsLock.Lock() - defer appsLock.Unlock() - +func (ui *UI) reloadUI(_ *api.Request) (msg string, err error) { // Close all archives. - for id, archiveFS := range apps { - err := archiveFS.Close() - if err != nil { - log.Warningf("ui: failed to close archive %s: %s", id, err) - } - } - - // Reset index. - for key := range apps { - delete(apps, key) - } + ui.CloseArchives() return "all ui archives successfully reloaded", nil } diff --git a/service/ui/module.go b/service/ui/module.go index a058c508..946cb1e2 100644 --- a/service/ui/module.go +++ b/service/ui/module.go @@ -1,27 +1,55 @@ package ui import ( - "errors" "os" "path/filepath" + "sync" "sync/atomic" "github.com/safing/portmaster/base/api" "github.com/safing/portmaster/base/log" "github.com/safing/portmaster/base/utils" "github.com/safing/portmaster/service/mgr" - "github.com/safing/portmaster/service/updates" + "github.com/spkg/zipfs" ) -func prep() error { - if err := registerAPIEndpoints(); err != nil { - return err - } +// UI serves the user interface files. +type UI struct { + mgr *mgr.Manager + instance instance - return registerRoutes() + archives map[string]*zipfs.FileSystem + archivesLock sync.RWMutex + + upgradeLock atomic.Bool } -func start() error { +// New returns a new UI module. +func New(instance instance) (*UI, error) { + m := mgr.New("UI") + ui := &UI{ + mgr: m, + instance: instance, + + archives: make(map[string]*zipfs.FileSystem), + } + + if err := ui.registerAPIEndpoints(); err != nil { + return nil, err + } + if err := ui.registerRoutes(); err != nil { + return nil, err + } + + return ui, nil +} + +func (ui *UI) Manager() *mgr.Manager { + return ui.mgr +} + +// Start starts the module. +func (ui *UI) Start() error { // Create a dummy directory to which processes change their working directory // to. Currently this includes the App and the Notifier. The aim is protect // all other directories and increase compatibility should any process want @@ -30,7 +58,7 @@ func start() error { // may seem dangerous, but proper permission on the parent directory provide // (some) protection. // Processes must _never_ read from this directory. - execDir := filepath.Join(module.instance.DataDir(), "exec") + execDir := filepath.Join(ui.instance.DataDir(), "exec") err := os.MkdirAll(execDir, 0o0777) //nolint:gosec // This is intentional. if err != nil { log.Warningf("ui: failed to create safe exec dir: %s", err) @@ -45,52 +73,67 @@ func start() error { return nil } -// UI serves the user interface files. -type UI struct { - mgr *mgr.Manager - - instance instance -} - -func (ui *UI) Manager() *mgr.Manager { - return ui.mgr -} - -// Start starts the module. -func (ui *UI) Start() error { - return start() -} - // Stop stops the module. func (ui *UI) Stop() error { return nil } -var ( - shimLoaded atomic.Bool - module *UI -) +func (ui *UI) getArchive(name string) (archive *zipfs.FileSystem, ok bool) { + ui.archivesLock.RLock() + defer ui.archivesLock.RUnlock() -// New returns a new UI module. -func New(instance instance) (*UI, error) { - if !shimLoaded.CompareAndSwap(false, true) { - return nil, errors.New("only one instance allowed") - } - m := mgr.New("UI") - module = &UI{ - mgr: m, - instance: instance, + archive, ok = ui.archives[name] + return +} + +func (ui *UI) setArchive(name string, archive *zipfs.FileSystem) { + ui.archivesLock.Lock() + defer ui.archivesLock.Unlock() + + ui.archives[name] = archive +} + +// CloseArchives closes all open archives. +func (ui *UI) CloseArchives() { + if ui == nil { + return } - if err := prep(); err != nil { - return nil, err + ui.archivesLock.Lock() + defer ui.archivesLock.Unlock() + + // Close archives. + for _, archive := range ui.archives { + if err := archive.Close(); err != nil { + ui.mgr.Warn("failed to close ui archive", "err", err) + } } - return module, nil + // Reset map. + clear(ui.archives) +} + +// EnableUpgradeLock enables the upgrade lock and closes all open archives. +func (ui *UI) EnableUpgradeLock() { + if ui == nil { + return + } + + ui.upgradeLock.Store(true) + ui.CloseArchives() +} + +// DisableUpgradeLock disables the upgrade lock. +func (ui *UI) DisableUpgradeLock() { + if ui == nil { + return + } + + ui.upgradeLock.Store(false) } type instance interface { DataDir() string API() *api.API - BinaryUpdates() *updates.Updater + GetBinaryUpdateFile(name string) (path string, err error) } diff --git a/service/ui/serve.go b/service/ui/serve.go index 68203552..24bda7aa 100644 --- a/service/ui/serve.go +++ b/service/ui/serve.go @@ -9,26 +9,19 @@ import ( "net/url" "path/filepath" "strings" - "sync" "github.com/spkg/zipfs" "github.com/safing/portmaster/base/api" "github.com/safing/portmaster/base/log" "github.com/safing/portmaster/base/utils" - "github.com/safing/portmaster/service/updates" ) -var ( - apps = make(map[string]*zipfs.FileSystem) - appsLock sync.RWMutex -) - -func registerRoutes() error { +func (ui *UI) registerRoutes() error { // Server assets. api.RegisterHandler( "/assets/{resPath:[a-zA-Z0-9/\\._-]+}", - &archiveServer{defaultModuleName: "assets"}, + &archiveServer{ui: ui, defaultModuleName: "assets"}, ) // Add slash to plain module namespaces. @@ -38,7 +31,7 @@ func registerRoutes() error { ) // Serve modules. - srv := &archiveServer{} + srv := &archiveServer{ui: ui} api.RegisterHandler("/ui/modules/{moduleName:[a-z]+}/", srv) api.RegisterHandler("/ui/modules/{moduleName:[a-z]+}/{resPath:[a-zA-Z0-9/\\._-]+}", srv) @@ -52,6 +45,7 @@ func registerRoutes() error { } type archiveServer struct { + ui *UI defaultModuleName string } @@ -82,39 +76,35 @@ func (bs *archiveServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { resPath = "index.html" } - appsLock.RLock() - archiveFS, ok := apps[moduleName] - appsLock.RUnlock() + archiveFS, ok := bs.ui.getArchive(moduleName) if ok { ServeFileFromArchive(w, r, moduleName, archiveFS, resPath) return } + // Check if the upgrade lock is enabled. + if bs.ui.upgradeLock.Load() { + http.Error(w, "Resources locked, upgrade in progress.", http.StatusLocked) + return + } + // get file from update system - zipFile, err := module.instance.BinaryUpdates().GetFile(fmt.Sprintf("%s.zip", moduleName)) + zipFile, err := bs.ui.instance.GetBinaryUpdateFile(fmt.Sprintf("%s.zip", moduleName)) if err != nil { - if errors.Is(err, updates.ErrNotFound) { - log.Tracef("ui: requested module %s does not exist", moduleName) - http.Error(w, err.Error(), http.StatusNotFound) - } else { - log.Tracef("ui: error loading module %s: %s", moduleName, err) - http.Error(w, err.Error(), http.StatusInternalServerError) - } + log.Tracef("ui: error loading module %s: %s", moduleName, err) + http.Error(w, err.Error(), http.StatusInternalServerError) return } // Open archive from disk. - archiveFS, err = zipfs.New(zipFile.Path()) + archiveFS, err = zipfs.New(zipFile) if err != nil { log.Tracef("ui: error prepping module %s: %s", moduleName, err) http.Error(w, err.Error(), http.StatusInternalServerError) return } - appsLock.Lock() - apps[moduleName] = archiveFS - appsLock.Unlock() - + bs.ui.setArchive(moduleName, archiveFS) ServeFileFromArchive(w, r, moduleName, archiveFS, resPath) } diff --git a/service/updates/module.go b/service/updates/module.go index 8a441ac9..024496c8 100644 --- a/service/updates/module.go +++ b/service/updates/module.go @@ -20,6 +20,7 @@ import ( "github.com/safing/portmaster/base/utils" "github.com/safing/portmaster/service/configure" "github.com/safing/portmaster/service/mgr" + "github.com/safing/portmaster/service/ui" ) const ( @@ -645,4 +646,5 @@ type instance interface { Restart() Shutdown() Notifications() *notifications.Notifications + UI() *ui.UI } diff --git a/service/updates/upgrade.go b/service/updates/upgrade.go index 250baede..7edf653f 100644 --- a/service/updates/upgrade.go +++ b/service/updates/upgrade.go @@ -24,6 +24,10 @@ func (u *Updater) upgrade(downloader *Downloader, ignoreVersion bool) error { } } + // Unload UI assets to be able to move files on Windows. + u.instance.UI().EnableUpgradeLock() + defer u.instance.UI().DisableUpgradeLock() + // Execute the upgrade. upgradeError := u.upgradeMoveFiles(downloader) if upgradeError == nil {