diff --git a/checker/collect.go b/checker/collect.go index 87deb16..d7d3d08 100644 --- a/checker/collect.go +++ b/checker/collect.go @@ -23,13 +23,10 @@ package checker import ( "context" - "crypto/sha256" - "encoding/hex" "encoding/json" "fmt" "log" "net" - "sort" "strconv" "strings" "sync" @@ -119,76 +116,6 @@ func (p *sshProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (any return data, nil } -// ShareKey implements sdk.ObservationSharer. An SSH probe (reachability, banner, -// KEX/host-key algorithm posture, host keys) depends only on the set of -// addresses and ports dialed and the probe knobs, never on which domain name -// points at the server: SSH has no SNI, so the same daemon answers identically -// behind every name. The declared SSHFP fingerprints are folded in because they -// live in the observation and drive the SSHFP-match rule — two services with the -// same endpoints but different SSHFP must not share a verdict. -// -// The host/Domain label is intentionally excluded, mirroring the ping checker's -// exclusion of "which domain the addresses belong to": it is a display label -// that does not change reachability, the negotiated algorithms, the host keys, -// or the SSHFP comparison. Inputs with no probable address return "" so the host -// falls back to the default per-target caching. -func (p *sshProvider) ShareKey(opts sdk.CheckerOptions) (string, error) { - server, err := resolveServer(opts) - if err != nil { - return "", nil - } - - apex := "" - if v, ok := sdk.GetOption[string](opts, OptionDomainName); ok { - apex = strings.TrimSuffix(v, ".") - } - subdomain := "" - if svc, ok := sdk.GetOption[happydns.ServiceMessage](opts, OptionService); ok { - subdomain = strings.TrimSuffix(svc.Domain, ".") - } - origin := sdk.JoinRelative(subdomain, apex) - _, ips := addressesFromServer(server, origin) - if len(ips) == 0 { - return "", nil - } - - ports := parsePorts(optString(opts, OptionPorts, "")) - if len(ports) == 0 { - ports = []uint16{DefaultSSHPort} - } - - timeoutMs := sdk.GetIntOption(opts, OptionProbeTimeoutMs, DefaultProbeTimeoutMs) - if timeoutMs <= 0 { - timeoutMs = DefaultProbeTimeoutMs - } - includeAuthProbe := sdk.GetBoolOption(opts, OptionIncludeAuthProbe, true) - - sortedIPs := append([]string(nil), ips...) - sort.Strings(sortedIPs) - - sortedPorts := append([]uint16(nil), ports...) - sort.Slice(sortedPorts, func(i, j int) bool { return sortedPorts[i] < sortedPorts[j] }) - portStrs := make([]string, len(sortedPorts)) - for i, p := range sortedPorts { - portStrs[i] = strconv.Itoa(int(p)) - } - - sshfp := sshfpFromServer(server) - fps := make([]string, 0, len(sshfp.Records)) - for _, r := range sshfp.Records { - fps = append(fps, fmt.Sprintf("%d:%d:%s", r.Algorithm, r.Type, r.Fingerprint)) - } - sort.Strings(fps) - - h := sha256.Sum256(fmt.Appendf(nil, "%d|%t|%s|%s|%s", - timeoutMs, includeAuthProbe, - strings.Join(sortedIPs, ","), - strings.Join(portStrs, ","), - strings.Join(fps, ","), - )) - return "ssh:" + hex.EncodeToString(h[:8]), nil -} - // resolveServer extracts the *abstract.Server payload from the options. // Two shapes are supported, same as the ping checker: // - "service": ServiceMessage (in-process plugin path, or HTTP after diff --git a/checker/rules_banner.go b/checker/rules_banner.go index 6ee1c19..6758838 100644 --- a/checker/rules_banner.go +++ b/checker/rules_banner.go @@ -113,7 +113,7 @@ func (r *knownVulnsRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter } var issues []Issue for _, ep := range data.Endpoints { - issues = append(issues, analyseBannerVulns(ep.Addr(), ep.Banner, ep.SoftVer, ep.Vendor)...) + issues = append(issues, analyseBannerVulns(ep.Addr(), ep.Banner, ep.SoftVer)...) } if len(issues) == 0 { return []sdk.CheckState{passState("ssh.known_vulnerabilities.ok", "No known CVE match against the advertised OpenSSH versions.")} diff --git a/checker/sharekey_test.go b/checker/sharekey_test.go deleted file mode 100644 index 968f0ab..0000000 --- a/checker/sharekey_test.go +++ /dev/null @@ -1,115 +0,0 @@ -package checker - -import ( - "encoding/json" - "net" - "strings" - "testing" - - "github.com/miekg/dns" - - sdk "git.happydns.org/checker-sdk-go/checker" - happydns "git.happydns.org/happyDomain/model" - "git.happydns.org/happyDomain/services/abstract" -) - -func serverOpts(t *testing.T, domain string, srv abstract.Server, extra map[string]any) sdk.CheckerOptions { - t.Helper() - raw, err := json.Marshal(srv) - if err != nil { - t.Fatal(err) - } - opts := sdk.CheckerOptions{ - OptionService: happydns.ServiceMessage{ - ServiceMeta: happydns.ServiceMeta{ - Type: "abstract.Server", - Domain: domain, - }, - Service: raw, - }, - OptionDomainName: "example.org", - } - for k, v := range extra { - opts[k] = v - } - return opts -} - -func TestShareKey_StableAndPrefixed(t *testing.T) { - p := &sshProvider{} - srv := abstract.Server{A: &dns.A{A: net.ParseIP("192.0.2.1").To4()}} - - a, err := p.ShareKey(serverOpts(t, "host", srv, nil)) - if err != nil { - t.Fatal(err) - } - b, _ := p.ShareKey(serverOpts(t, "host", srv, nil)) - if a != b { - t.Fatalf("non-deterministic key: %q vs %q", a, b) - } - if !strings.HasPrefix(a, "ssh:") { - t.Fatalf("missing prefix: %q", a) - } -} - -// The host/Domain label must not change the key: the same server answers -// identically behind every name (SSH has no SNI). -func TestShareKey_IgnoresDomainLabel(t *testing.T) { - p := &sshProvider{} - srv := abstract.Server{A: &dns.A{A: net.ParseIP("192.0.2.1").To4()}} - - a, _ := p.ShareKey(serverOpts(t, "alpha", srv, nil)) - b, _ := p.ShareKey(serverOpts(t, "beta", srv, nil)) - if a != b { - t.Fatalf("domain label must not affect the key: %q vs %q", a, b) - } -} - -func TestShareKey_DiffersByAddress(t *testing.T) { - p := &sshProvider{} - a, _ := p.ShareKey(serverOpts(t, "host", abstract.Server{A: &dns.A{A: net.ParseIP("192.0.2.1").To4()}}, nil)) - b, _ := p.ShareKey(serverOpts(t, "host", abstract.Server{A: &dns.A{A: net.ParseIP("192.0.2.2").To4()}}, nil)) - if a == b { - t.Fatalf("different addresses must yield different keys") - } -} - -func TestShareKey_DiffersByPortsAndKnobs(t *testing.T) { - p := &sshProvider{} - srv := abstract.Server{A: &dns.A{A: net.ParseIP("192.0.2.1").To4()}} - - def, _ := p.ShareKey(serverOpts(t, "host", srv, nil)) - ports, _ := p.ShareKey(serverOpts(t, "host", srv, map[string]any{OptionPorts: "22,2222"})) - auth, _ := p.ShareKey(serverOpts(t, "host", srv, map[string]any{OptionIncludeAuthProbe: false})) - timeout, _ := p.ShareKey(serverOpts(t, "host", srv, map[string]any{OptionProbeTimeoutMs: float64(1000)})) - - for name, got := range map[string]string{"ports": ports, "authProbe": auth, "timeout": timeout} { - if got == def { - t.Fatalf("%s must affect the key", name) - } - } -} - -// SSHFP fingerprints live in the observation and drive the SSHFP-match rule, so -// they must be part of the key. -func TestShareKey_DiffersBySSHFP(t *testing.T) { - p := &sshProvider{} - bare := abstract.Server{A: &dns.A{A: net.ParseIP("192.0.2.1").To4()}} - withFP := abstract.Server{ - A: &dns.A{A: net.ParseIP("192.0.2.1").To4()}, - SSHFP: []*dns.SSHFP{{Algorithm: 4, Type: 2, FingerPrint: "abcdef"}}, - } - - a, _ := p.ShareKey(serverOpts(t, "host", bare, nil)) - b, _ := p.ShareKey(serverOpts(t, "host", withFP, nil)) - if a == b { - t.Fatalf("declared SSHFP must affect the key") - } -} - -func TestShareKey_EmptyWhenNoAddress(t *testing.T) { - p := &sshProvider{} - if sk, err := p.ShareKey(sdk.CheckerOptions{}); err != nil || sk != "" { - t.Fatalf("expected empty key, got %q (err=%v)", sk, err) - } -} diff --git a/checker/vulns.go b/checker/vulns.go index 5be7740..d74415e 100644 --- a/checker/vulns.go +++ b/checker/vulns.go @@ -47,12 +47,6 @@ type opensshVuln struct { Description string Fix string AffectedRanges []opensshRange - // VendorFixes lists distribution packages that backport the fix - // without bumping the upstream version string. When the observed - // banner matches one of these (same vendor and upstream version, - // with a package revision at or above FixedFrom), the CVE is - // suppressed even though AffectedRanges would otherwise match. - VendorFixes []vendorFix } type opensshRange struct { @@ -60,19 +54,6 @@ type opensshRange struct { MaxExclusive string // "" means open-ended above } -// vendorFix records the earliest distribution package revision that -// carries a backported fix for a given upstream OpenSSH version. The -// Vendor is the leading token of the banner's vendor comment (e.g. -// "Debian" in "Debian-2+deb12u3"); Upstream is the upstream version -// the distro ships (e.g. "9.2p1"); FixedFrom is the package revision -// that first shipped the fix (e.g. "2+deb12u3"). Revisions are compared -// with the dpkg version-ordering algorithm. -type vendorFix struct { - Vendor string - Upstream string - FixedFrom string -} - var opensshVulns = []opensshVuln{ { Code: "cve_2024_6387_regreSSHion", @@ -87,16 +68,6 @@ var opensshVulns = []opensshVuln{ // The race also existed in < 4.4p1 (CVE-2006-5051 variant). {MaxExclusive: "4.4p1"}, }, - // Distributions backported the fix without changing the upstream - // version. Sources: DSA-5724-1 (Debian) and USN-6859-1 (Ubuntu). - // Debian bullseye (8.4p1) and Ubuntu focal (8.2p1) ship versions - // below 8.5p1 and are therefore not affected at all. - VendorFixes: []vendorFix{ - {Vendor: "Debian", Upstream: "9.2p1", FixedFrom: "2+deb12u3"}, // bookworm - {Vendor: "Ubuntu", Upstream: "9.6p1", FixedFrom: "3ubuntu13.3"}, // noble 24.04 - {Vendor: "Ubuntu", Upstream: "9.3p1", FixedFrom: "1ubuntu3.6"}, // mantic 23.10 - {Vendor: "Ubuntu", Upstream: "8.9p1", FixedFrom: "3ubuntu0.10"}, // jammy 22.04 - }, }, { Code: "cve_2023_38408_agent", @@ -176,15 +147,14 @@ func analyseBannerSoftware(addr, banner, software string) []Issue { } // analyseBannerVulns runs the banner through the OpenSSH CVE database -// and returns the matched issues. The upstream version match is -// deliberately loose, because distribution maintainers tend to backport -// fixes without changing the version string (e.g. "OpenSSH_9.2p1 -// Debian-2+deb12u3" reports the same 9.2p1 as a vulnerable build). To -// avoid false positives, a matched CVE is suppressed when the vendor -// comment identifies a distribution package known to carry the -// backported fix (see vendorFix). Any residual false positives can -// still be overridden at the UI layer, same as other checkers. -func analyseBannerVulns(addr, banner, software, vendor string) []Issue { +// and returns the matched issues. The banner parser is deliberately +// loose: a server running a vendor-patched OpenSSH (e.g. +// "OpenSSH_9.2p1 Debian-2+deb12u2") will still match the upstream +// version numbers, because distribution maintainers tend to backport +// fixes without changing the version string. Operators get to +// override these false positives at the UI layer, same as other +// checkers. +func analyseBannerVulns(addr, banner, software string) []Issue { if banner == "" { return nil } @@ -194,71 +164,25 @@ func analyseBannerVulns(addr, banner, software, vendor string) []Issue { } var issues []Issue for _, v := range opensshVulns { - if !rangesMatch(ver, v.AffectedRanges) { - continue + if rangesMatch(ver, v.AffectedRanges) { + issues = append(issues, Issue{ + Code: v.Code, + Severity: v.Severity, + Message: fmt.Sprintf("%s: %s", v.Title, v.Description), + Fix: v.Fix, + Endpoint: addr, + }) } - if vendorPatched(software, vendor, v.VendorFixes) { - continue - } - issues = append(issues, Issue{ - Code: v.Code, - Severity: v.Severity, - Message: fmt.Sprintf("%s: %s", v.Title, v.Description), - Fix: v.Fix, - Endpoint: addr, - }) } return issues } -// vendorPatched reports whether the banner's vendor comment identifies a -// distribution package that has backported the fix for this CVE. The -// vendor comment ("Debian-2+deb12u3") is split into a vendor name and a -// package revision; a fix applies when the vendor name and the upstream -// version both match and the observed revision is at or above the -// recorded FixedFrom revision (dpkg ordering). -func vendorPatched(software, vendor string, fixes []vendorFix) bool { - if vendor == "" || len(fixes) == 0 { - return false - } - name, revision := splitVendorComment(vendor) - if name == "" || revision == "" { - return false - } - upstream := upstreamVersionString(software) - for _, f := range fixes { - if !strings.EqualFold(f.Vendor, name) || f.Upstream != upstream { - continue - } - if dpkgVerCmp(revision, f.FixedFrom) >= 0 { - return true - } - } - return false -} - -// splitVendorComment splits a banner vendor comment such as -// "Debian-2+deb12u3" or "Ubuntu-3ubuntu0.10" into its vendor name and -// package revision on the first '-'. -func splitVendorComment(vendor string) (name, revision string) { - name, revision, _ = strings.Cut(vendor, "-") - return name, revision -} - -// upstreamVersionString returns the bare upstream version token from a -// software identifier, e.g. "OpenSSH_9.2p1" -> "9.2p1". It returns "" -// when the identifier is not a recognised OpenSSH banner. -func upstreamVersionString(software string) string { - m := opensshBannerRe.FindString(software) - return strings.TrimPrefix(m, "OpenSSH_") -} - // analyseBanner combines software-awareness and vulnerability matches. // Retained as a convenience for the HTML report, which surfaces both // concerns in a single "What to fix" list. -func analyseBanner(addr, banner, software, vendor string) []Issue { +func analyseBanner(addr, banner, software string) []Issue { out := analyseBannerSoftware(addr, banner, software) - out = append(out, analyseBannerVulns(addr, banner, software, vendor)...) + out = append(out, analyseBannerVulns(addr, banner, software)...) return out } @@ -323,77 +247,3 @@ func parseVersionString(s string) (opensshVersion, bool) { } return *v, true } - -// dpkgVerCmp compares two Debian/Ubuntu package version segments using -// the dpkg version-ordering algorithm (verrevcmp). It returns a negative -// value when a sorts before b, zero when equal, and a positive value -// when a sorts after b. We feed it the package-revision portion of a -// banner vendor comment (e.g. "2+deb12u3" vs "2+deb12u2"), which is -// sufficient because backport comparisons are always within the same -// upstream version. -func dpkgVerCmp(a, b string) int { - i, j := 0, 0 - for i < len(a) || j < len(b) { - // Compare the non-digit prefixes character by character using - // dpkg's special ordering (notably '~' sorts before everything, - // including the end of string). - for (i < len(a) && !isASCIIDigit(a[i])) || (j < len(b) && !isASCIIDigit(b[j])) { - ac, bc := 0, 0 - if i < len(a) { - ac = dpkgOrder(a[i]) - } - if j < len(b) { - bc = dpkgOrder(b[j]) - } - if ac != bc { - return ac - bc - } - i++ - j++ - } - // Skip leading zeros so digit runs compare by numeric value. - for i < len(a) && a[i] == '0' { - i++ - } - for j < len(b) && b[j] == '0' { - j++ - } - // Compare the digit runs: a longer run of significant digits is - // the larger number; on equal length the first differing digit - // decides. - firstDiff := 0 - for i < len(a) && isASCIIDigit(a[i]) && j < len(b) && isASCIIDigit(b[j]) { - if firstDiff == 0 { - firstDiff = int(a[i]) - int(b[j]) - } - i++ - j++ - } - if i < len(a) && isASCIIDigit(a[i]) { - return 1 - } - if j < len(b) && isASCIIDigit(b[j]) { - return -1 - } - if firstDiff != 0 { - return firstDiff - } - } - return 0 -} - -// dpkgOrder maps a non-digit byte to its dpkg sort weight: '~' sorts -// before everything (including end-of-string, weight 0), letters keep -// their ASCII value, and any other character sorts after letters. -func dpkgOrder(c byte) int { - switch { - case (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z'): - return int(c) - case c == '~': - return -1 - default: - return int(c) + 256 - } -} - -func isASCIIDigit(c byte) bool { return c >= '0' && c <= '9' } diff --git a/checker/vulns_vendorfix_test.go b/checker/vulns_vendorfix_test.go deleted file mode 100644 index 48c51d3..0000000 --- a/checker/vulns_vendorfix_test.go +++ /dev/null @@ -1,92 +0,0 @@ -// This file is part of the happyDomain (R) project. -// Copyright (c) 2020-2026 happyDomain -// Authors: Pierre-Olivier Mercier, et al. -// -// This program is offered under a commercial and under the AGPL license. -// For commercial licensing, contact us at . -// -// For AGPL licensing: -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -package checker - -import "testing" - -func hasIssue(issues []Issue, code string) bool { - for _, i := range issues { - if i.Code == code { - return true - } - } - return false -} - -func TestVendorFixSuppressesRegreSSHion(t *testing.T) { - const code = "cve_2024_6387_regreSSHion" - cases := []struct { - name string - soft string - vendor string - flagged bool - }{ - // Vanilla upstream in the affected window: must flag. - {"upstream 9.2p1", "OpenSSH_9.2p1", "", true}, - // Debian bookworm before / at / after the fix. - {"debian unpatched", "OpenSSH_9.2p1", "Debian-2+deb12u2", true}, - {"debian patched", "OpenSSH_9.2p1", "Debian-2+deb12u3", false}, - {"debian later point release", "OpenSSH_9.2p1", "Debian-2+deb12u10", false}, - // Ubuntu jammy: numeric ".10" must beat ".2" (dpkg numeric run). - {"ubuntu jammy unpatched", "OpenSSH_8.9p1", "Ubuntu-3ubuntu0.2", true}, - {"ubuntu jammy patched", "OpenSSH_8.9p1", "Ubuntu-3ubuntu0.10", false}, - // Ubuntu noble. - {"ubuntu noble patched", "OpenSSH_9.6p1", "Ubuntu-3ubuntu13.3", false}, - {"ubuntu noble unpatched", "OpenSSH_9.6p1", "Ubuntu-3ubuntu13.2", true}, - // A fix recorded for a different upstream version must not apply. - {"vendor mismatch upstream", "OpenSSH_9.3p1", "Debian-2+deb12u3", true}, - // Not affected at all (below 8.5p1): never flagged regardless. - {"debian bullseye", "OpenSSH_8.4p1", "Debian-5+deb11u1", false}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - banner := "SSH-2.0-" + tc.soft - if tc.vendor != "" { - banner += " " + tc.vendor - } - issues := analyseBannerVulns("host:22", banner, tc.soft, tc.vendor) - if got := hasIssue(issues, code); got != tc.flagged { - t.Fatalf("regreSSHion flagged=%v, want %v (issues=%v)", got, tc.flagged, issues) - } - }) - } -} - -func TestDpkgVerCmp(t *testing.T) { - cases := []struct { - a, b string - want int // sign - }{ - {"2+deb12u3", "2+deb12u2", 1}, - {"2+deb12u3", "2+deb12u3", 0}, - {"2+deb12u3", "2+deb12u10", -1}, - {"3ubuntu0.10", "3ubuntu0.2", 1}, - {"3ubuntu13.3", "3ubuntu13.2", 1}, - {"1.0", "1.0~rc1", 1}, // tilde sorts before everything - } - for _, tc := range cases { - got := dpkgVerCmp(tc.a, tc.b) - if (got > 0) != (tc.want > 0) || (got < 0) != (tc.want < 0) { - t.Errorf("dpkgVerCmp(%q,%q)=%d, want sign %d", tc.a, tc.b, got, tc.want) - } - } -}