From b30af8ecb9eed61266509d89c18bb91e74076b8d Mon Sep 17 00:00:00 2001 From: Ranganath Gunawardane Date: Fri, 24 Jul 2026 14:47:35 +1000 Subject: [PATCH 1/2] feat(updater): support custom HTTP headers via updater.conf --- README.md | 23 ++++ updater/update/export_test.go | 16 ++- updater/update/headers.go | 160 +++++++++++++++++++++++ updater/update/headers_test.go | 226 +++++++++++++++++++++++++++++++++ updater/update/profile.go | 4 + updater/update/update.go | 7 +- updater/updater.go | 16 ++- updater/updater_test.go | 55 ++++++++ 8 files changed, 502 insertions(+), 5 deletions(-) create mode 100644 updater/update/headers.go create mode 100644 updater/update/headers_test.go create mode 100644 updater/updater_test.go diff --git a/README.md b/README.md index 661a797..8d5c4f0 100644 --- a/README.md +++ b/README.md @@ -253,6 +253,27 @@ You can define a series of operations to run after the update is extracted: * `remove` / `rm` / `del`: Delete a file or directory. * `batchrename`: Recursively find and rename files in a directory. +### **Custom HTTP Headers** + +The `updater` can attach operator-defined HTTP headers to manifest check and package download requests. This allows Silver to work behind infrastructure that expects custom headers, such as routing tags for a multi-tenant CDN or enterprise API gateway. + +Headers are read from a JSON file named `updater.conf` located alongside the `updater` binary: + +```json +{ + "Headers": { + "X-Custom-Tenant": "tenant-42", + "X-Custom-Routing": "us-east-edge" + } +} +``` + +* If `updater.conf` is missing or invalid, the updater logs a warning and fails open. It proceeds without custom headers and never blocks an update check. +* Header names and values are validated against [RFC 9110](https://www.rfc-editor.org/rfc/rfc9110.html#fields). Individual values are capped at 4 KB, headers are capped at 20 per file, and the file size is capped at 64 KB. +* Custom headers can override legacy `X-profile-identity` and `X-profile-channel` headers from `updater-profile.conf`. They cannot override host headers (`User-Agent`, `X-OS-*`, `X-Binary-Arch`, `X-profile-timezone`). + +**Note**: Avoid placing credentials (such as an `Authorization` token) in `updater.conf`. The file is stored in plaintext on disk. Custom headers are also forwarded on HTTP redirects, including cross-domain redirects. + ## **A Robust Upgrade Strategy** Overwriting files in-place during an upgrade is risky. A partial update caused by a full disk, an inconveniently timed system reboot, or a permissions issue can leave your application in an unrecoverable state. @@ -405,6 +426,8 @@ While delivering manifests over a secure HTTPS connection is a fundamental first * `updater.exe profile-set-random-id`: Sets a unique random ID for this installation, sent to the update server. * `updater.exe profile-set-channel `: Sets the update channel (e.g., `beta`, `stable`), also sent to the update server for targeted rollouts. +These commands write to `updater-profile.conf`, which is deprecated in favour of `updater.conf` (see *Custom HTTP Headers* above) but still supported for backward compatibility. + --- ## **Building from Source** diff --git a/updater/update/export_test.go b/updater/update/export_test.go index 9ffd677..7b99241 100644 --- a/updater/update/export_test.go +++ b/updater/update/export_test.go @@ -10,7 +10,17 @@ package update // Test-only exports for black-box tests in package update_test. var ( - SetOSHeaders = setOSHeaders - TrimToNumericVersion = trimToNumericVersion - UserAgent = userAgent + SetOSHeaders = setOSHeaders + TrimToNumericVersion = trimToNumericVersion + UserAgent = userAgent + LoadCustomHeadersFromFile = loadCustomHeadersFromFile + ValidateHeaders = validateHeaders + IsValidHeaderFieldName = isValidHeaderFieldName + IsValidHeaderFieldValue = isValidHeaderFieldValue +) + +const ( + MaxHeadersFileBytes = maxHeadersFileBytes + MaxHeaderValueBytes = maxHeaderValueBytes + MaxCustomHeaders = maxCustomHeaders ) diff --git a/updater/update/headers.go b/updater/update/headers.go new file mode 100644 index 0000000..6c0c03d --- /dev/null +++ b/updater/update/headers.go @@ -0,0 +1,160 @@ +// SILVER - Service Wrapper +// Auto Updater +// +// Copyright (c) 2026 PaperCut Software http://www.papercut.com/ +// Use of this source code is governed by an MIT or GPL Version 2 license. +// See the project's LICENSE file for more information. +// + +package update + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/textproto" + "os" + "path/filepath" + "regexp" + "strings" +) + +const ( + headersFileName = "updater.conf" + maxHeadersFileBytes = 64 * 1024 + maxHeaderValueBytes = 4096 + maxCustomHeaders = 20 +) + +// systemHeaderKeys are headers describing this binary or host itself. +// A custom header sharing one of these names is dropped. +// This provides defense in depth alongside the load-order guarantee in Check(). +// +// Legacy profile identity and channel headers are deliberately omitted. +// The new updater.conf is allowed to override updater-profile.conf on collision. +var systemHeaderKeys = map[string]bool{ + textproto.CanonicalMIMEHeaderKey("User-Agent"): true, + textproto.CanonicalMIMEHeaderKey(headerProfileTimezoneKey): true, + textproto.CanonicalMIMEHeaderKey(headerOSTypeKey): true, + textproto.CanonicalMIMEHeaderKey(headerOSVersionKey): true, + textproto.CanonicalMIMEHeaderKey(headerOSArchKey): true, + textproto.CanonicalMIMEHeaderKey(headerBinaryArchKey): true, +} + +type headersFile struct { + Headers map[string]string `json:"Headers"` +} + +// AddCustomHeaders attaches operator-defined headers from updater.conf +// to req. It fails open if locating, reading, parsing, or validating fails. +// +// Callers must invoke AddCustomHeaders after legacy updater-profile.conf +// headers are set, but before User-Agent and OS headers. +func AddCustomHeaders(req *http.Request) { + headers, err := loadCustomHeaders() + if err != nil { + fmt.Printf("Couldn't load custom headers: %v.\n", err) + return + } + for key, value := range headers { + req.Header.Set(key, value) + } +} + +func loadCustomHeaders() (map[string]string, error) { + fn, err := getHeadersFileName() + if err != nil { + return nil, err + } + return loadCustomHeadersFromFile(fn) +} + +func loadCustomHeadersFromFile(fn string) (map[string]string, error) { + f, err := os.Open(fn) + if err != nil { + return nil, err + } + defer f.Close() + + // Read one byte past the limit so an oversized file is reported as an + // error rather than silently truncated. + data, err := io.ReadAll(io.LimitReader(f, maxHeadersFileBytes+1)) + if err != nil { + return nil, err + } + if len(data) > maxHeadersFileBytes { + return nil, fmt.Errorf("%s exceeds the %d byte limit", headersFileName, maxHeadersFileBytes) + } + + var conf headersFile + if err := json.Unmarshal(data, &conf); err != nil { + return nil, fmt.Errorf("invalid JSON in %s: %w", headersFileName, err) + } + + return validateHeaders(conf.Headers), nil +} + +func validateHeaders(raw map[string]string) map[string]string { + if len(raw) > maxCustomHeaders { + fmt.Printf("%s defines %d headers; only %d are allowed, the rest will be dropped.\n", + headersFileName, len(raw), maxCustomHeaders) + } + + valid := make(map[string]string, len(raw)) + for key, value := range raw { + if len(valid) >= maxCustomHeaders { + break + } + + key = strings.TrimSpace(key) + value = strings.TrimSpace(value) + + switch { + case !isValidHeaderFieldName(key): + fmt.Printf("Ignoring custom header %q: invalid header field name.\n", key) + case len(value) > maxHeaderValueBytes: + fmt.Printf("Ignoring custom header %q: value exceeds %d bytes.\n", key, maxHeaderValueBytes) + case !isValidHeaderFieldValue(value): + fmt.Printf("Ignoring custom header %q: invalid header field value.\n", key) + case systemHeaderKeys[textproto.CanonicalMIMEHeaderKey(key)]: + fmt.Printf("Ignoring custom header %q: reserved for internal use.\n", key) + default: + valid[key] = value + } + } + return valid +} + +func getHeadersFileName() (string, error) { + // File containing custom headers should exist alongside the updater binary. + updaterBin, err := os.Executable() + if err != nil { + return "", err + } + return filepath.Join(filepath.Dir(updaterBin), headersFileName), nil +} + +// headerFieldNameRE matches a non-empty RFC 9110 token, the valid form for +// an HTTP field-name: https://www.rfc-editor.org/rfc/rfc9110.html#section-5.6.2 +var headerFieldNameRE = regexp.MustCompile(`^[A-Za-z0-9!#$%&'*+\-.^_` + "`" + `|~]+$`) + +func isValidHeaderFieldName(s string) bool { + return headerFieldNameRE.MatchString(s) +} + +// isValidHeaderFieldValue reports whether s is a valid RFC 9110 field-value: +// visible ASCII/obs-text bytes, plus internal tabs, but no control +// characters (notably CR/LF) that could be used to inject header lines. +func isValidHeaderFieldValue(s string) bool { + for i := 0; i < len(s); i++ { + b := s[i] + if b == '\t' { + continue + } + if b < 0x20 || b == 0x7f { + return false + } + } + return true +} diff --git a/updater/update/headers_test.go b/updater/update/headers_test.go new file mode 100644 index 0000000..d64db65 --- /dev/null +++ b/updater/update/headers_test.go @@ -0,0 +1,226 @@ +// SILVER - Service Wrapper +// Auto Updater +// +// Copyright (c) 2026 PaperCut Software http://www.papercut.com/ +// Use of this source code is governed by an MIT or GPL Version 2 license. +// See the project's LICENSE file for more information. +// + +package update_test + +import ( + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/papercutsoftware/silver/updater/update" +) + +func TestIsValidHeaderFieldName(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + want bool + }{ + {"simple token", "X-Custom-Tenant", true}, + {"digits and dashes", "X-Tenant-42", true}, + {"empty", "", false}, + {"space", "X Custom", false}, + {"colon", "X-Custom:", false}, + {"contains CR", "X-Custom\r", false}, + {"contains LF", "X-Custom\n", false}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := update.IsValidHeaderFieldName(tt.input); got != tt.want { + t.Errorf("IsValidHeaderFieldName(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +func TestIsValidHeaderFieldValue(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + want bool + }{ + {"simple value", "tenant-42", true}, + {"multi-value with comma", "application/json, text/plain", true}, + {"internal tab", "a\tb", true}, + {"CRLF injection", "value\r\nX-Injected: evil", false}, + {"bare CR", "value\r", false}, + {"bare LF", "value\n", false}, + {"NUL byte", "value\x00", false}, + {"DEL byte", "value\x7f", false}, + {"empty", "", true}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := update.IsValidHeaderFieldValue(tt.input); got != tt.want { + t.Errorf("IsValidHeaderFieldValue(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +func TestValidateHeaders(t *testing.T) { + t.Run("trims keys and values", func(t *testing.T) { + got := update.ValidateHeaders(map[string]string{" X-Tenant ": " tenant-42 "}) + if got["X-Tenant"] != "tenant-42" { + t.Errorf("got %#v, want X-Tenant=tenant-42", got) + } + }) + + t.Run("drops invalid field name", func(t *testing.T) { + got := update.ValidateHeaders(map[string]string{"X Custom": "v"}) + if len(got) != 0 { + t.Errorf("got %#v, want empty", got) + } + }) + + t.Run("drops invalid field value", func(t *testing.T) { + got := update.ValidateHeaders(map[string]string{"X-Header": "invalid\r\nvalue"}) + if len(got) != 0 { + t.Errorf("got %#v, want empty", got) + } + }) + + t.Run("drops oversized value", func(t *testing.T) { + big := strings.Repeat("a", update.MaxHeaderValueBytes+1) + got := update.ValidateHeaders(map[string]string{"X-Big": big}) + if len(got) != 0 { + t.Errorf("got %#v, want empty", got) + } + }) + + t.Run("keeps value at the size limit", func(t *testing.T) { + fits := strings.Repeat("a", update.MaxHeaderValueBytes) + got := update.ValidateHeaders(map[string]string{"X-Fits": fits}) + if got["X-Fits"] != fits { + t.Errorf("expected value to survive at exactly the size limit") + } + }) + + t.Run("drops headers matching immutable system header names case-insensitively", func(t *testing.T) { + got := update.ValidateHeaders(map[string]string{ + "user-agent": "evil", + "X-PROFILE-TIMEZONE": "evil", + "X-OS-Type": "evil", + }) + if len(got) != 0 { + t.Errorf("got %#v, want immutable system headers to be dropped", got) + } + }) + + t.Run("allows overriding deprecated profile headers", func(t *testing.T) { + got := update.ValidateHeaders(map[string]string{ + "X-Profile-Identity": "custom-identity", + "X-Profile-Channel": "custom-channel", + }) + if got["X-Profile-Identity"] != "custom-identity" || got["X-Profile-Channel"] != "custom-channel" { + t.Errorf("got %#v, want profile identity/channel headers to pass through", got) + } + }) + + t.Run("caps total header count", func(t *testing.T) { + raw := make(map[string]string, update.MaxCustomHeaders+5) + for i := 0; i < update.MaxCustomHeaders+5; i++ { + raw[strings.Repeat("X", i+1)] = "v" + } + got := update.ValidateHeaders(raw) + if len(got) != update.MaxCustomHeaders { + t.Errorf("got %d headers, want %d", len(got), update.MaxCustomHeaders) + } + }) +} + +func TestLoadCustomHeadersFromFile(t *testing.T) { + t.Run("missing file errors", func(t *testing.T) { + _, err := update.LoadCustomHeadersFromFile(filepath.Join(t.TempDir(), "does-not-exist.conf")) + if err == nil { + t.Fatal("expected an error for a missing file") + } + }) + + t.Run("invalid JSON errors", func(t *testing.T) { + fn := writeTempConf(t, "not json") + _, err := update.LoadCustomHeadersFromFile(fn) + if err == nil { + t.Fatal("expected an error for invalid JSON") + } + }) + + t.Run("oversized file errors", func(t *testing.T) { + big := `{"Headers":{"X-Big":"` + strings.Repeat("a", update.MaxHeadersFileBytes) + `"}}` + fn := writeTempConf(t, big) + _, err := update.LoadCustomHeadersFromFile(fn) + if err == nil { + t.Fatal("expected an error for a file exceeding the size limit") + } + }) + + t.Run("valid file returns validated headers", func(t *testing.T) { + fn := writeTempConf(t, `{"Headers":{"X-Custom-Tenant":"tenant-42","X-Custom-Routing":"us-east-edge"}}`) + got, err := update.LoadCustomHeadersFromFile(fn) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got["X-Custom-Tenant"] != "tenant-42" || got["X-Custom-Routing"] != "us-east-edge" { + t.Errorf("got %#v", got) + } + }) +} + +func TestAddCustomHeaders(t *testing.T) { + t.Run("fails open gracefully when file missing", func(t *testing.T) { + req, _ := http.NewRequest("GET", "https://example.com", nil) + update.AddCustomHeaders(req) + if len(req.Header) != 0 { + t.Errorf("expected no headers added when updater.conf is missing, got %#v", req.Header) + } + }) + + t.Run("attaches headers when file present next to executable", func(t *testing.T) { + bin, err := os.Executable() + if err != nil { + t.Skipf("cannot resolve os.Executable: %v", err) + } + confPath := filepath.Join(filepath.Dir(bin), "updater.conf") + // Prevent overwriting an existing file + if _, err := os.Stat(confPath); err == nil { + t.Skip("updater.conf already exists in executable directory") + } + content := `{"Headers":{"X-Custom-Tenant":"tenant-42"}}` + if err := os.WriteFile(confPath, []byte(content), 0600); err != nil { + t.Skipf("cannot write temporary updater.conf next to executable: %v", err) + } + t.Cleanup(func() { + os.Remove(confPath) + }) + + req, _ := http.NewRequest("GET", "https://example.com", nil) + update.AddCustomHeaders(req) + + if req.Header.Get("X-Custom-Tenant") != "tenant-42" { + t.Errorf("got %q, want %q", req.Header.Get("X-Custom-Tenant"), "tenant-42") + } + }) +} + +func writeTempConf(t *testing.T, content string) string { + t.Helper() + fn := filepath.Join(t.TempDir(), "updater.conf") + if err := os.WriteFile(fn, []byte(content), 0600); err != nil { + t.Fatal(err) + } + return fn +} diff --git a/updater/update/profile.go b/updater/update/profile.go index 223c451..168fc5d 100644 --- a/updater/update/profile.go +++ b/updater/update/profile.go @@ -33,6 +33,7 @@ const ( headerProfileTimezoneKey string = "X-profile-timezone" ) +// Deprecated: Profile represents updater-profile.conf, which is deprecated in favor of updater.conf. type Profile struct { Id string `json:"id"` Channel string `json:"channel"` @@ -65,6 +66,7 @@ func saveProfile(prf *Profile) (err error) { return ioutil.WriteFile(fn, data, 0600) } +// Deprecated: SetRandomProfileID modifies updater-profile.conf, which is deprecated in favor of updater.conf. func SetRandomProfileID() int { strRand, err := generateRandomIDString() if err != nil { @@ -74,6 +76,7 @@ func SetRandomProfileID() int { return SetProfileID(strRand) } +// Deprecated: SetProfileID modifies updater-profile.conf, which is deprecated in favor of updater.conf. func SetProfileID(id string) int { prf := Profile{} err := loadProfile(&prf) @@ -90,6 +93,7 @@ func SetProfileID(id string) int { return 0 } +// Deprecated: SetProfileChannel modifies updater-profile.conf, which is deprecated in favor of updater.conf. func SetProfileChannel(channel string) int { prf := Profile{} err := loadProfile(&prf) diff --git a/updater/update/update.go b/updater/update/update.go index c84c9fb..2d7d2a1 100644 --- a/updater/update/update.go +++ b/updater/update/update.go @@ -39,8 +39,13 @@ func Check(updateURL string, currentVer string, publicKey string) (*UpgradeInfo, if err != nil { return nil, err } - req.Header.Set("User-Agent", userAgent()) + // Legacy updater-profile.conf headers are set first so that + // updater.conf (its replacement) can override them on collision. + // User-Agent and the OS headers describe this binary/host itself, + // not operator data, so they're set last and always win. addIDProfileToRequestHeader(req) + AddCustomHeaders(req) + req.Header.Set("User-Agent", userAgent()) setOSHeaders(req) res, err := client.Do(req) diff --git a/updater/updater.go b/updater/updater.go index 53ed3df..d3f180d 100644 --- a/updater/updater.go +++ b/updater/updater.go @@ -128,7 +128,15 @@ func download(url string) (string, error) { return "", err } - resp, err := http.Get(url) + req, err := http.NewRequest("GET", url, nil) + if err != nil { + _ = outfile.Close() + _ = os.Remove(outfile.Name()) + return "", err + } + update.AddCustomHeaders(req) + + resp, err := (&http.Client{}).Do(req) if err != nil { _ = outfile.Close() _ = os.Remove(outfile.Name()) @@ -136,6 +144,12 @@ func download(url string) (string, error) { } defer resp.Body.Close() + if resp.StatusCode >= http.StatusBadRequest { + _ = outfile.Close() + _ = os.Remove(outfile.Name()) + return "", fmt.Errorf("download HTTP error %d: %s", resp.StatusCode, resp.Status) + } + _, err = io.Copy(outfile, resp.Body) if err != nil { _ = outfile.Close() diff --git a/updater/updater_test.go b/updater/updater_test.go new file mode 100644 index 0000000..445814e --- /dev/null +++ b/updater/updater_test.go @@ -0,0 +1,55 @@ +// SILVER - Service Wrapper +// Auto Updater +// +// Copyright (c) 2026 PaperCut Software http://www.papercut.com/ +// Use of this source code is governed by an MIT or GPL Version 2 license. +// See the project's LICENSE file for more information. +// + +package main + +import ( + "net/http" + "net/http/httptest" + "os" + "testing" +) + +func TestDownload(t *testing.T) { + t.Run("writes the response body to a file", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("zip-content")) + })) + defer server.Close() + + fn, err := download(server.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer os.Remove(fn) + + got, err := os.ReadFile(fn) + if err != nil { + t.Fatalf("couldn't read downloaded file: %v", err) + } + if string(got) != "zip-content" { + t.Errorf("got %q, want %q", got, "zip-content") + } + }) + + t.Run("errors and cleans up on a non-2xx response", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "not found", http.StatusNotFound) + })) + defer server.Close() + + fn, err := download(server.URL) + if err == nil { + os.Remove(fn) + t.Fatal("expected an error for a 404 response") + } + if fn != "" { + t.Errorf("expected no filename on error, got %q", fn) + } + }) +} From f19a98f5fb64fd1ed32fe8393cabeea96ec9af28 Mon Sep 17 00:00:00 2001 From: Ranganath Gunawardane Date: Fri, 24 Jul 2026 21:11:49 +1000 Subject: [PATCH 2/2] fix(updater): harden custom header handling and reduce duplication --- .gitignore | 1 + README.md | 8 ++-- updater/update/headers.go | 80 ++++++++++++++++++++++------------ updater/update/headers_test.go | 49 +++++++++++++++++++-- updater/update/profile.go | 19 ++++---- updater/update/update.go | 10 ++--- updater/update/useragent.go | 2 + updater/updater.go | 26 ++++++----- 8 files changed, 135 insertions(+), 60 deletions(-) diff --git a/.gitignore b/.gitignore index e46d3b2..b6dfed9 100644 --- a/.gitignore +++ b/.gitignore @@ -28,6 +28,7 @@ _testmain.go *.exe *.test +coverage.out # Direnv stuff .envrc diff --git a/README.md b/README.md index 8d5c4f0..f873201 100644 --- a/README.md +++ b/README.md @@ -255,7 +255,7 @@ You can define a series of operations to run after the update is extracted: ### **Custom HTTP Headers** -The `updater` can attach operator-defined HTTP headers to manifest check and package download requests. This allows Silver to work behind infrastructure that expects custom headers, such as routing tags for a multi-tenant CDN or enterprise API gateway. +The `updater` can attach operator-defined HTTP headers to manifest check and package download requests. This allows Silver to work behind infrastructure that expects custom headers. Examples: routing tags for a multi-tenant CDN, or an enterprise API gateway. Headers are read from a JSON file named `updater.conf` located alongside the `updater` binary: @@ -269,10 +269,10 @@ Headers are read from a JSON file named `updater.conf` located alongside the `up ``` * If `updater.conf` is missing or invalid, the updater logs a warning and fails open. It proceeds without custom headers and never blocks an update check. -* Header names and values are validated against [RFC 9110](https://www.rfc-editor.org/rfc/rfc9110.html#fields). Individual values are capped at 4 KB, headers are capped at 20 per file, and the file size is capped at 64 KB. +* Header names and values are validated against [RFC 9110](https://www.rfc-editor.org/rfc/rfc9110.html#fields). Individual values are capped at 4 KB. Headers are capped at 20 per file. The file size is capped at 64 KB. * Custom headers can override legacy `X-profile-identity` and `X-profile-channel` headers from `updater-profile.conf`. They cannot override host headers (`User-Agent`, `X-OS-*`, `X-Binary-Arch`, `X-profile-timezone`). -**Note**: Avoid placing credentials (such as an `Authorization` token) in `updater.conf`. The file is stored in plaintext on disk. Custom headers are also forwarded on HTTP redirects, including cross-domain redirects. +**Note**: Avoid placing credentials (such as an `Authorization` token) in `updater.conf`. The file is stored in plaintext on disk. Custom headers are also forwarded on HTTP redirects. That includes cross-domain redirects. ## **A Robust Upgrade Strategy** @@ -426,7 +426,7 @@ While delivering manifests over a secure HTTPS connection is a fundamental first * `updater.exe profile-set-random-id`: Sets a unique random ID for this installation, sent to the update server. * `updater.exe profile-set-channel `: Sets the update channel (e.g., `beta`, `stable`), also sent to the update server for targeted rollouts. -These commands write to `updater-profile.conf`, which is deprecated in favour of `updater.conf` (see *Custom HTTP Headers* above) but still supported for backward compatibility. +These commands write to `updater-profile.conf`. That file is deprecated in favour of `updater.conf` (see *Custom HTTP Headers* above). It is still supported for backward compatibility. --- diff --git a/updater/update/headers.go b/updater/update/headers.go index 6c0c03d..e8e052a 100644 --- a/updater/update/headers.go +++ b/updater/update/headers.go @@ -15,8 +15,8 @@ import ( "net/http" "net/textproto" "os" - "path/filepath" "regexp" + "sort" "strings" ) @@ -33,13 +33,22 @@ const ( // // Legacy profile identity and channel headers are deliberately omitted. // The new updater.conf is allowed to override updater-profile.conf on collision. +// +// Host, Content-Length, Transfer-Encoding, and Connection are also denied. +// Go's net/http manages these itself. It ignores whatever is set on +// req.Header for them. Allowing them through would silently do nothing. +// That's a confusing trap for an operator configuring updater.conf. var systemHeaderKeys = map[string]bool{ - textproto.CanonicalMIMEHeaderKey("User-Agent"): true, + textproto.CanonicalMIMEHeaderKey(headerUserAgentKey): true, textproto.CanonicalMIMEHeaderKey(headerProfileTimezoneKey): true, textproto.CanonicalMIMEHeaderKey(headerOSTypeKey): true, textproto.CanonicalMIMEHeaderKey(headerOSVersionKey): true, textproto.CanonicalMIMEHeaderKey(headerOSArchKey): true, textproto.CanonicalMIMEHeaderKey(headerBinaryArchKey): true, + textproto.CanonicalMIMEHeaderKey("Host"): true, + textproto.CanonicalMIMEHeaderKey("Content-Length"): true, + textproto.CanonicalMIMEHeaderKey("Transfer-Encoding"): true, + textproto.CanonicalMIMEHeaderKey("Connection"): true, } type headersFile struct { @@ -47,14 +56,18 @@ type headersFile struct { } // AddCustomHeaders attaches operator-defined headers from updater.conf -// to req. It fails open if locating, reading, parsing, or validating fails. +// to req. It fails open on any error. // // Callers must invoke AddCustomHeaders after legacy updater-profile.conf -// headers are set, but before User-Agent and OS headers. +// headers are set. Call it before User-Agent and OS headers are set. func AddCustomHeaders(req *http.Request) { headers, err := loadCustomHeaders() if err != nil { - fmt.Printf("Couldn't load custom headers: %v.\n", err) + if os.IsNotExist(err) { + fmt.Printf("%s not found. Proceeding without custom headers.\n", headersFileName) + } else { + fmt.Printf("Couldn't load custom headers: %v.\n", err) + } return } for key, value := range headers { @@ -77,8 +90,8 @@ func loadCustomHeadersFromFile(fn string) (map[string]string, error) { } defer f.Close() - // Read one byte past the limit so an oversized file is reported as an - // error rather than silently truncated. + // Read one byte past the limit. An oversized file becomes an error. + // It is not silently truncated. data, err := io.ReadAll(io.LimitReader(f, maxHeadersFileBytes+1)) if err != nil { return nil, err @@ -96,56 +109,69 @@ func loadCustomHeadersFromFile(fn string) (map[string]string, error) { } func validateHeaders(raw map[string]string) map[string]string { - if len(raw) > maxCustomHeaders { - fmt.Printf("%s defines %d headers; only %d are allowed, the rest will be dropped.\n", - headersFileName, len(raw), maxCustomHeaders) + // Sort keys first. Map iteration order is random. + // Without sorting, which headers get dropped past maxCustomHeaders + // would differ from run to run. + keys := make([]string, 0, len(raw)) + for key := range raw { + keys = append(keys, key) } + sort.Strings(keys) + + // "X-Foo" and "x-foo" canonicalize to the same header. + // Keep only the first one seen. This keeps the winner deterministic. + // Otherwise random map iteration order would decide it later. + seenCanonical := make(map[string]bool, len(raw)) valid := make(map[string]string, len(raw)) - for key, value := range raw { + for i, key := range keys { if len(valid) >= maxCustomHeaders { + fmt.Printf("%s defines more than %d valid headers. Ignoring the remaining %d.\n", + headersFileName, maxCustomHeaders, len(keys)-i) break } + value := raw[key] key = strings.TrimSpace(key) value = strings.TrimSpace(value) + canonical := textproto.CanonicalMIMEHeaderKey(key) switch { case !isValidHeaderFieldName(key): - fmt.Printf("Ignoring custom header %q: invalid header field name.\n", key) + fmt.Printf("Ignoring custom header %q. Invalid header field name.\n", key) case len(value) > maxHeaderValueBytes: - fmt.Printf("Ignoring custom header %q: value exceeds %d bytes.\n", key, maxHeaderValueBytes) + fmt.Printf("Ignoring custom header %q. Value exceeds %d bytes.\n", key, maxHeaderValueBytes) case !isValidHeaderFieldValue(value): - fmt.Printf("Ignoring custom header %q: invalid header field value.\n", key) - case systemHeaderKeys[textproto.CanonicalMIMEHeaderKey(key)]: - fmt.Printf("Ignoring custom header %q: reserved for internal use.\n", key) + fmt.Printf("Ignoring custom header %q. Invalid header field value.\n", key) + case systemHeaderKeys[canonical]: + fmt.Printf("Ignoring custom header %q. Reserved for internal use.\n", key) + case seenCanonical[canonical]: + fmt.Printf("Ignoring custom header %q. Duplicate of an already-configured header.\n", key) default: valid[key] = value + seenCanonical[canonical] = true } } return valid } func getHeadersFileName() (string, error) { - // File containing custom headers should exist alongside the updater binary. - updaterBin, err := os.Executable() - if err != nil { - return "", err - } - return filepath.Join(filepath.Dir(updaterBin), headersFileName), nil + return fileNextToExecutable(headersFileName) } -// headerFieldNameRE matches a non-empty RFC 9110 token, the valid form for -// an HTTP field-name: https://www.rfc-editor.org/rfc/rfc9110.html#section-5.6.2 +// headerFieldNameRE matches a non-empty RFC 9110 token. +// That's the valid form for an HTTP field-name. +// See https://www.rfc-editor.org/rfc/rfc9110.html#section-5.6.2 var headerFieldNameRE = regexp.MustCompile(`^[A-Za-z0-9!#$%&'*+\-.^_` + "`" + `|~]+$`) func isValidHeaderFieldName(s string) bool { return headerFieldNameRE.MatchString(s) } -// isValidHeaderFieldValue reports whether s is a valid RFC 9110 field-value: -// visible ASCII/obs-text bytes, plus internal tabs, but no control -// characters (notably CR/LF) that could be used to inject header lines. +// isValidHeaderFieldValue reports whether s is a valid RFC 9110 field-value. +// Valid means visible ASCII or obs-text bytes, plus internal tabs. +// Control characters are rejected, notably CR/LF. +// Those could otherwise inject extra header lines. func isValidHeaderFieldValue(s string) bool { for i := 0; i < len(s); i++ { b := s[i] diff --git a/updater/update/headers_test.go b/updater/update/headers_test.go index d64db65..d67eb73 100644 --- a/updater/update/headers_test.go +++ b/updater/update/headers_test.go @@ -121,6 +121,31 @@ func TestValidateHeaders(t *testing.T) { } }) + t.Run("drops hop-by-hop headers Go would ignore anyway", func(t *testing.T) { + got := update.ValidateHeaders(map[string]string{ + "Host": "evil.example.com", + "Content-Length": "0", + "Transfer-Encoding": "chunked", + "Connection": "close", + }) + if len(got) != 0 { + t.Errorf("got %#v, want hop-by-hop headers to be dropped", got) + } + }) + + t.Run("dedupes case-variant duplicate keys deterministically", func(t *testing.T) { + got := update.ValidateHeaders(map[string]string{ + "X-Foo": "upper-value", + "x-foo": "lower-value", + }) + if len(got) != 1 { + t.Fatalf("got %#v, want exactly one surviving entry", got) + } + if got["X-Foo"] != "upper-value" { + t.Errorf("got %#v, want the alphabetically-first key to win", got) + } + }) + t.Run("allows overriding deprecated profile headers", func(t *testing.T) { got := update.ValidateHeaders(map[string]string{ "X-Profile-Identity": "custom-identity", @@ -131,14 +156,30 @@ func TestValidateHeaders(t *testing.T) { } }) - t.Run("caps total header count", func(t *testing.T) { + t.Run("caps total header count deterministically", func(t *testing.T) { raw := make(map[string]string, update.MaxCustomHeaders+5) for i := 0; i < update.MaxCustomHeaders+5; i++ { raw[strings.Repeat("X", i+1)] = "v" } - got := update.ValidateHeaders(raw) - if len(got) != update.MaxCustomHeaders { - t.Errorf("got %d headers, want %d", len(got), update.MaxCustomHeaders) + + // Run twice. Map iteration order is random. + // This catches a regression back to non-deterministic capping. + first := update.ValidateHeaders(raw) + second := update.ValidateHeaders(raw) + + if len(first) != update.MaxCustomHeaders { + t.Fatalf("got %d headers, want %d", len(first), update.MaxCustomHeaders) + } + for key := range first { + if _, ok := second[key]; !ok { + t.Errorf("header %q survived one run but not another, capping is not deterministic", key) + } + } + + // Keys are "X", "XX", "XXX", ... "XXXXX...". Sorted, the shortest + // keys always win. "X" through the 20-char key must survive. + if _, ok := first[strings.Repeat("X", update.MaxCustomHeaders)]; !ok { + t.Errorf("expected the shortest %d keys to survive, got %#v", update.MaxCustomHeaders, first) } }) } diff --git a/updater/update/profile.go b/updater/update/profile.go index 168fc5d..2526f5f 100644 --- a/updater/update/profile.go +++ b/updater/update/profile.go @@ -33,7 +33,7 @@ const ( headerProfileTimezoneKey string = "X-profile-timezone" ) -// Deprecated: Profile represents updater-profile.conf, which is deprecated in favor of updater.conf. +// Deprecated: Profile represents updater-profile.conf. That file is deprecated in favor of updater.conf. type Profile struct { Id string `json:"id"` Channel string `json:"channel"` @@ -66,7 +66,7 @@ func saveProfile(prf *Profile) (err error) { return ioutil.WriteFile(fn, data, 0600) } -// Deprecated: SetRandomProfileID modifies updater-profile.conf, which is deprecated in favor of updater.conf. +// Deprecated: SetRandomProfileID modifies updater-profile.conf. That file is deprecated in favor of updater.conf. func SetRandomProfileID() int { strRand, err := generateRandomIDString() if err != nil { @@ -76,7 +76,7 @@ func SetRandomProfileID() int { return SetProfileID(strRand) } -// Deprecated: SetProfileID modifies updater-profile.conf, which is deprecated in favor of updater.conf. +// Deprecated: SetProfileID modifies updater-profile.conf. That file is deprecated in favor of updater.conf. func SetProfileID(id string) int { prf := Profile{} err := loadProfile(&prf) @@ -93,7 +93,7 @@ func SetProfileID(id string) int { return 0 } -// Deprecated: SetProfileChannel modifies updater-profile.conf, which is deprecated in favor of updater.conf. +// Deprecated: SetProfileChannel modifies updater-profile.conf. That file is deprecated in favor of updater.conf. func SetProfileChannel(channel string) int { prf := Profile{} err := loadProfile(&prf) @@ -149,14 +149,17 @@ func generateRandomIDString() (string, error) { } func getProfileFileName() (string, error) { - // File containing the profile info should exist with the updater binary. + return fileNextToExecutable(profileFileName) +} + +// fileNextToExecutable resolves name to a path in the same directory +// as the running executable. +func fileNextToExecutable(name string) (string, error) { updaterBin, err := os.Executable() if err != nil { return "", err } - // Construct file name with absolute path. - profileFile := filepath.Join(filepath.Dir(updaterBin), profileFileName) - return profileFile, nil + return filepath.Join(filepath.Dir(updaterBin), name), nil } func validateProfile(prf *Profile) error { diff --git a/updater/update/update.go b/updater/update/update.go index 2d7d2a1..6915782 100644 --- a/updater/update/update.go +++ b/updater/update/update.go @@ -39,13 +39,13 @@ func Check(updateURL string, currentVer string, publicKey string) (*UpgradeInfo, if err != nil { return nil, err } - // Legacy updater-profile.conf headers are set first so that - // updater.conf (its replacement) can override them on collision. - // User-Agent and the OS headers describe this binary/host itself, - // not operator data, so they're set last and always win. + // Legacy updater-profile.conf headers are set first. + // updater.conf is its replacement. It can override them on collision. + // User-Agent and the OS headers describe this binary and host itself. + // They are not operator data. They are set last and always win. addIDProfileToRequestHeader(req) AddCustomHeaders(req) - req.Header.Set("User-Agent", userAgent()) + req.Header.Set(headerUserAgentKey, userAgent()) setOSHeaders(req) res, err := client.Do(req) diff --git a/updater/update/useragent.go b/updater/update/useragent.go index 287af02..698c7c8 100644 --- a/updater/update/useragent.go +++ b/updater/update/useragent.go @@ -10,6 +10,8 @@ package update import "runtime/debug" +const headerUserAgentKey = "User-Agent" + // userAgent returns the User-Agent for update check requests, e.g. // "silver-updater/v1.6.1". The version is stamped automatically by the Go // toolchain from the module's VCS tag; builds without VCS information diff --git a/updater/updater.go b/updater/updater.go index d3f180d..ee33289 100644 --- a/updater/updater.go +++ b/updater/updater.go @@ -122,40 +122,42 @@ func upgradeComplete(upgradeInfo *update.UpgradeInfo) { _ = ioutil.WriteFile(config.ReloadFileName, []byte(""), 0644) } -func download(url string) (string, error) { +func download(url string) (fn string, err error) { outfile, err := ioutil.TempFile("", "update-") if err != nil { return "", err } + // Close always. Remove only on error. This runs after every return + // below, so no error path needs to repeat that cleanup itself. + defer func() { + _ = outfile.Close() + if err != nil { + _ = os.Remove(outfile.Name()) + } + }() req, err := http.NewRequest("GET", url, nil) if err != nil { - _ = outfile.Close() - _ = os.Remove(outfile.Name()) return "", err } + // AddCustomHeaders sends every configured header to this URL. + // The URL comes from the manifest and may be unverified if no + // public key is set. Redirects can also forward these headers + // to a different host. Do not put secrets in updater.conf. update.AddCustomHeaders(req) resp, err := (&http.Client{}).Do(req) if err != nil { - _ = outfile.Close() - _ = os.Remove(outfile.Name()) return "", err } defer resp.Body.Close() if resp.StatusCode >= http.StatusBadRequest { - _ = outfile.Close() - _ = os.Remove(outfile.Name()) return "", fmt.Errorf("download HTTP error %d: %s", resp.StatusCode, resp.Status) } - _, err = io.Copy(outfile, resp.Body) - if err != nil { - _ = outfile.Close() - _ = os.Remove(outfile.Name()) + if _, err = io.Copy(outfile, resp.Body); err != nil { return "", err } - _ = outfile.Close() return outfile.Name(), nil }