From 2af16d3ab99cae04ee08414fbe37da86818b92a5 Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Sun, 26 Apr 2026 03:56:38 +0700 Subject: [PATCH] checker: harden HTTP collection and stabilize report ordering Validate the federation tester URI placeholder, escape the domain, set a client timeout, cap the response body, and ship CA certificates in the scratch image so HTTPS calls succeed. Sort hosts, connection reports, and errors when rendering so output is deterministic, and deduplicate TLS problems. Drop the deprecated aggregate Rule() and add tests for collection and rules. --- Dockerfile | 1 + checker/collect.go | 24 ++++- checker/collect_test.go | 91 ++++++++++++++++++ checker/definition.go | 2 +- checker/interactive.go | 2 +- checker/provider.go | 5 - checker/report.go | 26 +++++- checker/rule.go | 8 -- checker/rules_connection.go | 15 ++- checker/rules_federation.go | 12 ++- checker/rules_test.go | 178 ++++++++++++++++++++++++++++++++++++ checker/rules_tls.go | 37 ++++++-- plugin/plugin.go | 3 +- 13 files changed, 363 insertions(+), 41 deletions(-) create mode 100644 checker/collect_test.go create mode 100644 checker/rules_test.go diff --git a/Dockerfile b/Dockerfile index b7a7260..279b526 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,6 +9,7 @@ COPY . . RUN CGO_ENABLED=0 go build -tags standalone -ldflags "-X main.Version=${CHECKER_VERSION}" -o /checker-matrix . FROM scratch +COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt COPY --from=builder /checker-matrix /checker-matrix USER 65534:65534 EXPOSE 8080 diff --git a/checker/collect.go b/checker/collect.go index 9bc6340..e3175c4 100644 --- a/checker/collect.go +++ b/checker/collect.go @@ -4,12 +4,23 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" + "net/url" "strings" + "time" sdk "git.happydns.org/checker-sdk-go/checker" ) +const ( + defaultTesterURI = "https://federationtester.matrix.org/api/report?server_name=%s" + collectHTTPTimeout = 30 * time.Second + maxResponseBodySize = 5 << 20 // 5 MiB +) + +var collectHTTPClient = &http.Client{Timeout: collectHTTPTimeout} + func (p *matrixProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (any, error) { domain, _ := opts["serviceDomain"].(string) if domain == "" { @@ -19,15 +30,20 @@ func (p *matrixProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) ( testerURI, _ := opts["federationTesterServer"].(string) if testerURI == "" { - testerURI = "https://federationtester.matrix.org/api/report?server_name=%s" + testerURI = defaultTesterURI + } + if !strings.Contains(testerURI, "%s") { + return nil, fmt.Errorf("federationTesterServer must contain a %%s placeholder for the domain") } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf(testerURI, domain), nil) + reqURL := fmt.Sprintf(testerURI, url.QueryEscape(domain)) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) if err != nil { return nil, fmt.Errorf("unable to build the request: %w", err) } - resp, err := http.DefaultClient.Do(req) + resp, err := collectHTTPClient.Do(req) if err != nil { return nil, fmt.Errorf("unable to perform the test: %w", err) } @@ -38,7 +54,7 @@ func (p *matrixProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) ( } var data MatrixFederationData - if err := json.NewDecoder(resp.Body).Decode(&data); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, maxResponseBodySize)).Decode(&data); err != nil { return nil, fmt.Errorf("failed to decode federation tester response: %w", err) } diff --git a/checker/collect_test.go b/checker/collect_test.go new file mode 100644 index 0000000..b5d4a4b --- /dev/null +++ b/checker/collect_test.go @@ -0,0 +1,91 @@ +package checker + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + + sdk "git.happydns.org/checker-sdk-go/checker" +) + +func TestCollectMissingDomain(t *testing.T) { + p := &matrixProvider{} + if _, err := p.Collect(context.Background(), sdk.CheckerOptions{}); err == nil { + t.Fatal("expected error when serviceDomain is empty") + } +} + +func TestCollectSuccess(t *testing.T) { + const body = `{ + "WellKnownResult": {"m.server": "matrix.example.org:8448", "result": ""}, + "DNSResult": {"SRVSkipped": false, "SRVRecords": [{"Target": "matrix.example.org.", "Port": 8448, "Priority": 10, "Weight": 5}]}, + "ConnectionReports": {"1.2.3.4:8448": {"Checks": {"AllChecksOK": true, "MatchingServerName": true, "FutureValidUntilTS": true, "HasEd25519Key": true, "AllEd25519ChecksOK": true, "ValidCertificates": true}}}, + "ConnectionErrors": {}, + "Version": {"name": "Synapse", "version": "1.100.0"}, + "FederationOK": true + }` + + var gotURL string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotURL = r.URL.String() + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(body)) + })) + defer srv.Close() + + p := &matrixProvider{} + out, err := p.Collect(context.Background(), sdk.CheckerOptions{ + "serviceDomain": "example.org.", + "federationTesterServer": srv.URL + "/api/report?server_name=%s", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(gotURL, "server_name=example.org") { + t.Errorf("unexpected URL %q", gotURL) + } + data, ok := out.(*MatrixFederationData) + if !ok || data == nil { + t.Fatalf("expected *MatrixFederationData, got %T", out) + } + if !data.FederationOK { + t.Error("expected FederationOK=true") + } + if data.Version.Name != "Synapse" { + t.Errorf("unexpected version name %q", data.Version.Name) + } +} + +func TestCollectNon2xx(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadGateway) + })) + defer srv.Close() + + p := &matrixProvider{} + _, err := p.Collect(context.Background(), sdk.CheckerOptions{ + "serviceDomain": "example.org", + "federationTesterServer": srv.URL + "/?s=%s", + }) + if err == nil { + t.Fatal("expected error on 502 response") + } +} + +func TestCollectMalformedJSON(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte("not json")) + })) + defer srv.Close() + + p := &matrixProvider{} + _, err := p.Collect(context.Background(), sdk.CheckerOptions{ + "serviceDomain": "example.org", + "federationTesterServer": srv.URL + "/?s=%s", + }) + if err == nil { + t.Fatal("expected decode error") + } +} diff --git a/checker/definition.go b/checker/definition.go index ebf653e..cc296fe 100644 --- a/checker/definition.go +++ b/checker/definition.go @@ -17,7 +17,7 @@ import ( var Version = "built-in" // Definition returns the CheckerDefinition for the matrix federation checker. -func Definition() *sdk.CheckerDefinition { +func (p *matrixProvider) Definition() *sdk.CheckerDefinition { return &sdk.CheckerDefinition{ ID: "matrixim", Name: "Matrix Federation Tester", diff --git a/checker/interactive.go b/checker/interactive.go index 4f0deb8..9b35b79 100644 --- a/checker/interactive.go +++ b/checker/interactive.go @@ -35,7 +35,7 @@ func (p *matrixProvider) RenderForm() []sdk.CheckerOptionField { func (p *matrixProvider) ParseForm(r *http.Request) (sdk.CheckerOptions, error) { domain := strings.TrimSpace(r.FormValue("serviceDomain")) if domain == "" { - return nil, errors.New("Matrix domain is required") + return nil, errors.New("matrix domain is required") } opts := sdk.CheckerOptions{ diff --git a/checker/provider.go b/checker/provider.go index de7a43d..111b30c 100644 --- a/checker/provider.go +++ b/checker/provider.go @@ -14,8 +14,3 @@ type matrixProvider struct{} func (p *matrixProvider) Key() sdk.ObservationKey { return ObservationKeyMatrix } - -// Definition implements sdk.CheckerDefinitionProvider. -func (p *matrixProvider) Definition() *sdk.CheckerDefinition { - return Definition() -} diff --git a/checker/report.go b/checker/report.go index 7952ae0..ad88a39 100644 --- a/checker/report.go +++ b/checker/report.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "html/template" + "sort" "strings" sdk "git.happydns.org/checker-sdk-go/checker" @@ -326,7 +327,13 @@ func (p *matrixProvider) GetHTMLReport(ctx sdk.ReportContext) (string, error) { } // Hosts - for name, h := range r.DNSResult.Hosts { + hostNames := make([]string, 0, len(r.DNSResult.Hosts)) + for name := range r.DNSResult.Hosts { + hostNames = append(hostNames, name) + } + sort.Strings(hostNames) + for _, name := range hostNames { + h := r.DNSResult.Hosts[name] data.Hosts = append(data.Hosts, matrixHostData{ Name: name, CName: h.CName, @@ -335,7 +342,13 @@ func (p *matrixProvider) GetHTMLReport(ctx sdk.ReportContext) (string, error) { } // Successful connections - for addr, cr := range r.ConnectionReports { + connAddrs := make([]string, 0, len(r.ConnectionReports)) + for addr := range r.ConnectionReports { + connAddrs = append(connAddrs, addr) + } + sort.Strings(connAddrs) + for _, addr := range connAddrs { + cr := r.ConnectionReports[addr] conn := matrixConnectionData{ Address: addr, TLSVersion: cr.Cipher.Version, @@ -363,10 +376,15 @@ func (p *matrixProvider) GetHTMLReport(ctx sdk.ReportContext) (string, error) { } // Failed connections - for addr, ce := range r.ConnectionErrors { + errAddrs := make([]string, 0, len(r.ConnectionErrors)) + for addr := range r.ConnectionErrors { + errAddrs = append(errAddrs, addr) + } + sort.Strings(errAddrs) + for _, addr := range errAddrs { data.ConnectionErrors = append(data.ConnectionErrors, matrixConnErrData{ Address: addr, - Message: ce.Message, + Message: r.ConnectionErrors[addr].Message, }) } diff --git a/checker/rule.go b/checker/rule.go index 94d39f6..8506856 100644 --- a/checker/rule.go +++ b/checker/rule.go @@ -21,14 +21,6 @@ func Rules() []sdk.CheckRule { } } -// Rule returns the aggregate federation rule. -// -// Deprecated: prefer Rules() which exposes every concern individually. Kept -// for backward compatibility with callers that embed a single rule. -func Rule() sdk.CheckRule { - return &federationOKRule{} -} - // loadMatrixData fetches the Matrix observation. On error returns a // CheckState the caller should emit to short-circuit its rule. func loadMatrixData(ctx context.Context, obs sdk.ObservationGetter) (*MatrixFederationData, *sdk.CheckState) { diff --git a/checker/rules_connection.go b/checker/rules_connection.go index 4b6cf12..4cfa7ba 100644 --- a/checker/rules_connection.go +++ b/checker/rules_connection.go @@ -3,6 +3,7 @@ package checker import ( "context" "fmt" + "sort" sdk "git.happydns.org/checker-sdk-go/checker" ) @@ -23,16 +24,22 @@ func (r *connectionReachableRule) Evaluate(ctx context.Context, obs sdk.Observat } if len(data.ConnectionErrors) == 0 && len(data.ConnectionReports) == 0 { - return []sdk.CheckState{infoState("matrix.connection_reachable.unknown", "No endpoint was probed by the federation tester.")} + return []sdk.CheckState{unknownState("matrix.connection_reachable.unknown", "No endpoint was probed by the federation tester.")} } if len(data.ConnectionErrors) == 0 { return []sdk.CheckState{passState("matrix.connection_reachable.ok", fmt.Sprintf("All %d endpoint(s) accepted the connection.", len(data.ConnectionReports)))} } - out := make([]sdk.CheckState, 0, len(data.ConnectionErrors)) - for addr, cerr := range data.ConnectionErrors { - st := critState("matrix.connection_reachable.fail", cerr.Message) + addrs := make([]string, 0, len(data.ConnectionErrors)) + for addr := range data.ConnectionErrors { + addrs = append(addrs, addr) + } + sort.Strings(addrs) + + out := make([]sdk.CheckState, 0, len(addrs)) + for _, addr := range addrs { + st := critState("matrix.connection_reachable.fail", data.ConnectionErrors[addr].Message) st.Subject = addr out = append(out, st) } diff --git a/checker/rules_federation.go b/checker/rules_federation.go index 6261e50..60b8e34 100644 --- a/checker/rules_federation.go +++ b/checker/rules_federation.go @@ -3,6 +3,7 @@ package checker import ( "context" "fmt" + "sort" "strings" sdk "git.happydns.org/checker-sdk-go/checker" @@ -41,16 +42,21 @@ func (r *federationOKRule) Evaluate(ctx context.Context, obs sdk.ObservationGett var statusLine string switch { case data.DNSResult.SRVError != nil && data.WellKnownResult.Result != "": - statusLine = fmt.Sprintf("%s OR %s", data.DNSResult.SRVError.Message, data.WellKnownResult.Result) + statusLine = fmt.Sprintf("%s; %s", data.DNSResult.SRVError.Message, data.WellKnownResult.Result) case len(data.ConnectionErrors) > 0: + srvs := make([]string, 0, len(data.ConnectionErrors)) + for srv := range data.ConnectionErrors { + srvs = append(srvs, srv) + } + sort.Strings(srvs) var msg strings.Builder - for srv, cerr := range data.ConnectionErrors { + for _, srv := range srvs { if msg.Len() > 0 { msg.WriteString("; ") } msg.WriteString(srv) msg.WriteString(": ") - msg.WriteString(cerr.Message) + msg.WriteString(data.ConnectionErrors[srv].Message) } statusLine = fmt.Sprintf("Connection errors: %s", msg.String()) default: diff --git a/checker/rules_test.go b/checker/rules_test.go new file mode 100644 index 0000000..c865d58 --- /dev/null +++ b/checker/rules_test.go @@ -0,0 +1,178 @@ +package checker + +import ( + "context" + "encoding/json" + "strings" + "testing" + + sdk "git.happydns.org/checker-sdk-go/checker" +) + +type stubObs struct { + data *MatrixFederationData + err error +} + +func (s stubObs) Get(_ context.Context, _ sdk.ObservationKey, dest any) error { + if s.err != nil { + return s.err + } + b, err := json.Marshal(s.data) + if err != nil { + return err + } + return json.Unmarshal(b, dest) +} + +func (s stubObs) GetRelated(_ context.Context, _ sdk.ObservationKey) ([]sdk.RelatedObservation, error) { + return nil, nil +} + +func eval(t *testing.T, rule sdk.CheckRule, data *MatrixFederationData, opts sdk.CheckerOptions) []sdk.CheckState { + t.Helper() + return rule.Evaluate(context.Background(), stubObs{data: data}, opts) +} + +func TestFederationOKRulePass(t *testing.T) { + data := &MatrixFederationData{FederationOK: true} + data.Version.Name = "Synapse" + data.Version.Version = "1.100.0" + got := eval(t, &federationOKRule{}, data, sdk.CheckerOptions{"serviceDomain": "example.org."}) + if len(got) != 1 || got[0].Status != sdk.StatusOK { + t.Fatalf("expected single OK state, got %+v", got) + } + if !strings.Contains(got[0].Message, "Synapse 1.100.0") { + t.Errorf("expected version in message, got %q", got[0].Message) + } +} + +func TestFederationOKRuleFailDeterministicOrder(t *testing.T) { + data := &MatrixFederationData{} + data.ConnectionErrors = map[string]struct { + Message string `json:"Message"` + }{ + "z.example:8448": {Message: "boom z"}, + "a.example:8448": {Message: "boom a"}, + "m.example:8448": {Message: "boom m"}, + } + first := eval(t, &federationOKRule{}, data, nil)[0].Message + for range 5 { + if eval(t, &federationOKRule{}, data, nil)[0].Message != first { + t.Fatal("federation_ok message not stable across runs") + } + } + idxA := strings.Index(first, "a.example") + idxM := strings.Index(first, "m.example") + idxZ := strings.Index(first, "z.example") + if !(idxA < idxM && idxM < idxZ) { + t.Errorf("expected sorted order a