From ea8a6b076ef2359426687467d7465e4c092f6447 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 25 Nov 2020 22:29:13 +0100 Subject: conf: do exponential back off for sharing violation in hotfolder Windows gives us notifications about writes to files in a directory, but it does not give us notifications on when file handles are closed and when we can expect to be able to grab a handle to it; this would be racey at best. So, there always exists a race between the writer's last call to WriteFile() and its eventual CloseHandle(). Work around this by implementing a basic exponential back off of retrying the open call. While we're at it, clean up the "file already exists" logic to remove a basic toctou situation, and switch to using random temp file names in order to handle better the case of saving a new file from two different administrators at once. Reported-by: Jim Salter Signed-off-by: Jason A. Donenfeld --- conf/store.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) (limited to 'conf/store.go') diff --git a/conf/store.go b/conf/store.go index 0c4aa41f..4782aca0 100644 --- a/conf/store.go +++ b/conf/store.go @@ -11,6 +11,10 @@ import ( "os" "path/filepath" "strings" + "sync" + "time" + + "golang.org/x/sys/windows" "golang.zx2c4.com/wireguard/windows/conf/dpapi" ) @@ -47,7 +51,12 @@ func ListConfigNames() ([]string, error) { return configs[:i], nil } -func MigrateUnencryptedConfigs() (int, []error) { +var migrating sync.Mutex +var lastMigrationTimer *time.Timer + +func MigrateUnencryptedConfigs(sharingBase int) (int, []error) { + migrating.Lock() + defer migrating.Unlock() configFileDir, err := tunnelConfigurationsDirectory() if err != nil { return 0, []error{err} @@ -73,6 +82,13 @@ func MigrateUnencryptedConfigs() (int, []error) { // of Windows file locking for ensuring the file is finished being written. f, err := os.OpenFile(path, os.O_RDWR, 0) if err != nil { + if sharingBase > 0 && errors.Is(err, windows.ERROR_SHARING_VIOLATION) { + if lastMigrationTimer != nil { + lastMigrationTimer.Stop() + } + lastMigrationTimer = time.AfterFunc(time.Second/time.Duration(sharingBase*sharingBase), func() { MigrateUnencryptedConfigs(sharingBase - 1) }) + sharingBase = 0 + } errs[e] = err e++ continue @@ -98,12 +114,7 @@ func MigrateUnencryptedConfigs() (int, []error) { continue } dstFile := strings.TrimSuffix(path, configFileUnencryptedSuffix) + configFileSuffix - if _, err = os.Stat(dstFile); err != nil && !os.IsNotExist(err) { - errs[e] = errors.New("Unable to migrate to " + dstFile + " as it already exists") - e++ - continue - } - err = writeEncryptedFile(dstFile, bytes) + err = writeEncryptedFile(dstFile, false, bytes) if err != nil { errs[e] = err e++ @@ -185,7 +196,7 @@ func (config *Config) Save() error { if err != nil { return err } - return writeEncryptedFile(filename, bytes) + return writeEncryptedFile(filename, true, bytes) } func (config *Config) Path() (string, error) { -- cgit v1.2.3-59-g8ed1b