diff --git a/README.md b/README.md index 1318eb9..9904f6b 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,8 @@ directory: /data # The default permissions for users. This is a case insensitive option. Possible # permissions: C (Create), R (Read), U (Update), D (Delete). You can combine multiple # permissions. For example, to allow to read and create, set "RC". Default is "R". +# LOCK counts as a write: it needs U on a path that exists and C on one that does +# not, since locking a path that does not exist creates it. permissions: R # The default permissions rules for users. Default is none. Rules are applied @@ -240,6 +242,10 @@ A `path` rule is a prefix match. A rule written with a trailing slash also cover A `regex` rule is matched literally against the path, and gets none of the above handling. In particular `regex: "^/secret/"` does **not** match a request for `/secret` itself. Write `regex: "^/secret(/|$)"` if you want to cover the collection too. +Rules apply to every path an operation touches, not only the one it names. Collection listings leave out entries the rules deny, copying a collection leaves those entries behind, and a `MOVE` or `DELETE` that would act on a denied descendant is refused outright. + +Rules follow the case sensitivity of the file system, which each served directory is probed for at startup. Where names are case-insensitive, as on APFS and NTFS, `path: /secret/` also covers `/SECRET/`, a `regex` is matched against the folded path as well as the path as written, and Unicode normal forms count as one name. Elsewhere rules are matched exactly, since `/secret` and `/SECRET` are then different directories. + ### CORS The `allowed_*` properties are optional, the default value for each of them will be `*`. `exposed_headers` is optional as well, but is not set if not defined. Setting `allow_private_network` to `true` to allow Private-Network-Access preflight requests. Setting `credentials` to `true` will allow you to: diff --git a/go.mod b/go.mod index 2ebe4f2..04d7b83 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( golang.org/x/crypto v0.55.0 golang.org/x/crypto/x509roots/fallback v0.0.0-20260826144058-afebf4cb4efb golang.org/x/net v0.58.0 + golang.org/x/text v0.41.0 ) require ( @@ -29,5 +30,4 @@ require ( go.uber.org/multierr v1.11.0 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/sys v0.47.0 // indirect - golang.org/x/text v0.41.0 // indirect ) diff --git a/lib/casefold.go b/lib/casefold.go new file mode 100644 index 0000000..96fabe1 --- /dev/null +++ b/lib/casefold.go @@ -0,0 +1,76 @@ +package lib + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "unicode" + + "golang.org/x/text/unicode/norm" +) + +// defaultCaseInsensitiveFS is true on platforms that are case-insensitive by +// default, and false on platforms that are case-sensitive by default. It is used +// as a fallback when probing the file system fails. +const defaultCaseInsensitiveFS = runtime.GOOS == "darwin" || runtime.GOOS == "windows" + +// foldPath is used to compare paths in a case-insensitive manner, normalizing +// them to NFC and converting to lower case. +func foldPath(p string) string { + return norm.NFC.String(strings.ToLower(p)) +} + +// caseInsensitiveFS probes the file system backing dir to see if it is case +// insensitive. It falls back to the platform default when the probe fails. +func caseInsensitiveFS(dir string) bool { + info, err := os.Stat(dir) + if err != nil { + return defaultCaseInsensitiveFS + } + + flipped, ok := flipCase(dir) + if !ok { + return defaultCaseInsensitiveFS + } + + other, err := os.Stat(flipped) + if err != nil { + return false + } + + return os.SameFile(info, other) +} + +// flipCase swaps the case of the first letter in the last element of dir whose +// case mapping round-trips, reporting false when it holds none. +func flipCase(dir string) (string, bool) { + flipped := false + + base := strings.Map(func(r rune) rune { + if flipped { + return r + } + + switch { + case unicode.IsLower(r): + if u := unicode.ToUpper(r); unicode.ToLower(u) == r { + flipped = true + return u + } + case unicode.IsUpper(r): + if l := unicode.ToLower(r); unicode.ToUpper(l) == r { + flipped = true + return l + } + } + + return r + }, filepath.Base(dir)) + + if !flipped { + return "", false + } + + return filepath.Join(filepath.Dir(dir), base), true +} diff --git a/lib/handler.go b/lib/handler.go index 9af90c0..fecdba7 100644 --- a/lib/handler.go +++ b/lib/handler.go @@ -11,7 +11,8 @@ import ( type handlerUser struct { User - webdav.Handler + handler webdav.Handler + fs permissionsFS } type Handler struct { @@ -32,18 +33,12 @@ func NewHandler(c *Config) (http.Handler, error) { h := &Handler{ noPassword: c.NoPassword, behindProxy: c.BehindProxy, - user: &handlerUser{ - User: User{UserPermissions: c.UserPermissions}, - Handler: buildWebdavHandler(c.UserPermissions, c.Prefix, c.NoSniff, ls, logFunc), - }, - users: map[string]*handlerUser{}, + user: newHandlerUser(User{UserPermissions: c.UserPermissions}, c, ls, logFunc), + users: map[string]*handlerUser{}, } for _, u := range c.Users { - h.users[u.Username] = &handlerUser{ - User: u, - Handler: buildWebdavHandler(u.UserPermissions, c.Prefix, c.NoSniff, ls, logFunc), - } + h.users[u.Username] = newHandlerUser(u, c, ls, logFunc) } if c.CORS.Enabled { @@ -69,26 +64,46 @@ func NewHandler(c *Config) (http.Handler, error) { return h, nil } -// buildWebdavHandler creates the [webdav.Handler] for a set of user permissions, -// selecting between single-directory and multi-directory backing depending on -// whether directories are configured. -func buildWebdavHandler(p UserPermissions, prefix string, noSniff bool, ls webdav.LockSystem, logFunc func(*http.Request, error)) webdav.Handler { - h := webdav.Handler{ - Prefix: prefix, - Logger: logFunc, - } +// newHandlerUser prepares a user for serving, keeping the unwrapped file system +// alongside the handler. +func newHandlerUser(u User, c *Config, ls webdav.LockSystem, logFunc func(*http.Request, error)) *handlerUser { + fs := permissionsFS{fs: buildFileSystem(u.UserPermissions, c.NoSniff), perms: u.UserPermissions} + return &handlerUser{ + User: u, + handler: buildWebdavHandler(u.UserPermissions, fs, c.Prefix, ls, logFunc), + fs: fs, + } +} + +// buildFileSystem creates the unfiltered [webdav.FileSystem] for a set of user +// permissions, selecting between single-directory and multi-directory backing +// depending on whether directories are configured. +func buildFileSystem(p UserPermissions, noSniff bool) webdav.FileSystem { if p.useDirectories { - h.FileSystem = multiDir{ + return multiDir{ mounts: p.Directories, noSniff: noSniff, } + } + + return Dir{ + Dir: webdav.Dir(p.Directory), + noSniff: noSniff, + } +} + +// buildWebdavHandler creates the [webdav.Handler] for a set of user permissions. +func buildWebdavHandler(p UserPermissions, fs permissionsFS, prefix string, ls webdav.LockSystem, logFunc func(*http.Request, error)) webdav.Handler { + h := webdav.Handler{ + Prefix: prefix, + Logger: logFunc, + FileSystem: fs, + } + + if p.useDirectories { h.LockSystem = newMultiDirLockSystem(ls, p.Directories) } else { - h.FileSystem = Dir{ - Dir: webdav.Dir(p.Directory), - noSniff: noSniff, - } h.LockSystem = newLockSystem(ls, p.Directory) } @@ -132,18 +147,20 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Convert the HTTP request into an internal request type - req, err := newRequest(r, h.user.Prefix) + req, err := newRequest(r, h.user.handler.Prefix) if err != nil { lZap.Info("invalid request path or destination", zap.Error(err)) http.Error(w, "Invalid request path or destination", http.StatusBadRequest) return } - // Checks for user permissions relatively to this PATH. - allowed := user.Allowed(req, func(filename string) bool { - _, err := user.FileSystem.Stat(r.Context(), filename) + fileExists := func(filename string) bool { + _, err := user.fs.Stat(r.Context(), filename) return !os.IsNotExist(err) - }) + } + + // Checks for user permissions relatively to this PATH. + allowed := user.Allowed(req, fileExists) lZap.Debug("allowed & method & path", zap.Bool("allowed", allowed), zap.String("method", r.Method), zap.String("path", r.URL.Path)) @@ -152,6 +169,25 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + // MOVE and DELETE act on a whole subtree in one call, so every descendant + // needs authorizing here. COPY and PROPFIND go through permFS instead. + if r.Method == "MOVE" || r.Method == "DELETE" { + ok, err := user.fs.allowedThroughout(r.Context(), req.path, func(p Permissions) bool { + return p.Allowed(req, fileExists) + }) + if err != nil { + lZap.Error("could not authorize subtree", zap.String("path", req.path), zap.Error(err)) + w.WriteHeader(http.StatusForbidden) + return + } + + if !ok { + lZap.Info("denied by a rule on a descendant", zap.String("method", r.Method), zap.String("path", req.path)) + w.WriteHeader(http.StatusForbidden) + return + } + } + if r.Method == "HEAD" { w = responseWriterNoBody{w} } @@ -168,7 +204,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // // GET (or HEAD), when applied to collection, will return the same as PROPFIND method. if r.Method == "GET" || r.Method == "HEAD" { - info, err := user.FileSystem.Stat(r.Context(), req.path) + info, err := user.fs.Stat(r.Context(), req.path) if err == nil && info.IsDir() { r.Method = "PROPFIND" @@ -189,7 +225,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Runs the WebDAV. - user.ServeHTTP(w, r) + user.handler.ServeHTTP(w, r) } // getRequestLogger creates a zap.Logger using the request remote ip. diff --git a/lib/handler_test.go b/lib/handler_test.go index 9daecab..6a2c0b0 100644 --- a/lib/handler_test.go +++ b/lib/handler_test.go @@ -677,9 +677,13 @@ users: client := gowebdav.NewClient(srv.URL, "basic", "basic") + // A rule denying a path also hides it from the collection containing it. files, err := client.ReadDir("/") require.NoError(t, err) - require.Len(t, files, 5) + require.Len(t, files, 4) + for _, f := range files { + require.NotEqual(t, "c", f.Name()) + } err = client.Write("/foo.txt", []byte("new"), 0666) require.NoError(t, err) @@ -804,9 +808,13 @@ users: client := gowebdav.NewClient(srv.URL, "basic", "basic") + // A rule denying a path also hides it from the collection containing it. files, err := client.ReadDir("/prefix") require.NoError(t, err) - require.Len(t, files, 5) + require.Len(t, files, 4) + for _, f := range files { + require.NotEqual(t, "c", f.Name()) + } err = client.Write("/prefix/foo.txt", []byte("new"), 0666) require.NoError(t, err) @@ -1227,6 +1235,311 @@ rules: require.Equal(t, http.StatusForbidden, do(t, "PROPFIND", grantSrv.URL+"/pub")) } +// doRequest sends a raw request, for what gowebdav does not expose directly. +// An empty username sends no credentials. +func doRequest(t *testing.T, method, url, username, password string, headers map[string]string, body string) (int, string) { + t.Helper() + + var reader io.Reader + if body != "" { + reader = strings.NewReader(body) + } + + req, err := http.NewRequest(method, url, reader) + require.NoError(t, err) + + if username != "" { + req.SetBasicAuth(username, password) + } + + for k, v := range headers { + req.Header.Set(k, v) + } + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + + data, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + return resp.StatusCode, string(data) +} + +const exclusiveWriteLock = ` + + + + tester +` + +// TestServerRulesShadowedCollection covers a broader rule shadowing one that +// names a collection: resolving "/data/secret" through "/data/" would skip the +// deny rule, leaving the collection listable, relocatable and removable. +func TestServerRulesShadowedCollection(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + config string + username string + }{ + { + name: "path rule shadows", + config: ` +permissions: none +rules: + - path: "/data/" + permissions: CRUD + - path: "/data/secret/" + permissions: none`, + }, + { + name: "regex rule shadows", + config: ` +permissions: none +rules: + - regex: "^/data/" + permissions: CRUD + - path: "/data/secret/" + permissions: none`, + }, + { + // The shadowing rule need not sit next to the rule it shadows. + name: "appended global rule shadows", + username: "basic", + config: ` +permissions: none +rules: + - path: "/data/" + permissions: CRUD +users: + - username: basic + password: basic + rulesBehavior: append + rules: + - path: "/data/secret/" + permissions: none`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dir := makeTestDirectory(t, map[string][]byte{ + "data/secret/flag.txt": []byte("secret"), + }) + + srv := makeTestServer(t, fmt.Sprintf("directory: %s\n%s\n", dir, tc.config)) + defer srv.Close() + + do := func(method, path string, headers map[string]string) int { + t.Helper() + code, _ := doRequest(t, method, srv.URL+path, tc.username, "basic", headers, "") + return code + } + + // Controls: the deny rule holds inside the collection, and for the + // collection named with its trailing slash. + require.Equal(t, http.StatusForbidden, do("GET", "/data/secret/flag.txt", nil)) + require.Equal(t, http.StatusForbidden, do("PROPFIND", "/data/secret/", map[string]string{"Depth": "1"})) + + // Named without the trailing slash it is the same collection. + require.Equal(t, http.StatusForbidden, do("PROPFIND", "/data/secret", map[string]string{"Depth": "1"})) + require.Equal(t, http.StatusForbidden, do("DELETE", "/data/secret", nil)) + require.Equal(t, http.StatusForbidden, do("MOVE", "/data/secret", map[string]string{"Destination": srv.URL + "/data/exposed"})) + require.Equal(t, http.StatusForbidden, do("COPY", "/data/secret", map[string]string{"Destination": srv.URL + "/data/copied"})) + + require.FileExists(t, filepath.Join(dir, "data", "secret", "flag.txt")) + require.NoFileExists(t, filepath.Join(dir, "data", "exposed", "flag.txt")) + require.NoFileExists(t, filepath.Join(dir, "data", "copied", "flag.txt")) + }) + } +} + +// TestServerRulesCollectionGrantedByEnclosingRule guards the other direction: a +// rule naming a collection must not lose access an enclosing rule grants. +func TestServerRulesCollectionGrantedByEnclosingRule(t *testing.T) { + t.Parallel() + + dir := makeTestDirectory(t, map[string][]byte{"data/sub/foo.txt": []byte("foo")}) + + srv := makeTestServer(t, fmt.Sprintf(` +directory: %s +permissions: none +rules: + - path: "/data/" + permissions: CRUD + - path: "/data/sub/" + permissions: CRUD +`, dir)) + defer srv.Close() + + code, _ := doRequest(t, "PROPFIND", srv.URL+"/data/sub", "", "", map[string]string{"Depth": "1"}, "") + require.Equal(t, http.StatusMultiStatus, code) +} + +// TestServerRulesRecursiveDescendants covers operations acting on a whole +// subtree, which authorizing only the requested path let reach denied +// descendants: reading, enumerating, relocating and destroying them. +func TestServerRulesRecursiveDescendants(t *testing.T) { + t.Parallel() + + dir := makeTestDirectory(t, map[string][]byte{ + "secret/flag.txt": []byte("top secret"), + "secret/public-note.txt": []byte("note"), + }) + + srv := makeTestServer(t, fmt.Sprintf(` +directory: %s +permissions: CRUD +rules: + - path: "/secret/flag.txt" + permissions: none +`, dir)) + defer srv.Close() + + // Control: the rule holds when the request names the denied path. + code, _ := doRequest(t, "GET", srv.URL+"/secret/flag.txt", "", "", nil, "") + require.Equal(t, http.StatusForbidden, code) + + // Enumerating the parent must not report the denied descendant. + code, body := doRequest(t, "PROPFIND", srv.URL+"/secret", "", "", map[string]string{"Depth": "infinity"}, "") + require.Equal(t, http.StatusMultiStatus, code) + require.NotContains(t, body, "flag.txt") + require.Contains(t, body, "public-note.txt") + + // Copying the parent must leave the denied descendant behind, not carry it + // to a path no rule covers. The permitted sibling still copies. + code, _ = doRequest(t, "COPY", srv.URL+"/secret", "", "", map[string]string{"Destination": srv.URL + "/stolen", "Depth": "infinity"}, "") + require.Equal(t, http.StatusCreated, code) + + code, _ = doRequest(t, "GET", srv.URL+"/stolen/flag.txt", "", "", nil, "") + require.Equal(t, http.StatusNotFound, code) + + code, _ = doRequest(t, "GET", srv.URL+"/stolen/public-note.txt", "", "", nil, "") + require.Equal(t, http.StatusOK, code) + + // MOVE and DELETE act on the subtree in one call, so they cannot be partial. + code, _ = doRequest(t, "MOVE", srv.URL+"/secret", "", "", map[string]string{"Destination": srv.URL + "/moved"}, "") + require.Equal(t, http.StatusForbidden, code) + + code, _ = doRequest(t, "DELETE", srv.URL+"/secret", "", "", nil, "") + require.Equal(t, http.StatusForbidden, code) + + require.FileExists(t, filepath.Join(dir, "secret", "flag.txt")) +} + +// TestServerRulesRecursiveDescendantsMultiDir is the same defect across mounts, +// where copying out of one lands the denied file in another. +func TestServerRulesRecursiveDescendantsMultiDir(t *testing.T) { + t.Parallel() + + dirA := makeTestDirectory(t, map[string][]byte{"secret/flag.txt": []byte("mount secret")}) + dirB := makeTestDirectory(t, map[string][]byte{"keep.txt": []byte("keep")}) + + srv := makeTestServer(t, fmt.Sprintf(` +permissions: CRUD +directories: + - name: alpha + path: %s + - name: beta + path: %s +rules: + - path: "/alpha/secret/flag.txt" + permissions: none +`, dirA, dirB)) + defer srv.Close() + + code, _ := doRequest(t, "COPY", srv.URL+"/alpha/secret", "", "", map[string]string{"Destination": srv.URL + "/beta/stolen", "Depth": "infinity"}, "") + require.Equal(t, http.StatusCreated, code) + + code, _ = doRequest(t, "GET", srv.URL+"/beta/stolen/flag.txt", "", "", nil, "") + require.Equal(t, http.StatusNotFound, code) + + code, _ = doRequest(t, "DELETE", srv.URL+"/alpha/secret", "", "", nil, "") + require.Equal(t, http.StatusForbidden, code) + + require.FileExists(t, filepath.Join(dirA, "secret", "flag.txt")) +} + +// TestServerLockRequiresWritePermission covers LOCK being authorized by any +// permission at all, which let a read-only user create a file by locking a +// missing path, and hold a write lock that blocks legitimate writers. +func TestServerLockRequiresWritePermission(t *testing.T) { + t.Parallel() + + dir := makeTestDirectory(t, map[string][]byte{"existing.txt": []byte("x")}) + + srv := makeTestServer(t, fmt.Sprintf(` +directory: %s +permissions: none +users: + - username: reader + password: reader + permissions: R + - username: writer + password: writer + permissions: CRUD +`, dir)) + defer srv.Close() + + lock := func(username, path string) int { + t.Helper() + code, _ := doRequest(t, "LOCK", srv.URL+path, username, username, + map[string]string{"Timeout": "Infinite", "Content-Type": "application/xml"}, exclusiveWriteLock) + return code + } + + // A read-only user may neither lock a resource into existence nor reserve one. + require.Equal(t, http.StatusForbidden, lock("reader", "/created-by-lock.txt")) + require.NoFileExists(t, filepath.Join(dir, "created-by-lock.txt")) + require.Equal(t, http.StatusForbidden, lock("reader", "/existing.txt")) + + // Reading is unaffected. + code, _ := doRequest(t, "GET", srv.URL+"/existing.txt", "reader", "reader", nil, "") + require.Equal(t, http.StatusOK, code) + + // A user who can write still locks as before. + require.Equal(t, http.StatusOK, lock("writer", "/existing.txt")) + require.Equal(t, http.StatusCreated, lock("writer", "/new.txt")) +} + +// TestServerRulesCaseInsensitiveFilesystem covers a rule being evaded by asking +// for a differently cased spelling of the same file. +func TestServerRulesCaseInsensitiveFilesystem(t *testing.T) { + t.Parallel() + + dir := makeTestDirectory(t, map[string][]byte{"secret/flag.txt": []byte("secret")}) + + if _, err := os.Stat(filepath.Join(dir, "SECRET", "flag.txt")); err != nil { + t.Skip("backing file system distinguishes path case") + } + + // A regex rule has to follow the file system too, or it is evaded the same + // way while looking like it denies the path. + for _, rule := range []string{`path: "/secret/"`, `regex: "^/secret/"`} { + srv := makeTestServer(t, fmt.Sprintf(` +directory: %s +permissions: CRUD +rules: + - %s + permissions: none +`, dir, rule)) + + for _, path := range []string{"/secret/flag.txt", "/SECRET/flag.txt", "/Secret/flag.txt"} { + code, _ := doRequest(t, "GET", srv.URL+path, "", "", nil, "") + require.Equal(t, http.StatusForbidden, code, rule, path) + + code, _ = doRequest(t, "DELETE", srv.URL+path, "", "", nil, "") + require.Equal(t, http.StatusForbidden, code, rule, path) + } + + srv.Close() + } + + require.FileExists(t, filepath.Join(dir, "secret", "flag.txt")) +} + func TestServerRulesEmptyPrefixDestination(t *testing.T) { t.Parallel() diff --git a/lib/partial_update.go b/lib/partial_update.go index a796c86..7a43715 100644 --- a/lib/partial_update.go +++ b/lib/partial_update.go @@ -48,7 +48,7 @@ func writePartialUpdateError(w http.ResponseWriter, err error, fallbackStatus in func (u *handlerUser) handleOptions(w http.ResponseWriter, r *http.Request, reqPath string) { allow := "OPTIONS, LOCK, PUT, MKCOL, PATCH" - if fi, err := u.FileSystem.Stat(r.Context(), reqPath); err == nil { + if fi, err := u.fs.Stat(r.Context(), reqPath); err == nil { if fi.IsDir() { allow = "OPTIONS, LOCK, DELETE, PROPPATCH, COPY, MOVE, UNLOCK, PROPFIND" } else { @@ -97,7 +97,7 @@ func (u *handlerUser) handlePartialUpdate(w http.ResponseWriter, r *http.Request defer release() ctx := r.Context() - fi, statErr := u.FileSystem.Stat(ctx, reqPath) + fi, statErr := u.fs.Stat(ctx, reqPath) exists := statErr == nil if statErr != nil && !os.IsNotExist(statErr) { http.Error(w, statErr.Error(), http.StatusMethodNotAllowed) @@ -157,7 +157,7 @@ func (u *handlerUser) handlePartialUpdate(w http.ResponseWriter, r *http.Request if !exists { flag |= os.O_CREATE } - f, err := u.FileSystem.OpenFile(ctx, reqPath, flag, 0666) + f, err := u.fs.OpenFile(ctx, reqPath, flag, 0666) if err != nil { if os.IsNotExist(err) { http.Error(w, err.Error(), http.StatusConflict) @@ -361,7 +361,7 @@ func (u *handlerUser) confirmPartialUpdateLocks(r *http.Request, src string) (re hdr := r.Header.Get("If") if hdr == "" { now := time.Now() - token, err := u.LockSystem.Create(now, webdav.LockDetails{ + token, err := u.handler.LockSystem.Create(now, webdav.LockDetails{ Root: src, Duration: -1, ZeroDepth: true, @@ -373,7 +373,7 @@ func (u *handlerUser) confirmPartialUpdateLocks(r *http.Request, src string) (re return nil, http.StatusInternalServerError, err } return func() { - _ = u.LockSystem.Unlock(now, token) + _ = u.handler.LockSystem.Unlock(now, token) }, 0, nil } @@ -393,7 +393,7 @@ func (u *handlerUser) confirmPartialUpdateLocks(r *http.Request, src string) (re if parsedURL.Host != r.Host { continue } - lsrc, err = stripPartialPrefix(parsedURL.Path, u.Prefix) + lsrc, err = stripPartialPrefix(parsedURL.Path, u.handler.Prefix) if err != nil { return nil, http.StatusNotFound, err } @@ -401,7 +401,7 @@ func (u *handlerUser) confirmPartialUpdateLocks(r *http.Request, src string) (re lsrc = src } } - release, err = u.LockSystem.Confirm(time.Now(), lsrc, "", l.conditions...) + release, err = u.handler.LockSystem.Confirm(time.Now(), lsrc, "", l.conditions...) if errors.Is(err, webdav.ErrConfirmationFailed) { continue } diff --git a/lib/permissions.go b/lib/permissions.go index ca3456b..cf40e66 100644 --- a/lib/permissions.go +++ b/lib/permissions.go @@ -26,23 +26,38 @@ func (r *Rule) Validate() error { return nil } -// Matches checks if [Rule] matches the given path. -func (r *Rule) Matches(path string) bool { +// Matches checks if [Rule] matches the given path. When caseInsensitive is set +// the backing file system ignores case, so this must too. A regex is tried +// against the folded path as well as the path as written, which only widens it +// to spellings naming the same file. +func (r *Rule) Matches(path string, caseInsensitive bool) bool { if r.Regex != nil { + if caseInsensitive { + return r.Regex.MatchString(path) || r.Regex.MatchString(foldPath(path)) + } + return r.Regex.MatchString(path) } + if caseInsensitive { + return strings.HasPrefix(foldPath(path), foldPath(r.Path)) + } + return strings.HasPrefix(path, r.Path) } // matchesCollection checks if [Rule] names path as the collection it governs, // such as a rule for "/c/" and a request for "/c". Regex rules are matched // literally and are not considered here. -func (r *Rule) matchesCollection(path string) bool { +func (r *Rule) matchesCollection(path string, caseInsensitive bool) bool { if r.Regex != nil || !strings.HasSuffix(r.Path, "/") { return false } + if caseInsensitive { + return foldPath(path) == foldPath(strings.TrimSuffix(r.Path, "/")) + } + return path == strings.TrimSuffix(r.Path, "/") } @@ -63,6 +78,7 @@ type UserPermissions struct { directoryExplicit bool directoriesExplicit bool useDirectories bool + caseInsensitive bool } type DirectoryMount struct { @@ -93,26 +109,46 @@ func (p UserPermissions) Allowed(r *request, fileExists func(string) bool) bool // allowedAt resolves the permissions that govern path and applies check to them. func (p UserPermissions) allowedAt(path string, check func(Permissions) bool) bool { // Go through rules beginning from the last one. The first matched rule returns. + // Both senses of matching are tested per rule: in separate passes a broader + // rule would return first and shadow the narrower one naming the collection. for i := len(p.Rules) - 1; i >= 0; i-- { - if p.Rules[i].Matches(path) { + if p.Rules[i].Matches(path, p.caseInsensitive) { return check(p.Rules[i].Permissions) } - } - // A rule written with a trailing slash also governs the collection it names, - // so that a rule for "/c/" cannot be evaded by asking for "/c". Such a request - // acts on an entry of the parent collection, so it needs the permissions that - // apply there too. Requiring both means the rule can restrict the collection - // without granting access that would otherwise not exist. - for i := len(p.Rules) - 1; i >= 0; i-- { - if p.Rules[i].matchesCollection(path) { - return check(p.Rules[i].Permissions) && check(p.Permissions) + // A rule written with a trailing slash also governs the collection it names, + // so a rule for "/c/" cannot be evaded by asking for "/c". Such a request acts + // on an entry of the parent collection, so it needs those permissions too: the + // rule can restrict the collection, not grant access that would not exist. + if p.Rules[i].matchesCollection(path, p.caseInsensitive) { + if !check(p.Rules[i].Permissions) { + return false + } + + // Through the rules, not the global permissions alone, which would deny + // a collection that an enclosing rule grants. + if parent := parentCollection(path); parent != path { + return p.allowedAt(parent, check) + } + + return check(p.Permissions) } } return check(p.Permissions) } +// parentCollection returns the collection containing path, such as "/data/" for +// "/data/sub", or "/" for a top-level entry. That bounds allowedAt at the root. +func parentCollection(p string) string { + i := strings.LastIndex(strings.TrimSuffix(p, "/"), "/") + if i <= 0 { + return "/" + } + + return p[:i+1] +} + func (p *UserPermissions) Validate() error { var err error @@ -127,6 +163,8 @@ func (p *UserPermissions) Validate() error { } } + p.caseInsensitive = p.hasCaseInsensitiveBacking() + for _, r := range p.Rules { if err := r.Validate(); err != nil { return fmt.Errorf("invalid permissions: %w", err) @@ -143,6 +181,23 @@ func (p *UserPermissions) Validate() error { return nil } +// hasCaseInsensitiveBacking reports whether any backing directory resolves names +// regardless of case. Mounts spread over volumes that differ all fold, which +// keeps deny rules effective on the case-insensitive ones. +func (p *UserPermissions) hasCaseInsensitiveBacking() bool { + if p.useDirectories || len(p.Directories) > 0 { + for _, mount := range p.Directories { + if caseInsensitiveFS(mount.Path) { + return true + } + } + + return false + } + + return caseInsensitiveFS(p.Directory) +} + func (d *DirectoryMounts) Validate() error { names := map[string]struct{}{} @@ -237,8 +292,16 @@ func (p Permissions) Allowed(r *request, fileExists func(string) bool) bool { return p.Read && p.Delete case "DELETE": return p.Delete - case "LOCK", "UNLOCK": - return p.Create || p.Read || p.Update || p.Delete + case "LOCK": + // A lock is write-class: it reserves the resource against other writers, + // and locking a path that does not exist creates it. + if fileExists(r.path) { + return p.Update + } else { + return p.Create + } + case "UNLOCK": + return p.Create || p.Update default: return false } diff --git a/lib/permissions_fs.go b/lib/permissions_fs.go new file mode 100644 index 0000000..164bf92 --- /dev/null +++ b/lib/permissions_fs.go @@ -0,0 +1,158 @@ +package lib + +import ( + "context" + "errors" + "io" + "os" + "path" + + "golang.org/x/net/webdav" +) + +var _ webdav.FileSystem = permissionsFS{} + +// permissionsFS wraps a [webdav.FileSystem] so directory listings only report +// entries the user may read. PROPFIND and COPY reach descendants by enumerating +// through the file system, so rules have to be applied as those listings are produced. +// +// The wrapped file system is a named field rather than an embedded one on +// purpose. allowedThroughout has to read unfiltered listings, and a promoted +// OpenFile would make walking the filtered view by accident a one-character +// change that authorizes everything without failing any obvious way. +type permissionsFS struct { + fs webdav.FileSystem + perms UserPermissions +} + +func (f permissionsFS) Mkdir(ctx context.Context, name string, perm os.FileMode) error { + return f.fs.Mkdir(ctx, name, perm) +} + +func (f permissionsFS) RemoveAll(ctx context.Context, name string) error { + return f.fs.RemoveAll(ctx, name) +} + +func (f permissionsFS) Rename(ctx context.Context, oldName, newName string) error { + return f.fs.Rename(ctx, oldName, newName) +} + +func (f permissionsFS) Stat(ctx context.Context, name string) (os.FileInfo, error) { + return f.fs.Stat(ctx, name) +} + +func (f permissionsFS) OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (webdav.File, error) { + file, err := f.fs.OpenFile(ctx, name, flag, perm) + if err != nil { + return nil, err + } + + // The handler strips its prefix first, which for the default prefix "/" also + // drops the leading slash. Rules are written with one. + return &permissionsFile{File: file, name: cleanPath(name), perms: f.perms}, nil +} + +type permissionsFile struct { + webdav.File + name string + perms UserPermissions +} + +func (f *permissionsFile) Readdir(count int) ([]os.FileInfo, error) { + if count <= 0 { + fis, err := f.File.Readdir(count) + if err != nil { + return nil, err + } + + return f.readable(fis), nil + } + + // A positive count asks for that many entries, so keep reading until that + // many survive filtering: a short read would look like the end of the listing. + var entries []os.FileInfo + + for len(entries) < count { + fis, err := f.File.Readdir(count - len(entries)) + if err != nil { + if len(entries) > 0 && errors.Is(err, io.EOF) { + return entries, nil + } + + return nil, err + } + + if len(fis) == 0 { + break + } + + entries = append(entries, f.readable(fis)...) + } + + return entries, nil +} + +// readable returns the entries whose path the user is allowed to read. +func (f *permissionsFile) readable(fis []os.FileInfo) []os.FileInfo { + allowed := make([]os.FileInfo, 0, len(fis)) + + for _, fi := range fis { + if f.perms.allowedAt(path.Join(f.name, fi.Name()), func(p Permissions) bool { + return p.Read + }) { + allowed = append(allowed, fi) + } + } + + return allowed +} + +// allowedThroughout reports whether check holds for every descendant of name. +// Rename and RemoveAll act on a subtree in one call without consulting the file +// system per descendant, so MOVE and DELETE need this before dispatching. +// +// It reads f.fs directly rather than the [permissionsFS] view: the latter omits +// the very entries this needs to refuse on. +func (f permissionsFS) allowedThroughout(ctx context.Context, name string, check func(Permissions) bool) (bool, error) { + info, err := f.fs.Stat(ctx, name) + if err != nil { + if os.IsNotExist(err) { + // Nothing to walk; the request fails later on its own terms. + return true, nil + } + + return false, err + } + + if !info.IsDir() { + return true, nil + } + + file, err := f.fs.OpenFile(ctx, name, os.O_RDONLY, 0) + if err != nil { + return false, err + } + + entries, err := file.Readdir(-1) + err = errors.Join(err, file.Close()) + if err != nil { + return false, err + } + + for _, entry := range entries { + child := path.Join(name, entry.Name()) + + if !f.perms.allowedAt(child, check) { + return false, nil + } + + if entry.IsDir() { + ok, err := f.allowedThroughout(ctx, child, check) + if !ok || err != nil { + return ok, err + } + } + } + + return true, nil +} diff --git a/lib/permissions_test.go b/lib/permissions_test.go new file mode 100644 index 0000000..dfff88d --- /dev/null +++ b/lib/permissions_test.go @@ -0,0 +1,108 @@ +package lib + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRuleMatchesFolding(t *testing.T) { + t.Parallel() + + rule := &Rule{Path: "/pub/"} + + // Exact comparison: on a case-sensitive file system "/PUB/" is a different + // directory, which a rule granting "/pub/" must not reach. + require.True(t, rule.Matches("/pub/x.txt", false)) + require.False(t, rule.Matches("/PUB/x.txt", false)) + require.False(t, rule.Matches("/Pub/x.txt", false)) + + // Folded they name one directory, so the rule governs both. + require.True(t, rule.Matches("/pub/x.txt", true)) + require.True(t, rule.Matches("/PUB/x.txt", true)) + require.True(t, rule.Matches("/Pub/x.txt", true)) + + // A sibling merely starting with the same characters stays unaffected. + require.False(t, rule.Matches("/public/x.txt", true)) +} + +func TestRuleMatchesNormalization(t *testing.T) { + t.Parallel() + + const ( + nfc = "/café/" // café, composed + nfd = "/café/" // café, decomposed + ) + + rule := &Rule{Path: nfc} + + // A file system ignoring case treats these spellings as one file too, so + // folding has to normalize or the rule is evaded by retyping it. + require.True(t, rule.Matches(nfd+"flag.txt", true)) + require.False(t, rule.Matches(nfd+"flag.txt", false)) +} + +func TestRuleMatchesCollectionFolding(t *testing.T) { + t.Parallel() + + rule := &Rule{Path: "/c/"} + + require.True(t, rule.matchesCollection("/c", false)) + require.False(t, rule.matchesCollection("/C", false)) + require.True(t, rule.matchesCollection("/C", true)) + + // A rule without a trailing slash names a resource, not a collection. + require.False(t, (&Rule{Path: "/c"}).matchesCollection("/c", false)) +} + +func TestParentCollection(t *testing.T) { + t.Parallel() + + require.Equal(t, "/data/", parentCollection("/data/sub")) + require.Equal(t, "/data/", parentCollection("/data/sub/")) + require.Equal(t, "/data/sub/", parentCollection("/data/sub/leaf.txt")) + require.Equal(t, "/", parentCollection("/pub")) + require.Equal(t, "/", parentCollection("/")) +} + +func TestFlipCase(t *testing.T) { + t.Parallel() + + alt, ok := flipCase("/srv/dav") + require.True(t, ok) + require.Equal(t, "/srv/Dav", alt) + + alt, ok = flipCase("/srv/DAV") + require.True(t, ok) + require.Equal(t, "/srv/dAV", alt) + + // Only the first letter flips, so a name whose case mapping does not + // round-trip is left alone rather than changed by more than its case. + alt, ok = flipCase("/srv/ıstanbul") + require.True(t, ok) + require.Equal(t, "/srv/ıStanbul", alt) + + // Nothing to flip. + _, ok = flipCase("/srv/001") + require.False(t, ok) +} + +func TestRuleMatchesRegexFolding(t *testing.T) { + t.Parallel() + + rule := &Rule{Regex: regexp.MustCompile("^/secret/")} + + require.True(t, rule.Matches("/secret/flag.txt", false)) + require.False(t, rule.Matches("/SECRET/flag.txt", false)) + + // Where the file system serves both spellings as one file, the rule has to + // cover both or it denies nothing. + require.True(t, rule.Matches("/SECRET/flag.txt", true)) + + // A pattern written with upper case still relies on the path as written, so + // folding never takes a match away. + upper := &Rule{Regex: regexp.MustCompile("^/Secret/")} + require.True(t, upper.Matches("/Secret/flag.txt", true)) + require.False(t, upper.Matches("/public/flag.txt", true)) +}