WebDAV: Cap lock lifetime to prevent never-expiring locks photoprism#5736

Author: lastzero

internal/mutex/webdav.go

@@ -2,10 +2,16 @@ package mutex
 
 import (
 	"sync"
+	"time"
 
 	"golang.org/x/net/webdav"
 )
 
+// WebDAVMaxLockLifetime caps how long a WebDAV lock may be held, so a client
+// cannot mint infinite or over-long locks that leak memory and lock-null files
+// until restart. A negative value disables the cap (upstream infinite default).
+var WebDAVMaxLockLifetime = time.Hour
+
 var webdavMutex = sync.Mutex{}
 var webdavLocks = make(map[string]webdav.LockSystem)
 
@@ -17,11 +23,42 @@ func WebDAV(dir string) webdav.LockSystem {
 	// Return existing LockSystem, or create a new one otherwise.
 	if lock, ok := webdavLocks[dir]; ok {
 		return lock
-	} else if lock = webdav.NewMemLS(); lock != nil {
+	} else if memLS := webdav.NewMemLS(); memLS != nil {
+		lock = &webdavLockSystem{LockSystem: memLS}
 		webdavLocks[dir] = lock
 		return lock
 	}
 
 	// Should never happen.
 	return nil
 }
+
+// ClampLockDuration limits a requested lock duration to WebDAVMaxLockLifetime,
+// treating a negative request or cap as infinite (webdav's convention).
+func ClampLockDuration(d time.Duration) time.Duration {
+	if limit := WebDAVMaxLockLifetime; limit < 0 {
+		return d
+	} else if d < 0 || d > limit {
+		return limit
+	}
+
+	return d
+}
+
+// webdavLockSystem wraps a webdav.LockSystem to clamp lock lifetimes; it is the
+// authoritative cap, while the server also clamps the LOCK timeout header so the
+// granted lifetime reported to the client stays honest.
+type webdavLockSystem struct {
+	webdav.LockSystem
+}
+
+// Create clamps the requested lock duration before creating the lock.
+func (ls *webdavLockSystem) Create(now time.Time, details webdav.LockDetails) (string, error) {
+	details.Duration = ClampLockDuration(details.Duration)
+	return ls.LockSystem.Create(now, details)
+}
+
+// Refresh clamps the requested lock duration before refreshing the lock.
+func (ls *webdavLockSystem) Refresh(now time.Time, token string, duration time.Duration) (webdav.LockDetails, error) {
+	return ls.LockSystem.Refresh(now, token, ClampLockDuration(duration))
+}

internal/mutex/webdav_test.go

@@ -2,11 +2,94 @@ package mutex
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/webdav"
 )
 
 func TestWebDAV(t *testing.T) {
 	lockSystem := WebDAV("test")
 	assert.NotNil(t, lockSystem)
+	// Locks are clamped to a finite lifetime by default.
+	assert.IsType(t, &webdavLockSystem{}, lockSystem)
+}
+
+func TestClampLockDuration(t *testing.T) {
+	original := WebDAVMaxLockLifetime
+	defer func() { WebDAVMaxLockLifetime = original }()
+	t.Run("InfiniteRequestClampedToCap", func(t *testing.T) {
+		WebDAVMaxLockLifetime = time.Hour
+		assert.Equal(t, time.Hour, ClampLockDuration(-1))
+	})
+	t.Run("OverLongRequestClampedToCap", func(t *testing.T) {
+		WebDAVMaxLockLifetime = time.Hour
+		assert.Equal(t, time.Hour, ClampLockDuration(24*time.Hour))
+	})
+	t.Run("ShortRequestKept", func(t *testing.T) {
+		WebDAVMaxLockLifetime = time.Hour
+		assert.Equal(t, 10*time.Minute, ClampLockDuration(10*time.Minute))
+	})
+	t.Run("ExactCapKept", func(t *testing.T) {
+		WebDAVMaxLockLifetime = time.Hour
+		assert.Equal(t, time.Hour, ClampLockDuration(time.Hour))
+	})
+	t.Run("NegativeCapDisablesClamp", func(t *testing.T) {
+		WebDAVMaxLockLifetime = -1
+		assert.Equal(t, time.Duration(-1), ClampLockDuration(-1))
+		assert.Equal(t, 24*time.Hour, ClampLockDuration(24*time.Hour))
+	})
+}
+
+func TestWebDAVLockSystem_Create(t *testing.T) {
+	original := WebDAVMaxLockLifetime
+	defer func() { WebDAVMaxLockLifetime = original }()
+	WebDAVMaxLockLifetime = time.Hour
+	now := time.Unix(1700000000, 0)
+	t.Run("InfiniteLockExpires", func(t *testing.T) {
+		ls := &webdavLockSystem{LockSystem: webdav.NewMemLS()}
+		// Request an infinite lock; it must be clamped so it eventually expires.
+		// A conflicting Create on the same root is the observable: it is refused
+		// while the lock is held and succeeds once the clamped lifetime elapses.
+		token, err := ls.Create(now, webdav.LockDetails{Root: "/infinite", Duration: -1, ZeroDepth: true})
+		assert.NoError(t, err)
+		assert.NotEmpty(t, token)
+		// Still held just before the cap.
+		_, err = ls.Create(now.Add(59*time.Minute), webdav.LockDetails{Root: "/infinite", Duration: time.Minute, ZeroDepth: true})
+		assert.ErrorIs(t, err, webdav.ErrLocked)
+		// Expired after the cap, so the root can be locked again.
+		_, err = ls.Create(now.Add(61*time.Minute), webdav.LockDetails{Root: "/infinite", Duration: time.Minute, ZeroDepth: true})
+		assert.NoError(t, err)
+	})
+	t.Run("ShortLockKept", func(t *testing.T) {
+		ls := &webdavLockSystem{LockSystem: webdav.NewMemLS()}
+		token, err := ls.Create(now, webdav.LockDetails{Root: "/short", Duration: 5 * time.Minute, ZeroDepth: true})
+		assert.NoError(t, err)
+		assert.NotEmpty(t, token)
+		// Still held within the requested five minutes (under the cap, so kept).
+		_, err = ls.Create(now.Add(1*time.Minute), webdav.LockDetails{Root: "/short", Duration: time.Minute, ZeroDepth: true})
+		assert.ErrorIs(t, err, webdav.ErrLocked)
+		// Expired after the requested five minutes.
+		_, err = ls.Create(now.Add(6*time.Minute), webdav.LockDetails{Root: "/short", Duration: time.Minute, ZeroDepth: true})
+		assert.NoError(t, err)
+	})
+}
+
+func TestWebDAVLockSystem_Refresh(t *testing.T) {
+	original := WebDAVMaxLockLifetime
+	defer func() { WebDAVMaxLockLifetime = original }()
+	WebDAVMaxLockLifetime = time.Hour
+	now := time.Unix(1700000000, 0)
+	ls := &webdavLockSystem{LockSystem: webdav.NewMemLS()}
+	token, err := ls.Create(now, webdav.LockDetails{Root: "/refresh", Duration: 5 * time.Minute, ZeroDepth: true})
+	assert.NoError(t, err)
+	t.Run("InfiniteRefreshClampedToCap", func(t *testing.T) {
+		details, refreshErr := ls.Refresh(now, token, -1)
+		assert.NoError(t, refreshErr)
+		assert.Equal(t, time.Hour, details.Duration)
+	})
+	t.Run("NoSuchLock", func(t *testing.T) {
+		_, refreshErr := ls.Refresh(now, "unknown-token", time.Minute)
+		assert.Error(t, refreshErr)
+	})
 }

internal/server/README.md

@@ -58,6 +58,7 @@
   - Built-in security middleware skips browser-document headers (`Content-Security-Policy`, `X-Frame-Options`) on `/originals` and `/import` paths.
   - PROPFIND `207 Multi-Status` responses normalize XML media type to `application/xml; charset=utf-8`.
   - Request errors go to the console-only system log (`event.System*`), not the browser log stream, since `x/net/webdav` embeds absolute server paths in its messages. A `MKCOL` on an existing collection is a benign sync-client probe: it returns 405 and is logged at debug rather than as an error.
+  - `LOCK` lifetimes are capped at `mutex.WebDAVMaxLockLifetime` (default one hour) so a client cannot mint locks that never expire: `WebDAVClampLockTimeout` clamps the requested `Timeout` header and the `webdavLockSystem` wrapper enforces the same bound on the stored lock.
 - AutoTLS: uses `autocert` and spins up a redirect listener; ensure ports 80/443 are reachable.
 - Unix sockets: optional `force` query removes stale sockets; permissions can be set via `mode` query.
 - Health endpoints (`/livez`, `/health`, `/healthz`, `/readyz`) return `Cache-Control: no-store` and `Access-Control-Allow-Origin: *`.

internal/server/webdav.go

@@ -2,9 +2,11 @@ package server
 
 import (
 	"errors"
+	"fmt"
 	"net/http"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -135,6 +137,10 @@ func WebDAV(dir string, router *gin.RouterGroup, conf *config.Config) {
 			}
 		}
 
+		// Clamp the requested LOCK lifetime so a client cannot mint infinite or
+		// excessively long-lived locks; the lock system enforces the same cap.
+		WebDAVClampLockTimeout(c.Request)
+
 		// Invoke handler callback.
 		WebDAVHandler(c, router, srv)
 	}
@@ -184,6 +190,44 @@ func WebDAV(dir string, router *gin.RouterGroup, conf *config.Config) {
 	}
 }
 
+// WebDAVClampLockTimeout rewrites the LOCK "Timeout" request header so the lock the
+// upstream handler grants — and the lifetime it reports back — cannot exceed
+// mutex.WebDAVMaxLockLifetime. Clamping the header keeps the granted timeout the client
+// sees consistent with what is actually enforced, so conformant clients refresh in time.
+func WebDAVClampLockTimeout(request *http.Request) {
+	if request == nil || request.Method != header.MethodLock {
+		return
+	}
+
+	// A negative cap disables clamping (infinite locks allowed).
+	maxLifetime := mutex.WebDAVMaxLockLifetime
+	if maxLifetime < 0 {
+		return
+	}
+
+	// The upstream handler parses only the first comma-separated timeout value.
+	first := request.Header.Get(header.Timeout)
+	if i := strings.IndexByte(first, ','); i >= 0 {
+		first = first[:i]
+	}
+	first = strings.TrimSpace(first)
+
+	maxSeconds := int64(maxLifetime / time.Second)
+	capped := fmt.Sprintf("Second-%d", maxSeconds)
+
+	switch {
+	case first == "" || strings.EqualFold(first, "Infinite"):
+		// Absent or infinite request: grant the capped lifetime instead.
+		request.Header.Set(header.Timeout, capped)
+	case strings.HasPrefix(first, "Second-"):
+		// Numeric request: clamp only when it exceeds the cap, leave malformed
+		// values untouched so the upstream handler still rejects them.
+		if n, err := strconv.ParseInt(first[len("Second-"):], 10, 64); err == nil && n > maxSeconds {
+			request.Header.Set(header.Timeout, capped)
+		}
+	}
+}
+
 // WebDAVFileName determines the name and path of an uploaded file and returns its name if it exists.
 func WebDAVFileName(request *http.Request, router *gin.RouterGroup, conf *config.Config) (fileName string) {
 	// Check if this is a PUT request, as used for file uploads.

internal/server/webdav_lock_test.go

@@ -0,0 +1,69 @@
+package server
+
+import (
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/photoprism/photoprism/internal/mutex"
+	"github.com/photoprism/photoprism/pkg/http/header"
+)
+
+func TestWebDAVClampLockTimeout(t *testing.T) {
+	original := mutex.WebDAVMaxLockLifetime
+	defer func() { mutex.WebDAVMaxLockLifetime = original }()
+	mutex.WebDAVMaxLockLifetime = time.Hour
+
+	lockRequest := func(timeout string) *http.Request {
+		r, _ := http.NewRequest(header.MethodLock, "http://localhost/originals/x.txt", nil)
+		if timeout != "" {
+			r.Header.Set(header.Timeout, timeout)
+		}
+		return r
+	}
+
+	t.Run("InfiniteClampedToCap", func(t *testing.T) {
+		r := lockRequest("Infinite")
+		WebDAVClampLockTimeout(r)
+		assert.Equal(t, "Second-3600", r.Header.Get(header.Timeout))
+	})
+	t.Run("AbsentClampedToCap", func(t *testing.T) {
+		r := lockRequest("")
+		WebDAVClampLockTimeout(r)
+		assert.Equal(t, "Second-3600", r.Header.Get(header.Timeout))
+	})
+	t.Run("OverCapClamped", func(t *testing.T) {
+		r := lockRequest("Second-99999")
+		WebDAVClampLockTimeout(r)
+		assert.Equal(t, "Second-3600", r.Header.Get(header.Timeout))
+	})
+	t.Run("UnderCapKept", func(t *testing.T) {
+		r := lockRequest("Second-600")
+		WebDAVClampLockTimeout(r)
+		assert.Equal(t, "Second-600", r.Header.Get(header.Timeout))
+	})
+	t.Run("FirstValueClamped", func(t *testing.T) {
+		r := lockRequest("Infinite, Second-300")
+		WebDAVClampLockTimeout(r)
+		assert.Equal(t, "Second-3600", r.Header.Get(header.Timeout))
+	})
+	t.Run("MalformedLeftUntouched", func(t *testing.T) {
+		r := lockRequest("bogus")
+		WebDAVClampLockTimeout(r)
+		assert.Equal(t, "bogus", r.Header.Get(header.Timeout))
+	})
+	t.Run("NonLockMethodIgnored", func(t *testing.T) {
+		r, _ := http.NewRequest(header.MethodPut, "http://localhost/originals/x.txt", nil)
+		r.Header.Set(header.Timeout, "Infinite")
+		WebDAVClampLockTimeout(r)
+		assert.Equal(t, "Infinite", r.Header.Get(header.Timeout))
+	})
+	t.Run("NegativeCapDisablesClamp", func(t *testing.T) {
+		mutex.WebDAVMaxLockLifetime = -1
+		r := lockRequest("Infinite")
+		WebDAVClampLockTimeout(r)
+		assert.Equal(t, "Infinite", r.Header.Get(header.Timeout))
+	})
+}

pkg/http/header/webdav.go

@@ -5,4 +5,6 @@ const (
 	XFavorite = "X-Favorite"
 	// XModTime conveys modification time in WebDAV headers.
 	XModTime = "X-OC-MTime"
+	// Timeout is the WebDAV LOCK request header that carries the requested lock lifetime.
+	Timeout = "Timeout"
 )