diff --git a/cmd/ingestor/mqtt_credentials_r3_118_test.go b/cmd/ingestor/mqtt_credentials_r3_118_test.go index 5ff184598..974a00245 100644 --- a/cmd/ingestor/mqtt_credentials_r3_118_test.go +++ b/cmd/ingestor/mqtt_credentials_r3_118_test.go @@ -42,13 +42,13 @@ func TestBrokerForLogMarksRemovedUserinfo_118(t *testing.T) { // scheme, or the configured user name and password, would pass it. func TestErrForLogMasksKnownSecrets_118(t *testing.T) { src := MQTTSource{ - Broker: "wss://" + credUser + ":" + credPass + "@host/mqtt?token=abc", + Broker: "wss://" + credUser + ":" + credPass + "@host/mqtt?token=" + credQuery, Username: "cfg-user", Password: "cfg-pass", } secrets := mqttSourceSecrets(src) for _, c := range []struct{ in, want string }{ - {"dial host/mqtt?token=abc: refused", "dial host/mqtt?****: refused"}, + {"dial host/mqtt?token=" + credQuery + ": refused", "dial host/mqtt?****: refused"}, {"not authorized: cfg-user/cfg-pass", "not authorized: ****/****"}, {"auth " + credUser + ":" + credPass + " rejected", "auth **** rejected"}, {"connect tcp://x:y@host failed", "connect tcp://****@host failed"}, // MaskText still runs @@ -78,15 +78,15 @@ func TestDisconnectErrorMasksKnownSecrets_118(t *testing.T) { resetSourceStatusRegistry() t.Cleanup(resetSourceStatusRegistry) buf := captureLog118(t) - src := MQTTSource{Name: "feed", Broker: "wss://host/mqtt?token=abc", Password: "cfg-pass"} + src := MQTTSource{Name: "feed", Broker: "wss://host/mqtt?token=" + credQuery, Password: "cfg-pass"} setup := prepareMQTTSource(src, "feed") opts, status := setup.opts, setup.status - opts.OnConnectionLost(nil, errors.New("dial host/mqtt?token=abc: cfg-pass rejected")) + opts.OnConnectionLost(nil, errors.New("dial host/mqtt?token="+credQuery+": cfg-pass rejected")) const want = "dial host/mqtt?****: **** rejected" if got := status.snapshot(time.Now()).LastError; got != want { t.Errorf("lastError = %q, want %q", got, want) } - if out := buf.String(); !strings.Contains(out, want) || strings.Contains(out, "abc") || strings.Contains(out, "cfg-pass") { + if out := buf.String(); !strings.Contains(out, want) || strings.Contains(out, credQuery) || strings.Contains(out, "cfg-pass") { t.Errorf("log:\n%s", out) } } diff --git a/cmd/ingestor/mqtt_status_credentials_118_test.go b/cmd/ingestor/mqtt_status_credentials_118_test.go index 5c94f35f5..788e29dc1 100644 --- a/cmd/ingestor/mqtt_status_credentials_118_test.go +++ b/cmd/ingestor/mqtt_status_credentials_118_test.go @@ -17,8 +17,18 @@ import ( // status registry, the stats file (which feeds the public /api/mqtt/status // and /api/healthz) or the client ID, whatever form the URL takes. +// Password and query parts of the brokers below. They are not plain +// numbers or short words, so a timestamp, counter or size in a checked +// string cannot contain one by chance (#250: a stats-file number that +// contained "1234" failed TestStatsFileHasNoCredentials_118). +const ( + credPassHead = "pw2024pw" // a password cut at '/' + credPassCut = "zq1234zq" // a password cut at '?' or '#' + credQuery = "qxabcxq" // the rest after '?' or '#', and a token +) + // secretParts118 are the credential parts used in the brokers below. -var secretParts118 = []string{credUser, credPass, "tok3n", "2024", "1234", "abc", "p%zz"} +var secretParts118 = []string{credUser, credPass, "tok3n", credPassHead, credPassCut, credQuery, "p%zz"} func assertNoSecretParts118(t *testing.T, what, s string) { t.Helper() @@ -33,12 +43,12 @@ func assertNoSecretParts118(t *testing.T, what, s string) { // url.Parse, so the password (or its first part) used to be logged. func TestBrokerForLogAmbiguousPassword_118(t *testing.T) { for _, c := range []struct{ in, want string }{ - {"tcp://" + credUser + ":2024/" + credPass + "@host:1883", "tcp://****@host:1883"}, - {"tcp://" + credUser + ":1234?abc@host", "tcp://****@host"}, - {"tcp://" + credUser + ":1234#abc@host", "tcp://****@host"}, + {"tcp://" + credUser + ":" + credPassHead + "/" + credPass + "@host:1883", "tcp://****@host:1883"}, + {"tcp://" + credUser + ":" + credPassCut + "?" + credQuery + "@host", "tcp://****@host"}, + {"tcp://" + credUser + ":" + credPassCut + "#" + credQuery + "@host", "tcp://****@host"}, {credUser + ":" + credPass + "@host:1883", "tcp://****@host:1883"}, {"tcp://tok3n@host", "tcp://****@host"}, - {"wss://host/mqtt?token=abc", "wss://host/mqtt"}, + {"wss://host/mqtt?token=" + credQuery, "wss://host/mqtt"}, {"tcp://" + credUser + ":p%zz@host", "tcp://****@host"}, } { got := brokerForLog(c.in) @@ -52,7 +62,7 @@ func TestBrokerForLogAmbiguousPassword_118(t *testing.T) { // The generated client ID is logged ("as client …"), so its host part must // come from the stripped broker, not from url.Parse's idea of the host. func TestMQTTClientIDBaseHasNoCredentials_118(t *testing.T) { - id := mqttClientID(MQTTSource{Broker: "tcp://" + credUser + ":2024/" + credPass + "@host:1883"}) + id := mqttClientID(MQTTSource{Broker: "tcp://" + credUser + ":" + credPassHead + "/" + credPass + "@host:1883"}) if !regexp.MustCompile(`^corescope-host-[0-9a-f]{8}$`).MatchString(id) { t.Fatalf("client id %q", id) } @@ -103,8 +113,8 @@ func TestMQTTSourceTagsAreUnique_118(t *testing.T) { func TestSourceStatusHoldsNoCredentials_118(t *testing.T) { resetSourceStatusRegistry() t.Cleanup(resetSourceStatusRegistry) - s := RegisterSourceStatus("t", "tcp://"+credUser+":1234?abc@host") - s.MarkDisconnect(time.Now(), errors.New(`dial "wss://`+credUser+`:`+credPass+`@host/mqtt?token=abc": refused`)) + s := RegisterSourceStatus("t", "tcp://"+credUser+":"+credPassCut+"?"+credQuery+"@host") + s.MarkDisconnect(time.Now(), errors.New(`dial "wss://`+credUser+`:`+credPass+`@host/mqtt?token=`+credQuery+`": refused`)) snap := s.snapshot(time.Now()) b, _ := json.Marshal(snap) assertNoSecretParts118(t, "status snapshot", string(b))