diff --git a/weed/sftpd/sftp_filer.go b/weed/sftpd/sftp_filer.go index 7eea186b0..6287c0f98 100644 --- a/weed/sftpd/sftp_filer.go +++ b/weed/sftpd/sftp_filer.go @@ -150,9 +150,8 @@ func (fs *SftpServer) newFileWriter(r *sftp.Request) (io.WriterAt, error) { if err != nil { return nil, err } - dir, _ := util.FullPath(absPath).DirAndName() - if err := fs.checkFilePermission(dir, "write"); err != nil { - glog.Errorf("Permission denied for %s", dir) + if err := fs.checkFilePermission(absPath, "write"); err != nil { + glog.Errorf("Permission denied for %s", absPath) return nil, err } // Create a temporary file to buffer writes diff --git a/weed/sftpd/sftp_permissions_test.go b/weed/sftpd/sftp_permissions_test.go index 739072894..ae0c69355 100644 --- a/weed/sftpd/sftp_permissions_test.go +++ b/weed/sftpd/sftp_permissions_test.go @@ -1,9 +1,19 @@ package sftpd import ( + "context" + "net" + "os" "testing" + "github.com/pkg/sftp" + "github.com/seaweedfs/seaweedfs/weed/pb" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/sftpd/user" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" ) func TestPathWithin(t *testing.T) { @@ -62,3 +72,100 @@ func TestIsPathInHomeDirectorySiblingPrefix(t *testing.T) { t.Error("home /tenants/alice must not match sibling /tenants/alice-archive") } } + +// newFileWriter must honor a more specific ACL on the target file, not just +// the parent directory's: a file the user cannot write stays read-only even +// in a writable directory. Missing paths still defer to the parent directory. +func TestNewFileWriterChecksTargetFile(t *testing.T) { + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + filer := &permissionTestFiler{entries: map[string]*filer_pb.Entry{ + "/": {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{FileMode: 0755}}, + "/home": {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{FileMode: 0755}}, + "/home/alice": {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}}, + "/home/alice/locked.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}}, + "/home/alice/ok.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}}, + "/home/alice/nested": {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}}, + "/home/alice/nested/ok.f": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}}, + }} + grpcServer := grpc.NewServer() + filer_pb.RegisterSeaweedFilerServer(grpcServer, filer) + go func() { _ = grpcServer.Serve(lis) }() + t.Cleanup(grpcServer.Stop) + + _, port, _ := net.SplitHostPort(lis.Addr().String()) + fs := &SftpServer{ + filerAddr: pb.ServerAddress("127.0.0.1:0." + port), + grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()), + user: &user.User{ + Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice", + Permissions: map[string][]string{ + "/home/alice": {"readwrite"}, + "/home/alice/locked.txt": {"read"}, + }, + }, + } + + write := func(path string) error { + w, err := fs.newFileWriter(&sftp.Request{Filepath: path}) + if writer, ok := w.(*SeaweedSftpFileWriter); ok { + _ = os.Remove(writer.tmpFile.Name()) + _ = writer.tmpFile.Close() + } + return err + } + + if err := write("/locked.txt"); err == nil { + t.Error("expected write denial on a file with an explicit read-only ACL") + } + if err := write("/ok.txt"); err != nil { + t.Errorf("expected write under the directory ACL, got %v", err) + } + if err := write("/new.txt"); err != nil { + t.Errorf("expected write on a new file in a writable dir, got %v", err) + } + if err := write("/nested/new.txt"); err != nil { + t.Errorf("expected write on a new file under a missing intermediate check, got %v", err) + } + delete(filer.entries, "/home/alice/nested") + delete(filer.entries, "/home/alice/nested/ok.f") + if err := write("/gone/deeper.txt"); err != nil { + t.Errorf("expected write when ancestors resolve to the writable dir, got %v", err) + } +} + +func TestCheckFilePermissionMissingAncestorsTerminates(t *testing.T) { + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + filer := &permissionTestFiler{entries: map[string]*filer_pb.Entry{}} + grpcServer := grpc.NewServer() + filer_pb.RegisterSeaweedFilerServer(grpcServer, filer) + go func() { _ = grpcServer.Serve(lis) }() + t.Cleanup(grpcServer.Stop) + + _, port, _ := net.SplitHostPort(lis.Addr().String()) + fs := &SftpServer{ + filerAddr: pb.ServerAddress("127.0.0.1:0." + port), + grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()), + user: &user.User{Username: "alice", Uid: 1000, HomeDir: "/home/alice"}, + } + if err := fs.checkFilePermission("/a/b/c", "write"); err == nil { + t.Error("expected denial when no ancestor exists") + } +} + +type permissionTestFiler struct { + filer_pb.UnimplementedSeaweedFilerServer + entries map[string]*filer_pb.Entry +} + +func (f *permissionTestFiler) LookupDirectoryEntry(_ context.Context, req *filer_pb.LookupDirectoryEntryRequest) (*filer_pb.LookupDirectoryEntryResponse, error) { + if entry, ok := f.entries[req.Directory+"/"+req.Name]; ok { + return &filer_pb.LookupDirectoryEntryResponse{Entry: entry}, nil + } + return nil, status.Error(codes.NotFound, "no entry is found") +}