diff --git a/README.md b/README.md index f39d2a1..89edc07 100644 --- a/README.md +++ b/README.md @@ -11,9 +11,9 @@ ever-changing **vulnerability feed**, and understand **Alpine's APK versioning** rules to tell whether an installed version is actually older than the one that fixes a given CVE. Sentinel automates exactly that loop. -Point it at an image and it works as a **pipeline**: it pulls and unpacks the -image, reads the **Alpine package database** out of the layers to build a -lightweight **SBOM** (software bill of materials), detects the Alpine OS version, +Point it at an image and it works as a **pipeline**: it pulls and saves the +image, then **streams** the **Alpine package database** out of the layers to +build a lightweight **SBOM** (software bill of materials), detects the Alpine OS version, downloads the matching **Alpine Security Database (SecDB)**, and compares every installed package against the known fixes using an **APK-aware version comparator**. The result is a clear report of vulnerable packages with their @@ -27,7 +27,7 @@ pulls in **no third-party dependencies**. - Scans **Docker images** from the command line (`--image`). - Machine-readable **JSON output** for **CI pipelines** (`--format json`). - **Policy-based build failure** thresholds (`--fail-on`). -- Extracts and analyzes **image layers** to build an **SBOM**. +- **Streams image layers** from the saved tar (no full extraction to disk) to build an **SBOM**. - Detects the Alpine version from `alpine-release` (with safe fallback to image-tag parsing). - Fetches and merges **Alpine SecDB** from both the `main` and `community` repositories. - **APK-aware version comparison**, including pre/post-release suffix semantics (see below). @@ -47,13 +47,12 @@ next, and the whole thing is orchestrated by `cmd/scanner/main.go`. ┌───────────────────┐ │ docker pull │ ensure the image exists locally │ docker save │ export it to image.tar ── internal/extractor -│ unpack tarball │ untar into a temp dir (zip-slip guarded) └─────────┬─────────┘ - │ unpacked image (manifest.json + layer tarballs) + │ saved image.tar (manifest.json + layer blobs) ▼ ┌───────────────────┐ -│ read manifest │ ordered list of layers -│ walk each layer │ gzip/plain auto-detected ── internal/analyzer +│ stream outer tar │ find manifest.json (any order) +│ walk each layer │ gzip/plain, in-memory, cap-guarded ── internal/analyzer │ parse apk DB │ lib/apk/db/installed → SBOM └─────────┬─────────┘ │ SBOM: []{name, version} @@ -77,14 +76,14 @@ next, and the whole thing is orchestrated by `cmd/scanner/main.go`. Pipeline in one line: ``` -docker pull → docker save → unpack → SBOM → SecDB → match → report +docker pull → docker save → stream layers → SBOM → SecDB → match → report ``` ### Project structure - `cmd/scanner/main.go` — **CLI entrypoint**, matching loop, scoring, and report rendering. -- `internal/extractor/` — pulls, saves, and unpacks **Docker images**. -- `internal/analyzer/` — reads the manifest, walks layers, and builds the **SBOM** from the Alpine package DB. +- `internal/extractor/` — pulls and saves **Docker images**, and provides the cap-guarded streaming tar walker. +- `internal/analyzer/` — reads the manifest and streams layers to build the **SBOM** from the Alpine package DB. - `internal/matcher/` — fetches/merges **SecDB** and provides the **apk-aware version comparison**. ## Why a custom APK version comparator? @@ -158,9 +157,8 @@ version, so nothing is flagged: Target: alpine:3.14 [*] Phase 1: Extracting Image... -Pulling and saving image 'alpine:3.14' (this might take a moment)... Pulling image 'alpine:3.14' from registry... -Unpacking image layers... +Saving image 'alpine:3.14' (using binary: /usr/local/bin/docker)... [*] Phase 2: Analyzing Layers... Found Alpine package database in layer: blobs/sha256/422ed46b1a92... -> Generated SBOM with 14 installed packages. @@ -189,9 +187,8 @@ reports a vulnerable `apk-tools` with its CVE, severity, and remediation: Target: alpine:3.10 [*] Phase 1: Extracting Image... -Pulling and saving image 'alpine:3.10' (this might take a moment)... Pulling image 'alpine:3.10' from registry... -Unpacking image layers... +Saving image 'alpine:3.10' (using binary: /usr/local/bin/docker)... [*] Phase 2: Analyzing Layers... Found Alpine package database in layer: blobs/sha256/26d14edc4f17... -> Generated SBOM with 14 installed packages. diff --git a/ROADMAP.md b/ROADMAP.md index 4a96e24..27da2dd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -6,7 +6,7 @@ Living document. Tick items off as they land; reorder when priorities change. - [x] **Real APK version comparison parity.** Current tokenizer in `internal/matcher/version.go` misses APK suffix semantics (`_alpha`, `_beta`, `_rc`, `_pre`, `_p`, `_git`). E.g. `1.2.2_pre2` should rank lower than `1.2.2`. Port logic from `apk-tools` `apk_version.c` and add a golden-file test suite. - [x] **Bounded tar extraction.** `internal/extractor/extractor.go` does unbounded `io.Copy` — malicious layer can fill disk (tar bomb). Wrap in `io.LimitReader` with configurable cap and reject symlinks/hardlinks pointing outside target. -- [ ] **Stream layers instead of extracting to disk.** Today we untar the whole image then walk layers again. Read layers directly from the saved tar stream — faster, lower disk footprint, removes temp dir need for most scans. +- [x] **Stream layers instead of extracting to disk.** Today we untar the whole image then walk layers again. Read layers directly from the saved tar stream — faster, lower disk footprint, removes temp dir need for most scans. - [ ] **SecDB caching.** On-disk cache keyed by `(version, repo, ETag/Last-Modified)` under `$XDG_CACHE_HOME/sentinel`. Adds offline support and cuts repeated network calls in CI. - [ ] **Context + cancellation.** Plumb `context.Context` through `extractor`, `analyzer`, `matcher`. A hung `docker pull` is currently unkillable. diff --git a/cmd/scanner/main.go b/cmd/scanner/main.go index 3e217f3..cbcc99f 100644 --- a/cmd/scanner/main.go +++ b/cmd/scanner/main.go @@ -72,7 +72,7 @@ func main() { fmt.Println("[*] Phase 1: Extracting Image...") } - extractedPath, cleanup, err := extractor.ExtractImage(*imageFlag, textOutput) + imageTarPath, cleanup, err := extractor.SaveImage(*imageFlag, extractor.Options{Verbose: textOutput}) if err != nil { log.Fatalf("Extraction failed: %v", err) } @@ -81,12 +81,8 @@ func main() { if textOutput { fmt.Println("[*] Phase 2: Analyzing Layers...") } - layers, err := analyzer.GetImageLayers(extractedPath) - if err != nil { - log.Fatalf("Failed to read manifest: %v", err) - } - sbom, err := analyzer.BuildSBOM(extractedPath, layers, textOutput) + sbom, err := analyzer.BuildSBOM(imageTarPath, extractor.New(), analyzer.Options{Verbose: textOutput}) if err != nil { log.Fatalf("Failed to build SBOM: %v", err) } diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 4d7820e..0220fa0 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -1,17 +1,20 @@ -// Package analyzer reads unpacked image layers to locate and parse the Alpine -// package database, producing the software bill of materials (SBOM). +// Package analyzer streams the layers of a saved Docker image tarball to locate +// and parse the Alpine package database, producing the software bill of +// materials (SBOM) without unpacking the image to disk. package analyzer import ( "archive/tar" "bufio" + "bytes" "compress/gzip" "encoding/json" "fmt" "io" "os" - "path/filepath" "strings" + + "sentinel-scanner/internal/extractor" ) // Manifest represents the structure of the Docker manifest.json file. @@ -21,26 +24,19 @@ type Manifest []struct { Layers []string `json:"Layers"` } -// GetImageLayers reads the manifest.json and returns the ordered list of layer tarballs. -func GetImageLayers(extractPath string) ([]string, error) { - manifestPath := filepath.Join(extractPath, "manifest.json") - - file, err := os.Open(manifestPath) - if err != nil { - return nil, fmt.Errorf("could not open manifest.json: %w", err) +// GetImageLayers decodes a Docker manifest.json and returns the ordered list of +// layer tarball paths. +func GetImageLayers(manifest io.Reader) ([]string, error) { + var m Manifest + if err := json.NewDecoder(manifest).Decode(&m); err != nil { + return nil, fmt.Errorf("parse manifest.json: %w", err) } - defer func() { _ = file.Close() }() - var manifest Manifest - if err := json.NewDecoder(file).Decode(&manifest); err != nil { - return nil, fmt.Errorf("failed to parse manifest.json: %w", err) - } - - if len(manifest) == 0 || len(manifest[0].Layers) == 0 { + if len(m) == 0 || len(m[0].Layers) == 0 { return nil, fmt.Errorf("invalid manifest: no layers found") } - return manifest[0].Layers, nil + return m[0].Layers, nil } // Package represents a single installed software package. @@ -49,80 +45,179 @@ type Package struct { Version string } -// BuildSBOM searches through the container layers to find the Alpine package database, -// parses it, and returns a list of installed packages (the SBOM). -func BuildSBOM(extractPath string, layers []string, verbose bool) ([]Package, error) { - var sbom []Package +// Options configures SBOM construction. +type Options struct { + // Verbose enables progress output to stdout. + Verbose bool +} - // Iterate through the layers in the exact order specified by the manifest - for _, layer := range layers { - // Optimization: We wrap the file logic in an anonymous function (a closure). - // This ensures 'defer file.Close()' runs immediately after each layer finishes processing, - // preventing a file-descriptor memory leak on massive images with 100+ layers. - err := func() error { - layerPath := filepath.Join(extractPath, layer) +const ( + manifestName = "manifest.json" + apkDBPath = "lib/apk/db/installed" + apkDBWhiteout = "lib/apk/db/.wh.installed" + apkDBOpaque = "lib/apk/db/.wh..wh..opq" +) - file, err := os.Open(layerPath) - if err != nil { - return fmt.Errorf("failed to open layer %s: %w", layer, err) - } - defer func() { _ = file.Close() }() // Safely closes at the end of this anonymous function +// layerResult records what a single layer did to the apk database: it either +// wrote a new copy (db != nil) or deleted it via an OCI whiteout. +type layerResult struct { + db []byte + whiteout bool +} - // We use bufio.Reader so we can "peek" ahead into the file stream without consuming it. - br := bufio.NewReader(file) +// BuildSBOM streams the layers of the saved Docker image at imageTarPath, finds +// the Alpine package database as it exists in the final merged filesystem, +// parses it, and returns the installed packages (the SBOM). Layers are never +// written to disk; ext supplies the size caps that bound the work against tar +// bombs, shared cumulatively across every layer. +func BuildSBOM(imageTarPath string, ext *extractor.Extractor, opts Options) ([]Package, error) { + walker := ext.NewStreamWalker() + + // Pass 1: locate and decode manifest.json. Its position in the outer archive + // is not guaranteed (Docker emits layer blobs before it), so we read the + // whole archive to find it. + layers, err := readLayers(imageTarPath, walker) + if err != nil { + return nil, err + } + layerSet := make(map[string]bool, len(layers)) + for _, l := range layers { + layerSet[l] = true + } - // Peek at the first 2 bytes to check for the GZIP magic signature - headerBytes, err := br.Peek(2) - if err != nil && err != io.EOF { - return err + // Pass 2: scan each layer blob for the apk database, keyed by layer name. + // We cannot resolve "last write wins" during the walk because the outer tar + // is not ordered by the manifest, so we record per-layer results and resolve + // afterwards. + results := make(map[string]layerResult, len(layers)) + scan := func(hdr *tar.Header, body io.Reader) error { + if !layerSet[hdr.Name] { + return nil // config blob or other non-layer entry + } + res, err := scanLayer(walker, body) + if err != nil { + return fmt.Errorf("scan layer %q: %w", hdr.Name, err) + } + if res.db != nil || res.whiteout { + results[hdr.Name] = res + if opts.Verbose && res.db != nil { + fmt.Println("Found Alpine package database in layer:", hdr.Name) } + } + return nil + } + if err := walkArchive(imageTarPath, walker, scan); err != nil { + return nil, err + } - var tr *tar.Reader - - // GZIP magic numbers are 0x1f and 0x8b - if len(headerBytes) == 2 && headerBytes[0] == 0x1f && headerBytes[1] == 0x8b { - // Layer is COMPRESSED. Wrap our stream in a gzip decompressor. - gzr, err := gzip.NewReader(br) - if err != nil { - return fmt.Errorf("failed to create gzip reader: %w", err) - } - defer func() { _ = gzr.Close() }() - tr = tar.NewReader(gzr) - } else { - // Layer is UNCOMPRESSED plain tar. - tr = tar.NewReader(br) - } + // Resolve in manifest order: the last layer to write the database wins, and + // a whiteout deletes any database carried from earlier layers. + var winning []byte + for _, l := range layers { + res, ok := results[l] + if !ok { + continue + } + if res.whiteout { + winning = nil + continue + } + winning = res.db + } - // Search inside the layer stream - for { - header, err := tr.Next() - if err == io.EOF { - break // End of this layer's tarball - } - if err != nil { - return err - } - - if header.Name == "lib/apk/db/installed" { - if verbose { - fmt.Println("Found Alpine package database in layer:", layer) - } - sbom = parseAlpineDB(tr) - } - } + // A whiteout (winning == nil) or an empty/unparseable database both yield + // zero packages; report that as "not found" rather than an empty SBOM, the + // same contract the disk-based path enforced. + sbom := parseAlpineDB(bytes.NewReader(winning)) + if len(sbom) == 0 { + return nil, fmt.Errorf("could not find %s in any layer (is this an Alpine image?)", apkDBPath) + } + + return sbom, nil +} + +// walkArchive opens the saved image tarball and walks its outer (uncompressed) +// tar stream with fn under the walker's shared size budget. +func walkArchive(imageTarPath string, walker *extractor.StreamWalker, fn extractor.WalkFunc) error { + file, err := os.Open(imageTarPath) + if err != nil { + return fmt.Errorf("open image tarball: %w", err) + } + defer func() { _ = file.Close() }() + + return walker.Walk(file, fn) +} + +// readLayers walks the outer archive to find manifest.json and returns its +// ordered layer list. +func readLayers(imageTarPath string, walker *extractor.StreamWalker) ([]string, error) { + var layers []string + var found bool + + err := walkArchive(imageTarPath, walker, func(hdr *tar.Header, body io.Reader) error { + if hdr.Name != manifestName { return nil - }() + } + l, err := GetImageLayers(body) + if err != nil { + return err + } + layers, found = l, true + return nil + }) + if err != nil { + return nil, err + } + if !found { + return nil, fmt.Errorf("could not find manifest.json in image tarball") + } + + return layers, nil +} + +// scanLayer descends into a single layer blob — gzip-compressed or plain tar — +// and reports whether it writes or deletes the apk database. The descent reuses +// walker so inner entries draw against the same cumulative size budget. +func scanLayer(walker *extractor.StreamWalker, body io.Reader) (layerResult, error) { + // Peek at the first 2 bytes to detect the gzip magic signature without + // consuming the stream. + br := bufio.NewReader(body) + magic, err := br.Peek(2) + if err != nil && err != io.EOF { + return layerResult{}, err + } + var src io.Reader = br + if len(magic) == 2 && magic[0] == 0x1f && magic[1] == 0x8b { + gzr, err := gzip.NewReader(br) if err != nil { - return nil, err + return layerResult{}, fmt.Errorf("gzip reader: %w", err) } + defer func() { _ = gzr.Close() }() + src = gzr } - if len(sbom) == 0 { - return nil, fmt.Errorf("could not find lib/apk/db/installed in any layer (is this an Alpine image?)") + var res layerResult + err = walker.Walk(src, func(hdr *tar.Header, entry io.Reader) error { + switch hdr.Name { + case apkDBPath: + b, err := io.ReadAll(entry) + if err != nil { + return fmt.Errorf("read apk database: %w", err) + } + res.db = b + res.whiteout = false + case apkDBWhiteout, apkDBOpaque: + res.db = nil + res.whiteout = true + } + return nil + }) + if err != nil { + return layerResult{}, err } - return sbom, nil + return res, nil } // parseAlpineDB reads the custom text format of the Alpine installed database diff --git a/internal/analyzer/analyzer_test.go b/internal/analyzer/analyzer_test.go new file mode 100644 index 0000000..c2da2c1 --- /dev/null +++ b/internal/analyzer/analyzer_test.go @@ -0,0 +1,385 @@ +package analyzer + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "sentinel-scanner/internal/extractor" +) + +// file is one entry inside a synthetic layer tarball. +type file struct { + name string + body string +} + +// layer is a synthetic image layer: a tar of files, optionally gzip-compressed, +// stored in the outer archive under blobName. +type layer struct { + blobName string + gzip bool + files []file +} + +// layerTar builds a single layer's tarball, gzipped when requested. +func layerTar(t *testing.T, l layer) []byte { + t.Helper() + + var raw bytes.Buffer + tw := tar.NewWriter(&raw) + for _, f := range l.files { + hdr := &tar.Header{Name: f.name, Typeflag: tar.TypeReg, Size: int64(len(f.body)), Mode: 0o644} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("write layer header %q: %v", f.name, err) + } + if _, err := tw.Write([]byte(f.body)); err != nil { + t.Fatalf("write layer body %q: %v", f.name, err) + } + } + if err := tw.Close(); err != nil { + t.Fatalf("close layer tar: %v", err) + } + + if !l.gzip { + return raw.Bytes() + } + + var gzbuf bytes.Buffer + gw := gzip.NewWriter(&gzbuf) + if _, err := gw.Write(raw.Bytes()); err != nil { + t.Fatalf("gzip layer: %v", err) + } + if err := gw.Close(); err != nil { + t.Fatalf("close gzip: %v", err) + } + return gzbuf.Bytes() +} + +// imageTar builds a saved-image tarball (the outer, uncompressed archive) on +// disk from the given layers and manifest layer order, and returns its path. +// manifestFirst controls whether manifest.json precedes or follows the layer +// blobs in the outer archive, exercising both orderings. +func imageTar(t *testing.T, layers []layer, manifestOrder []string, manifestFirst bool) string { + t.Helper() + + manifest := Manifest{{Config: "config.json", Layers: manifestOrder}} + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatalf("marshal manifest: %v", err) + } + + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + + writeEntry := func(name string, body []byte) { + hdr := &tar.Header{Name: name, Typeflag: tar.TypeReg, Size: int64(len(body)), Mode: 0o644} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("write outer header %q: %v", name, err) + } + if _, err := tw.Write(body); err != nil { + t.Fatalf("write outer body %q: %v", name, err) + } + } + + if manifestFirst { + writeEntry(manifestName, manifestJSON) + } + for _, l := range layers { + writeEntry(l.blobName, layerTar(t, l)) + } + if !manifestFirst { + writeEntry(manifestName, manifestJSON) + } + + if err := tw.Close(); err != nil { + t.Fatalf("close outer tar: %v", err) + } + + path := filepath.Join(t.TempDir(), "image.tar") + if err := os.WriteFile(path, buf.Bytes(), 0o600); err != nil { + t.Fatalf("write image tar: %v", err) + } + return path +} + +const apkDB = "C:Q1\nP:musl\nV:1.2.4-r0\n\nC:Q2\nP:busybox\nV:1.36.1-r0\n" + +func TestBuildSBOM(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + layers []layer + manifestOrder []string + manifestFirst bool + want []Package + wantErr string + }{ + { + name: "single plain layer with apk db", + layers: []layer{ + {blobName: "l1", files: []file{{name: apkDBPath, body: apkDB}}}, + }, + manifestOrder: []string{"l1"}, + manifestFirst: true, + want: []Package{ + {Name: "musl", Version: "1.2.4-r0"}, + {Name: "busybox", Version: "1.36.1-r0"}, + }, + }, + { + name: "gzip-compressed layer", + layers: []layer{ + {blobName: "l1", gzip: true, files: []file{{name: apkDBPath, body: apkDB}}}, + }, + manifestOrder: []string{"l1"}, + manifestFirst: true, + want: []Package{ + {Name: "musl", Version: "1.2.4-r0"}, + {Name: "busybox", Version: "1.36.1-r0"}, + }, + }, + { + name: "manifest after layers in outer tar", + layers: []layer{ + {blobName: "l1", files: []file{{name: apkDBPath, body: apkDB}}}, + }, + manifestOrder: []string{"l1"}, + manifestFirst: false, + want: []Package{ + {Name: "musl", Version: "1.2.4-r0"}, + {Name: "busybox", Version: "1.36.1-r0"}, + }, + }, + { + name: "later layer rewrites db wins in manifest order", + layers: []layer{ + {blobName: "l1", files: []file{{name: apkDBPath, body: "P:musl\nV:1.0.0-r0\n"}}}, + {blobName: "l2", files: []file{{name: apkDBPath, body: "P:musl\nV:9.9.9-r0\n"}}}, + }, + manifestOrder: []string{"l1", "l2"}, + manifestFirst: true, + want: []Package{{Name: "musl", Version: "9.9.9-r0"}}, + }, + { + name: "winner follows manifest order not tar order", + // l2 (the manifest's last layer) carries the winning db, but it is + // written to the outer archive BEFORE l1 — proving resolution uses + // manifest order, not the order blobs appear in the tar. + layers: []layer{ + {blobName: "l2", files: []file{{name: apkDBPath, body: "P:musl\nV:9.9.9-r0\n"}}}, + {blobName: "l1", files: []file{{name: apkDBPath, body: "P:musl\nV:1.0.0-r0\n"}}}, + }, + manifestOrder: []string{"l1", "l2"}, + manifestFirst: true, + want: []Package{{Name: "musl", Version: "9.9.9-r0"}}, + }, + { + name: "whiteout in later layer deletes db", + layers: []layer{ + {blobName: "l1", files: []file{{name: apkDBPath, body: apkDB}}}, + {blobName: "l2", files: []file{{name: apkDBWhiteout, body: ""}}}, + }, + manifestOrder: []string{"l1", "l2"}, + manifestFirst: true, + wantErr: "could not find", + }, + { + name: "opaque whiteout in later layer deletes db", + layers: []layer{ + {blobName: "l1", files: []file{{name: apkDBPath, body: apkDB}}}, + {blobName: "l2", files: []file{{name: apkDBOpaque, body: ""}}}, + }, + manifestOrder: []string{"l1", "l2"}, + manifestFirst: true, + wantErr: "could not find", + }, + { + name: "db readded after whiteout", + layers: []layer{ + {blobName: "l1", files: []file{{name: apkDBPath, body: "P:musl\nV:1.0.0-r0\n"}}}, + {blobName: "l2", files: []file{{name: apkDBWhiteout, body: ""}}}, + {blobName: "l3", files: []file{{name: apkDBPath, body: "P:musl\nV:2.0.0-r0\n"}}}, + }, + manifestOrder: []string{"l1", "l2", "l3"}, + manifestFirst: true, + want: []Package{{Name: "musl", Version: "2.0.0-r0"}}, + }, + { + name: "no apk db anywhere", + layers: []layer{ + {blobName: "l1", files: []file{{name: "etc/hostname", body: "host"}}}, + }, + manifestOrder: []string{"l1"}, + manifestFirst: true, + wantErr: "could not find", + }, + { + name: "empty apk db yields not-found, not empty sbom", + layers: []layer{ + {blobName: "l1", files: []file{{name: apkDBPath, body: ""}}}, + }, + manifestOrder: []string{"l1"}, + manifestFirst: true, + wantErr: "could not find", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + path := imageTar(t, tc.layers, tc.manifestOrder, tc.manifestFirst) + got, err := BuildSBOM(path, extractor.New(), Options{}) + + if tc.wantErr != "" { + if err == nil { + t.Fatalf("BuildSBOM succeeded, want error containing %q", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("BuildSBOM error = %q, want substring %q", err.Error(), tc.wantErr) + } + return + } + + if err != nil { + t.Fatalf("BuildSBOM returned error: %v", err) + } + if !equalPackages(got, tc.want) { + t.Fatalf("BuildSBOM packages = %v, want %v", got, tc.want) + } + }) + } +} + +// TestBuildSBOM_OversizedEntryTrippedCap confirms the streaming path still +// enforces the per-entry size cap inherited from the extractor: an apk database +// larger than the cap fails rather than being buffered whole. +func TestBuildSBOM_OversizedEntryTrippedCap(t *testing.T) { + t.Parallel() + + big := strings.Repeat("P:x\nV:1\n", 200) + path := imageTar(t, + []layer{{blobName: "l1", files: []file{{name: apkDBPath, body: big}}}}, + []string{"l1"}, true) + + ext := &extractor.Extractor{FileSizeCap: 16, TotalSizeCap: extractor.DefaultTotalSizeCap} + if _, err := BuildSBOM(path, ext, Options{}); err == nil { + t.Fatal("BuildSBOM succeeded, want per-entry cap error") + } +} + +func TestGetImageLayers(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + want []string + wantErr string + }{ + { + name: "valid manifest", + input: `[{"Config":"c.json","RepoTags":["alpine:3.19"],"Layers":["a","b"]}]`, + want: []string{"a", "b"}, + }, + { + name: "empty manifest array", + input: `[]`, + wantErr: "no layers found", + }, + { + name: "no layers", + input: `[{"Config":"c.json","Layers":[]}]`, + wantErr: "no layers found", + }, + { + name: "malformed json", + input: `{not json`, + wantErr: "parse manifest.json", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := GetImageLayers(strings.NewReader(tc.input)) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("GetImageLayers error = %v, want substring %q", err, tc.wantErr) + } + return + } + if err != nil { + t.Fatalf("GetImageLayers returned error: %v", err) + } + if strings.Join(got, ",") != strings.Join(tc.want, ",") { + t.Fatalf("GetImageLayers = %v, want %v", got, tc.want) + } + }) + } +} + +func TestParseAlpineDB(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + want []Package + }{ + { + name: "two packages", + input: "P:musl\nV:1.2.4-r0\n\nP:busybox\nV:1.36.1-r0\n", + want: []Package{ + {Name: "musl", Version: "1.2.4-r0"}, + {Name: "busybox", Version: "1.36.1-r0"}, + }, + }, + { + name: "version without name is skipped until paired", + input: "P:zlib\nV:1.3-r0\n", + want: []Package{{Name: "zlib", Version: "1.3-r0"}}, + }, + { + name: "name without version produces nothing", + input: "P:orphan\n", + want: nil, + }, + { + name: "empty input", + input: "", + want: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := parseAlpineDB(strings.NewReader(tc.input)) + if !equalPackages(got, tc.want) { + t.Fatalf("parseAlpineDB = %v, want %v", got, tc.want) + } + }) + } +} + +func equalPackages(a, b []Package) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/internal/extractor/extractor.go b/internal/extractor/extractor.go index 4f15559..a412364 100644 --- a/internal/extractor/extractor.go +++ b/internal/extractor/extractor.go @@ -1,5 +1,6 @@ -// Package extractor saves a Docker image via the docker CLI and unpacks its -// layers into a temporary directory for analysis. +// Package extractor saves a Docker image via the docker CLI and provides +// primitives for reading its layers — either unpacking them to disk or walking +// them in memory — under tar-bomb size guards. package extractor import ( @@ -12,36 +13,38 @@ import ( "strings" ) -// ExtractImage saves the docker image to a temporary directory and unpacks it. -// It returns the path to the unpacked directory, and a cleanup function. -func ExtractImage(imageName string, verbose bool) (string, func(), error) { +// Options configures image extraction. +type Options struct { + // Verbose enables progress output to stdout. + Verbose bool +} + +// SaveImage pulls imageName and writes it to a tarball via `docker save`, +// returning the path to that tarball and a cleanup function that removes the +// temporary directory holding it. The image is not unpacked; callers stream its +// layers directly from the saved tar (see Extractor.NewStreamWalker). +func SaveImage(imageName string, opts Options) (string, func(), error) { // 1. Create a secure temporary directory tempDir, err := os.MkdirTemp("", "sentinel-scanner-*") if err != nil { - return "", nil, fmt.Errorf("failed to create temp dir: %w", err) + return "", nil, fmt.Errorf("create temp dir: %w", err) } // Setup our cleanup function to wipe the temp files when we are done cleanup := func() { _ = os.RemoveAll(tempDir) - if verbose { + if opts.Verbose { fmt.Println("Cleaned up temporary files.") } } tarPath := filepath.Join(tempDir, "image.tar") - extractPath := filepath.Join(tempDir, "unpacked") - - // 2. Shell out to Docker to save the image - if verbose { - fmt.Printf("Pulling and saving image '%s' (this might take a moment)...\n", imageName) - } // Find the docker binary safely dockerBinary := getDockerPath() - // 1.5. Pull the image from the registry to ensure it exists locally - if verbose { + // Pull the image from the registry to ensure it exists locally + if opts.Verbose { fmt.Printf("Pulling image '%s' from registry...\n", imageName) } pullCmd := exec.Command(dockerBinary, "pull", imageName) @@ -49,9 +52,9 @@ func ExtractImage(imageName string, verbose bool) (string, func(), error) { return "", cleanup, fmt.Errorf("docker pull failed: %v\nOutput: %s", err, output) } - // 2. Shell out to Docker to save the image - if verbose { - fmt.Printf("Pulling and saving image '%s' (using binary: %s)...\n", imageName, dockerBinary) + // Shell out to Docker to save the image + if opts.Verbose { + fmt.Printf("Saving image '%s' (using binary: %s)...\n", imageName, dockerBinary) } cmd := exec.Command(dockerBinary, "save", "-o", tarPath, imageName) @@ -60,19 +63,7 @@ func ExtractImage(imageName string, verbose bool) (string, func(), error) { return "", cleanup, fmt.Errorf("docker save failed: %v\nOutput: %s", err, output) } - // 3. Unpack the tarball - if err := os.Mkdir(extractPath, 0750); err != nil { - return "", cleanup, fmt.Errorf("failed to create extract dir: %w", err) - } - - if verbose { - fmt.Println("Unpacking image layers...") - } - if err := New().Untar(tarPath, extractPath); err != nil { - return "", cleanup, fmt.Errorf("failed to untar image: %w", err) - } - - return extractPath, cleanup, nil + return tarPath, cleanup, nil } // DefaultFileSizeCap is the maximum number of bytes extracted for a single tar @@ -224,6 +215,111 @@ func (e *Extractor) Untar(tarball, targetDir string) error { return nil } +// WalkFunc is invoked for each regular-file entry encountered by +// StreamWalker.Walk. body is capped at the per-entry size limit; reading past it +// causes the walk to fail. Returning a non-nil error stops the walk and is +// propagated to the caller. body must not be retained after fn returns. +type WalkFunc func(hdr *tar.Header, body io.Reader) error + +// StreamWalker walks one or more tar streams under a single shared cumulative +// size budget, enforcing the same per-entry and total caps as Untar without +// writing anything to disk. A walker is created per image so the cumulative cap +// spans every layer (and the bytes drawn descending into them), mirroring the +// whole-image budget the disk-based Untar enforced. +type StreamWalker struct { + ext *Extractor + total int64 +} + +// NewStreamWalker returns a StreamWalker that draws against the extractor's +// configured caps. Reuse one walker across an image's layers so the total cap +// is cumulative; create a fresh walker per image. +func (e *Extractor) NewStreamWalker() *StreamWalker { + return &StreamWalker{ext: e} +} + +// Walk reads the tar stream r and calls fn for each regular-file entry. Entry +// names that are absolute or contain a ".." path component are rejected. Each +// body fn receives is bounded by the per-entry cap, and every byte fn reads — +// at this level or any nested Walk sharing this StreamWalker — draws down the +// shared total budget. Non-regular entries (directories, links) are skipped; +// because nothing is written to disk, their link targets need no resolution. +func (w *StreamWalker) Walk(r io.Reader, fn WalkFunc) error { + tr := tar.NewReader(r) + fileCap := w.ext.fileSizeCap() + totalCap := w.ext.totalSizeCap() + + for { + header, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return err + } + + // Reject path traversal up front. Unlike Untar's coarse substring check + // (which writes to disk and so errs on the side of rejection), a stream + // walk only matches entry names, so we reject a ".." path component + // precisely — this lets through legitimate OCI whiteout markers such as + // ".wh..wh..opq" whose names embed "..". + if filepath.IsAbs(header.Name) || hasDotDotComponent(header.Name) { + return fmt.Errorf("walk entry %q: path traversal detected", header.Name) + } + + if header.Typeflag != tar.TypeReg { + continue + } + + // Reject any single entry whose declared size blows the per-entry cap + // before we read or skip it. + if header.Size > fileCap { + return fmt.Errorf("walk entry %q: file size exceeds cap of %d bytes", header.Name, fileCap) + } + + // Cap the body at fileCap+1 so a callback that reads it fully can tell + // "fit exactly" from "overflowed", and count every consumed byte against + // the shared cumulative budget. + body := &countingReader{r: io.LimitReader(tr, fileCap+1), total: &w.total} + if err := fn(header, body); err != nil { + return err + } + if body.n > fileCap { + return fmt.Errorf("walk entry %q: file size exceeds cap of %d bytes", header.Name, fileCap) + } + if w.total > totalCap { + return fmt.Errorf("walk entry %q: total extraction size exceeds cap of %d bytes", header.Name, totalCap) + } + } + return nil +} + +// countingReader tracks how many bytes have been read both for the current +// entry (n) and across the whole walk (the shared total). +type countingReader struct { + r io.Reader + n int64 + total *int64 +} + +func (c *countingReader) Read(p []byte) (int, error) { + n, err := c.r.Read(p) + c.n += int64(n) + *c.total += int64(n) + return n, err +} + +// hasDotDotComponent reports whether name contains a ".." path component, +// treating "/" as the separator regardless of host OS (tar paths are slash-based). +func hasDotDotComponent(name string) bool { + for _, part := range strings.Split(filepath.ToSlash(name), "/") { + if part == ".." { + return true + } + } + return false +} + // isSafePath reports whether joining root and name stays within root after // cleaning, i.e. the entry does not escape the extraction directory. func isSafePath(root, name string) bool { diff --git a/internal/extractor/extractor_test.go b/internal/extractor/extractor_test.go index 45624af..f1f1b04 100644 --- a/internal/extractor/extractor_test.go +++ b/internal/extractor/extractor_test.go @@ -3,6 +3,8 @@ package extractor import ( "archive/tar" "bytes" + "fmt" + "io" "os" "path/filepath" "strings" @@ -54,6 +56,200 @@ func writeTar(t *testing.T, dir string, entries []entry) string { return path } +// member is a synthetic tar entry carrying a body, used to build archives +// in-memory for streaming tests. +type member struct { + name string + typeflag byte + body []byte +} + +// tarBytes builds a tarball in memory and returns its bytes. +func tarBytes(t *testing.T, members []member) []byte { + t.Helper() + + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + for _, m := range members { + flag := m.typeflag + if flag == 0 { + flag = tar.TypeReg + } + hdr := &tar.Header{Name: m.name, Typeflag: flag, Size: int64(len(m.body)), Mode: 0o644} + if flag == tar.TypeDir { + hdr.Mode = 0o755 + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("write header %q: %v", m.name, err) + } + if len(m.body) > 0 { + if _, err := tw.Write(m.body); err != nil { + t.Fatalf("write body %q: %v", m.name, err) + } + } + } + if err := tw.Close(); err != nil { + t.Fatalf("close tar writer: %v", err) + } + return buf.Bytes() +} + +func TestWalkStream_DeliversRegularFiles(t *testing.T) { + t.Parallel() + + archive := tarBytes(t, []member{ + {name: "dir/", typeflag: tar.TypeDir}, + {name: "dir/a.txt", body: []byte("alpha")}, + {name: "link", typeflag: tar.TypeSymlink}, + {name: "b.txt", body: []byte("bravo")}, + // A legitimate OCI opaque whiteout marker: its name embeds ".." but has + // no ".." path component, so it must be delivered, not rejected. + {name: "lib/apk/db/.wh..wh..opq"}, + }) + + got := make(map[string]string) + w := New().NewStreamWalker() + err := w.Walk(bytes.NewReader(archive), func(hdr *tar.Header, body io.Reader) error { + b, err := io.ReadAll(body) + if err != nil { + return err + } + got[hdr.Name] = string(b) + return nil + }) + if err != nil { + t.Fatalf("Walk returned error: %v", err) + } + + want := map[string]string{ + "dir/a.txt": "alpha", + "b.txt": "bravo", + "lib/apk/db/.wh..wh..opq": "", + } + if len(got) != len(want) { + t.Fatalf("delivered %d regular entries, want %d (%v)", len(got), len(want), got) + } + for name, body := range want { + if got[name] != body { + t.Errorf("entry %q body = %q, want %q", name, got[name], body) + } + } +} + +func TestWalkStream_RejectedEntries(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + members []member + fileCap int64 + totalCap int64 + readBody bool // whether the callback consumes each body (for total cap) + wantErr string + }{ + { + name: "path traversal via ..", + members: []member{{name: "../evil.txt", body: []byte("x")}}, + wantErr: "path traversal", + }, + { + name: "absolute path", + members: []member{{name: "/etc/passwd", body: []byte("x")}}, + wantErr: "path traversal", + }, + { + name: "nested dotdot component", + members: []member{{name: "a/../../b", body: []byte("x")}}, + wantErr: "path traversal", + }, + { + name: "single entry over file cap", + members: []member{{name: "big.txt", body: make([]byte, 1025)}}, + fileCap: 1024, + wantErr: "file size exceeds cap", + }, + { + name: "cumulative over total cap", + members: []member{ + {name: "a.txt", body: make([]byte, 1024)}, + {name: "b.txt", body: make([]byte, 1024)}, + }, + fileCap: 1024, + totalCap: 1024, + readBody: true, + wantErr: "total extraction size exceeds cap", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + archive := tarBytes(t, tc.members) + e := &Extractor{FileSizeCap: tc.fileCap, TotalSizeCap: tc.totalCap} + err := e.NewStreamWalker().Walk(bytes.NewReader(archive), func(_ *tar.Header, body io.Reader) error { + if tc.readBody { + _, err := io.Copy(io.Discard, body) + return err + } + return nil + }) + if err == nil { + t.Fatalf("Walk succeeded, want error containing %q", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("Walk error = %q, want substring %q", err.Error(), tc.wantErr) + } + }) + } +} + +// TestWalkStream_TotalCapSpansNestedWalks locks in the whole-image cumulative +// budget: bytes read while descending into one stream and then a second, via +// the same StreamWalker, accumulate against a single total cap. +func TestWalkStream_TotalCapSpansNestedWalks(t *testing.T) { + t.Parallel() + + inner := tarBytes(t, []member{{name: "file", body: make([]byte, 1024)}}) + outer := tarBytes(t, []member{ + {name: "layer-a", body: inner}, + {name: "layer-b", body: inner}, + }) + + // Per-entry cap is generous; the total cap is only large enough for one + // inner file's worth of bytes, so reading both layers must trip it. + e := &Extractor{FileSizeCap: 1 << 20, TotalSizeCap: 1500} + w := e.NewStreamWalker() + err := w.Walk(bytes.NewReader(outer), func(_ *tar.Header, body io.Reader) error { + // Descend into the layer with the SAME walker so the budget is shared. + return w.Walk(body, func(_ *tar.Header, entry io.Reader) error { + _, err := io.Copy(io.Discard, entry) + return err + }) + }) + if err == nil { + t.Fatal("Walk succeeded, want cumulative total cap error across nested walks") + } + if !strings.Contains(err.Error(), "total extraction size exceeds cap") { + t.Fatalf("Walk error = %q, want total cap error", err.Error()) + } +} + +// TestWalkStream_PropagatesCallbackError confirms a callback error stops the +// walk and surfaces unchanged. +func TestWalkStream_PropagatesCallbackError(t *testing.T) { + t.Parallel() + + archive := tarBytes(t, []member{{name: "a.txt", body: []byte("x")}}) + sentinel := fmt.Errorf("boom") + err := New().NewStreamWalker().Walk(bytes.NewReader(archive), func(_ *tar.Header, _ io.Reader) error { + return sentinel + }) + if err != sentinel { + t.Fatalf("Walk error = %v, want %v", err, sentinel) + } +} + func TestUntar_SafeEntries(t *testing.T) { t.Parallel()