diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index 5b759ec..6e331d5 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -251,6 +251,11 @@ jobs: echo "sha512_base64=${SHA512_BASE64}" } > build/release/BUILD_INFO.txt + - name: Verify release assets + run: | + ./scripts/verify-release-assets.sh \ + build/release "${{ steps.version.outputs.version }}" + - name: Summary env: VERSION: ${{ steps.version.outputs.version }} diff --git a/scripts/README.md b/scripts/README.md index c4eca27..edc47c4 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -16,9 +16,9 @@ over the air: `nanokvm_.tar.gz` plus its `latest.json` manifest. The format is not arbitrary — it is fixed by the on-device updater in `server/service/application/`: -- **One root directory.** `install.go` untars the package and moves the single - top-level directory over `/kvmapp`, so the tarball must contain exactly - `nanokvm_/`. +- **One safe root directory.** The updater scans before extracting and accepts + only directories and regular files under `nanokvm_/`; links, + special files, duplicate paths, and path traversal are rejected. - **`name` is the file name.** `version.go` builds the download URL as `/`, where `` is `https://cdn.sipeed.com/nanokvm` (or `.../preview` when `/etc/kvm/preview_updates` exists). @@ -27,9 +27,13 @@ The format is not arbitrary — it is fixed by the on-device updater in - **`/kvmapp/version`** is what the device reports as its installed version, so it must match the `version` field. -`size` is parsed into `Latest.Size` but never read anywhere, so nothing on the -device depends on its units. Published manifests have carried a kilobyte-ish -value; `package.sh` writes the exact byte count instead. +New releases use manifest v2. `size` remains the exact compressed byte count +for older clients, while v2 clients use `size_bytes` and +`unpacked_size_bytes` for storage preflight checks. Historical v1 `size` +values have inconsistent units, so v2 clients only require v1 `size` to be +non-zero and verify the downloaded SHA-512 instead. Publish v2 metadata before +releasing clients that consume it; clients retain v1 compatibility for custom +update servers. ## Building a release diff --git a/scripts/package.sh b/scripts/package.sh index 87e21d9..3c0f18a 100755 --- a/scripts/package.sh +++ b/scripts/package.sh @@ -7,9 +7,9 @@ # download URL as "/", so "name" must be the tarball file name. # - server/service/application/update.go verifies the download against # "sha512", which is the *base64* encoding of the raw SHA-512 digest. -# - server/service/application/install.go untars the package and moves the -# single top-level directory over /kvmapp, so the tarball must contain -# exactly one root directory: nanokvm_/. +# - server/service/application/archive.go validates and extracts the package +# before install.go moves its single top-level directory over /kvmapp, so +# the tarball must contain exactly one root directory: nanokvm_/. # # Build artifacts are expected to be in place already (see # scripts/build-in-container.sh and the "web" target in the Makefile): @@ -199,14 +199,29 @@ fi # --- manifest ---------------------------------------------------------------- # update.go compares base64(raw sha512), not the hex digest. SHA512="$(openssl dgst -sha512 -binary "$TARBALL" | openssl base64 -A)" -SIZE="$(wc -c < "$TARBALL" | tr -d ' ')" +SIZE_BYTES="$(wc -c < "$TARBALL" | tr -d ' ')" +UNPACKED_SIZE_BYTES="$(python3 - "$TARBALL" <<'PY' +import sys +import tarfile + +total = 0 +with tarfile.open(sys.argv[1], "r:gz") as archive: + for member in archive: + if member.isfile(): + total += member.size +print(total) +PY +)" cat > "$MANIFEST" <&2 + exit 1 + fi case "$entry" in "$PACKAGE_ROOT"|"$PACKAGE_ROOT"/*) ;; *) @@ -92,6 +99,9 @@ fi MANIFEST_VERSION=$(jq -er '.version | select(type == "string" and length > 0)' "$MANIFEST") MANIFEST_NAME=$(jq -er '.name | select(type == "string" and length > 0)' "$MANIFEST") MANIFEST_SIZE=$(jq -er '.size | select(type == "number" and . >= 0 and floor == .)' "$MANIFEST") +MANIFEST_FORMAT=$(jq -er '.manifest_version | select(type == "number" and . == 2)' "$MANIFEST") +MANIFEST_SIZE_BYTES=$(jq -er '.size_bytes | select(type == "number" and . > 0 and floor == .)' "$MANIFEST") +MANIFEST_UNPACKED_SIZE_BYTES=$(jq -er '.unpacked_size_bytes | select(type == "number" and . > 0 and floor == .)' "$MANIFEST") MANIFEST_SHA512=$(jq -er '.sha512 | select(type == "string" and length > 0)' "$MANIFEST") if [ "$MANIFEST_VERSION" != "$VERSION" ]; then @@ -104,10 +114,39 @@ if [ "$MANIFEST_NAME" != "$TARBALL_NAME" ]; then fi ACTUAL_SIZE=$(wc -c < "$TARBALL" | tr -d ' ') -if [ "$MANIFEST_SIZE" != "$ACTUAL_SIZE" ]; then +if [ "$MANIFEST_FORMAT" != "2" ]; then + echo "[ERROR] latest.json manifest_version must be 2" >&2 + exit 1 +fi +if [ "$MANIFEST_SIZE" != "$ACTUAL_SIZE" ] || [ "$MANIFEST_SIZE_BYTES" != "$ACTUAL_SIZE" ]; then echo "[ERROR] latest.json size '$MANIFEST_SIZE' does not match '$ACTUAL_SIZE'" >&2 exit 1 fi +if [ "$ACTUAL_SIZE" -gt "$MAX_PACKAGE_SIZE" ]; then + echo "[ERROR] release tarball exceeds device limit of $MAX_PACKAGE_SIZE bytes" >&2 + exit 1 +fi + +ACTUAL_UNPACKED_SIZE=$(python3 - "$TARBALL" <<'PY' +import sys +import tarfile + +total = 0 +with tarfile.open(sys.argv[1], "r:gz") as archive: + for member in archive: + if member.isfile(): + total += member.size +print(total) +PY +) +if [ "$MANIFEST_UNPACKED_SIZE_BYTES" != "$ACTUAL_UNPACKED_SIZE" ]; then + echo "[ERROR] latest.json unpacked_size_bytes '$MANIFEST_UNPACKED_SIZE_BYTES' does not match '$ACTUAL_UNPACKED_SIZE'" >&2 + exit 1 +fi +if [ "$ACTUAL_UNPACKED_SIZE" -gt "$MAX_UNPACKED_SIZE" ]; then + echo "[ERROR] unpacked package exceeds device limit of $MAX_UNPACKED_SIZE bytes" >&2 + exit 1 +fi ACTUAL_SHA512=$(openssl dgst -sha512 -binary "$TARBALL" | openssl base64 -A) if [ "$MANIFEST_SHA512" != "$ACTUAL_SHA512" ]; then diff --git a/server/service/application/archive.go b/server/service/application/archive.go new file mode 100644 index 0000000..241751a --- /dev/null +++ b/server/service/application/archive.go @@ -0,0 +1,203 @@ +package application + +import ( + "archive/tar" + "compress/gzip" + "errors" + "fmt" + "io" + "math" + "os" + "path" + "path/filepath" + "strings" +) + +type updateArchiveInfo struct { + root string + expandedBytes uint64 + entries int +} + +func inspectUpdateArchive(archivePath, expectedRoot string) (updateArchiveInfo, error) { + file, err := os.Open(archivePath) + if err != nil { + return updateArchiveInfo{}, err + } + defer file.Close() + reader, err := gzip.NewReader(file) + if err != nil { + return updateArchiveInfo{}, fmt.Errorf("open gzip archive: %w", err) + } + defer reader.Close() + + info := updateArchiveInfo{root: expectedRoot} + seen := make(map[string]struct{}) + hasRoot, hasVersion := false, false + tarReader := tar.NewReader(reader) + for { + header, err := tarReader.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return updateArchiveInfo{}, fmt.Errorf("read tar archive: %w", err) + } + name, err := validateArchiveHeader(header, expectedRoot) + if err != nil { + return updateArchiveInfo{}, err + } + if _, ok := seen[name]; ok { + return updateArchiveInfo{}, fmt.Errorf("duplicate archive entry %q", name) + } + seen[name] = struct{}{} + info.entries++ + if info.entries > maxArchiveEntries { + return updateArchiveInfo{}, fmt.Errorf("update archive has more than %d entries", maxArchiveEntries) + } + if name == expectedRoot && header.Typeflag == tar.TypeDir { + hasRoot = true + } + if name == expectedRoot+"/version" && isRegularType(header.Typeflag) { + hasVersion = true + } + if isRegularType(header.Typeflag) { + if header.Size < 0 { + return updateArchiveInfo{}, fmt.Errorf("negative archive file size") + } + size := uint64(header.Size) + if size > maxExpandedSize-info.expandedBytes { + return updateArchiveInfo{}, fmt.Errorf("update archive expanded size exceeds %d bytes", maxExpandedSize) + } + if info.expandedBytes > math.MaxUint64-size { + return updateArchiveInfo{}, fmt.Errorf("update archive expanded size overflow") + } + info.expandedBytes += size + } + } + if !hasRoot { + return updateArchiveInfo{}, fmt.Errorf("invalid update package layout: missing top-level directory") + } + if !hasVersion { + return updateArchiveInfo{}, fmt.Errorf("invalid update package layout: missing version file") + } + return info, nil +} + +func extractUpdateArchive(archivePath, destDir, expectedRoot string) (string, error) { + file, err := os.Open(archivePath) + if err != nil { + return "", err + } + defer file.Close() + reader, err := gzip.NewReader(file) + if err != nil { + return "", fmt.Errorf("open gzip archive: %w", err) + } + defer reader.Close() + + seen := make(map[string]struct{}) + entries := 0 + expanded := uint64(0) + tarReader := tar.NewReader(reader) + for { + header, err := tarReader.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return "", fmt.Errorf("read tar archive: %w", err) + } + name, err := validateArchiveHeader(header, expectedRoot) + if err != nil { + return "", err + } + if _, ok := seen[name]; ok { + return "", fmt.Errorf("duplicate archive entry %q", name) + } + seen[name] = struct{}{} + entries++ + if entries > maxArchiveEntries { + return "", fmt.Errorf("update archive has more than %d entries", maxArchiveEntries) + } + + filename := filepath.Join(destDir, filepath.FromSlash(name)) + switch header.Typeflag { + case tar.TypeDir: + if err := os.MkdirAll(filename, 0o755); err != nil { + return "", fmt.Errorf("create archive directory: %w", err) + } + case tar.TypeReg, tar.TypeRegA: + if header.Size < 0 { + return "", fmt.Errorf("negative archive file size") + } + size := uint64(header.Size) + if size > maxExpandedSize-expanded { + return "", fmt.Errorf("update archive expanded size exceeds %d bytes", maxExpandedSize) + } + expanded += size + if err := os.MkdirAll(filepath.Dir(filename), 0o755); err != nil { + return "", fmt.Errorf("create archive parent directory: %w", err) + } + out, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_EXCL, os.FileMode(header.Mode)&0o777) + if err != nil { + return "", fmt.Errorf("create archive file: %w", err) + } + written, copyErr := io.CopyN(out, tarReader, header.Size) + closeErr := out.Close() + if copyErr != nil { + return "", fmt.Errorf("write archive file: %w", copyErr) + } + if written != header.Size { + return "", fmt.Errorf("truncated archive file %q", name) + } + if closeErr != nil { + return "", fmt.Errorf("close archive file: %w", closeErr) + } + } + } + return filepath.Join(destDir, expectedRoot), nil +} + +func validateArchiveHeader(header *tar.Header, expectedRoot string) (string, error) { + if header.Name == "" || path.IsAbs(header.Name) { + return "", fmt.Errorf("invalid archive path %q", header.Name) + } + name := path.Clean(header.Name) + if name == "." || name == ".." || strings.HasPrefix(name, "../") { + return "", fmt.Errorf("invalid archive path %q", header.Name) + } + if name != expectedRoot && !strings.HasPrefix(name, expectedRoot+"/") { + return "", fmt.Errorf("invalid update package layout: entry %q is outside %s", header.Name, expectedRoot) + } + if !isRegularType(header.Typeflag) && header.Typeflag != tar.TypeDir { + return "", fmt.Errorf("unsupported archive entry type for %q", header.Name) + } + return name, nil +} + +func isRegularType(typeFlag byte) bool { + return typeFlag == tar.TypeReg || typeFlag == tar.TypeRegA +} + +func validateExtractedPackage(rootDir, expectedVersion string) error { + version, err := os.ReadFile(filepath.Join(rootDir, "version")) + if err != nil { + return fmt.Errorf("invalid update package layout: read version: %w", err) + } + if strings.TrimSpace(string(version)) != expectedVersion { + return fmt.Errorf("invalid update package layout: version mismatch") + } + for _, required := range []string{ + "version", + "server/NanoKVM-Server", + "kvm_system/kvm_system", + "system/init.d/S95nanokvm", + } { + info, err := os.Stat(filepath.Join(rootDir, filepath.FromSlash(required))) + if err != nil || !info.Mode().IsRegular() { + return fmt.Errorf("invalid update package layout: missing required file %s", required) + } + } + return nil +} diff --git a/server/service/application/archive_test.go b/server/service/application/archive_test.go new file mode 100644 index 0000000..dbfc65b --- /dev/null +++ b/server/service/application/archive_test.go @@ -0,0 +1,90 @@ +package application + +import ( + "archive/tar" + "compress/gzip" + "os" + "path/filepath" + "testing" +) + +type archiveEntry struct { + name string + typeFlag byte + data string +} + +func writeTestArchive(t *testing.T, entries []archiveEntry) string { + t.Helper() + file := filepath.Join(t.TempDir(), "update.tar.gz") + out, err := os.Create(file) + if err != nil { + t.Fatal(err) + } + gzipWriter := gzip.NewWriter(out) + tarWriter := tar.NewWriter(gzipWriter) + for _, entry := range entries { + header := &tar.Header{Name: entry.name, Typeflag: entry.typeFlag, Mode: 0o755} + if entry.typeFlag == tar.TypeReg || entry.typeFlag == tar.TypeRegA { + header.Size = int64(len(entry.data)) + } + if err := tarWriter.WriteHeader(header); err != nil { + t.Fatal(err) + } + if entry.data != "" { + if _, err := tarWriter.Write([]byte(entry.data)); err != nil { + t.Fatal(err) + } + } + } + if err := tarWriter.Close(); err != nil { + t.Fatal(err) + } + if err := gzipWriter.Close(); err != nil { + t.Fatal(err) + } + if err := out.Close(); err != nil { + t.Fatal(err) + } + return file +} + +func TestInspectArchiveRejectsUnsafeEntries(t *testing.T) { + root := "nanokvm_1.2.3" + for _, entry := range []archiveEntry{ + {name: "../escape", typeFlag: tar.TypeReg, data: "x"}, + {name: "/escape", typeFlag: tar.TypeReg, data: "x"}, + {name: root + "/link", typeFlag: tar.TypeSymlink}, + } { + archive := writeTestArchive(t, []archiveEntry{{name: root, typeFlag: tar.TypeDir}, entry}) + if _, err := inspectUpdateArchive(archive, root); err == nil { + t.Fatalf("unsafe entry %+v was accepted", entry) + } + } +} + +func TestInspectAndExtractArchive(t *testing.T) { + root := "nanokvm_1.2.3" + archive := writeTestArchive(t, []archiveEntry{ + {name: root, typeFlag: tar.TypeDir}, + {name: root + "/version", typeFlag: tar.TypeReg, data: "1.2.3\n"}, + {name: root + "/server/NanoKVM-Server", typeFlag: tar.TypeReg, data: "server"}, + {name: root + "/kvm_system/kvm_system", typeFlag: tar.TypeReg, data: "system"}, + {name: root + "/system/init.d/S95nanokvm", typeFlag: tar.TypeReg, data: "init"}, + }) + info, err := inspectUpdateArchive(archive, root) + if err != nil { + t.Fatal(err) + } + if info.expandedBytes != 22 { // data totals: 6 + 6 + 6 + 4 + t.Fatalf("unexpected expanded size %d", info.expandedBytes) + } + destination := t.TempDir() + extracted, err := extractUpdateArchive(archive, destination, root) + if err != nil { + t.Fatal(err) + } + if err := validateExtractedPackage(extracted, "1.2.3"); err != nil { + t.Fatal(err) + } +} diff --git a/server/service/application/install.go b/server/service/application/install.go index 79a3013..74cc865 100644 --- a/server/service/application/install.go +++ b/server/service/application/install.go @@ -31,17 +31,12 @@ func releaseUpdateLock() { isUpdating = false } -func installPackage(source string) error { - dir, err := utils.UnTarGz(source, CacheDir) - if err != nil { - return fmt.Errorf("failed to decompress app: %w", err) - } - +func installPreparedPackage(sourceDir string) error { if err := backupCurrentApp(); err != nil { return err } - if err := applyUpdate(dir); err != nil { + if err := applyUpdate(sourceDir); err != nil { return err } diff --git a/server/service/application/progress_writer.go b/server/service/application/progress_writer.go index 79bc391..0b9482a 100644 --- a/server/service/application/progress_writer.go +++ b/server/service/application/progress_writer.go @@ -7,11 +7,13 @@ import ( "strings" "time" + "NanoKVM-Server/utils" + log "github.com/sirupsen/logrus" ) const ( - sentinelPath = "/tmp/.download_in_progress" + sentinelPath = utils.TransferSentinelPath tickerInterval = 2500 * time.Millisecond sentinelPermission = 0644 ) diff --git a/server/service/application/service.go b/server/service/application/service.go index 03bbf88..9584145 100644 --- a/server/service/application/service.go +++ b/server/service/application/service.go @@ -7,6 +7,14 @@ const ( AppDir = "/kvmapp" BackupDir = "/root/old" CacheDir = "/root/.kvmcache" + + updateWorkspacePrefix = "nanokvm-update-" + cacheDirMode = 0o700 + maxPackageSize = uint64(1 << 30) + maxExpandedSize = uint64(2 << 30) + maxArchiveEntries = 100_000 + minFreeReserve = uint64(128 << 20) + freeReservePercent = uint64(5) ) type Service struct{} diff --git a/server/service/application/storage.go b/server/service/application/storage.go new file mode 100644 index 0000000..54570b9 --- /dev/null +++ b/server/service/application/storage.go @@ -0,0 +1,103 @@ +package application + +import ( + "errors" + "fmt" + "math" + "os" + "path/filepath" + + "golang.org/x/sys/unix" +) + +var ErrInsufficientStorage = errors.New("insufficient storage") + +type filesystemSpace struct { + total uint64 + available uint64 +} + +func getFilesystemSpace(path string) (filesystemSpace, error) { + var stat unix.Statfs_t + if err := unix.Statfs(path, &stat); err != nil { + return filesystemSpace{}, fmt.Errorf("stat filesystem %s: %w", path, err) + } + return filesystemSpaceFromStats(uint64(stat.Blocks), uint64(stat.Bavail), uint64(stat.Bsize)) +} + +func filesystemSpaceFromStats(blocks, availableBlocks, blockSize uint64) (filesystemSpace, error) { + if blockSize != 0 && (blocks > math.MaxUint64/blockSize || availableBlocks > math.MaxUint64/blockSize) { + return filesystemSpace{}, fmt.Errorf("filesystem size overflow") + } + return filesystemSpace{total: blocks * blockSize, available: availableBlocks * blockSize}, nil +} + +func reserveBytes(total uint64) uint64 { + percent := total / 100 * freeReservePercent + percent += total % 100 * freeReservePercent / 100 + if percent > minFreeReserve { + return percent + } + return minFreeReserve +} + +func hasFreeSpace(space filesystemSpace, payload uint64) (bool, uint64, error) { + reserve := reserveBytes(space.total) + if payload > math.MaxUint64-reserve { + return false, 0, fmt.Errorf("storage requirement overflow") + } + required := payload + reserve + return space.available >= required, required, nil +} + +func ensureFreeSpace(path string, payloadBytes uint64) error { + space, err := getFilesystemSpace(path) + if err != nil { + return err + } + ok, required, err := hasFreeSpace(space, payloadBytes) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("%w: need %d MiB including reserve, %d MiB available on %s", ErrInsufficientStorage, required>>20, space.available>>20, path) + } + return nil +} + +func ensureExpandedSpace(path string, expandedBytes uint64) error { + // Keep the last known-good backup until the new package is fully prepared. + // Reclaiming it here can leave the device without rollback material even + // when extraction later fails or the reclaimed space is still insufficient. + return ensureFreeSpace(path, expandedBytes) +} + +func ensureInstallFilesystem(workspaceDir, appDir, backupDir string) error { + backupPath := backupDir + if _, err := os.Stat(backupPath); err != nil { + if !os.IsNotExist(err) { + return fmt.Errorf("stat backup path %s: %w", backupPath, err) + } + backupPath = filepath.Dir(backupPath) + } + + // AppDir already exists (prepareCacheForUpdate verifies it), so stat the + // directory itself. Its parent can be on another filesystem when AppDir is + // a mount point. + paths := []string{workspaceDir, appDir, backupPath} + var device uint64 + for i, path := range paths { + var stat unix.Stat_t + if err := unix.Stat(path, &stat); err != nil { + return fmt.Errorf("stat install filesystem %s: %w", path, err) + } + if i == 0 { + device = uint64(stat.Dev) + continue + } + if uint64(stat.Dev) != device { + return fmt.Errorf("update workspace, application and backup must be on the same filesystem") + } + } + return nil +} diff --git a/server/service/application/storage_test.go b/server/service/application/storage_test.go new file mode 100644 index 0000000..d269cc7 --- /dev/null +++ b/server/service/application/storage_test.go @@ -0,0 +1,46 @@ +package application + +import ( + "math" + "path/filepath" + "testing" +) + +func TestReserveBytesAndSpaceBoundary(t *testing.T) { + if got := reserveBytes(1 << 30); got != minFreeReserve { + t.Fatalf("reserve for 1 GiB = %d, want %d", got, minFreeReserve) + } + if got := reserveBytes(4 << 30); got != 214748364 { + t.Fatalf("reserve for 4 GiB = %d, want 214748364", got) + } + space := filesystemSpace{total: 1 << 30, available: (1 << 20) + minFreeReserve} + ok, required, err := hasFreeSpace(space, 1<<20) + if err != nil || !ok || required != space.available { + t.Fatalf("boundary should fit: ok=%v required=%d err=%v", ok, required, err) + } + space.available-- + ok, _, err = hasFreeSpace(space, 1<<20) + if err != nil || ok { + t.Fatalf("one byte short should fail: ok=%v err=%v", ok, err) + } +} + +func TestStorageOverflowIsRejected(t *testing.T) { + if _, err := filesystemSpaceFromStats(math.MaxUint64, 1, 2); err == nil { + t.Fatal("expected filesystem overflow") + } + if _, _, err := hasFreeSpace(filesystemSpace{total: 1}, math.MaxUint64); err == nil { + t.Fatal("expected requirement overflow") + } +} + +func TestInstallFilesystemChecksAppMountPoint(t *testing.T) { + workspace := t.TempDir() + backup := filepath.Join(t.TempDir(), "backup") + + // /proc is a mount point whose parent is /. Checking filepath.Dir(appDir) + // would therefore miss the cross-filesystem layout on a normal Linux host. + if err := ensureInstallFilesystem(workspace, "/proc", backup); err == nil { + t.Fatal("separate application mount point was accepted") + } +} diff --git a/server/service/application/update.go b/server/service/application/update.go index 610529b..7b4d81e 100644 --- a/server/service/application/update.go +++ b/server/service/application/update.go @@ -9,6 +9,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "time" "github.com/gin-gonic/gin" @@ -52,33 +53,62 @@ func restartServices() { } func update() error { - _ = os.RemoveAll(CacheDir) - _ = os.MkdirAll(CacheDir, 0o755) - defer func() { - _ = os.RemoveAll(CacheDir) - }() - - // get latest information latest, err := getLatest() if err != nil { return err } - - // download - target := filepath.Join(CacheDir, latest.Name) - if err := download(latest.Url, target); err != nil { - log.Errorf("download app failed: %s", err) + if err := prepareCacheForUpdate(); err != nil { + return err + } + workspace, err := newUpdateWorkspace(CacheDir) + if err != nil { + return err + } + defer func() { + if err := workspace.Close(); err != nil { + log.Warnf("failed to clean update workspace %s: %v", workspace.dir, err) + } + }() + if err := ensureInstallFilesystem(workspace.dir, AppDir, BackupDir); err != nil { + return err + } + if err := preflightManifestSpace(workspace.dir, latest); err != nil { + return err + } + + target := filepath.Join(workspace.dir, latest.Name) + downloadInfo, err := download(latest, target) + if err != nil { + log.Errorf("download app failed: %s", err) + return err + } + if err := validateDownloadedSize(latest, uint64(downloadInfo.Written)); err != nil { return err } - // check sha512 if err := checksum(target, latest.Sha512); err != nil { log.Errorf("check sha512 failed: %s", err) return err } - - // install - if err := installPackage(target); err != nil { + expectedRoot := strings.TrimSuffix(latest.Name, ".tar.gz") + info, err := inspectUpdateArchive(target, expectedRoot) + if err != nil { + return fmt.Errorf("inspect update package: %w", err) + } + if err := validateExpandedSize(latest, info.expandedBytes); err != nil { + return err + } + if err := ensureExpandedSpace(workspace.dir, info.expandedBytes); err != nil { + return err + } + sourceDir, err := extractUpdateArchive(target, workspace.dir, expectedRoot) + if err != nil { + return fmt.Errorf("extract update package: %w", err) + } + if err := validateExtractedPackage(sourceDir, latest.Version); err != nil { + return err + } + if err := installPreparedPackage(sourceDir); err != nil { log.Errorf("failed to install package: %v", err) return err } @@ -86,7 +116,7 @@ func update() error { return nil } -func download(url string, target string) (err error) { +func download(latest *Latest, target string) (info utils.DownloadInfo, err error) { for i := range maxTries { log.Debugf("attempt #%d/%d", i+1, maxTries) if i > 0 { @@ -94,21 +124,26 @@ func download(url string, target string) (err error) { } var req *http.Request - req, err = utils.NewAuthenticatedRequest("GET", url, nil) + req, err = utils.NewAuthenticatedRequest("GET", latest.Url, nil) if err != nil { log.Errorf("new request err: %s", err) continue } log.Debugf("update will be saved to: %s", target) - err = utils.Download(req, target) + info, err = utils.Download(req, target, int64(maxPackageSize), func(contentLength int64) error { + if latest.ManifestVersion == 2 && uint64(contentLength) != latest.SizeBytes { + return fmt.Errorf("update package size mismatch: manifest has %d bytes, response has %d", latest.SizeBytes, contentLength) + } + return ensureFreeSpace(filepath.Dir(target), uint64(contentLength)) + }) if err != nil { log.Errorf("downloading latest application failed, try again...") continue } - return nil + return info, nil } - return err + return utils.DownloadInfo{}, err } func checksum(filePath string, expectedHash string) error { diff --git a/server/service/application/update_offline.go b/server/service/application/update_offline.go index 0031f7a..552b208 100644 --- a/server/service/application/update_offline.go +++ b/server/service/application/update_offline.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "mime/multipart" + "net/http" "os" "path/filepath" "regexp" @@ -45,11 +46,33 @@ func offlineUpdate(c *gin.Context) error { return err } - _ = os.RemoveAll(CacheDir) - _ = os.MkdirAll(CacheDir, 0o755) + if err := prepareCacheForUpdate(); err != nil { + return err + } + workspace, err := newUpdateWorkspace(CacheDir) + if err != nil { + return err + } defer func() { - _ = os.RemoveAll(CacheDir) + if err := workspace.Close(); err != nil { + log.Warnf("failed to clean update workspace %s: %v", workspace.dir, err) + } }() + if err := ensureInstallFilesystem(workspace.dir, AppDir, BackupDir); err != nil { + return err + } + + maxRequestBytes := int64(maxPackageSize) + (1 << 20) + contentLength := c.Request.ContentLength + if contentLength > maxRequestBytes { + return fmt.Errorf("offline update request exceeds %d bytes", maxRequestBytes) + } + if contentLength > 0 { + if err := ensureFreeSpace(workspace.dir, uint64(contentLength)); err != nil { + return err + } + } + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxRequestBytes) if err := createSentinelFile(); err != nil { return err @@ -62,7 +85,7 @@ func offlineUpdate(c *gin.Context) error { return fmt.Errorf("invalid multipart data: %w", err) } - target, err := processUpload(reader, c.Request.ContentLength) + target, err := processUpload(reader, contentLength, workspace.dir) if err != nil { log.Errorf("failed to upload install package: %v", err) return err @@ -73,7 +96,24 @@ func offlineUpdate(c *gin.Context) error { return err } - if err := installPackage(target); err != nil { + archiveName := filepath.Base(target) + expectedRoot := strings.TrimSuffix(archiveName, ".tar.gz") + expectedVersion := strings.TrimPrefix(expectedRoot, "nanokvm_") + info, err := inspectUpdateArchive(target, expectedRoot) + if err != nil { + return fmt.Errorf("inspect update package: %w", err) + } + if err := ensureExpandedSpace(workspace.dir, info.expandedBytes); err != nil { + return err + } + sourceDir, err := extractUpdateArchive(target, workspace.dir, expectedRoot) + if err != nil { + return fmt.Errorf("extract update package: %w", err) + } + if err := validateExtractedPackage(sourceDir, expectedVersion); err != nil { + return err + } + if err := installPreparedPackage(sourceDir); err != nil { log.Errorf("failed to install package: %v", err) return err } @@ -145,7 +185,7 @@ func createSentinelFile() error { return nil } -func processUpload(reader *multipart.Reader, contentLength int64) (string, error) { +func processUpload(reader *multipart.Reader, contentLength int64, workspaceDir string) (string, error) { var outPath string for { @@ -160,8 +200,11 @@ func processUpload(reader *multipart.Reader, contentLength int64) (string, error if part.FormName() != "file" { continue } + if outPath != "" { + return "", fmt.Errorf("multiple files uploaded") + } - outPath, err = saveUploadedFile(part, contentLength) + outPath, err = saveUploadedFile(part, contentLength, workspaceDir) if err != nil { return "", err } @@ -174,7 +217,7 @@ func processUpload(reader *multipart.Reader, contentLength int64) (string, error return outPath, nil } -func saveUploadedFile(part *multipart.Part, contentLength int64) (string, error) { +func saveUploadedFile(part *multipart.Part, contentLength int64, workspaceDir string) (string, error) { filename := part.FileName() if filename == "" { return "", fmt.Errorf("no filename provided") @@ -184,19 +227,36 @@ func saveUploadedFile(part *multipart.Part, contentLength int64) (string, error) return "", err } - outPath := filepath.Join(CacheDir, filename) - out, err := os.Create(outPath) + outPath := filepath.Join(workspaceDir, filename) + out, err := os.OpenFile(outPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) if err != nil { return "", fmt.Errorf("failed to create output file: %w", err) } - defer out.Close() + success := false + defer func() { + _ = out.Close() + if !success { + _ = os.Remove(outPath) + } + }() + if contentLength < 0 { + contentLength = 0 + } pw := newProgressWriter(out, contentLength) defer pw.Stop() - if _, err := io.Copy(pw, part); err != nil { + written, err := io.Copy(pw, io.LimitReader(part, int64(maxPackageSize)+1)) + if err != nil { return "", fmt.Errorf("failed to write file: %w", err) } + if written > int64(maxPackageSize) { + return "", fmt.Errorf("uploaded package exceeds %d bytes", maxPackageSize) + } + if err := out.Close(); err != nil { + return "", fmt.Errorf("failed to close output file: %w", err) + } + success = true return outPath, nil } @@ -221,6 +281,9 @@ func validateFilename(filename string) error { log.Warnf("Invalid filename characters: %s", filename) return fmt.Errorf("invalid filename: contains invalid characters") } + if !packageNamePattern.MatchString(filename) { + return fmt.Errorf("invalid update package name") + } return nil } diff --git a/server/service/application/version.go b/server/service/application/version.go index 05cca56..a4e9e5d 100644 --- a/server/service/application/version.go +++ b/server/service/application/version.go @@ -21,11 +21,14 @@ import ( ) type Latest struct { - Version string `json:"version"` - Name string `json:"name"` - Sha512 string `json:"sha512"` - Size uint64 `json:"size"` - Url string `json:"-"` + ManifestVersion int `json:"manifest_version,omitempty"` + Version string `json:"version"` + Name string `json:"name"` + Sha512 string `json:"sha512"` + LegacySize uint64 `json:"size"` + SizeBytes uint64 `json:"size_bytes,omitempty"` + UnpackedSizeBytes uint64 `json:"unpacked_size_bytes,omitempty"` + Url string `json:"-"` } const ( @@ -149,8 +152,45 @@ func validateLatest(latest *Latest) error { if err != nil || len(digest) != 64 { return errors.New("invalid update package sha512") } - if latest.Size == 0 { + if latest.LegacySize == 0 { return errors.New("invalid update package size") } + switch latest.ManifestVersion { + case 0, 1: + return nil + case 2: + if latest.SizeBytes == 0 || latest.SizeBytes > maxPackageSize { + return errors.New("invalid update package size_bytes") + } + if latest.UnpackedSizeBytes == 0 || latest.UnpackedSizeBytes > maxExpandedSize { + return errors.New("invalid update package unpacked_size_bytes") + } + return nil + default: + return errors.New("unsupported update manifest version") + } +} + +func preflightManifestSpace(path string, latest *Latest) error { + if latest.ManifestVersion != 2 { + return nil + } + return ensureFreeSpace(path, latest.SizeBytes) +} + +func validateDownloadedSize(latest *Latest, written uint64) error { + if written > maxPackageSize { + return fmt.Errorf("update package exceeds %d bytes", maxPackageSize) + } + if latest.ManifestVersion == 2 && written != latest.SizeBytes { + return fmt.Errorf("update package size mismatch: expected %d bytes, got %d", latest.SizeBytes, written) + } + return nil +} + +func validateExpandedSize(latest *Latest, expanded uint64) error { + if latest.ManifestVersion == 2 && expanded != latest.UnpackedSizeBytes { + return fmt.Errorf("update package expanded size mismatch: expected %d bytes, got %d", latest.UnpackedSizeBytes, expanded) + } return nil } diff --git a/server/service/application/version_test.go b/server/service/application/version_test.go new file mode 100644 index 0000000..294b8a7 --- /dev/null +++ b/server/service/application/version_test.go @@ -0,0 +1,46 @@ +package application + +import ( + "crypto/sha512" + "encoding/base64" + "testing" +) + +func validLatest() Latest { + digest := sha512.Sum512([]byte("package")) + return Latest{ + Version: "1.2.3", Name: "nanokvm_1.2.3.tar.gz", + Sha512: base64.StdEncoding.EncodeToString(digest[:]), LegacySize: 1, + } +} + +func TestValidateLatestV1DoesNotInterpretLegacySizeAsBytes(t *testing.T) { + latest := validLatest() + latest.LegacySize = 15048 // historic stable manifests used a non-byte value + if err := validateLatest(&latest); err != nil { + t.Fatal(err) + } + if err := validateDownloadedSize(&latest, 15406125); err != nil { + t.Fatalf("v1 must not require equality with legacy size: %v", err) + } +} + +func TestValidateLatestV2RequiresExactByteFields(t *testing.T) { + latest := validLatest() + latest.ManifestVersion = 2 + latest.SizeBytes = 100 + latest.UnpackedSizeBytes = 200 + if err := validateLatest(&latest); err != nil { + t.Fatal(err) + } + if err := validateDownloadedSize(&latest, 99); err == nil { + t.Fatal("v2 size mismatch was accepted") + } + if err := validateExpandedSize(&latest, 199); err == nil { + t.Fatal("v2 expanded size mismatch was accepted") + } + latest.ManifestVersion = 3 + if err := validateLatest(&latest); err == nil { + t.Fatal("unknown manifest version was accepted") + } +} diff --git a/server/service/application/workspace.go b/server/service/application/workspace.go new file mode 100644 index 0000000..38532ee --- /dev/null +++ b/server/service/application/workspace.go @@ -0,0 +1,69 @@ +package application + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +type updateWorkspace struct { + dir string +} + +func newUpdateWorkspace(baseDir string) (*updateWorkspace, error) { + if err := os.MkdirAll(baseDir, cacheDirMode); err != nil { + return nil, fmt.Errorf("create update cache: %w", err) + } + if err := os.Chmod(baseDir, cacheDirMode); err != nil { + return nil, fmt.Errorf("chmod update cache: %w", err) + } + dir, err := os.MkdirTemp(baseDir, updateWorkspacePrefix) + if err != nil { + return nil, fmt.Errorf("create update workspace: %w", err) + } + return &updateWorkspace{dir: dir}, nil +} + +func (w *updateWorkspace) Close() error { + return os.RemoveAll(w.dir) +} + +func cleanupStaleWorkspaces(baseDir string) error { + entries, err := os.ReadDir(baseDir) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + for _, entry := range entries { + if !strings.HasPrefix(entry.Name(), updateWorkspacePrefix) { + continue + } + if err := os.RemoveAll(filepath.Join(baseDir, entry.Name())); err != nil { + return err + } + } + return nil +} + +func prepareCacheForUpdate() error { + info, err := os.Stat(AppDir) + if err != nil { + return fmt.Errorf("stat current application: %w", err) + } + if !info.IsDir() { + return fmt.Errorf("current application is not a directory") + } + if err := os.MkdirAll(CacheDir, cacheDirMode); err != nil { + return fmt.Errorf("create update cache: %w", err) + } + if err := os.Chmod(CacheDir, cacheDirMode); err != nil { + return fmt.Errorf("chmod update cache: %w", err) + } + if err := cleanupStaleWorkspaces(CacheDir); err != nil { + return fmt.Errorf("clean stale update workspace: %w", err) + } + return nil +} diff --git a/server/service/application/workspace_test.go b/server/service/application/workspace_test.go new file mode 100644 index 0000000..b416cf2 --- /dev/null +++ b/server/service/application/workspace_test.go @@ -0,0 +1,49 @@ +package application + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestWorkspaceCleanupIsScopedToUpdaterDirectories(t *testing.T) { + base := t.TempDir() + for _, name := range []string{"nanokvm-update-old", "update-other", "firmware.tar.xz", "nanokvm-go_1.0.0"} { + if err := os.Mkdir(filepath.Join(base, name), 0o755); err != nil { + t.Fatal(err) + } + } + if err := cleanupStaleWorkspaces(base); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(base, "nanokvm-update-old")); !os.IsNotExist(err) { + t.Fatalf("stale workspace still exists: %v", err) + } + for _, name := range []string{"update-other", "firmware.tar.xz", "nanokvm-go_1.0.0"} { + if _, err := os.Stat(filepath.Join(base, name)); err != nil { + t.Fatalf("unrelated cache entry %s was removed: %v", name, err) + } + } +} + +func TestNewWorkspaceIsPrivateAndCloseOnlyRemovesItself(t *testing.T) { + base := t.TempDir() + keep := filepath.Join(base, "keep") + if err := os.WriteFile(keep, []byte("keep"), 0o600); err != nil { + t.Fatal(err) + } + workspace, err := newUpdateWorkspace(base) + if err != nil { + t.Fatal(err) + } + if filepath.Dir(workspace.dir) != base || !strings.HasPrefix(filepath.Base(workspace.dir), updateWorkspacePrefix) { + t.Fatalf("unexpected workspace path %q", workspace.dir) + } + if err := workspace.Close(); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(keep); err != nil { + t.Fatalf("unrelated cache entry was removed: %v", err) + } +} diff --git a/server/service/download/service.go b/server/service/download/service.go index 3ea0b5d..473dd22 100644 --- a/server/service/download/service.go +++ b/server/service/download/service.go @@ -2,6 +2,7 @@ package download import ( "NanoKVM-Server/proto" + "NanoKVM-Server/utils" "bytes" "context" "crypto/sha256" @@ -26,7 +27,7 @@ import ( type downloadStatus string const ( - transferSentinelPath = "/tmp/.download_in_progress" + transferSentinelPath = utils.TransferSentinelPath downloadStatusIdle downloadStatus = "idle" downloadStatusInProgress downloadStatus = "in_progress" downloadStatusSuccess downloadStatus = "success" diff --git a/server/utils/http.go b/server/utils/http.go index dfa4fc9..4b4538d 100644 --- a/server/utils/http.go +++ b/server/utils/http.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "mime" "net/http" "net/url" "os" @@ -14,10 +15,13 @@ import ( log "github.com/sirupsen/logrus" ) -const maxDownloadSize = int64(1024 * 1024 * 1024) - var downloadClient = NewUpdateHTTPClient(15 * time.Minute) +type DownloadInfo struct { + ContentLength int64 + Written int64 +} + func NewAuthenticatedRequest(method string, rawURL string, body io.Reader) (*http.Request, error) { req, err := http.NewRequest(method, rawURL, body) if err != nil { @@ -66,26 +70,12 @@ func sameUpdateHost(left *url.URL, right *url.URL) bool { return strings.EqualFold(left.Host, right.Host) } -func Download(req *http.Request, target string) error { +func Download(req *http.Request, target string, maxBytes int64, beforeWrite func(contentLength int64) error) (DownloadInfo, error) { log.Debugf("downloading %s to %s", req.URL.Redacted(), target) - err := os.MkdirAll(filepath.Dir(target), 0o755) - if err != nil { - log.Errorf("create dir %s err: %s", filepath.Dir(target), err) - return err - } - out, err := os.OpenFile(target, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o755) - if err != nil { - log.Errorf("cannot create file '%s', error: %s", target, err) - return err - } - defer func() { - _ = out.Close() - }() - resp, err := downloadClient.Do(req) if err != nil { log.Errorf("request to %s failed", req.URL.Redacted()) - return errors.New("update website is inaccessible right now") + return DownloadInfo{}, errors.New("update website is inaccessible right now") } defer func() { _ = resp.Body.Close() @@ -93,23 +83,57 @@ func Download(req *http.Request, target string) error { if resp.StatusCode != http.StatusOK { log.Errorf("request failed, status code: %d", resp.StatusCode) - return errors.New("update website is inaccessible right now") + return DownloadInfo{}, errors.New("update website is inaccessible right now") } - - contentType := resp.Header.Get("Content-Type") - if contentType != "application/octet-stream" && contentType != "application/zip" && contentType != "application/gzip" { - log.Debugf("unexpected content-type, it should be either octet-stream or (g)zip, but got: %s", contentType) - return errors.New("unsupported content type") + contentType, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type")) + if err != nil || !allowedDownloadContentType(contentType) { + log.Debugf("unexpected content-type: %s", resp.Header.Get("Content-Type")) + return DownloadInfo{}, errors.New("unsupported content type") } - - written, err := io.Copy(out, io.LimitReader(resp.Body, maxDownloadSize+1)) + if resp.ContentLength > maxBytes { + return DownloadInfo{}, fmt.Errorf("download exceeds %d bytes", maxBytes) + } + if resp.ContentLength > 0 && beforeWrite != nil { + if err := beforeWrite(resp.ContentLength); err != nil { + return DownloadInfo{}, err + } + } + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + log.Errorf("create dir %s err: %s", filepath.Dir(target), err) + return DownloadInfo{}, err + } + out, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + log.Errorf("cannot create file '%s', error: %s", target, err) + return DownloadInfo{}, err + } + success := false + defer func() { + _ = out.Close() + if !success { + _ = os.Remove(target) + } + }() + written, err := io.Copy(out, io.LimitReader(resp.Body, maxBytes+1)) if err != nil { log.Errorf("download file to %s err: %s", target, err) - return err + return DownloadInfo{}, err } - if written > maxDownloadSize { - return fmt.Errorf("download exceeds %d bytes", maxDownloadSize) + if written > maxBytes { + return DownloadInfo{}, fmt.Errorf("download exceeds %d bytes", maxBytes) + } + if err := out.Close(); err != nil { + return DownloadInfo{}, err + } + success = true + return DownloadInfo{ContentLength: resp.ContentLength, Written: written}, nil +} + +func allowedDownloadContentType(contentType string) bool { + switch contentType { + case "application/octet-stream", "application/gzip", "application/x-gzip", "application/x-compressed", "application/zip": + return true + default: + return false } - - return nil } diff --git a/server/utils/http_test.go b/server/utils/http_test.go new file mode 100644 index 0000000..231dcf3 --- /dev/null +++ b/server/utils/http_test.go @@ -0,0 +1,76 @@ +package utils + +import ( + "errors" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" +) + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (fn roundTripFunc) RoundTrip(request *http.Request) (*http.Response, error) { + return fn(request) +} + +func testDownloadClient(body, contentType string) *http.Client { + return &http.Client{Transport: roundTripFunc(func(request *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{contentType}}, + Body: io.NopCloser(strings.NewReader(body)), + ContentLength: int64(len(body)), + Request: request, + }, nil + })} +} + +func TestDownloadPreflightRunsBeforeTargetCreation(t *testing.T) { + oldClient := downloadClient + downloadClient = testDownloadClient("package", "application/x-compressed; charset=binary") + defer func() { downloadClient = oldClient }() + + target := filepath.Join(t.TempDir(), "package.tar.gz") + req, err := http.NewRequest(http.MethodGet, "https://updates.example/package", nil) + if err != nil { + t.Fatal(err) + } + _, err = Download(req, target, 1024, func(int64) error { return errors.New("no space") }) + if err == nil { + t.Fatal("download succeeded despite failed preflight") + } + if _, statErr := os.Stat(target); !os.IsNotExist(statErr) { + t.Fatalf("target was created before preflight: %v", statErr) + } +} + +func TestDownloadRemovesPartialFileAndReportsWrittenBytes(t *testing.T) { + oldClient := downloadClient + downloadClient = testDownloadClient("12345", "application/gzip") + defer func() { downloadClient = oldClient }() + + request := func() *http.Request { + req, err := http.NewRequest(http.MethodGet, "https://updates.example/package", nil) + if err != nil { + t.Fatal(err) + } + return req + } + target := filepath.Join(t.TempDir(), "package.tar.gz") + if _, err := Download(request(), target, 4, nil); err == nil { + t.Fatal("oversized download was accepted") + } + if _, err := os.Stat(target); !os.IsNotExist(err) { + t.Fatalf("partial file remains: %v", err) + } + info, err := Download(request(), target, 5, nil) + if err != nil { + t.Fatal(err) + } + if info.Written != 5 { + t.Fatalf("written = %d, want 5", info.Written) + } +} diff --git a/server/utils/transfer.go b/server/utils/transfer.go new file mode 100644 index 0000000..d291513 --- /dev/null +++ b/server/utils/transfer.go @@ -0,0 +1,6 @@ +package utils + +// TransferSentinelPath coordinates application uploads and virtual-media +// transfers. /run is already required by the device init scripts and preserves +// the previous reboot-clears-stale-state behaviour without using /tmp. +const TransferSentinelPath = "/run/nanokvm-transfer-in-progress"