Skip to content

Commit 59be38f

Browse files
sarg3ntclaude
andcommitted
address Copilot review: stale doc comment + case-insensitive DNS SAN match
- config.go: TLSHosts doc no longer claims os.Hostname() is auto-added; explains why we explicitly skip it (random container ID forces regen). - tls.go: verifyCertCoversHosts now lowercases DNS SANs and strips a trailing dot on both sides before comparison. Per RFC 6125 §6.4 DNS host matching is case-insensitive, so "Example.COM." and "example.com" must compare equal — otherwise a casing-only env-var edit would force a spurious cert regeneration. IP comparison is unchanged. - crypto_test.go: extends TestLoadOrCreateTLSCert_RegenOnMissingSAN with two new assertions covering the casing and trailing-dot equivalences. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f024c71 commit 59be38f

3 files changed

Lines changed: 52 additions & 6 deletions

File tree

gearbox-agent/internal/framework/config/config.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,14 @@ type Config struct {
2222
// auto-generated self-signed certificate. Ignored when TLSCustom is
2323
// true (the user-provided cert is used verbatim).
2424
//
25-
// The agent already includes "localhost", "127.0.0.1", "::1", and the
26-
// container's os.Hostname() — TLSHosts is for the address the operator
27-
// will actually point clients at (e.g. a static container IP, an FQDN
28-
// behind a reverse proxy, or both).
25+
// The cert generator always covers "localhost", "127.0.0.1", and
26+
// "::1"; TLSHosts is for any other address clients will actually
27+
// dial (e.g. a static container IP, an FQDN behind a reverse proxy,
28+
// or both). The container's os.Hostname() is deliberately NOT added
29+
// automatically — in a container it's a random short ID that
30+
// changes per recreation, which would force needless cert
31+
// regeneration since LoadOrCreateTLSCert regenerates when the
32+
// existing cert is missing a requested SAN.
2933
//
3034
// Parsed from HAPROXY_AGENT_TLS_HOSTS as a comma-separated list;
3135
// whitespace around each entry is trimmed, empty entries are dropped.

gearbox-agent/internal/framework/crypto/crypto_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,29 @@ func TestLoadOrCreateTLSCert_RegenOnMissingSAN(t *testing.T) {
461461
if !containsString(regenerated.DNSNames, "example.com") {
462462
t.Errorf("regenerated cert lost example.com SAN: %v", regenerated.DNSNames)
463463
}
464+
regeneratedSerial := regenerated.SerialNumber.String()
465+
466+
// Casing change on the same DNS name → reuse, no spurious regen
467+
// (RFC 6125 §6.4: DNS host matching is case-insensitive).
468+
_, isNew, err = LoadOrCreateTLSCert(certPath, keyPath, []string{"EXAMPLE.com", "172.16.2.3"})
469+
if err != nil {
470+
t.Fatalf("fourth LoadOrCreateTLSCert() error = %v", err)
471+
}
472+
if isNew {
473+
t.Error("call differing only in DNS casing should not regenerate")
474+
}
475+
if loadCertForTest(t, certPath).SerialNumber.String() != regeneratedSerial {
476+
t.Error("cert serial changed across casing-only difference (unexpected regeneration)")
477+
}
478+
479+
// Trailing-dot equivalence (example.com. == example.com per DNS).
480+
_, isNew, err = LoadOrCreateTLSCert(certPath, keyPath, []string{"example.com.", "172.16.2.3"})
481+
if err != nil {
482+
t.Fatalf("fifth LoadOrCreateTLSCert() error = %v", err)
483+
}
484+
if isNew {
485+
t.Error("trailing-dot variant should not regenerate")
486+
}
464487
}
465488

466489
func loadCertForTest(t *testing.T, certPath string) *x509.Certificate {

gearbox-agent/internal/framework/crypto/tls.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"net"
1313
"os"
1414
"path/filepath"
15+
"strings"
1516
"time"
1617
)
1718

@@ -130,6 +131,12 @@ func verifyCert(certPath string) error {
130131
// verifyCertCoversHosts loads the cert at certPath and confirms every entry
131132
// in `hosts` is present as a SAN (DNS name for hostnames, IPAddresses entry
132133
// for IPs). Returns nil if all hosts are covered.
134+
//
135+
// DNS comparison is case-insensitive per RFC 6125 §6.4 (TLS host matching),
136+
// so changing only the casing of an entry in HAPROXY_AGENT_TLS_HOSTS will
137+
// not force a spurious regeneration. A single trailing dot on FQDNs is also
138+
// stripped on both sides ("example.com." and "example.com" are equivalent
139+
// per DNS).
133140
func verifyCertCoversHosts(certPath string, hosts []string) error {
134141
data, err := os.ReadFile(certPath)
135142
if err != nil {
@@ -146,7 +153,7 @@ func verifyCertCoversHosts(certPath string, hosts []string) error {
146153

147154
dnsSet := make(map[string]struct{}, len(cert.DNSNames))
148155
for _, d := range cert.DNSNames {
149-
dnsSet[d] = struct{}{}
156+
dnsSet[normaliseDNSName(d)] = struct{}{}
150157
}
151158
ipSet := make(map[string]struct{}, len(cert.IPAddresses))
152159
for _, ip := range cert.IPAddresses {
@@ -160,13 +167,25 @@ func verifyCertCoversHosts(certPath string, hosts []string) error {
160167
}
161168
continue
162169
}
163-
if _, ok := dnsSet[h]; !ok {
170+
if _, ok := dnsSet[normaliseDNSName(h)]; !ok {
164171
return fmt.Errorf("certificate missing DNS SAN %s", h)
165172
}
166173
}
167174
return nil
168175
}
169176

177+
// normaliseDNSName lowercases and strips a single trailing dot so that
178+
// "Example.COM.", "example.com.", and "example.com" all compare equal.
179+
// DNS names are case-insensitive and the trailing dot is the
180+
// FQDN/relative distinction, not a real character.
181+
func normaliseDNSName(s string) string {
182+
s = strings.ToLower(s)
183+
if len(s) > 0 && s[len(s)-1] == '.' {
184+
s = s[:len(s)-1]
185+
}
186+
return s
187+
}
188+
170189
// generateSelfSignedCert creates a new self-signed TLS certificate and key.
171190
// The certificate is valid for 1 year and includes the specified hosts as SANs.
172191
func generateSelfSignedCert(certPath, keyPath string, hosts []string) error {

0 commit comments

Comments
 (0)