diff --git a/internal/builder/upstream_file.go b/internal/builder/upstream_file.go index d5ef52ee..f6ece406 100644 --- a/internal/builder/upstream_file.go +++ b/internal/builder/upstream_file.go @@ -39,6 +39,22 @@ import ( // unpacks no archive larger than this either (mesh-host internal/apply/archive.go, maxArchive). const maxUpstream = 512 << 20 +// maxEntries is the most entries an upstream archive may hold: millions of empty ones would fill the build +// seat's inodes and the node's. Betterbird's release holds 64. +const maxEntries = 100_000 + +// noDowngrade refuses a redirect off https: the digest still guards the bytes, and a fetch in the clear says +// to whoever is on the path what the build seat fetches. +func noDowngrade(req *http.Request, via []*http.Request) error { + if req.URL.Scheme != "https" { + return fmt.Errorf("%s redirected to %s, which is not https", via[0].URL, req.URL) + } + if len(via) >= 10 { + return errors.New("more than 10 redirects") + } + return nil +} + // upstreamClient fetches upstream files: a variable so a test serves its own. var upstreamClient = &http.Client{Timeout: 15 * time.Minute} @@ -98,7 +114,9 @@ func upstreamFile(ctx context.Context, workspace string, a catalogue.Artifact, if err != nil { return "", err } - response, err := upstreamClient.Do(request) + client := *upstreamClient + client.CheckRedirect = noDowngrade + response, err := client.Do(request) if err != nil { return "", fmt.Errorf("cannot fetch %s: %w", a.URL, err) } @@ -213,6 +231,10 @@ func unpackUpstream(ctx context.Context, file, into, within string) error { return err } if wait != nil { + // Whatever follows the tar's end is read before xz is waited for: xz blocks on a full pipe otherwise. + if _, err := io.Copy(io.Discard, stream); err != nil { + return err + } w := wait wait = nil return w() @@ -228,7 +250,7 @@ func untar(stream io.Reader, into, within string) error { prefix = path.Clean(within) + "/" } var total int64 - files := 0 + files, entries := 0, 0 for { header, err := reader.Next() if err == io.EOF { @@ -237,11 +259,20 @@ func untar(stream io.Reader, into, within string) error { if err != nil { return err } + if entries++; entries > maxEntries { + return fmt.Errorf("it holds more than %d entries", maxEntries) + } + if header.Typeflag == tar.TypeXGlobalHeader { + // A pax global header (git archive writes one) names no file. + continue + } name := path.Clean(strings.TrimPrefix(header.Name, "./")) if path.IsAbs(name) || name == ".." || strings.HasPrefix(name, "../") { return fmt.Errorf("%s names a path outside the archive", header.Name) } - if name == "." || name+"/" == prefix { + if name == "." || name+"/" == prefix || + (header.Typeflag == tar.TypeDir && strings.HasPrefix(prefix, name+"/")) { + // The archive's root, the directory taken, or one of its parents. continue } if prefix != "" { diff --git a/internal/builder/upstream_file_test.go b/internal/builder/upstream_file_test.go index 6e233dce..c2b61a91 100644 --- a/internal/builder/upstream_file_test.go +++ b/internal/builder/upstream_file_test.go @@ -7,7 +7,9 @@ import ( "context" "crypto/sha256" "encoding/hex" + "fmt" "io" + "net" "net/http" "net/http/httptest" "os" @@ -16,6 +18,7 @@ import ( "sort" "strings" "testing" + "time" "github.com/novox/mesh-controller/internal/catalogue" ) @@ -97,9 +100,16 @@ func serveVendor(t *testing.T, v *vendor) string { })) t.Cleanup(server.Close) before := upstreamClient - upstreamClient = server.Client() + // Under a public name, as a manifest must say it: example.com, which the test server's certificate + // names, dialled at the server. + transport := server.Client().Transport.(*http.Transport).Clone() + addr := server.Listener.Addr().String() + transport.DialContext = func(ctx context.Context, network, _ string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, network, addr) + } + upstreamClient = &http.Client{Transport: transport} t.Cleanup(func() { upstreamClient = before }) - return server.URL + return "https://example.com" } // entriesOf lists a packed archive: each file with its mode and content. @@ -297,3 +307,79 @@ func TestAnUpstreamArchiveCompressedWithXzIsRead(t *testing.T) { t.Fatalf("the xz release unpacked to %v", names) } } + +func TestAnUpstreamTarWithAGlobalHeaderNestedWithinAndTrailingDataIsRead(t *testing.T) { + // git archive writes a pax global header; a release may unpack under a/b with a/ listed first; xz may be + // followed by bytes after the tar's end, which must be read before xz is waited for. + var raw bytes.Buffer + w := tar.NewWriter(&raw) + w.WriteHeader(&tar.Header{Typeflag: tar.TypeXGlobalHeader, Name: "pax_global_header", PAXRecords: map[string]string{"comment": "abc"}}) + w.WriteHeader(&tar.Header{Name: "a/", Typeflag: tar.TypeDir, Mode: 0o755}) + w.WriteHeader(&tar.Header{Name: "a/b/", Typeflag: tar.TypeDir, Mode: 0o755}) + w.WriteHeader(&tar.Header{Name: "a/b/f", Typeflag: tar.TypeReg, Mode: 0o644, Size: 1}) + w.Write([]byte("x")) + w.Close() + body := gzipped(t, raw.Bytes()) + v := &vendor{files: map[string][]byte{"/r.tar.gz": body}} + base := serveVendor(t, v) + packed, err := upstreamArchive(context.Background(), t.TempDir(), catalogue.Artifact{Name: "a", Kind: "archive", + URL: base + "/r.tar.gz", SHA256: sumHex(body), Version: "1", Within: "a/b"}, func(string, string, ...any) {}) + if err != nil { + t.Fatal(err) + } + if e := entriesOf(t, packed); len(e) != 1 || e["f"] == "" { + t.Fatalf("%v", e) + } + if _, err := exec.LookPath("xz"); err != nil { + t.Skip("NOT TESTED here: no xz") + } + // A megabyte after the tar's end: more than a pipe holds. + trailing := append(append([]byte{}, raw.Bytes()...), bytes.Repeat([]byte{0}, 1<<20)...) + cmd := exec.Command("xz", "-zc") + cmd.Stdin = bytes.NewReader(trailing) + compressed, err := cmd.Output() + if err != nil { + t.Fatal(err) + } + v.files["/r.tar.xz"] = compressed + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + if _, err := upstreamArchive(ctx, t.TempDir(), catalogue.Artifact{Name: "a", Kind: "archive", URL: base + "/r.tar.xz", + SHA256: sumHex(compressed), Version: "1", Within: "a/b"}, func(string, string, ...any) {}); err != nil { + t.Fatal(err) + } +} + +func TestAnUpstreamArchiveWithTooManyEntriesIsRefused(t *testing.T) { + var raw bytes.Buffer + w := tar.NewWriter(&raw) + for i := 0; i <= maxEntries; i++ { + w.WriteHeader(&tar.Header{Name: fmt.Sprintf("d%d/", i), Typeflag: tar.TypeDir, Mode: 0o755}) + } + w.Close() + body := gzipped(t, raw.Bytes()) + v := &vendor{files: map[string][]byte{"/r.tar.gz": body}} + base := serveVendor(t, v) + _, err := upstreamArchive(context.Background(), t.TempDir(), catalogue.Artifact{Name: "a", Kind: "archive", + URL: base + "/r.tar.gz", SHA256: sumHex(body), Version: "1"}, func(string, string, ...any) {}) + if err == nil || !strings.Contains(err.Error(), "entries") { + t.Fatalf("%v", err) + } +} + +func TestARedirectOffHttpsIsNotFollowed(t *testing.T) { + plain := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("x")) })) + t.Cleanup(plain.Close) + secure := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, plain.URL+"/r", http.StatusFound) + })) + t.Cleanup(secure.Close) + before := upstreamClient + upstreamClient = secure.Client() + t.Cleanup(func() { upstreamClient = before }) + _, err := upstreamFile(context.Background(), t.TempDir(), catalogue.Artifact{Name: "a", URL: secure.URL + "/r", + SHA256: sumHex([]byte("x")), Version: "1", File: "a"}, func(string, string, ...any) {}) + if err == nil || !strings.Contains(err.Error(), "not https") { + t.Fatalf("%v", err) + } +} diff --git a/internal/catalogue/build.go b/internal/catalogue/build.go index 74815e39..8341916b 100644 --- a/internal/catalogue/build.go +++ b/internal/catalogue/build.go @@ -2,6 +2,7 @@ package catalogue import ( "fmt" + "net" "net/url" "path" "regexp" @@ -357,6 +358,27 @@ func (b *Build) problems(module string) []string { return problems } +// reservedSuffixes are names no public vendor publishes under: an installation's own and documentation's. +var reservedSuffixes = []string{".internal", ".local", ".lan", ".home", ".corp", ".localhost", ".home.arpa", + ".test", ".invalid", ".example", ".intranet", ".private"} + +// notAPublicHost says why a host is not a public vendor's name, or "". +func notAPublicHost(host string) string { + h := strings.ToLower(strings.TrimSuffix(host, ".")) + if net.ParseIP(h) != nil { + return "an address rather than a name" + } + if !strings.Contains(h, ".") { + return "a name with no domain" + } + for _, s := range reservedSuffixes { + if strings.HasSuffix(h, s) { + return "a name under " + s + ", which is nobody's public domain" + } + } + return "" +} + // sha256Hex is a pinned sha256 as a manifest writes it: 64 lower-case hexadecimal characters. var sha256Hex = regexp.MustCompile(`^[0-9a-f]{64}$`) @@ -386,6 +408,8 @@ func upstreamFileProblems(module string, a Artifact) []string { } if u, err := url.Parse(a.URL); err != nil || u.Scheme != "https" || u.Host == "" { say("is fetched from %q, and an upstream file is fetched over https from a named host", a.URL) + } else if why := notAPublicHost(u.Hostname()); why != "" { + say("is fetched from %s, %s: an upstream file is somebody else's release, published under a public name", a.URL, why) } if !sha256Hex.MatchString(a.SHA256) { say("is fetched from %s without a pinned sha256 (64 lower-case hexadecimal characters): what that "+ diff --git a/internal/catalogue/installation.go b/internal/catalogue/installation.go index 94733e05..b257806b 100644 --- a/internal/catalogue/installation.go +++ b/internal/catalogue/installation.go @@ -71,9 +71,8 @@ var hostname = regexp.MustCompile( `(?:be|nl|de|fr|uk|eu|com|net|org|io|dev|app|cloud|site|online|me|co|ch|at|lu|` + `internal|example|tld|test|invalid)`) -// upstreamPin is where an artifact pinned by URL says where it is published and which release it is: the -// vendor's address and the vendor's version, which name no installation (Artifact.URL). -var upstreamPin = regexp.MustCompile(`^build\.artifacts\[[0-9]+\]\.(url|version)$`) +// upstreamVersion is where an artifact pinned by URL says which release it is (Artifact.Version). +var upstreamVersion = regexp.MustCompile(`^build\.artifacts\[[0-9]+\]\.version$`) // address is a dotted quad. var address = regexp.MustCompile(`(?:[0-9]{1,3}\.){3}[0-9]{1,3}`) @@ -117,11 +116,6 @@ func InstallationProblems(m Manifest) []string { "%s is named after %s, and a module is named for what it is, not for where it runs", m.Module, name)) } walk(tree, "", nil, func(at string, value string, meant map[string]bool, isImage bool) { - // An archive pinned by URL names the vendor that publishes it, and the version it pins, which may - // have four dotted numbers (novox/hq ADR 0316): the world's, as a public registry is an image's. - if upstreamPin.MatchString(at) { - return - } for _, name := range namesIn(value) { if (isImage && worldsRegistries[strings.ToLower(name)]) || meant[name] { continue @@ -129,7 +123,10 @@ func InstallationProblems(m Manifest) []string { problems = append(problems, fmt.Sprintf("%s names %s at %s", m.Module, name, at)) } for _, ip := range addressesIn(value) { - if meant[ip] { + // An upstream pin's version may have four dotted numbers (Owl 1.5.6.1), which is no address; a name + // in it is still judged above. Its URL is judged whole: the vendor's host goes through the artifact's + // names-on-purpose like any name a definition means (novox/hq ADR 0316). + if meant[ip] || upstreamVersion.MatchString(at) { continue } problems = append(problems, fmt.Sprintf("%s names the public address %s at %s", m.Module, ip, at)) diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 6ba3f51c..35295a23 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -1001,6 +1001,9 @@ type Artifact struct { // release that unpacks into a directory of its own name (`betterbird/`). An entry outside it refuses // the build. Empty means the archive's root. Within string `json:"within,omitempty"` + // NamesOnPurpose are the names this artifact means to name, each with why: the vendor's host an upstream + // file is fetched from, which the installation check judges like any name (InstallationProblems). + NamesOnPurpose map[string]string `json:"names-on-purpose,omitempty"` // Entrypoints are the compiled files a tool host should load from this module, relative to the // bundle's root. diff --git a/internal/catalogue/upstream_file_test.go b/internal/catalogue/upstream_file_test.go index c4bbcda9..ca3016df 100644 --- a/internal/catalogue/upstream_file_test.go +++ b/internal/catalogue/upstream_file_test.go @@ -16,9 +16,9 @@ func upstreamManifest(artifact string) string { func TestAnArchivePinnedByURLAndSha256IsAccepted(t *testing.T) { for _, a := range []string{ - `{"name":"archive","kind":"archive","url":"https://vendor.invalid/release-1.tar.xz","sha256":"` + pinnedSum + + `{"name":"archive","kind":"archive","url":"https://vendor.example.org/release-1.tar.xz","sha256":"` + pinnedSum + `","version":"1.0","within":"release"}`, - `{"name":"addon","kind":"archive","url":"https://vendor.invalid/addon.xpi","sha256":"` + pinnedSum + + `{"name":"addon","kind":"archive","url":"https://vendor.example.org/addon.xpi","sha256":"` + pinnedSum + `","version":"1.5.6.1","file":"addon.xpi"}`, } { if _, err := ParseManifest([]byte(upstreamManifest(a))); err != nil { @@ -64,19 +64,38 @@ func TestAnUpstreamFileThatIsNotPinnedIsRefused(t *testing.T) { } } -func TestAnUpstreamFilesVendorAndVersionNameNoInstallation(t *testing.T) { - // The vendor's address is the world's, as a public registry is an image's; a version of four dotted numbers - // is not an address. Anything else still is judged. +func TestAnUpstreamFilesVendorIsNamedOnPurposeAndAnInstallationsIsRefused(t *testing.T) { + // The vendor's host is judged like any name, and passes through names-on-purpose; a version of four dotted + // numbers is not an address, and a name in it is still judged. m, err := ParseManifest([]byte(upstreamManifest(`{"name":"owl","kind":"archive","url":"https://vendor.be/owl/owl.xpi","sha256":"` + - pinnedSum + `","version":"1.5.6.1","file":"owl.xpi"}`))) + pinnedSum + `","version":"1.5.6.1","file":"owl.xpi","names-on-purpose":{"vendor.be":"Owl's vendor publishes it there"}}`))) if err != nil { t.Fatal(err) } if problems := InstallationProblems(m); len(problems) != 0 { - t.Fatalf("the vendor and the version were taken for an installation: %v", problems) + t.Fatalf("the declared vendor and the version were taken for an installation: %v", problems) } - m.Build.Artifacts[0].File = "mail.novox.be" + m.Build.Artifacts[0].NamesOnPurpose = nil + if problems := InstallationProblems(m); len(problems) != 1 || !strings.Contains(problems[0], "vendor.be") { + t.Fatalf("an undeclared vendor host is judged: %v", problems) + } + m.Build.Artifacts[0].Version = "git.novox.be 10.0.0.1" + if problems := InstallationProblems(m); len(problems) != 2 { + t.Fatalf("a name in the version is judged: %v", problems) + } + m.Build.Artifacts[0].URL, m.Build.Artifacts[0].Version = "https://mail.novox.be/owl.xpi", "1" if problems := InstallationProblems(m); len(problems) != 1 || !strings.Contains(problems[0], "mail.novox.be") { - t.Fatalf("a name elsewhere in the artifact is still judged: %v", problems) + t.Fatalf("an installation's host in the url is judged: %v", problems) + } +} + +func TestAnUpstreamFileFromAnAddressOrAReservedNameIsRefused(t *testing.T) { + for _, u := range []string{"https://git.novox.internal/r.tar.xz", "https://10.1.2.3/r.tar.xz", + "https://203.0.113.9/r.tar.xz", "https://[2001:db8::1]/r.tar.xz", "https://nas.lan/r.tar.xz", "https://vendor/r.tar.xz"} { + _, err := ParseManifest([]byte(upstreamManifest(`{"name":"x","kind":"archive","url":"` + u + `","sha256":"` + + pinnedSum + `","version":"1"}`))) + if err == nil || !strings.Contains(err.Error(), "published under a public name") { + t.Errorf("%s: %v", u, err) + } } }