refactor(pki): simplify after code review

- Reuse ValidateDomainParam from pkg/validation in validateDomain
  instead of duplicating control char/path checks
- Restore cache-first in GetCertificate for hot-path performance;
  domain revocation already handled by sweepExpiredCerts every 10m
- Remove redundant comment

commit: 5bf3a89200d7a1e1b0d416e17e0cf4909514bf34
author: brice · 6mo ago · 2026-03-30 18:46 UTC
committer: brice · 6mo ago · 2026-03-30 18:48 UTC
parent: 450331c
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +8 -17

M internal/adapters/out/pki/ca.go +3 -10


@@ -24,6 +24,7 @@ import (
24	24		"github.com/bnema/zerowrap"
25	25
26	26		out "github.com/bnema/gordon/internal/boundaries/out"
	27	+	"github.com/bnema/gordon/pkg/validation"
27	28		)
28	29
29	30		// Compile-time check: *CA satisfies the CertificateAuthority boundary.

@@ -435,13 +436,8 @@ func parseCertAndKey(certPEM, keyPEM []byte) (*x509.Certificate, crypto.Signer,
435	436		// validateDomain performs defense-in-depth validation of a domain name
436	437		// before issuing a certificate.
437	438		func validateDomain(domain string) error {
438		-	if domain == "" {
439		-	return fmt.Errorf("empty domain")
440		-	}
441		-	for _, r := range domain {
442		-	if r < 32 {
443		-	return fmt.Errorf("domain contains control characters")
444		-	}
	439	+	if err := validation.ValidateDomainParam(domain); err != nil {
	440	+	return err
445	441		}
446	442		if len(domain) > 253 {
447	443		return fmt.Errorf("domain too long (%d chars), limit is 253", len(domain))

@@ -449,9 +445,6 @@ func validateDomain(domain string) error {
449	445		if strings.HasPrefix(domain, "*") {
450	446		return fmt.Errorf("wildcard domains not allowed")
451	447		}
452		-	if strings.ContainsAny(domain, "/\\") {
453		-	return fmt.Errorf("domain contains path separators")
454		-	}
455	448		if net.ParseIP(domain) != nil {
456	449		return fmt.Errorf("IP addresses not allowed, must be a domain name")
457	450		}

 	"github.com/bnema/zerowrap"
 	out "github.com/bnema/gordon/internal/boundaries/out"
+	"github.com/bnema/gordon/pkg/validation"
 )
 // Compile-time check: *CA satisfies the CertificateAuthority boundary.
 // validateDomain performs defense-in-depth validation of a domain name
 // before issuing a certificate.
 func validateDomain(domain string) error {
-	if domain == "" {
+	if err := validation.ValidateDomainParam(domain); err != nil {
-		return fmt.Errorf("empty domain")
+		return err
-	}
-	for _, r := range domain {
-		if r < 32 {
-			return fmt.Errorf("domain contains control characters")
-		}
 	}
 	if len(domain) > 253 {
 		return fmt.Errorf("domain too long (%d chars), limit is 253", len(domain))
 	if strings.HasPrefix(domain, "*") {
 		return fmt.Errorf("wildcard domains not allowed")
 	}
-	if strings.ContainsAny(domain, "/\\") {
-		return fmt.Errorf("domain contains path separators")
-	}
 	if net.ParseIP(domain) != nil {
 		return fmt.Errorf("IP addresses not allowed, must be a domain name")
 	}

M internal/app/run.go +1 -2


@@ -375,7 +375,6 @@ func createServices(ctx context.Context, v *viper.Viper, cfg Config, log zerowra
375	375		return nil, log.WrapErr(err, "failed to load configuration")
376	376		}
377	377
378		-	// Create PKI (internal CA) — disabled when tls_port is 0.
379	378		if cfg.Server.TLSPort != 0 {
380	379		caAdapter, err := pkiadapter.NewCA(resolveDataDir(cfg.Server.DataDir), log)
381	380		if err != nil {

@@ -1908,7 +1907,7 @@ func runServers(ctx context.Context, v viper.Viper, cfg Config, svc services,
1908	1907		logEvent := log.Info().
1909	1908		Int("proxy_port", cfg.Server.Port).
1910	1909		Int("registry_port", cfg.Server.RegistryPort)
1911		-	if cfg.Server.TLSPort != 0 {
	1910	+	if tlsSrv != nil {
1912	1911		logEvent = logEvent.Int("tls_port", cfg.Server.TLSPort)
1913	1912		}
1914	1913		logEvent.Msg("Gordon is running")

 		return nil, log.WrapErr(err, "failed to load configuration")
 	}
-	// Create PKI (internal CA) — disabled when tls_port is 0.
 	if cfg.Server.TLSPort != 0 {
 		caAdapter, err := pkiadapter.NewCA(resolveDataDir(cfg.Server.DataDir), log)
 		if err != nil {
 	logEvent := log.Info().
 		Int("proxy_port", cfg.Server.Port).
 		Int("registry_port", cfg.Server.RegistryPort)
-	if cfg.Server.TLSPort != 0 {
+	if tlsSrv != nil {
 		logEvent = logEvent.Int("tls_port", cfg.Server.TLSPort)
 	}
 	logEvent.Msg("Gordon is running")

M internal/usecase/pki/service.go +4 -5


@@ -74,11 +74,6 @@ func (s Service) GetCertificate(hello tls.ClientHelloInfo) (*tls.Certificate,
74	74		return nil, fmt.Errorf("no SNI in ClientHello")
75	75		}
76	76
77		-	if !s.isDomainAllowed(hello.Context(), domain) {
78		-	s.cache.Delete(domain)
79		-	return nil, fmt.Errorf("domain %q not in route table", domain)
80		-	}
81		-
82	77		if entry, ok := s.cache.Load(domain); ok {
83	78		if cached, ok := entry.(*cachedCert); ok {
84	79		if time.Now().Before(cached.expiresAt) {

@@ -88,6 +83,10 @@ func (s Service) GetCertificate(hello tls.ClientHelloInfo) (*tls.Certificate,
88	83		s.cache.Delete(domain)
89	84		}
90	85
	86	+	if !s.isDomainAllowed(hello.Context(), domain) {
	87	+	return nil, fmt.Errorf("domain %q not in route table", domain)
	88	+	}
	89	+
91	90		cert, err := s.ca.IssueCertificate(domain)
92	91		if err != nil {
93	92		s.log.Error().Err(err).Str("domain", domain).Msg("failed to issue leaf cert")

 		return nil, fmt.Errorf("no SNI in ClientHello")
 	}
-	if !s.isDomainAllowed(hello.Context(), domain) {
-		s.cache.Delete(domain)
-		return nil, fmt.Errorf("domain %q not in route table", domain)
-	}
 	if entry, ok := s.cache.Load(domain); ok {
 		if cached, ok := entry.(*cachedCert); ok {
 			if time.Now().Before(cached.expiresAt) {
 		s.cache.Delete(domain)
 	}
+	if !s.isDomainAllowed(hello.Context(), domain) {
+		return nil, fmt.Errorf("domain %q not in route table", domain)
+	}
 	cert, err := s.ca.IssueCertificate(domain)
 	if err != nil {
 		s.log.Error().Err(err).Str("domain", domain).Msg("failed to issue leaf cert")