diff --git a/CHANGELOG.md b/CHANGELOG.md index 79d37cca..e74ecb2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed +- Prevent `assetkey` from leaking into wwclient logs. - Remove requisite dependency between ignition disk target and ignition service. #2083 - Return HTTP 409 status when creating an existing overlay - Allow whitespace to be trimmed for wwdoc comments. #2109 diff --git a/internal/app/wwclient/root.go b/internal/app/wwclient/root.go index be8c2fd5..54bb0ead 100644 --- a/internal/app/wwclient/root.go +++ b/internal/app/wwclient/root.go @@ -28,6 +28,7 @@ import ( "github.com/warewulf/warewulf/internal/pkg/pidfile" "github.com/warewulf/warewulf/internal/pkg/version" "github.com/warewulf/warewulf/internal/pkg/wwlog" + "github.com/warewulf/warewulf/internal/pkg/wwurl" ) var ( @@ -340,7 +341,7 @@ func updateSystem(target string, ipaddr string, port int, wwid string, tag strin counter = 0 } if counter == 0 { - wwlog.Error("%s", err) + wwlog.Error("%s", wwurl.SanitizeError(err)) } counter++ } diff --git a/internal/pkg/warewulfd/parser.go b/internal/pkg/warewulfd/parser.go index c7ae0285..ffad2fc3 100644 --- a/internal/pkg/warewulfd/parser.go +++ b/internal/pkg/warewulfd/parser.go @@ -53,7 +53,7 @@ func initHandleRequest(w http.ResponseWriter, req *http.Request) (*requestContex if remoteNode.AssetKey != "" && remoteNode.AssetKey != rinfo.assetkey { w.WriteHeader(http.StatusUnauthorized) - wwlog.Denied("incorrect asset key: node %s: %s", remoteNode.Id(), rinfo.assetkey) + wwlog.Denied("incorrect asset key for node %s:", remoteNode.Id()) updateStatus(remoteNode.Id(), rinfo.stage, "BAD_ASSET", rinfo.ipaddr) return nil, fmt.Errorf("incorrect asset key") } diff --git a/internal/pkg/wwurl/sanitize.go b/internal/pkg/wwurl/sanitize.go new file mode 100644 index 00000000..73bddb65 --- /dev/null +++ b/internal/pkg/wwurl/sanitize.go @@ -0,0 +1,41 @@ +package wwurl + +import ( + "net/url" + "regexp" +) + +// sensitiveParams are query parameter keys that should be redacted in logs. +var sensitiveParams = []string{"assetkey"} + +// embeddedURLPattern matches an http(s) URL within a larger string, stopping at +// whitespace or a double quote so it works whether the URL is quoted or not. +var embeddedURLPattern = regexp.MustCompile(`https?://[^\s"]+`) + +// SanitizeURL returns a copy of the URL string with sensitive query parameters +// replaced by "REDACTED". Useful for logging URLs without leaking secrets. +func SanitizeURL(rawURL string) string { + u, err := url.Parse(rawURL) + if err != nil { + return rawURL + } + q := u.Query() + redacted := false + for _, key := range sensitiveParams { + if q.Has(key) { + q.Set(key, "REDACTED") + redacted = true + } + } + if redacted { + u.RawQuery = q.Encode() + } + return u.String() +} + +// SanitizeError returns the error message with sensitive URL query parameters +// redacted. Go's net/http embeds the full request URL (including query params) +// in transport errors, so this prevents secrets from appearing in log output. +func SanitizeError(err error) string { + return embeddedURLPattern.ReplaceAllStringFunc(err.Error(), SanitizeURL) +} diff --git a/internal/pkg/wwurl/sanitize_test.go b/internal/pkg/wwurl/sanitize_test.go new file mode 100644 index 00000000..fee0e0c8 --- /dev/null +++ b/internal/pkg/wwurl/sanitize_test.go @@ -0,0 +1,60 @@ +package wwurl + +import ( + "fmt" + "strings" + "testing" +) + +func TestSanitizeURL(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "redacts assetkey", + input: "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?assetkey=secretvalue&compress=gz&stage=runtime&uuid=62184d56-6d53-9895-0b51-035f457c496f", + want: "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?assetkey=REDACTED&compress=gz&stage=runtime&uuid=62184d56-6d53-9895-0b51-035f457c496f", + }, + { + name: "no assetkey unchanged", + input: "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?compress=gz&stage=runtime", + want: "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?compress=gz&stage=runtime", + }, + { + name: "invalid URL returned as-is", + input: "not a url ://bad", + want: "not a url ://bad", + }, + { + name: "empty string", + input: "", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := SanitizeURL(tt.input) + if got != tt.want { + t.Errorf("SanitizeURL(%q)\n got: %q\n want: %q", tt.input, got, tt.want) + } + }) + } +} + +func TestSanitizeError(t *testing.T) { + // Simulate a Go net/http transport error that embeds the full URL. + errMsg := `Get "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?assetkey=mysecret&compress=gz&stage=runtime&uuid=abc123": dial tcp 192.168.3.1:9874: connect: connection refused` + err := fmt.Errorf("%s", errMsg) + got := SanitizeError(err) + if got == errMsg { + t.Error("SanitizeError did not redact assetkey from error message") + } + if strings.Contains(got, "mysecret") { + t.Errorf("SanitizeError still contains secret: %q", got) + } + if !strings.Contains(got, "REDACTED") { + t.Errorf("SanitizeError missing REDACTED marker: %q", got) + } +}