Files
seaweedFS/weed/sftpd/sftp_service.go
Chris Lu e361daa754 fix: SFTP HomeDir path translation for user operations (#7611)
* fix: SFTP HomeDir path translation for user operations

When users have a non-root HomeDir (e.g., '/sftp/user'), their SFTP
operations should be relative to that directory. Previously, when a
user uploaded to '/' via SFTP, the path was not translated to their
home directory, causing 'permission denied for / for permission write'.

This fix adds a toAbsolutePath() method that implements chroot-like
behavior where the user's HomeDir becomes their root. All file and
directory operations now translate paths through this method.

Example: User with HomeDir='/sftp/user' uploading to '/' now correctly
maps to '/sftp/user'.

Fixes: https://github.com/seaweedfs/seaweedfs/issues/7470

* test: add SFTP integration tests

Add comprehensive integration tests for the SFTP server including:
- HomeDir path translation tests (verifies fix for issue #7470)
- Basic file upload/download operations
- Directory operations (mkdir, rmdir, list)
- Large file handling (1MB test)
- File rename operations
- Stat/Lstat operations
- Path edge cases (trailing slashes, .., unicode filenames)
- Admin root access verification

The test framework starts a complete SeaweedFS cluster with:
- Master server
- Volume server
- Filer server
- SFTP server with test user credentials

Test users are configured in testdata/userstore.json:
- admin: HomeDir=/ with full access
- testuser: HomeDir=/sftp/testuser with access to home
- readonly: HomeDir=/public with read-only access

* fix: correct SFTP HomeDir path translation and add CI

Fix path.Join issue where paths starting with '/' weren't joined correctly.
path.Join('/sftp/user', '/file') returns '/file' instead of '/sftp/user/file'.
Now we strip the leading '/' before joining.

Test improvements:
- Update go.mod to Go 1.24
- Fix weed binary discovery to prefer local build over PATH
- Add stabilization delay after service startup
- All 8 SFTP integration tests pass locally

Add GitHub Actions workflow for SFTP tests:
- Runs on push/PR affecting sftpd code or tests
- Tests HomeDir path translation, file ops, directory ops
- Covers issue #7470 fix verification

* security: update golang.org/x/crypto to v0.45.0

Addresses security vulnerability in golang.org/x/crypto < 0.45.0

* security: use proper SSH host key verification in tests

Replace ssh.InsecureIgnoreHostKey() with ssh.FixedHostKey() that
verifies the server's host key matches the known test key we generated.
This addresses CodeQL warning go/insecure-hostkeycallback.

Also updates go.mod to specify go 1.24.0 explicitly.

* security: fix path traversal vulnerability in SFTP toAbsolutePath

The previous implementation had a critical security vulnerability:
- Path traversal via '../..' could escape the HomeDir chroot jail
- Absolute paths were not correctly prefixed with HomeDir

The fix:
1. Concatenate HomeDir with userPath directly, then clean
2. Add security check to ensure final path stays within HomeDir
3. If traversal detected, safely return HomeDir instead

Also adds path traversal prevention tests to verify the fix.

* fix: address PR review comments

1. Fix SkipCleanup check to use actual test config instead of default
   - Added skipCleanup field to SftpTestFramework struct
   - Store config.SkipCleanup during Setup()
   - Use f.skipCleanup in Cleanup() instead of DefaultTestConfig()

2. Fix path prefix check false positive in mkdir
   - Changed from strings.HasPrefix(absPath, fs.user.HomeDir)
   - To: absPath == fs.user.HomeDir || strings.HasPrefix(absPath, fs.user.HomeDir+"/")
   - Prevents matching partial directory names (e.g., /sftp/username when HomeDir is /sftp/user)

* fix: check write permission on parent dir for mkdir

Aligns makeDir's permission check with newFileWriter for consistency.
To create a directory, a user needs write permission on the parent
directory, not mkdir permission on the new directory path.

* fix: refine SFTP path traversal logic and tests

1. Refine toAbsolutePath:
   - Use path.Join with strings.TrimPrefix for idiomatic path construction
   - Return explicit error on path traversal attempt instead of clamping
   - Updated all call sites to handle the error

2. Add Unit Tests:
   - Added sftp_server_test.go to verify toAbsolutePath logic
   - Covers normal paths, root path, and various traversal attempts

3. Update Integration Tests:
   - Updated PathTraversalPrevention test to reflect that standard SFTP clients
     sanitize paths before sending. The test now verifies successful containment
     within the jail rather than blocking (since the server receives a clean path).
   - The server-side blocking is verified by the new unit tests.

4. Makefile:
   - Removed -v from default test target

* fix: address PR comments on tests and makefile

1. Enhanced Unit Tests:
   - Added edge cases (empty path, multiple slashes, trailing slash) to sftp_server_test.go

2. Makefile Improvements:
   - Added 'all' target as default entry point

3. Code Clarity:
   - Added comment to mkdir permission check explaining defensive nature of HomeDir check

* fix: address PR review comments on permissions and tests

1. Security:
   - Added write permission check on target directory in renameEntry

2. Logging:
   - Changed dispatch log verbosity from V(0) to V(1)

3. Testing:
   - Updated Makefile .PHONY targets
   - Added unit test cases for empty/root HomeDir behavior in toAbsolutePath

* fix: set SFTP starting directory to virtual root

1. Critical Fix:
   - Changed sftp.WithStartDirectory from fs.user.HomeDir to '/'
   - Prevents double-prefixing when toAbsolutePath translates paths
   - Users now correctly start at their virtual root which maps to HomeDir

2. Test Improvements:
   - Use pointer for homeDir in tests for clearer nil vs empty distinction

* fix: clean HomeDir at config load time

Clean HomeDir path when loading users from JSON config.
This handles trailing slashes and other path anomalies at the source,
ensuring consistency throughout the codebase and avoiding repeated
cleaning on every toAbsolutePath call.

* test: strengthen assertions and add error checking in SFTP tests

1. Add error checking for cleanup operations in TestWalk
2. Strengthen cwd assertion to expect '/' explicitly in TestCurrentWorkingDirectory
3. Add error checking for cleanup in PathTraversalPrevention test
2025-12-03 13:42:05 -08:00

303 lines
8.5 KiB
Go

// sftp_service.go
package sftpd
import (
"context"
"fmt"
"io"
"net"
"os"
"path/filepath"
"time"
"github.com/pkg/sftp"
"github.com/seaweedfs/seaweedfs/weed/glog"
"github.com/seaweedfs/seaweedfs/weed/pb"
"github.com/seaweedfs/seaweedfs/weed/sftpd/auth"
"github.com/seaweedfs/seaweedfs/weed/sftpd/user"
"golang.org/x/crypto/ssh"
"google.golang.org/grpc"
)
// SFTPService holds configuration for the SFTP service.
type SFTPService struct {
options SFTPServiceOptions
userStore user.Store
authManager *auth.Manager
}
// SFTPServiceOptions contains all configuration options for the SFTP service.
type SFTPServiceOptions struct {
GrpcDialOption grpc.DialOption
DataCenter string
FilerGroup string
Filer pb.ServerAddress
// SSH Configuration
SshPrivateKey string // Legacy single host key
HostKeysFolder string // Multiple host keys for different algorithms
AuthMethods []string // Enabled auth methods: "password", "publickey", "keyboard-interactive"
MaxAuthTries int // Limit authentication attempts
BannerMessage string // Pre-auth banner message
LoginGraceTime time.Duration // Timeout for authentication
// Connection Management
ClientAliveInterval time.Duration // Keep-alive check interval
ClientAliveCountMax int // Max missed keep-alives before disconnect
// User Management
UserStoreFile string // Path to user store file
}
// NewSFTPService creates a new service instance.
func NewSFTPService(options *SFTPServiceOptions) *SFTPService {
service := SFTPService{options: *options}
// Initialize user store
userStore, err := user.NewFileStore(options.UserStoreFile)
if err != nil {
glog.Fatalf("Failed to initialize user store: %v", err)
}
service.userStore = userStore
// Initialize auth manager
service.authManager = auth.NewManager(userStore, options.AuthMethods)
return &service
}
// Serve accepts incoming connections on the provided listener and handles them.
func (s *SFTPService) Serve(listener net.Listener) error {
// Build SSH server config
sshConfig, err := s.buildSSHConfig()
if err != nil {
return fmt.Errorf("failed to create SSH config: %w", err)
}
glog.V(0).Infof("Starting Seaweed SFTP service on %s", listener.Addr().String())
for {
conn, err := listener.Accept()
if err != nil {
return fmt.Errorf("failed to accept incoming connection: %w", err)
}
go s.handleSSHConnection(conn, sshConfig)
}
}
// buildSSHConfig creates the SSH server configuration with proper authentication.
func (s *SFTPService) buildSSHConfig() (*ssh.ServerConfig, error) {
// Get base config from auth manager
config := s.authManager.GetSSHServerConfig()
// Set additional options
config.MaxAuthTries = s.options.MaxAuthTries
config.BannerCallback = func(conn ssh.ConnMetadata) string {
return s.options.BannerMessage
}
config.ServerVersion = "SSH-2.0-SeaweedFS-SFTP" // Custom server version
hostKeysAdded := 0
// Add legacy host key if specified
if s.options.SshPrivateKey != "" {
if err := s.addHostKey(config, s.options.SshPrivateKey); err != nil {
return nil, err
}
hostKeysAdded++
}
// Add all host keys from the specified folder
if s.options.HostKeysFolder != "" {
files, err := os.ReadDir(s.options.HostKeysFolder)
if err != nil {
return nil, fmt.Errorf("failed to read host keys folder: %w", err)
}
for _, file := range files {
if file.IsDir() {
continue // Skip directories
}
keyPath := filepath.Join(s.options.HostKeysFolder, file.Name())
if err := s.addHostKey(config, keyPath); err != nil {
// Log the error but continue with other keys
glog.V(0).Info(fmt.Sprintf("Failed to add host key %s: %v", keyPath, err))
continue
}
hostKeysAdded++
}
if hostKeysAdded == 0 {
glog.V(0).Info(fmt.Sprintf("Warning: no valid host keys found in folder %s", s.options.HostKeysFolder))
}
}
// Ensure we have at least one host key
if hostKeysAdded == 0 {
return nil, fmt.Errorf("no host keys provided")
}
return config, nil
}
// addHostKey adds a host key to the SSH server configuration.
func (s *SFTPService) addHostKey(config *ssh.ServerConfig, keyPath string) error {
keyBytes, err := os.ReadFile(keyPath)
if err != nil {
return fmt.Errorf("failed to read host key %s: %v", keyPath, err)
}
// Try parsing as private key
signer, err := ssh.ParsePrivateKey(keyBytes)
if err != nil {
// Try parsing with passphrase if available
if passphraseErr, ok := err.(*ssh.PassphraseMissingError); ok {
return fmt.Errorf("host key %s requires passphrase: %v", keyPath, passphraseErr)
}
return fmt.Errorf("failed to parse host key %s: %v", keyPath, err)
}
config.AddHostKey(signer)
glog.V(0).Infof("Added host key %s (%s)", keyPath, signer.PublicKey().Type())
return nil
}
// handleSSHConnection handles an incoming SSH connection.
func (s *SFTPService) handleSSHConnection(conn net.Conn, config *ssh.ServerConfig) {
// Set connection deadline for handshake
_ = conn.SetDeadline(time.Now().Add(s.options.LoginGraceTime))
// Perform SSH handshake
sshConn, chans, reqs, err := ssh.NewServerConn(conn, config)
if err != nil {
glog.Errorf("Failed to handshake: %v", err)
conn.Close()
return
}
// Clear deadline after successful handshake
_ = conn.SetDeadline(time.Time{})
// Set up connection monitoring
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
// Start keep-alive monitoring
go s.monitorConnection(ctx, sshConn)
username := sshConn.Permissions.Extensions["username"]
glog.V(0).Infof("New SSH connection from %s (%s) as user %s",
sshConn.RemoteAddr(), sshConn.ClientVersion(), username)
// Get user from store
sftpUser, err := s.authManager.GetUser(username)
if err != nil {
glog.Errorf("Failed to retrieve user %s: %v", username, err)
sshConn.Close()
return
}
// Create user-specific filesystem
userFs := NewSftpServer(
s.options.Filer,
s.options.GrpcDialOption,
s.options.DataCenter,
s.options.FilerGroup,
sftpUser,
)
// Ensure home directory exists with proper permissions
if err := userFs.EnsureHomeDirectory(); err != nil {
glog.Errorf("Failed to ensure home directory for user %s: %v", username, err)
// We don't close the connection here, as the user might still be able to access other directories
}
// Handle SSH requests and channels
go ssh.DiscardRequests(reqs)
for newChannel := range chans {
go s.handleChannel(newChannel, &userFs)
}
}
// monitorConnection monitors an SSH connection with keep-alives.
func (s *SFTPService) monitorConnection(ctx context.Context, sshConn *ssh.ServerConn) {
if s.options.ClientAliveInterval <= 0 {
return
}
ticker := time.NewTicker(s.options.ClientAliveInterval)
defer ticker.Stop()
missedCount := 0
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
// Send keep-alive request
_, _, err := sshConn.SendRequest("keepalive@openssh.com", true, nil)
if err != nil {
missedCount++
glog.V(0).Infof("Keep-alive missed for %s: %v (%d/%d)",
sshConn.RemoteAddr(), err, missedCount, s.options.ClientAliveCountMax)
if missedCount >= s.options.ClientAliveCountMax {
glog.Warningf("Closing unresponsive connection from %s", sshConn.RemoteAddr())
sshConn.Close()
return
}
} else {
missedCount = 0
}
}
}
}
// handleChannel handles a single SSH channel.
func (s *SFTPService) handleChannel(newChannel ssh.NewChannel, fs *SftpServer) {
if newChannel.ChannelType() != "session" {
_ = newChannel.Reject(ssh.UnknownChannelType, "unknown channel type")
return
}
channel, requests, err := newChannel.Accept()
if err != nil {
glog.Errorf("Could not accept channel: %v", err)
return
}
go func(in <-chan *ssh.Request) {
for req := range in {
switch req.Type {
case "subsystem":
// Check that the subsystem is "sftp".
if string(req.Payload[4:]) == "sftp" {
_ = req.Reply(true, nil)
s.handleSFTP(channel, fs)
} else {
_ = req.Reply(false, nil)
}
default:
_ = req.Reply(false, nil)
}
}
}(requests)
}
// handleSFTP starts the SFTP server on the SSH channel.
func (s *SFTPService) handleSFTP(channel ssh.Channel, fs *SftpServer) {
// Start at virtual root "/" - toAbsolutePath translates this to the user's HomeDir
serverOptions := sftp.WithStartDirectory("/")
server := sftp.NewRequestServer(channel, sftp.Handlers{
FileGet: fs,
FilePut: fs,
FileCmd: fs,
FileList: fs,
}, serverOptions)
if err := server.Serve(); err == io.EOF {
server.Close()
glog.V(0).Info("SFTP client exited session.")
} else if err != nil {
glog.Errorf("SFTP server finished with error: %v", err)
}
}