diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 8020912a..19fdad80 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -16,7 +16,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-22.04] - go: ["1.25.11", "1.26.4"] + go: ["1.25.12", "1.26.5"] goos: [linux] goarch: [amd64, arm64] @@ -64,14 +64,14 @@ jobs: - name: Upload Release Artifact uses: actions/upload-artifact@v7 - if: ${{ (github.ref == 'refs/heads/main' || github.event_name == 'pull_request') && matrix.go == '1.25.11' }} + if: ${{ (github.ref == 'refs/heads/main' || github.event_name == 'pull_request') && matrix.go == '1.25.12' }} with: name: wings_linux_${{ matrix.goarch }} path: dist/wings - name: Upload Debug Artifact uses: actions/upload-artifact@v7 - if: ${{ (github.ref == 'refs/heads/main' || github.event_name == 'pull_request') && matrix.go == '1.25.11' }} + if: ${{ (github.ref == 'refs/heads/main' || github.event_name == 'pull_request') && matrix.go == '1.25.12' }} with: name: wings_linux_${{ matrix.goarch }}_debug path: dist/wings_debug @@ -82,7 +82,7 @@ jobs: strategy: fail-fast: false matrix: - go: ["1.25.11", "1.26.4"] + go: ["1.25.12", "1.26.5"] steps: - name: Setup Go diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 335170da..ec3a8e08 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -20,7 +20,7 @@ jobs: - name: Setup Go uses: actions/setup-go@v6 with: - go-version: "1.25.11" + go-version: "1.25.12" - name: Build release binaries env: diff --git a/Dockerfile b/Dockerfile index f0242995..4e2e6e77 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Stage 1 (Build) -FROM golang:1.25.11-alpine AS builder +FROM golang:1.25.12-alpine AS builder ARG VERSION RUN apk add --update --no-cache git make diff --git a/config/config.go b/config/config.go index 904095a0..c235ee4d 100644 --- a/config/config.go +++ b/config/config.go @@ -289,6 +289,11 @@ type Backups struct { // Defaults to "best_speed" (level 1) CompressionLevel string `default:"best_speed" yaml:"compression_level"` + + // RestoreHostAllowlist allows backup restore downloads to connect to otherwise blocked + // private/internal destinations. Entries may be hostnames, IP addresses, or CIDR ranges. + RestoreHostAllowlist []string `yaml:"restore_host_allowlist"` + // RemoveBackupsOnServerDelete deletes backups associated with a server when the server is deleted RemoveBackupsOnServerDelete bool `default:"true" yaml:"remove_backups_on_server_delete"` } diff --git a/config/config_docker.go b/config/config_docker.go index f3e846b0..df10679b 100644 --- a/config/config_docker.go +++ b/config/config_docker.go @@ -2,8 +2,11 @@ package config import ( "encoding/base64" + "net/url" "sort" + "strings" + "github.com/distribution/reference" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/registry" "github.com/goccy/go-json" @@ -108,6 +111,92 @@ func (c DockerConfiguration) ContainerLogConfig() container.LogConfig { Config: c.LogConfig.Config, } } +// RegistryCredentialsForImage returns registry credentials for an image only +// when the configured registry and image reference share the same registry +// identity. +func (c DockerConfiguration) RegistryCredentialsForImage(img string) (string, *RegistryConfiguration) { + named, err := reference.ParseNormalizedNamed(img) + if err != nil { + return "", nil + } + + imageDomain := strings.ToLower(reference.Domain(named)) + imagePath := reference.Path(named) + var matchedRegistry string + var matchedCredentials RegistryConfiguration + matchedScore := -1 + + for registry, cfg := range c.Registries { + domain, path, ok := parseDockerRegistryReference(registry) + if !ok || domain != imageDomain || !registryPathMatchesImage(path, imagePath) { + continue + } + + score := len(domain) + len(path) + if score > matchedScore { + matchedRegistry = registry + matchedCredentials = cfg + matchedScore = score + } + } + + if matchedScore == -1 { + return "", nil + } + + return matchedRegistry, &matchedCredentials +} + +func parseDockerRegistryReference(registry string) (string, string, bool) { + registry = strings.TrimSpace(registry) + if registry == "" { + return "", "", false + } + + if u, err := url.Parse(registry); err == nil && u.Host != "" { + p := strings.Trim(u.Path, "/") + if p == "" || p == "v1" || p == "v2" { + registry = u.Host + } else { + registry = u.Host + "/" + p + } + } + + registry = strings.Trim(registry, "/") + if registry == "" { + return "", "", false + } + + hasPath := strings.Contains(registry, "/") + ref := registry + if !hasPath { + ref += "/wings" + } + + named, err := reference.ParseNormalizedNamed(ref) + if err != nil { + return "", "", false + } + + path := "" + if hasPath { + path = reference.Path(named) + } + domain := strings.ToLower(reference.Domain(named)) + if domain == "docker.io" && (path == "v1" || path == "v2") { + path = "" + } + + return domain, path, true +} + +func registryPathMatchesImage(registryPath string, imagePath string) bool { + if registryPath == "" { + return true + } + + return imagePath == registryPath || strings.HasPrefix(imagePath, registryPath+"/") +} // RegistryConfiguration defines the authentication credentials for a given // Docker registry. diff --git a/config/config_docker_test.go b/config/config_docker_test.go new file mode 100644 index 00000000..416ba0f7 --- /dev/null +++ b/config/config_docker_test.go @@ -0,0 +1,99 @@ +package config + +import "testing" + +func TestDockerRegistryCredentialsForImage(t *testing.T) { + cfg := DockerConfiguration{ + Registries: map[string]RegistryConfiguration{ + "registry.example.com": { + Username: "registry", + Password: "secret", + }, + "registry.example.com/team": { + Username: "team", + Password: "secret", + }, + "registry.example.com:5000": { + Username: "port", + Password: "secret", + }, + "https://index.docker.io/v1/": { + Username: "docker", + Password: "secret", + }, + }, + } + + tests := []struct { + name string + image string + username string + }{ + { + name: "registry domain", + image: "registry.example.com/project/image:latest", + username: "registry", + }, + { + name: "registry with port", + image: "registry.example.com:5000/project/image:latest", + username: "port", + }, + { + name: "registry path", + image: "registry.example.com/team/image:latest", + username: "team", + }, + { + name: "registry prefix is not domain", + image: "registry.example.com.evil/project/image:latest", + }, + { + name: "registry path prefix falls back to domain", + image: "registry.example.com/team-evil/image:latest", + username: "registry", + }, + { + name: "legacy docker hub registry", + image: "docker.io/library/busybox:latest", + username: "docker", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, registry := cfg.RegistryCredentialsForImage(tt.image) + if tt.username == "" { + if registry != nil { + t.Fatalf("expected no registry credentials, got username %q", registry.Username) + } + + return + } + + if registry == nil { + t.Fatalf("expected registry credentials for %q", tt.image) + } + + if registry.Username != tt.username { + t.Fatalf("expected username %q, got %q", tt.username, registry.Username) + } + }) + } +} + +func TestDockerRegistryPathCredentialsDoNotMatchSiblingPath(t *testing.T) { + cfg := DockerConfiguration{ + Registries: map[string]RegistryConfiguration{ + "registry.example.com/team": { + Username: "team", + Password: "secret", + }, + }, + } + + _, registry := cfg.RegistryCredentialsForImage("registry.example.com/team-evil/image:latest") + if registry != nil { + t.Fatalf("expected no registry credentials, got username %q", registry.Username) + } +} \ No newline at end of file diff --git a/environment/docker/container.go b/environment/docker/container.go index 4c779298..7cc2190b 100644 --- a/environment/docker/container.go +++ b/environment/docker/container.go @@ -390,15 +390,9 @@ func (e *Environment) ensureImageExists(image string) error { defer cancel() // Get a registry auth configuration from the config. - var registryAuth *config.RegistryConfiguration - for registry, c := range config.Get().Docker.Registries { - if !strings.HasPrefix(image, registry) { - continue - } - + registry, registryAuth := config.Get().Docker.RegistryCredentialsForImage(image) + if registryAuth != nil { log.WithField("registry", registry).Debug("using authentication for registry") - registryAuth = &c - break } // Get the ImagePullOptions. diff --git a/internal/ufs/fs_darwin.go b/internal/ufs/fs_darwin.go index 2a680aee..edea6011 100644 --- a/internal/ufs/fs_darwin.go +++ b/internal/ufs/fs_darwin.go @@ -59,5 +59,5 @@ func (fs *UnixFS) Chtimesat(dirfd int, name string, atime, mtime time.Time) erro unix.NsecToTimespec(atime.UnixNano()), unix.NsecToTimespec(mtime.UnixNano()), } - return ensurePathError(unix.UtimesNanoAt(dirfd, name, utimes[0:], 0), "chtimes", name) + return ensurePathError(unix.UtimesNanoAt(dirfd, name, utimes[0:], AT_SYMLINK_NOFOLLOW), "chtimes", name) } diff --git a/internal/ufs/fs_linux.go b/internal/ufs/fs_linux.go index a77d6b9b..7ef1d094 100644 --- a/internal/ufs/fs_linux.go +++ b/internal/ufs/fs_linux.go @@ -70,5 +70,5 @@ func (fs *UnixFS) Chtimesat(dirfd int, name string, atime, mtime time.Time) erro set(1, mtime) // This does support `AT_SYMLINK_NOFOLLOW` as well if needed. - return ensurePathError(unix.UtimesNanoAt(dirfd, name, utimes[0:], 0), "chtimes", name) + return ensurePathError(unix.UtimesNanoAt(dirfd, name, utimes[0:], AT_SYMLINK_NOFOLLOW), "chtimes", name) } diff --git a/internal/ufs/fs_quota.go b/internal/ufs/fs_quota.go index cc89cbd0..bf8a99b8 100644 --- a/internal/ufs/fs_quota.go +++ b/internal/ufs/fs_quota.go @@ -4,6 +4,7 @@ package ufs import ( + "math" "sync/atomic" ) @@ -67,14 +68,33 @@ func (fs *Quota) SetUsage(newUsage int64) int64 { // Add adds `i` to the tracked usage total. func (fs *Quota) Add(i int64) int64 { - usage := fs.Usage() + for { + usage := fs.Usage() + var next int64 + + switch { + case i > 0: + if usage > math.MaxInt64-i { + next = math.MaxInt64 + } else { + next = usage + i + } + case i < 0: + if i == math.MinInt64 { + next = 0 + } else if usage <= -i { + next = 0 + } else { + next = usage + i + } + default: + return usage + } - // If adding `i` to the usage will put us below 0, cap it. (`i` can be negative) - if usage+i < 0 { - fs.usage.Store(0) - return 0 + if fs.usage.CompareAndSwap(usage, next) { + return next + } } - return fs.usage.Add(i) } // CanFit checks if the given size can fit in the filesystem without exceeding @@ -98,14 +118,15 @@ func (fs *Quota) CanFit(size int64) bool { return true } - // If the current usage + the requested size are under the limit of the - // filesystem, allow it. - if usage+size <= limit { + if size <= 0 { return true } - // Welp, the size would exceed the limit of the filesystem, deny it. - return false + if usage >= limit { + return false + } + + return size <= limit-usage } // Remove removes the named file or (empty) directory. diff --git a/internal/ufs/fs_quota_test.go b/internal/ufs/fs_quota_test.go new file mode 100644 index 00000000..3943822a --- /dev/null +++ b/internal/ufs/fs_quota_test.go @@ -0,0 +1,47 @@ +package ufs + +import ( + "math" + "testing" +) + +func TestQuotaCanFitRejectsOverflowingSize(t *testing.T) { + q := NewQuota(nil, 1<<30) + q.SetUsage(1) + + if q.CanFit(math.MaxInt64) { + t.Fatal("expected oversized write to be rejected") + } + + q.SetUsage(1 << 20) + if q.CanFit(math.MaxInt64 - 100) { + t.Fatal("expected oversized write to be rejected") + } +} + +func TestQuotaCanFitAllowsShrinkingWrite(t *testing.T) { + q := NewQuota(nil, 10) + q.SetUsage(20) + + if !q.CanFit(-5) { + t.Fatal("expected shrinking write to be allowed") + } +} + +func TestQuotaAddDoesNotResetOnPositiveOverflow(t *testing.T) { + q := NewQuota(nil, 0) + q.SetUsage(math.MaxInt64 - 1) + + if got := q.Add(10); got != math.MaxInt64 { + t.Fatalf("expected usage to saturate at MaxInt64, got %d", got) + } +} + +func TestQuotaAddClampsSubtractionAtZero(t *testing.T) { + q := NewQuota(nil, 0) + q.SetUsage(5) + + if got := q.Add(-10); got != 0 { + t.Fatalf("expected usage to clamp at zero, got %d", got) + } +} \ No newline at end of file diff --git a/internal/ufs/fs_unix_test.go b/internal/ufs/fs_unix_test.go index a8bab4b6..7d2a27fa 100644 --- a/internal/ufs/fs_unix_test.go +++ b/internal/ufs/fs_unix_test.go @@ -13,6 +13,7 @@ import ( "slices" "strconv" "testing" + "time" "github.com/pelican-dev/wings/internal/ufs" ) @@ -283,15 +284,56 @@ func TestUnixFS_Lchown(t *testing.T) { } func TestUnixFS_Chtimes(t *testing.T) { - t.Parallel() - fs, err := newTestUnixFS() + tmpDir := t.TempDir() + root := filepath.Join(tmpDir, "root") + if err := os.Mkdir(root, 0o755); err != nil { + t.Fatal(err) + } + fs, err := ufs.NewUnixFS(root, false) if err != nil { t.Fatal(err) - return } - defer fs.Cleanup() - // TODO: implement + regular := filepath.Join(root, "regular") + if err := os.WriteFile(regular, []byte("regular"), 0o644); err != nil { + t.Fatal(err) + } + regularTime := time.Unix(1_700_100_000, 0) + if err := fs.Chtimes("regular", regularTime, regularTime); err != nil { + t.Fatal(err) + } + regularStat, err := os.Lstat(regular) + if err != nil { + t.Fatal(err) + } + if !regularStat.ModTime().Equal(regularTime) { + t.Fatalf("expected regular file mtime to be %s, got %s", regularTime, regularStat.ModTime()) + } + + target := filepath.Join(tmpDir, "target") + if err := os.WriteFile(target, []byte("target"), 0o644); err != nil { + t.Fatal(err) + } + original := time.Unix(1_700_000_000, 0) + if err := os.Chtimes(target, original, original); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(root, "link")); err != nil { + t.Fatal(err) + } + + changed := original.Add(-24 * time.Hour) + if err := fs.Chtimes("link", changed, changed); err != nil { + t.Fatal(err) + } + + st, err := os.Lstat(target) + if err != nil { + t.Fatal(err) + } + if !st.ModTime().Equal(original) { + t.Fatalf("expected target mtime to remain %s, got %s", original, st.ModTime()) + } } func TestUnixFS_Create(t *testing.T) { diff --git a/router/router_download.go b/router/router_download.go index 108d5ee6..5445f553 100644 --- a/router/router_download.go +++ b/router/router_download.go @@ -28,7 +28,7 @@ func getDownloadBackup(c *gin.Context) { } // Get the server using the UUID from the token. - if _, ok := manager.Get(token.ServerUuid); !ok || !token.IsUniqueRequest() || !token.HasScope(tokens.BackupDownload) { + if _, ok := manager.Get(token.ServerUuid); !ok || token.Denylisted() || !token.IsUniqueRequest() || !token.HasScope(tokens.BackupDownload) { c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ "error": "The requested resource was not found on this server.", }) @@ -43,7 +43,7 @@ func getDownloadBackup(c *gin.Context) { } // Locate the backup on the local disk. - b, st, err := backup.LocateLocal(client, token.BackupUuid, token.ServerUuid) + b, st, err := backup.LocateLocal(client, token.BackupUuid) if err != nil { if errors.Is(err, os.ErrNotExist) { c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ @@ -82,7 +82,7 @@ func getDownloadFile(c *gin.Context) { } s, ok := manager.Get(token.ServerUuid) - if !ok || !token.IsUniqueRequest() || !token.HasScope(tokens.FileDownload) { + if !ok || token.Denylisted() || !token.IsUniqueRequest() || !token.HasScope(tokens.FileDownload) { c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ "error": "The requested resource was not found on this server.", }) diff --git a/router/router_server_backup.go b/router/router_server_backup.go index e64c56d4..eb607888 100644 --- a/router/router_server_backup.go +++ b/router/router_server_backup.go @@ -1,19 +1,39 @@ package router import ( + "context" + stderrors "errors" + "mime" + "net" "net/http" + "net/netip" + "net/url" "os" "strings" + "time" "emperror.dev/errors" "github.com/apex/log" "github.com/gin-gonic/gin" + "github.com/google/uuid" + "github.com/pelican-dev/wings/config" "github.com/pelican-dev/wings/router/middleware" "github.com/pelican-dev/wings/server" "github.com/pelican-dev/wings/server/backup" ) +var blockedBackupRestorePrefixes = []netip.Prefix{ + netip.MustParsePrefix("100.64.0.0/10"), + netip.MustParsePrefix("198.18.0.0/15"), +} + +type backupDownloadError string + +func (e backupDownloadError) Error() string { + return string(e) +} + // postServerBackup performs a backup against a given server instance using the // provided backup adapter. func postServerBackup(c *gin.Context) { @@ -29,12 +49,16 @@ func postServerBackup(c *gin.Context) { return } + backupUuid, ok := parseBackupUuid(c, data.Uuid) + if !ok { + return + } var adapter backup.BackupInterface switch data.Adapter { case backup.LocalBackupAdapter: - adapter = backup.NewLocal(client, data.Uuid, s.ID(), data.Ignore) + adapter = backup.NewLocal(client, backupUuid, data.Ignore) case backup.S3BackupAdapter: - adapter = backup.NewS3(client, data.Uuid, s.ID(), data.Ignore) + adapter = backup.NewS3(client, backupUuid, data.Ignore) default: middleware.CaptureAndAbort(c, errors.New("router/backups: provided adapter is not valid: "+string(data.Adapter))) return @@ -80,11 +104,20 @@ func postServerRestoreBackup(c *gin.Context) { if err := c.BindJSON(&data); err != nil { return } + backupUuid, ok := parseBackupUuid(c, c.Param("backup")) + if !ok { + return + } if data.Adapter == backup.S3BackupAdapter && data.DownloadUrl == "" { c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": "The download_url field is required when the backup adapter is set to S3."}) return } - + if data.Adapter == backup.S3BackupAdapter { + if err := validateBackupDownloadUrl(data.DownloadUrl); err != nil { + c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + } s.SetRestoring(true) hasError := true defer func() { @@ -104,10 +137,11 @@ func postServerRestoreBackup(c *gin.Context) { } } + // Now that we've cleaned up the data directory if necessary, grab the backup file // and attempt to restore it into the server directory. if data.Adapter == backup.LocalBackupAdapter { - b, _, err := backup.LocateLocal(client, c.Param("backup"), s.ID()) + b, _, err := backup.LocateLocal(client, backupUuid) if err != nil { middleware.CaptureAndAbort(c, err) return @@ -129,7 +163,7 @@ func postServerRestoreBackup(c *gin.Context) { // Since this is not a local backup we need to stream the archive and then // parse over the contents as we go in order to restore it to the server. - httpClient := http.Client{} + httpClient := backupRestoreHttpClient() logger.Info("downloading backup from remote location...") // TODO: this will hang if there is an issue. We can't use c.Request.Context() (or really any) // since it will be canceled when the request is closed which happens quickly since we push @@ -144,11 +178,21 @@ func postServerRestoreBackup(c *gin.Context) { } res, err := httpClient.Do(req) if err != nil { + var downloadErr backupDownloadError + if stderrors.As(err, &downloadErr) { + c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": downloadErr.Error()}) + return + } middleware.CaptureAndAbort(c, err) return } + if res.StatusCode != http.StatusOK { + _ = res.Body.Close() + c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": "The provided backup link returned an invalid response status: " + res.Status}) + return + } // Don't allow content types that we know are going to give us problems. - if res.Header.Get("Content-Type") == "" || !strings.Contains("application/x-gzip application/gzip", res.Header.Get("Content-Type")) { + if !isSupportedBackupRestoreContentType(res.Header.Get("Content-Type")) { _ = res.Body.Close() c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{ "error": "The provided backup link is not a supported content type. \"" + res.Header.Get("Content-Type") + "\" is not application/x-gzip.", @@ -158,14 +202,14 @@ func postServerRestoreBackup(c *gin.Context) { go func(s *server.Server, uuid string, logger *log.Entry) { logger.Info("starting restoration process for server backup using S3 driver") - if err := s.RestoreBackup(backup.NewS3(client, uuid, s.ID(), ""), res.Body); err != nil { + if err := s.RestoreBackup(backup.NewS3(client, uuid, ""), res.Body); err != nil { logger.WithField("error", errors.WithStack(err)).Error("failed to restore remote S3 backup to server") } s.Events().Publish(server.DaemonMessageEvent, "Completed server restoration from S3 backup.") s.Events().Publish(server.BackupRestoreCompletedEvent, "") logger.Info("completed server restoration from S3 backup") s.SetRestoring(false) - }(s, c.Param("backup"), logger) + }(s, backupUuid, logger) hasError = false c.Status(http.StatusAccepted) @@ -176,7 +220,11 @@ func postServerRestoreBackup(c *gin.Context) { // endpoint can make its own decisions as to how it wants to handle that // response. func deleteServerBackup(c *gin.Context) { - b, _, err := backup.LocateLocal(middleware.ExtractApiClient(c), c.Param("backup"), middleware.ExtractServer(c).ID()) + backupUuid, ok := parseBackupUuid(c, c.Param("backup")) + if !ok { + return + } + b, _, err := backup.LocateLocal(middleware.ExtractApiClient(c), backupUuid) if err != nil { // Just return from the function at this point if the backup was not located. if errors.Is(err, os.ErrNotExist) { @@ -197,3 +245,126 @@ func deleteServerBackup(c *gin.Context) { } c.Status(http.StatusNoContent) } + +func parseBackupUuid(c *gin.Context, value string) (string, bool) { + parsed, err := uuid.Parse(value) + if err == nil && len(value) == len(parsed.String()) && parsed.String() == strings.ToLower(value) { + return parsed.String(), true + } + c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{"error": "The backup identifier must be a valid UUID."}) + return "", false +} + +func validateBackupDownloadUrl(raw string) error { + parsed, err := url.Parse(raw) + if err != nil || parsed.Host == "" { + return backupDownloadError("The provided backup link is not a valid URL.") + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return backupDownloadError("The provided backup link must use HTTP or HTTPS.") + } + if ip := net.ParseIP(parsed.Hostname()); ip != nil && isBlockedBackupRestoreIP(parsed.Hostname(), ip) { + return backupDownloadError("The provided backup link resolves to a blocked address.") + } + return nil +} + +func backupRestoreHttpClient() http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = nil + transport.ResponseHeaderTimeout = 30 * time.Second + dialer := &net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + } + transport.DialContext = func(ctx context.Context, network string, address string) (net.Conn, error) { + host, port, err := net.SplitHostPort(address) + if err != nil { + return nil, err + } + ips, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if err != nil { + return nil, err + } + if len(ips) == 0 { + return nil, errors.New("router/backups: backup download host did not resolve to any addresses") + } + for _, resolved := range ips { + if isBlockedBackupRestoreIP(host, resolved.IP) { + return nil, backupDownloadError("The provided backup link resolves to a blocked address.") + } + } + var lastErr error + for _, resolved := range ips { + conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(resolved.IP.String(), port)) + if err == nil { + return conn, nil + } + lastErr = err + } + return nil, lastErr + } + return http.Client{ + Transport: transport, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return backupDownloadError("The provided backup link redirects too many times.") + } + return validateBackupDownloadUrl(req.URL.String()) + }, + } +} + +func isBlockedBackupRestoreIP(host string, ip net.IP) bool { + addr, ok := netip.AddrFromSlice(ip) + if !ok { + return true + } + addr = addr.Unmap() + if !addr.IsGlobalUnicast() || addr.IsPrivate() || addr.IsLoopback() || addr.IsLinkLocalUnicast() || isExplicitlyBlockedBackupRestoreIP(addr) { + return !isAllowedBackupRestoreDestination(host, addr) + } + return false +} + +func isExplicitlyBlockedBackupRestoreIP(addr netip.Addr) bool { + for _, prefix := range blockedBackupRestorePrefixes { + if prefix.Contains(addr) { + return true + } + } + return false +} + +func isAllowedBackupRestoreDestination(host string, addr netip.Addr) bool { + host = strings.TrimSuffix(strings.ToLower(host), ".") + for _, entry := range config.Get().System.Backups.RestoreHostAllowlist { + entry = strings.TrimSuffix(strings.ToLower(strings.TrimSpace(entry)), ".") + if entry == "" { + continue + } + if entry == host { + return true + } + if allowedAddr, err := netip.ParseAddr(entry); err == nil && allowedAddr.Unmap() == addr { + return true + } + if prefix, err := netip.ParsePrefix(entry); err == nil && prefix.Contains(addr) { + return true + } + } + return false +} + +func isSupportedBackupRestoreContentType(value string) bool { + mediaType, _, err := mime.ParseMediaType(value) + if err != nil { + mediaType = strings.TrimSpace(value) + } + switch strings.ToLower(mediaType) { + case "application/x-gzip", "application/gzip": + return true + default: + return false + } +} \ No newline at end of file diff --git a/router/router_server_backup_test.go b/router/router_server_backup_test.go new file mode 100644 index 00000000..2e56e95f --- /dev/null +++ b/router/router_server_backup_test.go @@ -0,0 +1,342 @@ +package router + +import ( + "context" + "fmt" + "net" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/apex/log" + "github.com/gin-gonic/gin" + + "github.com/pelican-dev/wings/config" + "github.com/pelican-dev/wings/environment" + "github.com/pelican-dev/wings/events" + "github.com/pelican-dev/wings/internal/models" + "github.com/pelican-dev/wings/remote" + wserver "github.com/pelican-dev/wings/server" +) + +func init() { + config.Set(&config.Configuration{AuthenticationToken: "test-token"}) +} + +type backupTestRemoteClient struct { + restoreStatus chan string +} + +func (c backupTestRemoteClient) GetBackupRemoteUploadURLs(context.Context, string, int64) (remote.BackupRemoteUploadResponse, error) { + return remote.BackupRemoteUploadResponse{}, nil +} + +func (c backupTestRemoteClient) GetInstallationScript(context.Context, string) (remote.InstallationScript, error) { + return remote.InstallationScript{}, nil +} + +func (c backupTestRemoteClient) GetServerConfiguration(context.Context, string) (remote.ServerConfigurationResponse, error) { + return remote.ServerConfigurationResponse{}, nil +} + +func (c backupTestRemoteClient) GetServers(context.Context, int) ([]remote.RawServerData, error) { + return nil, nil +} + +func (c backupTestRemoteClient) ResetServersState(context.Context) error { + return nil +} + +func (c backupTestRemoteClient) SetArchiveStatus(context.Context, string, bool) error { + return nil +} + +func (c backupTestRemoteClient) SetBackupStatus(context.Context, string, remote.BackupRequest) error { + return nil +} + +func (c backupTestRemoteClient) SendRestorationStatus(_ context.Context, backup string, _ bool) error { + if c.restoreStatus != nil { + select { + case c.restoreStatus <- backup: + default: + } + } + return nil +} + +func (c backupTestRemoteClient) SetInstallationStatus(context.Context, string, remote.InstallStatusRequest) error { + return nil +} + +func (c backupTestRemoteClient) SetTransferStatus(context.Context, string, bool) error { + return nil +} + +func (c backupTestRemoteClient) ValidateSftpCredentials(context.Context, remote.SftpAuthRequest) (remote.SftpAuthResponse, error) { + return remote.SftpAuthResponse{}, nil +} + +func (c backupTestRemoteClient) SendActivityLogs(context.Context, []models.Activity) error { + return nil +} + + +func (c backupTestRemoteClient) PushServerStateChange(context.Context, string, remote.ServerStateChange) error { + return nil +} + +type backupTestEnvironment struct{} + +func (backupTestEnvironment) Type() string { return "test" } + +func (backupTestEnvironment) Config() *environment.Configuration { + return &environment.Configuration{} +} + +func (backupTestEnvironment) Events() *events.Bus { return events.NewBus() } + +func (backupTestEnvironment) Exists() (bool, error) { return true, nil } + +func (backupTestEnvironment) IsRunning(context.Context) (bool, error) { return false, nil } + +func (backupTestEnvironment) InSituUpdate() error { return nil } + +func (backupTestEnvironment) OnBeforeStart(context.Context) error { return nil } + +func (backupTestEnvironment) Start(context.Context) error { return nil } + +func (backupTestEnvironment) Stop(context.Context) error { return nil } + +func (backupTestEnvironment) WaitForStop(context.Context, time.Duration, bool) error { + return nil +} + +func (backupTestEnvironment) Terminate(context.Context, string) error { return nil } + +func (backupTestEnvironment) Destroy() error { return nil } + +func (backupTestEnvironment) ExitState() (uint32, bool, error) { return 0, false, nil } + +func (backupTestEnvironment) Create() error { return nil } + +func (backupTestEnvironment) Attach(context.Context) error { return nil } + +func (backupTestEnvironment) SendCommand(string) error { return nil } + +func (backupTestEnvironment) Readlog(int) ([]string, error) { return nil, nil } + +func (backupTestEnvironment) State() string { return environment.ProcessOfflineState } + +func (backupTestEnvironment) SetState(string) {} + +func (backupTestEnvironment) Uptime(context.Context) (int64, error) { return 0, nil } + +func (backupTestEnvironment) SetLogCallback(func([]byte)) {} + +func newBackupRestoreContext(t *testing.T, client backupTestRemoteClient, backupID string, body string) (*gin.Context, *httptest.ResponseRecorder, *wserver.Server) { + t.Helper() + + gin.SetMode(gin.TestMode) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest(http.MethodPost, "/api/servers/server/backup/"+backupID+"/restore", strings.NewReader(body)) + c.Request.Header.Set("Content-Type", "application/json") + c.Params = gin.Params{ + {Key: "server", Value: "server"}, + {Key: "backup", Value: backupID}, + } + + s, err := wserver.New(client) + if err != nil { + t.Fatal(err) + } + s.Config().Uuid = "server" + s.Environment = backupTestEnvironment{} + + c.Set("server", s) + c.Set("api_client", client) + c.Set("logger", log.WithField("test", t.Name())) + + return c, w, s +} + +func TestPostServerRestoreBackupRejectsLoopbackDownloadURL(t *testing.T) { + hit := make(chan struct{}, 1) + internal := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hit <- struct{}{} + w.Header().Set("Content-Type", "application") + _, _ = w.Write([]byte("not a gzip archive")) + })) + defer internal.Close() + downloadURL := strings.Replace(internal.URL, "127.0.0.1", "localhost", 1) + if downloadURL == internal.URL { + t.Fatalf("expected test server URL to use 127.0.0.1, got %s", internal.URL) + } + + client := backupTestRemoteClient{restoreStatus: make(chan string, 1)} + backupID := "11111111-1111-1111-1111-111111111111" + c, w, s := newBackupRestoreContext(t, client, backupID, fmt.Sprintf(`{"adapter":"s3","download_url":%q}`, downloadURL)) + defer s.CtxCancel() + + postServerRestoreBackup(c) + + if c.Writer.Status() != http.StatusBadRequest { + t.Fatalf("expected restore request to be rejected, got status %d body %s", c.Writer.Status(), w.Body.String()) + } + + select { + case <-hit: + t.Fatal("expected loopback server not to receive restore download request") + case <-time.After(100 * time.Millisecond): + } +} + +func TestPostServerRestoreBackupRejectsNonUuidBackupID(t *testing.T) { + client := backupTestRemoteClient{restoreStatus: make(chan string, 1)} + c, w, s := newBackupRestoreContext(t, client, "../target/archive", `{"adapter":"s3","download_url":"https://example.com/archive.tar.gz"}`) + defer s.CtxCancel() + + postServerRestoreBackup(c) + + if c.Writer.Status() != http.StatusBadRequest { + t.Fatalf("expected non-UUID backup id to be rejected, got status %d body %s", c.Writer.Status(), w.Body.String()) + } +} + +func TestPostServerRestoreBackupRejectsBadDownloadStatus(t *testing.T) { + setBackupRestoreAllowlist(t, []string{"127.0.0.1"}) + + remote := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/x-gzip") + http.Error(w, "missing", http.StatusNotFound) + })) + defer remote.Close() + + client := backupTestRemoteClient{restoreStatus: make(chan string, 1)} + backupID := "11111111-1111-1111-1111-111111111111" + c, w, s := newBackupRestoreContext(t, client, backupID, fmt.Sprintf(`{"adapter":"s3","download_url":%q}`, remote.URL)) + defer s.CtxCancel() + + postServerRestoreBackup(c) + + if c.Writer.Status() != http.StatusBadRequest { + t.Fatalf("expected restore request to be rejected, got status %d body %s", c.Writer.Status(), w.Body.String()) + } +} + +func TestBackupRestoreContentTypeValidation(t *testing.T) { + tests := map[string]bool{ + "application/gzip": true, + "application/gzip; charset=binary": true, + "application/x-gzip": true, + "application/x-gzip; charset=binary": true, + "application": false, + "gzip": false, + "text/plain": false, + "": false, + } + + for contentType, expected := range tests { + if got := isSupportedBackupRestoreContentType(contentType); got != expected { + t.Fatalf("expected content type %q support to be %v, got %v", contentType, expected, got) + } + } +} + +func TestParseBackupUuid(t *testing.T) { + tests := map[string]struct { + expected string + valid bool + }{ + "11111111-1111-1111-1111-111111111111": {expected: "11111111-1111-1111-1111-111111111111", valid: true}, + "11111111-1111-1111-1111-AAAAAAAAAAAA": {expected: "11111111-1111-1111-1111-aaaaaaaaaaaa", valid: true}, + "11111111111111111111111111111111": {valid: false}, + "../target/archive": {valid: false}, + } + + for value, test := range tests { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + got, ok := parseBackupUuid(c, value) + if ok != test.valid { + t.Fatalf("expected validity for %q to be %v, got %v", value, test.valid, ok) + } + if got != test.expected { + t.Fatalf("expected normalized backup UUID %q, got %q", test.expected, got) + } + } +} + +func TestBackupRestoreBlockedIPValidation(t *testing.T) { + setBackupRestoreAllowlist(t, nil) + + tests := map[string]bool{ + "127.0.0.1": true, + "10.0.0.1": true, + "169.254.1.1": true, + "100.64.0.1": true, + "198.18.0.1": true, + "::1": true, + "fe80::1": true, + "8.8.8.8": false, + "2606:4700::11": false, + } + + for raw, expected := range tests { + if got := isBlockedBackupRestoreIP("", net.ParseIP(raw)); got != expected { + t.Fatalf("expected blocked state for %q to be %v, got %v", raw, expected, got) + } + } +} + +func TestBackupRestoreDestinationAllowlist(t *testing.T) { + setBackupRestoreAllowlist(t, []string{ + "minio.internal", + "10.0.0.10", + "192.168.50.0/24", + }) + + tests := []struct { + name string + host string + ip string + blocked bool + }{ + {name: "hostname", host: "minio.internal", ip: "10.0.0.20", blocked: false}, + {name: "ip", host: "10.0.0.10", ip: "10.0.0.10", blocked: false}, + {name: "cidr", host: "192.168.50.10", ip: "192.168.50.10", blocked: false}, + {name: "not listed", host: "10.0.0.11", ip: "10.0.0.11", blocked: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := isBlockedBackupRestoreIP(test.host, net.ParseIP(test.ip)); got != test.blocked { + t.Fatalf("expected blocked state for %q/%q to be %v, got %v", test.host, test.ip, test.blocked, got) + } + }) + } +} + +func TestBackupRestoreHTTPClientDoesNotLimitResponseBodyRead(t *testing.T) { + client := backupRestoreHttpClient() + if client.Timeout != 0 { + t.Fatalf("expected restore client not to set total request timeout, got %s", client.Timeout) + } +} + +func setBackupRestoreAllowlist(t *testing.T, entries []string) { + t.Helper() + + previous := config.Get() + t.Cleanup(func() { + config.Set(previous) + }) + + next := *previous + next.System.Backups.RestoreHostAllowlist = entries + config.Set(&next) +} \ No newline at end of file diff --git a/router/router_server_files.go b/router/router_server_files.go index 8e6443d3..a428741d 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -599,7 +599,7 @@ func postServerUploadFiles(c *gin.Context) { } s, ok := manager.Get(token.ServerUuid) - if !ok || !token.IsUniqueRequest() || !token.HasScope(tokens.FileUpload) { + if !ok || token.Denylisted() || !token.IsUniqueRequest() || !token.HasScope(tokens.FileUpload) { c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ "error": "The requested resource was not found on this server.", }) diff --git a/router/tokens/backup.go b/router/tokens/backup.go index 90022231..da3abb93 100644 --- a/router/tokens/backup.go +++ b/router/tokens/backup.go @@ -8,6 +8,7 @@ type BackupPayload struct { jwt.Payload ServerUuid string `json:"server_uuid"` + UserUuid string `json:"user_uuid"` BackupUuid string `json:"backup_uuid"` UniqueId string `json:"unique_id"` Scoped @@ -25,3 +26,9 @@ func (p *BackupPayload) GetPayload() *jwt.Payload { func (p *BackupPayload) IsUniqueRequest() bool { return getTokenStore().IsValidToken(p.UniqueId) } + +// Denylisted returns true if this token was issued before the user's access to +// the server was revoked. +func (p *BackupPayload) Denylisted() bool { + return isDenylisted(&p.Payload, p.ServerUuid, p.UserUuid) +} \ No newline at end of file diff --git a/router/tokens/denylist_test.go b/router/tokens/denylist_test.go new file mode 100644 index 00000000..53c073ab --- /dev/null +++ b/router/tokens/denylist_test.go @@ -0,0 +1,173 @@ +package tokens + +import ( + "encoding/json" + "testing" + "time" + + "github.com/gbrlsnchs/jwt/v3" +) + +type denylistable interface { + Denylisted() bool +} + +func resetDenylistState(t *testing.T, bootTime time.Time) { + t.Helper() + + originalBootTime := wingsBootTime + wingsBootTime = bootTime + denylist.Clear() + userDenylist.Clear() + + t.Cleanup(func() { + wingsBootTime = originalBootTime + denylist.Clear() + userDenylist.Clear() + }) +} + +func payloadsIssuedAt(issuedAt time.Time, serverUUID, userUUID string) map[string]denylistable { + payload := func() jwt.Payload { + return jwt.Payload{ + IssuedAt: jwt.NumericDate(issuedAt), + JWTID: "token-id", + } + } + + return map[string]denylistable{ + "websocket": &WebsocketPayload{ + Payload: payload(), + ServerUUID: serverUUID, + UserUUID: userUUID, + }, + "file download": &FilePayload{ + Payload: payload(), + ServerUuid: serverUUID, + UserUuid: userUUID, + }, + "backup download": &BackupPayload{ + Payload: payload(), + ServerUuid: serverUUID, + UserUuid: userUUID, + }, + "file upload": &UploadPayload{ + Payload: payload(), + ServerUuid: serverUUID, + UserUuid: userUUID, + }, + } +} + +func TestUserBoundPayloadsHonorServerRevocation(t *testing.T) { + issuedAt := time.Now().Add(-time.Minute) + resetDenylistState(t, issuedAt.Add(-time.Minute)) + + payloads := payloadsIssuedAt(issuedAt, "server", "user") + for name, payload := range payloads { + if payload.Denylisted() { + t.Fatalf("expected fresh %s token to be accepted", name) + } + } + + DenyForServer("server", "user") + + for name, payload := range payloads { + if !payload.Denylisted() { + t.Fatalf("expected revoked %s token to be denied", name) + } + } +} + +func TestUserBoundPayloadsAllowTokensIssuedAfterRevocation(t *testing.T) { + revokedAt := time.Now() + resetDenylistState(t, revokedAt.Add(-time.Hour)) + userDenylist.Store("server:user", revokedAt) + + for name, payload := range payloadsIssuedAt(revokedAt, "server", "user") { + if !payload.Denylisted() { + t.Fatalf("expected %s token issued at revocation time to be denied", name) + } + } + + for name, payload := range payloadsIssuedAt(revokedAt.Add(time.Second), "server", "user") { + if payload.Denylisted() { + t.Fatalf("expected reissued %s token to be accepted", name) + } + } +} + +func TestDownloadPayloadsDecodeUserUUID(t *testing.T) { + const encoded = `{"server_uuid":"server","user_uuid":"user"}` + + var file FilePayload + if err := json.Unmarshal([]byte(encoded), &file); err != nil { + t.Fatal(err) + } + if file.UserUuid != "user" { + t.Fatalf("expected file payload user UUID to be decoded, got %q", file.UserUuid) + } + + var backup BackupPayload + if err := json.Unmarshal([]byte(encoded), &backup); err != nil { + t.Fatal(err) + } + if backup.UserUuid != "user" { + t.Fatalf("expected backup payload user UUID to be decoded, got %q", backup.UserUuid) + } +} + +func TestUserBoundPayloadsFailClosedWithoutRevocationClaims(t *testing.T) { + issuedAt := time.Now() + resetDenylistState(t, issuedAt.Add(-time.Hour)) + + tests := map[string]denylistable{ + "missing issued at": &FilePayload{ + ServerUuid: "server", + UserUuid: "user", + }, + "missing server UUID": &BackupPayload{ + Payload: jwt.Payload{IssuedAt: jwt.NumericDate(issuedAt)}, + UserUuid: "user", + }, + "missing user UUID": &UploadPayload{ + Payload: jwt.Payload{IssuedAt: jwt.NumericDate(issuedAt)}, + ServerUuid: "server", + }, + } + + for name, payload := range tests { + if !payload.Denylisted() { + t.Fatalf("expected token with %s to be denied", name) + } + } +} + +func TestUserBoundPayloadsRejectTokensIssuedBeforeBoot(t *testing.T) { + bootTime := time.Now() + resetDenylistState(t, bootTime) + + for name, payload := range payloadsIssuedAt(bootTime.Add(-time.Second), "server", "user") { + if !payload.Denylisted() { + t.Fatalf("expected pre-boot %s token to be denied", name) + } + } +} + +func TestUserRevocationIsScopedToServerAndUser(t *testing.T) { + issuedAt := time.Now().Add(-time.Minute) + resetDenylistState(t, issuedAt.Add(-time.Minute)) + DenyForServer("server", "user") + + for name, payload := range payloadsIssuedAt(issuedAt, "other-server", "user") { + if payload.Denylisted() { + t.Fatalf("expected %s token for another server to be accepted", name) + } + } + + for name, payload := range payloadsIssuedAt(issuedAt, "server", "other-user") { + if payload.Denylisted() { + t.Fatalf("expected %s token for another user to be accepted", name) + } + } +} \ No newline at end of file diff --git a/router/tokens/file.go b/router/tokens/file.go index b4b27955..4805a252 100644 --- a/router/tokens/file.go +++ b/router/tokens/file.go @@ -8,6 +8,7 @@ type FilePayload struct { jwt.Payload FilePath string `json:"file_path"` ServerUuid string `json:"server_uuid"` + UserUuid string `json:"user_uuid"` UniqueId string `json:"unique_id"` Scoped } @@ -24,3 +25,10 @@ func (p *FilePayload) GetPayload() *jwt.Payload { func (p *FilePayload) IsUniqueRequest() bool { return getTokenStore().IsValidToken(p.UniqueId) } + + +// Denylisted returns true if this token was issued before the user's access to +// the server was revoked. +func (p *FilePayload) Denylisted() bool { + return isDenylisted(&p.Payload, p.ServerUuid, p.UserUuid) +} \ No newline at end of file diff --git a/router/tokens/upload.go b/router/tokens/upload.go index 18fd963d..adceb41d 100644 --- a/router/tokens/upload.go +++ b/router/tokens/upload.go @@ -25,3 +25,9 @@ func (p *UploadPayload) GetPayload() *jwt.Payload { func (p *UploadPayload) IsUniqueRequest() bool { return getTokenStore().IsValidToken(p.UniqueId) } + +// Denylisted returns true if this token was issued before the user's access to +// the server was revoked. +func (p *UploadPayload) Denylisted() bool { + return isDenylisted(&p.Payload, p.ServerUuid, p.UserUuid) +} diff --git a/router/tokens/websocket.go b/router/tokens/websocket.go index 8ad3fb55..07ad8f4e 100644 --- a/router/tokens/websocket.go +++ b/router/tokens/websocket.go @@ -47,6 +47,36 @@ func DenyForServer(s string, u string) { userDenylist.Store(strings.Join([]string{s, u}, ":"), time.Now()) } +// Checks if a user-bound JWT has been denied because it was issued before Wings +// booted, or before the Panel revoked the user's access to a server. +func isDenylisted(payload *jwt.Payload, serverUUID, userUUID string) bool { + // A token without all of the claims needed to check revocation cannot be + // safely accepted. + if payload.IssuedAt == nil || serverUUID == "" || userUUID == "" { + return true + } + + if payload.IssuedAt.Time.Before(wingsBootTime) { + return true + } + + // This list is deprecated, but we maintain the check here so that custom + // instances are able to continue working. We'll remove it in a future release. + if t, ok := denylist.Load(payload.JWTID); ok { + if !payload.IssuedAt.Time.After(t.(time.Time)) { + return true + } + } + + if t, ok := userDenylist.Load(strings.Join([]string{serverUUID, userUUID}, ":")); ok { + if !payload.IssuedAt.Time.After(t.(time.Time)) { + return true + } + } + + return false +} + // WebsocketPayload defines the JWT payload for a websocket connection. This JWT is passed along to // the websocket after it has been connected to by sending an "auth" event. type WebsocketPayload struct { @@ -79,36 +109,7 @@ func (p *WebsocketPayload) GetServerUuid() string { // before Wings was booted, or because we have denied all tokens with the same JTI // occurring before a set time. func (p *WebsocketPayload) Denylisted() bool { - // If there is no IssuedAt present for the token, we cannot validate the token so - // just immediately mark it as not valid. - if p.IssuedAt == nil { - return true - } - - // If the time that the token was issued is before the time at which Wings was booted - // then the token is invalid for our purposes, even if the token "has permission". - if p.IssuedAt.Time.Before(wingsBootTime) { - return true - } - - // Finally, if the token was issued before a time that is currently denied for this - // token instance, ignore the permissions response. - // - // This list is deprecated, but we maintain the check here so that custom instances - // are able to continue working. We'll remove it in a future release. - if t, ok := denylist.Load(p.JWTID); ok { - if p.IssuedAt.Time.Before(t.(time.Time)) { - return true - } - } - - if t, ok := userDenylist.Load(strings.Join([]string{p.ServerUUID, p.UserUUID}, ":")); ok { - if p.IssuedAt.Time.Before(t.(time.Time)) { - return true - } - } - - return false + return isDenylisted(&p.Payload, p.ServerUUID, p.UserUUID) } // Checks if the given token payload has a permission string. diff --git a/server/backup/backup.go b/server/backup/backup.go index fbd64dd1..e7a28d4a 100644 --- a/server/backup/backup.go +++ b/server/backup/backup.go @@ -8,9 +8,11 @@ import ( "io/fs" "os" "path" + "strings" "emperror.dev/errors" "github.com/apex/log" + "github.com/google/uuid" "github.com/mholt/archives" "golang.org/x/sync/errgroup" @@ -96,15 +98,39 @@ func (b *Backup) Identifier() string { return b.Uuid } +func (b *Backup) normalizedIdentifier() (string, error) { + parsed, err := uuid.Parse(b.Identifier()) + if err != nil || len(b.Identifier()) != len(parsed.String()) || parsed.String() != strings.ToLower(b.Identifier()) { + return "", errors.New("backup: identifier must be a valid UUID") + } + return parsed.String(), nil +} + +func (b *Backup) validateIdentifier() error { + identifier, err := b.normalizedIdentifier() + if err != nil { + return err + } + b.Uuid = identifier + return nil +} + func (b *Backup) ServerId() string { return b.ServerUuid } // Path returns the path for this specific backup. func (b *Backup) Path() string { - return path.Join(config.Get().System.BackupDirectory, b.ServerId(), b.Identifier()+".tar.gz") + identifier, err := b.normalizedIdentifier() + if err != nil { + identifier = path.Base(b.Identifier()) + } + return path.Join(config.Get().System.BackupDirectory, identifier+".tar.gz") } // Size returns the size of the generated backup. func (b *Backup) Size() (int64, error) { + if err := b.validateIdentifier(); err != nil { + return 0, err + } st, err := os.Stat(b.Path()) if err != nil { return 0, err @@ -115,6 +141,9 @@ func (b *Backup) Size() (int64, error) { // Checksum returns the SHA256 checksum of a backup. func (b *Backup) Checksum() ([]byte, error) { + if err := b.validateIdentifier(); err != nil { + return nil, err + } h := sha1.New() f, err := os.Open(b.Path()) diff --git a/server/backup/backup_local.go b/server/backup/backup_local.go index 9513a7ed..977671a7 100644 --- a/server/backup/backup_local.go +++ b/server/backup/backup_local.go @@ -21,22 +21,24 @@ type LocalBackup struct { var _ BackupInterface = (*LocalBackup)(nil) -func NewLocal(client remote.Client, uuid string, suuid string, ignore string) *LocalBackup { +func NewLocal(client remote.Client, uuid string, ignore string) *LocalBackup { return &LocalBackup{ Backup{ - client: client, - Uuid: uuid, - ServerUuid: suuid, - Ignore: ignore, - adapter: LocalBackupAdapter, + client: client, + Uuid: uuid, + Ignore: ignore, + adapter: LocalBackupAdapter, }, } } // LocateLocal finds the backup for a server and returns the local path. This // will obviously only work if the backup was created as a local backup. -func LocateLocal(client remote.Client, uuid string, suuid string) (*LocalBackup, os.FileInfo, error) { - b := NewLocal(client, uuid, suuid, "") +func LocateLocal(client remote.Client, uuid string) (*LocalBackup, os.FileInfo, error) { + b := NewLocal(client, uuid, "") + if err := b.validateIdentifier(); err != nil { + return nil, nil, err + } st, err := os.Stat(b.Path()) if err != nil { return nil, nil, err @@ -51,6 +53,9 @@ func LocateLocal(client remote.Client, uuid string, suuid string) (*LocalBackup, // Remove removes a backup from the system. func (b *LocalBackup) Remove() error { + if err := b.validateIdentifier(); err != nil { + return err + } err := os.Remove(b.Path()) if err != nil { return err @@ -76,6 +81,9 @@ func (b *LocalBackup) WithLogContext(c map[string]interface{}) { // Generate generates a backup of the selected files and pushes it to the // defined location for this instance. func (b *LocalBackup) Generate(ctx context.Context, fsys *filesystem.Filesystem, ignore string) (*ArchiveDetails, error) { + if err := b.validateIdentifier(); err != nil { + return nil, err + } a := &filesystem.Archive{ Filesystem: fsys, Ignore: ignore, @@ -103,6 +111,9 @@ func (b *LocalBackup) Generate(ctx context.Context, fsys *filesystem.Filesystem, // Restore will walk over the archive and call the callback function for each // file encountered. func (b *LocalBackup) Restore(ctx context.Context, _ io.Reader, callback RestoreCallback) error { + if err := b.validateIdentifier(); err != nil { + return err + } f, err := os.Open(b.Path()) if err != nil { return err diff --git a/server/backup/backup_s3.go b/server/backup/backup_s3.go index de4e8727..3f01cc59 100644 --- a/server/backup/backup_s3.go +++ b/server/backup/backup_s3.go @@ -26,20 +26,22 @@ type S3Backup struct { var _ BackupInterface = (*S3Backup)(nil) -func NewS3(client remote.Client, uuid string, suuid string, ignore string) *S3Backup { +func NewS3(client remote.Client, uuid string, ignore string) *S3Backup { return &S3Backup{ Backup{ - client: client, - Uuid: uuid, - ServerUuid: suuid, - Ignore: ignore, - adapter: S3BackupAdapter, + client: client, + Uuid: uuid, + Ignore: ignore, + adapter: S3BackupAdapter, }, } } // Remove removes a backup from the system. func (s *S3Backup) Remove() error { + if err := s.validateIdentifier(); err != nil { + return err + } return os.Remove(s.Path()) } @@ -51,6 +53,9 @@ func (s *S3Backup) WithLogContext(c map[string]interface{}) { // Generate creates a new backup on the disk, moves it into the S3 bucket via // the provided presigned URL, and then deletes the backup from the disk. func (s *S3Backup) Generate(ctx context.Context, fsys *filesystem.Filesystem, ignore string) (*ArchiveDetails, error) { + if err := s.validateIdentifier(); err != nil { + return nil, err + } defer s.Remove() a := &filesystem.Archive{ @@ -64,7 +69,7 @@ func (s *S3Backup) Generate(ctx context.Context, fsys *filesystem.Filesystem, ig if err != nil { return nil, err } - } + } if err := a.Create(ctx, s.Path()); err != nil { return nil, err } diff --git a/server/backup/backup_test.go b/server/backup/backup_test.go new file mode 100644 index 00000000..091a66e7 --- /dev/null +++ b/server/backup/backup_test.go @@ -0,0 +1,103 @@ +package backup + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/pelican-dev/wings/config" + "github.com/pelican-dev/wings/server/filesystem" +) + +func TestBackupGenerateRequiresUuidIdentifier(t *testing.T) { + tests := map[string]func(string) BackupInterface{ + "local": func(identifier string) BackupInterface { + return NewLocal(nil, identifier, "") + }, + "s3": func(identifier string) BackupInterface { + return NewS3(nil, identifier, "") + }, + } + + for name, createBackup := range tests { + t.Run(name, func(t *testing.T) { + testBackupGenerateRequiresUuidIdentifier(t, createBackup) + }) + } +} + +func TestBackupPathUsesBackupDirectory(t *testing.T) { + backupDir := t.TempDir() + config.Set(&config.Configuration{ + AuthenticationToken: "test-token", + System: config.SystemConfiguration{ + BackupDirectory: backupDir, + }, + }) + + for _, identifier := range []string{ + "11111111-1111-1111-1111-111111111111", + "../target/archive", + "nested/archive", + } { + b := NewLocal(nil, identifier, "") + rel, err := filepath.Rel(backupDir, b.Path()) + if err != nil { + t.Fatal(err) + } + if filepath.IsAbs(rel) || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + t.Fatalf("expected backup path %q to remain under %q", b.Path(), backupDir) + } + } +} + +func testBackupGenerateRequiresUuidIdentifier(t *testing.T, createBackup func(string) BackupInterface) { + t.Helper() + + root := t.TempDir() + backupDir := filepath.Join(root, "backups") + targetDir := filepath.Join(root, "target") + serverDir := filepath.Join(root, "server") + for _, dir := range []string{backupDir, targetDir, serverDir} { + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + } + config.Set(&config.Configuration{ + AuthenticationToken: "test-token", + System: config.SystemConfiguration{ + BackupDirectory: backupDir, + }, + }) + + if err := os.WriteFile(filepath.Join(serverDir, "file.txt"), []byte("server data"), 0o600); err != nil { + t.Fatal(err) + } + fsys, err := filesystem.New(serverDir, 0, nil) + if err != nil { + t.Fatal(err) + } + + existingArchive := filepath.Join(targetDir, "archive.tar.gz") + existingArchiveContents := []byte("existing archive") + if err := os.WriteFile(existingArchive, existingArchiveContents, 0o600); err != nil { + t.Fatal(err) + } + + b := createBackup("../target/archive") + if _, err := b.Generate(context.Background(), fsys, ""); err == nil { + t.Fatal("expected invalid backup identifier to be rejected") + } + + got, err := os.ReadFile(existingArchive) + if err != nil { + t.Fatal(err) + } + if bytes.Equal(got, existingArchiveContents) { + return + } + t.Fatal("expected backup generation not to overwrite existing archive") +} \ No newline at end of file diff --git a/server/filesystem/compress.go b/server/filesystem/compress.go index fd559c06..e6975faa 100644 --- a/server/filesystem/compress.go +++ b/server/filesystem/compress.go @@ -5,6 +5,7 @@ import ( "fmt" "io" iofs "io/fs" + "math" "path" "path/filepath" "strings" @@ -152,7 +153,7 @@ func (fs *Filesystem) CompressFiles(dir string, name string, paths []string, ext } } - if !fs.unixFS.CanFit(cw.BytesWritten()) { + if cw.BytesWritten() < 0 || !fs.unixFS.CanFit(cw.BytesWritten()) { _ = fs.unixFS.Remove(destPath) return nil, "", newFilesystemError(ErrCodeDiskSpace, nil) } @@ -243,9 +244,19 @@ func (fs *Filesystem) SpaceAvailableForDecompression(ctx context.Context, dir st if err != nil { return err } - if !fs.unixFS.CanFit(size.Add(info.Size())) { + fileSize := info.Size() + if fileSize <= 0 { + return nil + } + current := size.Load() + if fileSize > math.MaxInt64-current { + return newFilesystemError(ErrCodeDiskSpace, nil) + } + next := current + fileSize + if !fs.unixFS.CanFit(next) { return newFilesystemError(ErrCodeDiskSpace, nil) } + size.Store(next) return nil } }) diff --git a/server/filesystem/disk_space.go b/server/filesystem/disk_space.go index 5280b53d..d3a5dca8 100644 --- a/server/filesystem/disk_space.go +++ b/server/filesystem/disk_space.go @@ -4,7 +4,7 @@ package filesystem import ( "golang.org/x/sys/unix" - "slices" + "slices" "sync" "sync/atomic" "time" @@ -167,9 +167,9 @@ func (fs *Filesystem) DirectorySize(root string) (int64, error) { if err != nil { return 0, err } - + var hardLinks []uint64 - + var size atomic.Int64 err = fs.unixFS.WalkDirat(dirfd, name, func(dirfd int, name, _ string, d ufs.DirEntry, err error) error { if err != nil { @@ -196,7 +196,7 @@ func (fs *Filesystem) DirectorySize(root string) (int64, error) { hardLinks = append(hardLinks, sysFileInfo.Ino) } } - + size.Add(info.Size()) return nil }) @@ -210,7 +210,33 @@ func (fs *Filesystem) HasSpaceFor(size int64) error { return nil } +func (fs *Filesystem) reserveDisk(size int64) error { + if size <= 0 { + return nil + } + + fs.mu.Lock() + defer fs.mu.Unlock() + + if err := fs.HasSpaceFor(size); err != nil { + return err + } + fs.unixFS.Add(size) + return nil +} + +func (fs *Filesystem) adjustDisk(size int64) int64 { + if size == 0 { + return fs.CachedUsage() + } + + fs.mu.Lock() + defer fs.mu.Unlock() + + return fs.unixFS.Add(size) +} + // Updates the disk usage for the Filesystem instance. func (fs *Filesystem) addDisk(i int64) int64 { return fs.unixFS.Add(i) -} +} \ No newline at end of file diff --git a/server/filesystem/filesystem.go b/server/filesystem/filesystem.go index a798db43..2ef8238d 100644 --- a/server/filesystem/filesystem.go +++ b/server/filesystem/filesystem.go @@ -96,7 +96,19 @@ func (fs *Filesystem) UnixFS() *ufs.UnixFS { // already. If it is present, the file is opened using the defaults which will truncate // the contents. The opened file is then returned to the caller. func (fs *Filesystem) Touch(p string, flag int) (ufs.File, error) { - return fs.unixFS.Touch(p, flag, 0o644) + var currentSize int64 + st, err := fs.unixFS.Stat(p) + if err != nil && !errors.Is(err, ufs.ErrNotExist) { + return nil, err + } else if err == nil && !st.IsDir() { + currentSize = st.Size() + } + + file, err := fs.unixFS.Touch(p, flag, 0o644) + if err != nil { + return nil, err + } + return newQuotaFile(fs, file, currentSize), nil } // Writefile writes a file to the system. If the file does not already exist one diff --git a/server/filesystem/filesystem_test.go b/server/filesystem/filesystem_test.go index 644ea53a..f84dd4be 100644 --- a/server/filesystem/filesystem_test.go +++ b/server/filesystem/filesystem_test.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "errors" + "math" "math/rand" "os" "path/filepath" @@ -115,10 +116,93 @@ func TestFilesystem_Openfile(t *testing.T) { }) } -func TestFilesystem_Writefile(t *testing.T) { +func TestFilesystem_Touch(t *testing.T) { g := Goblin(t) fs, _ := NewFs() + g.Describe("Touch", func() { + g.It("enforces disk limits while writing to the returned handle", func() { + fs.SetDiskLimit(10) + + f, err := fs.Touch("quota.txt", ufs.O_RDWR|ufs.O_TRUNC) + g.Assert(err).IsNil() + defer f.Close() + + n, err := f.WriteAt([]byte("1234567890"), 0) + g.Assert(err).IsNil() + g.Assert(n).Equal(10) + g.Assert(fs.CachedUsage()).Equal(int64(10)) + + n, err = f.WriteAt([]byte("1"), 10) + g.Assert(err).IsNotNil() + g.Assert(n).Equal(0) + g.Assert(IsErrorCode(err, ErrCodeDiskSpace)).IsTrue() + g.Assert(fs.CachedUsage()).Equal(int64(10)) + }) + + g.It("enforces disk limits while sequentially writing to the returned handle", func() { + fs.SetDiskLimit(10) + + f, err := fs.Touch("quota.txt", ufs.O_RDWR|ufs.O_TRUNC) + g.Assert(err).IsNil() + defer f.Close() + + n, err := f.Write([]byte("1234567890")) + g.Assert(err).IsNil() + g.Assert(n).Equal(10) + + n, err = f.Write([]byte("1")) + g.Assert(err).IsNotNil() + g.Assert(n).Equal(0) + g.Assert(IsErrorCode(err, ErrCodeDiskSpace)).IsTrue() + g.Assert(fs.CachedUsage()).Equal(int64(10)) + }) + + g.It("updates disk usage when a truncated file is closed smaller", func() { + r := bytes.NewReader([]byte("1234567890")) + err := fs.Write("quota.txt", r, r.Size(), 0o644) + g.Assert(err).IsNil() + g.Assert(fs.CachedUsage()).Equal(int64(10)) + + f, err := fs.Touch("quota.txt", ufs.O_RDWR|ufs.O_TRUNC) + g.Assert(err).IsNil() + + n, err := f.WriteAt([]byte("1234"), 0) + g.Assert(err).IsNil() + g.Assert(n).Equal(4) + + err = f.Close() + g.Assert(err).IsNil() + g.Assert(fs.CachedUsage()).Equal(int64(4)) + }) + + g.It("does not reset disk usage after a failed huge-offset write", func() { + const usage = int64(5 * 1024 * 1024) + fs.SetDiskLimit(10 * 1024 * 1024) + fs.unixFS.SetUsage(usage) + + f, err := fs.Touch("quota.txt", ufs.O_RDWR|ufs.O_TRUNC) + g.Assert(err).IsNil() + + n, err := f.WriteAt([]byte("x"), math.MaxInt64) + g.Assert(err).IsNotNil() + g.Assert(n).Equal(0) + + err = f.Close() + g.Assert(err).IsNil() + g.Assert(fs.CachedUsage()).Equal(usage) + }) + + g.AfterEach(func() { + _ = fs.TruncateRootDirectory() + }) + }) +} + +func TestFilesystem_Writefile(t *testing.T) { + g := Goblin(t) + fs, rfs := NewFs() + g.Describe("Open and WriteFile", func() { buf := &bytes.Buffer{} @@ -185,6 +269,19 @@ func TestFilesystem_Writefile(t *testing.T) { g.Assert(IsErrorCode(err, ErrCodeDiskSpace)).IsTrue() }) + g.It("cannot write a file whose claimed size overflows the quota check", func() { + fs.SetDiskLimit(1024) + fs.unixFS.SetUsage(1) + + r := bytes.NewReader([]byte("small body")) + err := fs.Write("overflow.txt", r, math.MaxInt64, 0o644) + g.Assert(err).IsNotNil() + g.Assert(IsErrorCode(err, ErrCodeDiskSpace)).IsTrue() + + _, err = rfs.StatServerFile("overflow.txt") + g.Assert(errors.Is(err, os.ErrNotExist)).IsTrue("err is not os.ErrNotExist") + }) + g.It("truncates the file when writing new contents", func() { r := bytes.NewReader([]byte("original data")) err := fs.Write("test.txt", r, r.Size(), 0o644) diff --git a/server/filesystem/quota_file.go b/server/filesystem/quota_file.go new file mode 100644 index 00000000..b6619957 --- /dev/null +++ b/server/filesystem/quota_file.go @@ -0,0 +1,120 @@ +package filesystem + +import ( + "io" + "math" + "sync" + + "github.com/pelican-dev/wings/internal/ufs" +) + +type quotaFile struct { + ufs.File + + fs *Filesystem + mu sync.Mutex + size int64 +} + +func newQuotaFile(fs *Filesystem, file ufs.File, size int64) ufs.File { + return "aFile{File: file, fs: fs, size: size} +} + +func (f *quotaFile) Write(p []byte) (int, error) { + if len(p) == 0 { + return f.File.Write(p) + } + + f.mu.Lock() + defer f.mu.Unlock() + + off, err := f.File.Seek(0, io.SeekCurrent) + if err != nil { + return 0, err + } + + return f.writeAtLocked(p, off, func() (int, error) { + return f.File.Write(p) + }) +} + +func (f *quotaFile) WriteAt(p []byte, off int64) (int, error) { + if off < 0 || len(p) == 0 { + return f.File.WriteAt(p, off) + } + + f.mu.Lock() + defer f.mu.Unlock() + + return f.writeAtLocked(p, off, func() (int, error) { + return f.File.WriteAt(p, off) + }) +} + +func (f *quotaFile) writeAtLocked(p []byte, off int64, write func() (int, error)) (int, error) { + previousSize := f.size + end, ok := quotaWriteEnd(off, len(p)) + if !ok { + return 0, newFilesystemError(ErrCodeDiskSpace, nil) + } + if growth := end - previousSize; growth > 0 { + if err := f.fs.reserveDisk(growth); err != nil { + return 0, err + } + } + + n, err := write() + writtenEnd := previousSize + if n > 0 { + writtenEnd, _ = quotaWriteEnd(off, n) + if writtenEnd > previousSize { + f.size = writtenEnd + } + } + + if reserved := end - previousSize; reserved > 0 { + actual := int64(0) + if writtenEnd > previousSize { + actual = writtenEnd - previousSize + } + if actual < reserved { + f.fs.adjustDisk(actual - reserved) + } + } + + return n, err +} + +func quotaWriteEnd(off int64, size int) (int64, bool) { + if size < 0 || off > math.MaxInt64-int64(size) { + return 0, false + } + return off + int64(size), true +} + +func (f *quotaFile) ReadFrom(r io.Reader) (int64, error) { + return io.Copy(quotaFileWriter{file: f}, r) +} + +func (f *quotaFile) Close() error { + f.mu.Lock() + defer f.mu.Unlock() + + st, statErr := f.File.Stat() + closeErr := f.File.Close() + if statErr == nil { + f.fs.adjustDisk(st.Size() - f.size) + } + if statErr != nil { + return statErr + } + return closeErr +} + +type quotaFileWriter struct { + file *quotaFile +} + +func (w quotaFileWriter) Write(p []byte) (int, error) { + return w.file.Write(p) +} \ No newline at end of file diff --git a/server/install.go b/server/install.go index 8307517e..d3956ab3 100644 --- a/server/install.go +++ b/server/install.go @@ -147,12 +147,22 @@ func (s *Server) IsInstalling() bool { return s.installing.Load() } +func (s *Server) SetInstalling(state bool) { + s.installing.Store(state) + if state { + s.Sftp().CancelAll() + } +} + func (s *Server) IsTransferring() bool { return s.transferring.Load() } func (s *Server) SetTransferring(state bool) { s.transferring.Store(state) + if state { + s.Sftp().CancelAll() + } } func (s *Server) IsRestoring() bool { @@ -161,6 +171,13 @@ func (s *Server) IsRestoring() bool { func (s *Server) SetRestoring(state bool) { s.restoring.Store(state) + if state { + s.Sftp().CancelAll() + } +} + +func (s *Server) IsInProtectedState() bool { + return s.IsInstalling() || s.IsTransferring() || s.IsRestoring() } // RemoveContainer removes the installation container for the server. @@ -184,7 +201,7 @@ func (ip *InstallationProcess) Run() error { if !ip.Server.installing.SwapIf(true) { return errors.New("install: cannot obtain installation lock") } - + ip.Server.Sftp().CancelAll() // We now have an exclusive lock on this installation process. Ensure that whenever this // process is finished that the semaphore is released so that other processes and be executed // without encountering a wait timeout. @@ -239,15 +256,9 @@ func (ip *InstallationProcess) writeScriptToDisk() error { // Pulls the docker image to be used for the installation container. func (ip *InstallationProcess) pullInstallationImage() error { // Get a registry auth configuration from the config. - var registryAuth *config.RegistryConfiguration - for registry, c := range config.Get().Docker.Registries { - if !strings.HasPrefix(ip.Script.ContainerImage, registry) { - continue - } - + registry, registryAuth := config.Get().Docker.RegistryCredentialsForImage(ip.Script.ContainerImage) + if registryAuth != nil { log.WithField("registry", registry).Debug("using authentication for registry") - registryAuth = &c - break } // Get the ImagePullOptions. diff --git a/server/state_test.go b/server/state_test.go new file mode 100644 index 00000000..decc9f1f --- /dev/null +++ b/server/state_test.go @@ -0,0 +1,47 @@ +package server + +import "testing" + +func TestProtectedStateCancelsSftpSessions(t *testing.T) { + tests := []struct { + name string + set func(*Server) + }{ + { + name: "installing", + set: func(s *Server) { + s.SetInstalling(true) + }, + }, + { + name: "transferring", + set: func(s *Server) { + s.SetTransferring(true) + }, + }, + { + name: "restoring", + set: func(s *Server) { + s.SetRestoring(true) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv, err := New(nil) + if err != nil { + t.Fatal(err) + } + + ctx := srv.Sftp().Context("user") + tt.set(srv) + + select { + case <-ctx.Done(): + default: + t.Fatal("expected SFTP session context to be canceled") + } + }) + } +} \ No newline at end of file diff --git a/sftp/handler.go b/sftp/handler.go index 04cb3ad2..94996594 100644 --- a/sftp/handler.go +++ b/sftp/handler.go @@ -35,6 +35,30 @@ type Handler struct { ro bool } +type quotaWriterAt struct { + io.WriterAt + server *server.Server +} + +func (w quotaWriterAt) WriteAt(p []byte, off int64) (int, error) { + if w.server != nil && w.server.IsInProtectedState() { + return 0, sftp.ErrSSHFxPermissionDenied + } + + n, err := w.WriterAt.WriteAt(p, off) + if filesystem.IsErrorCode(err, filesystem.ErrCodeDiskSpace) { + return n, ErrSSHQuotaExceeded + } + return n, err +} + +func (w quotaWriterAt) Close() error { + if c, ok := w.WriterAt.(io.Closer); ok { + return c.Close() + } + return nil +} + // NewHandler returns a new connection handler for the SFTP server. This allows a given user // to access the underlying filesystem. func NewHandler(sc *ssh.ServerConn, srv *server.Server) (*Handler, error) { @@ -81,7 +105,7 @@ func (h *Handler) Fileread(request *sftp.Request) (io.ReaderAt, error) { defer h.mu.Unlock() if err := h.fs.IsIgnored(request.Filepath); err != nil { return nil, err - } + } f, _, err := h.fs.File(request.Filepath) if err != nil { if !errors.Is(err, os.ErrNotExist) { @@ -101,7 +125,7 @@ func (h *Handler) Filewrite(request *sftp.Request) (io.WriterAt, error) { l := h.logger.WithField("source", request.Filepath) // If the user doesn't have enough space left on the server it should respond with an // error since we won't be letting them write this file to the disk. - if !h.fs.HasSpaceAvailable(true) { + if !h.fs.HasSpaceAvailable(false) { return nil, ErrSSHQuotaExceeded } @@ -110,7 +134,7 @@ func (h *Handler) Filewrite(request *sftp.Request) (io.WriterAt, error) { if err := h.fs.IsIgnored(request.Filepath); err != nil { return nil, err - } + } // The specific permission required to perform this action. If the file exists on the // system already it only needs to be an update, otherwise we'll check for a create. permission := PermissionFileUpdate @@ -141,7 +165,7 @@ func (h *Handler) Filewrite(request *sftp.Request) (io.WriterAt, error) { event = server.ActivitySftpCreate } h.events.MustLog(event, FileAction{Entity: request.Filepath}) - return f, nil + return quotaWriterAt{WriterAt: f, server: h.server}, nil } // Filecmd hander for basic SFTP system calls related to files, but not anything to do with reading @@ -154,10 +178,10 @@ func (h *Handler) Filecmd(request *sftp.Request) error { if request.Target != "" { l = l.WithField("target", request.Target) } - + if err := h.fs.IsIgnored(request.Filepath); err != nil { return err - } + } switch request.Method { // Allows a user to make changes to the permissions of a given file or directory @@ -300,7 +324,7 @@ func (h *Handler) Filelist(request *sftp.Request) (sftp.ListerAt, error) { // Determines if a user has permission to perform a specific action on the SFTP server. These // permissions are defined and returned by the Panel API. func (h *Handler) can(permission string) bool { - if h.server.IsSuspended() { + if h.server.IsSuspended() || h.server.IsInProtectedState() { return false } for _, p := range h.permissions { diff --git a/sftp/handler_test.go b/sftp/handler_test.go new file mode 100644 index 00000000..5eb940fc --- /dev/null +++ b/sftp/handler_test.go @@ -0,0 +1,140 @@ +package sftp + +import ( + "errors" + "io" + "testing" + + pkgsftp "github.com/pkg/sftp" + + "github.com/pelican-dev/wings/server" +) + +type writeAtFunc func([]byte, int64) (int, error) + +func (f writeAtFunc) WriteAt(p []byte, off int64) (int, error) { + return f(p, off) +} + +func TestHandlerDeniesAccessWhenServerIsInProtectedState(t *testing.T) { + tests := []struct { + name string + set func(*server.Server) + }{ + { + name: "installing", + set: func(s *server.Server) { + s.SetInstalling(true) + }, + }, + { + name: "transferring", + set: func(s *server.Server) { + s.SetTransferring(true) + }, + }, + { + name: "restoring", + set: func(s *server.Server) { + s.SetRestoring(true) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv, err := server.New(nil) + if err != nil { + t.Fatal(err) + } + tt.set(srv) + + h := Handler{ + server: srv, + permissions: []string{"*"}, + } + + if h.can(PermissionFileCreate) { + t.Fatal("expected SFTP access to be denied") + } + }) + } +} + +func TestWriterDeniesWritesWhenServerEntersProtectedState(t *testing.T) { + tests := []struct { + name string + set func(*server.Server) + }{ + { + name: "installing", + set: func(s *server.Server) { + s.SetInstalling(true) + }, + }, + { + name: "transferring", + set: func(s *server.Server) { + s.SetTransferring(true) + }, + }, + { + name: "restoring", + set: func(s *server.Server) { + s.SetRestoring(true) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv, err := server.New(nil) + if err != nil { + t.Fatal(err) + } + + var called bool + writer := quotaWriterAt{ + server: srv, + WriterAt: writeAtFunc(func(_ []byte, _ int64) (int, error) { + called = true + return 1, nil + }), + } + tt.set(srv) + + n, err := writer.WriteAt([]byte("x"), 0) + if !errors.Is(err, pkgsftp.ErrSSHFxPermissionDenied) { + t.Fatalf("expected permission denied, got %v", err) + } + if n != 0 { + t.Fatalf("expected zero bytes written, got %d", n) + } + if called { + t.Fatal("expected underlying writer not to be called") + } + }) + } +} + +func TestWriterForwardsWritesWhenServerIsAvailable(t *testing.T) { + srv, err := server.New(nil) + if err != nil { + t.Fatal(err) + } + + writer := quotaWriterAt{ + server: srv, + WriterAt: writeAtFunc(func(p []byte, _ int64) (int, error) { + return len(p), io.EOF + }), + } + + n, err := writer.WriteAt([]byte("test"), 0) + if !errors.Is(err, io.EOF) { + t.Fatalf("expected forwarded error, got %v", err) + } + if n != 4 { + t.Fatalf("expected forwarded byte count, got %d", n) + } +} \ No newline at end of file