From b331bc736d54bad2ac30cb3aa55abd1dca87ae29 Mon Sep 17 00:00:00 2001 From: "a.gritsenko" Date: Fri, 2 Oct 2026 22:48:31 +0300 Subject: [PATCH] Shares: allow any folder, not only ZFS datasets SMB shares and NFS exports may now point to any existing directory (USB disks under /mnt or /media, /srv, home folders...) instead of only ZFS dataset mountpoints. System trees (/etc, /usr, /var, /boot, /proc, /root, ...) are refused, also after resolving symlinks. The path suggestions list mounted data filesystems from /proc/self/mounts in addition to ZFS mountpoints. Co-Authored-By: Claude Opus 5.5 --- internal/shares/mounts.go | 55 +++++++++++++++++++++++++++++ internal/shares/render.go | 2 +- internal/shares/shares_test.go | 29 ++++++++++++++-- internal/shares/validate.go | 24 +++++++++---- internal/web/server.go | 9 +++-- internal/web/shares.go | 56 ++++++++++++++++++++---------- internal/web/shares_test.go | 20 +++++++++-- internal/web/templates/shares.html | 7 ++-- internal/web/web_test.go | 13 ++++++- packaging/README.md | 5 ++- 10 files changed, 183 insertions(+), 37 deletions(-) create mode 100644 internal/shares/mounts.go diff --git a/internal/shares/mounts.go b/internal/shares/mounts.go new file mode 100644 index 0000000..04a7d46 --- /dev/null +++ b/internal/shares/mounts.go @@ -0,0 +1,55 @@ +package shares + +import ( + "sort" + "strconv" + "strings" +) + +// pseudoFS are filesystem types that never hold user data. +var pseudoFS = map[string]bool{ + "proc": true, "sysfs": true, "tmpfs": true, "devtmpfs": true, "devpts": true, "cgroup": true, "cgroup2": true, + "securityfs": true, "pstore": true, "bpf": true, "debugfs": true, "tracefs": true, "configfs": true, + "fusectl": true, "mqueue": true, "hugetlbfs": true, "autofs": true, "binfmt_misc": true, "nsfs": true, + "overlay": true, "squashfs": true, "efivarfs": true, "rpc_pipefs": true, "nfsd": true, "ramfs": true, +} + +// ParseMounts extracts shareable mountpoints from /proc/self/mounts content: +// real filesystems outside system directories, sorted and de-duplicated. +func ParseMounts(data string) []string { + seen := map[string]bool{} + var out []string + for _, line := range strings.Split(data, "\n") { + f := strings.Fields(line) + if len(f) < 3 || pseudoFS[f[2]] { + continue + } + mp := unescapeMount(f[1]) + if !strings.HasPrefix(mp, "/") || ValidateNotSystem(mp) != nil || seen[mp] { + continue + } + seen[mp] = true + out = append(out, mp) + } + sort.Strings(out) + return out +} + +// unescapeMount decodes the octal escapes (\040 etc.) the kernel uses in mounts. +func unescapeMount(s string) string { + if !strings.Contains(s, `\`) { + return s + } + var b strings.Builder + for i := 0; i < len(s); i++ { + if s[i] == '\\' && i+4 <= len(s) { + if n, err := strconv.ParseUint(s[i+1:i+4], 8, 8); err == nil { + b.WriteByte(byte(n)) + i += 3 + continue + } + } + b.WriteByte(s[i]) + } + return b.String() +} diff --git a/internal/shares/render.go b/internal/shares/render.go index 99b9f1d..e5a578e 100644 --- a/internal/shares/render.go +++ b/internal/shares/render.go @@ -102,7 +102,7 @@ func RenderExports(c Config, uid, gid string) string { default: opts = append(opts, "root_squash", "anonuid="+uid, "anongid="+gid) } - // crossmnt: child ZFS datasets are separate mounts and must be reachable + // crossmnt: child mounts (ZFS datasets or other filesystems) must be reachable opts = append(opts, "crossmnt", "no_subtree_check") var parts []string for _, cl := range e.Clients { diff --git a/internal/shares/shares_test.go b/internal/shares/shares_test.go index 0933002..de484bb 100644 --- a/internal/shares/shares_test.go +++ b/internal/shares/shares_test.go @@ -70,12 +70,13 @@ func TestValidation(t *testing.T) { t.Errorf("%q accepted", n) } } - for _, p := range []string{"/nas/x", "/tmp_pool"} { + for _, p := range []string{"/nas/x", "/tmp_pool", "/mnt/usb", "/srv/media", "/home/alice/docs", "/media/disk", "/variable", "/etcetera"} { if err := ValidatePath(p); err != nil { t.Errorf("%q rejected: %v", p, err) } } - for _, p := range []string{"", "/", "nas/x", "/nas/../etc", "/nas/", "/nas/x\"y", "/nas/x\n"} { + for _, p := range []string{"", "/", "nas/x", "/nas/../etc", "/nas/", "/nas/x\"y", "/nas/x\n", + "/etc", "/etc/samba", "/var/lib/docker", "/usr", "/proc/1", "/root", "/boot/efi", "/run/user/1000"} { if ValidatePath(p) == nil { t.Errorf("%q accepted", p) } @@ -336,3 +337,27 @@ func TestUsers(t *testing.T) { t.Errorf("owner account deleted") } } + +func TestParseMounts(t *testing.T) { + in := `proc /proc proc rw,nosuid 0 0 +tmpfs /run tmpfs rw 0 0 +/dev/sda2 / ext4 rw 0 0 +/dev/sda1 /boot/efi vfat rw 0 0 +tank /tank zfs rw,xattr 0 0 +tank /tank zfs rw,xattr 0 0 +/dev/sdc1 /mnt/usb\040disk ext4 rw 0 0 +overlay /srv/docker/merged overlay rw 0 0 +` + got := strings.Join(ParseMounts(in), "|") + if got != "/mnt/usb disk|/tank" { + t.Errorf("ParseMounts = %q", got) + } +} + +func TestConfigRefusesSystemPath(t *testing.T) { + c := Defaults() + c.SMB.Shares = []SMBShare{{Name: "etc", Path: "/etc", Guest: true}} + if c.Validate() == nil { + t.Errorf("share on /etc accepted") + } +} diff --git a/internal/shares/validate.go b/internal/shares/validate.go index 2f3bcbd..0cabfb1 100644 --- a/internal/shares/validate.go +++ b/internal/shares/validate.go @@ -54,8 +54,23 @@ func ValidateComment(s string) error { return nil } -// ValidatePath accepts an absolute, clean path without control characters. -// Whether it lies under a pool is checked by the caller (needs live state). +// systemRoots are OS trees that must never be shared. +var systemRoots = []string{"/etc", "/proc", "/sys", "/dev", "/boot", "/run", "/usr", "/bin", "/sbin", + "/lib", "/lib32", "/lib64", "/libx32", "/var", "/root", "/snap", "/lost+found"} + +// ValidateNotSystem refuses the root filesystem and system directories. +func ValidateNotSystem(p string) error { + if p == "/" { + return errors.New("refusing to share the root filesystem") + } + if UnderRoots(p, systemRoots) { + return fmt.Errorf("%s is a system directory and cannot be shared", p) + } + return nil +} + +// ValidatePath accepts an absolute, clean, non-system path without control +// characters. Existence and symlinks are checked by the caller. func ValidatePath(p string) error { if p == "" { return errors.New("path is required") @@ -63,10 +78,7 @@ func ValidatePath(p string) error { if !strings.HasPrefix(p, "/") || path.Clean(p) != p || strings.ContainsAny(p, "\"\\\r\n\t") { return errors.New("path must be absolute and normalised (no .., trailing / or quotes)") } - if p == "/" { - return errors.New("refusing to share the root filesystem") - } - return nil + return ValidateNotSystem(p) } // UnderRoots reports whether p equals or lies below one of the roots. diff --git a/internal/web/server.go b/internal/web/server.go index 64846cc..347a186 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -9,6 +9,7 @@ import ( "log" "math" "net/http" + "path/filepath" "strings" "sync" "time" @@ -38,7 +39,10 @@ type Server struct { pages map[string]*template.Template mux *http.ServeMux - dirExists func(string) bool // share path check; replaced in tests + // share path checks; replaced in tests + dirExists func(string) bool + realPath func(string) (string, error) + readMounts func() string mu sync.Mutex shareErr string // last failed Apply of the share config @@ -47,7 +51,8 @@ type Server struct { func New(cfg *config.Store, st *store.Store, ring *history.Ring, sch *sched.Scheduler, sh *shares.Store) *Server { run := hostrun.Auto() s := &Server{cfg: cfg, st: st, ring: ring, sch: sch, send: alert.SendMail, - admin: zfsadm.New(zfsadm.NoInput(run)), shares: sh, shm: shares.New(run), dirExists: statDir, mux: http.NewServeMux()} + admin: zfsadm.New(zfsadm.NoInput(run)), shares: sh, shm: shares.New(run), + dirExists: statDir, realPath: filepath.EvalSymlinks, readMounts: procMounts, mux: http.NewServeMux()} s.pages = map[string]*template.Template{} layout := template.Must(template.New("layout").Funcs(funcs).ParseFS(assets, "templates/layout.html")) for _, name := range []string{"dashboard", "disk", "zfs", "alerts", "settings", "shares"} { diff --git a/internal/web/shares.go b/internal/web/shares.go index 7ff7870..fb61b18 100644 --- a/internal/web/shares.go +++ b/internal/web/shares.go @@ -53,21 +53,39 @@ func (s *Server) setApplyErr(e string) { s.mu.Unlock() } -// mountpoints lists dataset mountpoints — the only places shares may point to. -func mountpoints(st sched.Status) []string { +// pathSuggestions lists ZFS dataset mountpoints plus other mounted data +// filesystems (USB disks, /mnt, /srv…) for the path input. Any non-system +// folder may be shared; these are only hints. +func (s *Server) pathSuggestions(st sched.Status) []string { + seen := map[string]bool{} var out []string - for _, d := range st.Datasets { - if strings.HasPrefix(d.Mountpoint, "/") { - out = append(out, d.Mountpoint) + add := func(p string) { + if strings.HasPrefix(p, "/") && !seen[p] && shares.ValidateNotSystem(p) == nil { + seen[p] = true + out = append(out, p) } } + for _, d := range st.Datasets { + add(d.Mountpoint) + } + for _, p := range shares.ParseMounts(s.readMounts()) { + add(p) + } sort.Strings(out) return out } +func procMounts() string { + b, err := os.ReadFile("/proc/self/mounts") + if err != nil { + return "" + } + return string(b) +} + func (s *Server) sharesView(st sched.Status, form smbForm, nform nfsForm) sharesVM { c := s.shares.Get() - vm := sharesVM{C: c, Svc: st.Services, Mountpoints: mountpoints(st), Owner: s.shm.Owner, Host: st.Host, + vm := sharesVM{C: c, Svc: st.Services, Mountpoints: s.pathSuggestions(st), Owner: s.shm.Owner, Host: st.Host, Form: form, NForm: nform, ApplyErr: s.applyErr(), Squashes: []string{"root", "all", "none"}} if vm.Form.Name == "" && vm.Form.Orig == "" { vm.Form.New, vm.Form.Guest, vm.Form.Browseable, vm.Form.HideSys = true, true, true, true @@ -147,17 +165,22 @@ func (s *Server) sharesDone(w http.ResponseWriter, r *http.Request, msg, anchor http.Redirect(w, r, "/shares?msg="+url.QueryEscape(msg)+anchor, http.StatusSeeOther) } -// pathOK checks that a share path exists and lies inside a ZFS dataset. -func (s *Server) pathOK(st sched.Status, p string) error { +// pathOK checks that a share path is an existing, non-system directory, also +// after resolving symlinks (so /tank/link -> /etc is refused). +func (s *Server) pathOK(p string) error { if err := shares.ValidatePath(p); err != nil { return err } - if !shares.UnderRoots(p, mountpoints(st)) { - return errPathOutside - } if !s.dirExists(p) { return errPathMissing } + rp, err := s.realPath(p) + if err != nil { + return errPathMissing + } + if rp != p && shares.ValidateNotSystem(rp) != nil { + return strErr("path resolves to " + rp + ", a system directory") + } return nil } @@ -170,10 +193,7 @@ type strErr string func (e strErr) Error() string { return string(e) } -const ( - errPathOutside = strErr("path must be inside a ZFS dataset (see the list of mountpoints)") - errPathMissing = strErr("path does not exist or is not a directory") -) +const errPathMissing = strErr("path does not exist or is not a directory") func (s *Server) smbSettings(w http.ResponseWriter, r *http.Request) { if err := r.ParseForm(); err != nil { @@ -210,7 +230,6 @@ func (s *Server) smbSave(w http.ResponseWriter, r *http.Request) { if len(f.Path) > 1 { f.Path = strings.TrimRight(f.Path, "/") } - st := s.sch.Status() sh := shares.SMBShare{Name: f.Name, Path: f.Path, Comment: f.Comment, ReadOnly: f.ReadOnly, Guest: f.Guest, Users: f.Users, Browseable: f.Browseable, HideSysFiles: f.HideSys} if sh.Guest { @@ -220,7 +239,7 @@ func (s *Server) smbSave(w http.ResponseWriter, r *http.Request) { s.sharesFail(w, r, "Not saved: "+err.Error(), f, nfsForm{}) return } - if err := s.pathOK(st, sh.Path); err != nil { + if err := s.pathOK(sh.Path); err != nil { s.sharesFail(w, r, "Not saved: "+err.Error(), f, nfsForm{}) return } @@ -276,13 +295,12 @@ func (s *Server) nfsSave(w http.ResponseWriter, r *http.Request) { if len(f.Path) > 1 { f.Path = strings.TrimRight(f.Path, "/") } - st := s.sch.Status() e := shares.NFSExport{Path: f.Path, Clients: shares.SplitList(f.Clients), ReadOnly: f.ReadOnly, Squash: f.Squash, Async: f.Async} if err := e.Validate(); err != nil { s.sharesFail(w, r, "Not saved: "+err.Error(), smbForm{}, f) return } - if err := s.pathOK(st, e.Path); err != nil { + if err := s.pathOK(e.Path); err != nil { s.sharesFail(w, r, "Not saved: "+err.Error(), smbForm{}, f) return } diff --git a/internal/web/shares_test.go b/internal/web/shares_test.go index f6c718c..0e59c0e 100644 --- a/internal/web/shares_test.go +++ b/internal/web/shares_test.go @@ -68,7 +68,7 @@ func TestSharesSMBFlow(t *testing.T) { // page lists users and mountpoints, prefills from ?path= body := get(srv, "/shares?path=/tank/data").Body.String() - for _, want := range []string{`value="/tank/data"`, `value="data"`, `value="alice"`, "No SMB shares yet"} { + for _, want := range []string{`value="/tank/data"`, `value="data"`, `value="alice"`, "No SMB shares yet", `