refactor(pki): simplify after code review
- Remove redundant rootDER field; derive from rootCert.Raw - Cache mobileconfig at handler construction instead of per-request - Use direct map lookup in isDomainAllowed for external routes - Remove narrating comments in service.go and ca.go
- commit
- 32363a88abdbda80c8e5f8fad2819734fdb3f299
- author
- brice · · 2026-03-29 17:57 UTC
- parent
- dddcd1a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +10 -22
M internal/adapters/in/http/onboarding/handler.go +7 -10
| @@ -10,19 +10,17 @@ import ( | |||
| 10 | 10 | ||
| 11 | 11 | // Handler serves CA onboarding endpoints on the HTTP port. | |
| 12 | 12 | type Handler struct { | |
| 13 | - | rootPEM []byte | |
| 14 | - | rootDER []byte | |
| 15 | - | rootCN string | |
| 16 | - | cidrs []*net.IPNet // Cloudflare CIDRs — if source IP matches, don't serve onboarding | |
| 13 | + | rootPEM []byte | |
| 14 | + | mobileconfig []byte | |
| 15 | + | cidrs []*net.IPNet | |
| 17 | 16 | } | |
| 18 | 17 | ||
| 19 | 18 | // NewHandler creates an onboarding handler. | |
| 20 | 19 | func NewHandler(rootPEM, rootDER []byte, rootCN string, cloudflareCIDRs []*net.IPNet) *Handler { | |
| 21 | 20 | return &Handler{ | |
| 22 | - | rootPEM: rootPEM, | |
| 23 | - | rootDER: rootDER, | |
| 24 | - | rootCN: rootCN, | |
| 25 | - | cidrs: cloudflareCIDRs, | |
| 21 | + | rootPEM: rootPEM, | |
| 22 | + | mobileconfig: pkiadapter.GenerateMobileconfig(rootDER, rootCN), | |
| 23 | + | cidrs: cloudflareCIDRs, | |
| 26 | 24 | } | |
| 27 | 25 | } | |
| 28 | 26 | ||
| @@ -35,10 +33,9 @@ func (h *Handler) ServeCACert(w http.ResponseWriter, _ *http.Request) { | |||
| 35 | 33 | ||
| 36 | 34 | // ServeMobileconfig serves an iOS configuration profile with the root CA. | |
| 37 | 35 | func (h *Handler) ServeMobileconfig(w http.ResponseWriter, _ *http.Request) { | |
| 38 | - | mc := pkiadapter.GenerateMobileconfig(h.rootDER, h.rootCN) | |
| 39 | 36 | w.Header().Set("Content-Type", "application/x-apple-aspen-config") | |
| 40 | 37 | w.Header().Set("Content-Disposition", "attachment; filename=gordon-ca.mobileconfig") | |
| 41 | - | _, _ = w.Write(mc) | |
| 38 | + | _, _ = w.Write(h.mobileconfig) | |
| 42 | 39 | } | |
| 43 | 40 | ||
| 44 | 41 | // ServeOnboardingPage serves the CA trust onboarding HTML page. | |
| @@ -10,19 +10,17 @@ import ( | |||||
| 10 | 10 | ||||
| 11 | // Handler serves CA onboarding endpoints on the HTTP port. | 11 | // Handler serves CA onboarding endpoints on the HTTP port. | ||
| 12 | type Handler struct { | 12 | type Handler struct { | ||
| 13 | - | rootPEM []byte | 13 | + | rootPEM []byte |
| 14 | - | rootDER []byte | 14 | + | mobileconfig []byte |
| 15 | - | rootCN string | 15 | + | cidrs []*net.IPNet |
| 16 | - | cidrs []*net.IPNet // Cloudflare CIDRs — if source IP matches, don't serve onboarding | |||
| 17 | } | 16 | } | ||
| 18 | 17 | ||||
| 19 | // NewHandler creates an onboarding handler. | 18 | // NewHandler creates an onboarding handler. | ||
| 20 | func NewHandler(rootPEM, rootDER []byte, rootCN string, cloudflareCIDRs []*net.IPNet) *Handler { | 19 | func NewHandler(rootPEM, rootDER []byte, rootCN string, cloudflareCIDRs []*net.IPNet) *Handler { | ||
| 21 | return &Handler{ | 20 | return &Handler{ | ||
| 22 | - | rootPEM: rootPEM, | 21 | + | rootPEM: rootPEM, |
| 23 | - | rootDER: rootDER, | 22 | + | mobileconfig: pkiadapter.GenerateMobileconfig(rootDER, rootCN), |
| 24 | - | rootCN: rootCN, | 23 | + | cidrs: cloudflareCIDRs, |
| 25 | - | cidrs: cloudflareCIDRs, | |||
| 26 | } | 24 | } | ||
| 27 | } | 25 | } | ||
| 28 | 26 | ||||
| @@ -35,10 +33,9 @@ func (h *Handler) ServeCACert(w http.ResponseWriter, _ *http.Request) { | |||||
| 35 | 33 | ||||
| 36 | // ServeMobileconfig serves an iOS configuration profile with the root CA. | 34 | // ServeMobileconfig serves an iOS configuration profile with the root CA. | ||
| 37 | func (h *Handler) ServeMobileconfig(w http.ResponseWriter, _ *http.Request) { | 35 | func (h *Handler) ServeMobileconfig(w http.ResponseWriter, _ *http.Request) { | ||
| 38 | - | mc := pkiadapter.GenerateMobileconfig(h.rootDER, h.rootCN) | |||
| 39 | w.Header().Set("Content-Type", "application/x-apple-aspen-config") | 36 | w.Header().Set("Content-Type", "application/x-apple-aspen-config") | ||
| 40 | w.Header().Set("Content-Disposition", "attachment; filename=gordon-ca.mobileconfig") | 37 | w.Header().Set("Content-Disposition", "attachment; filename=gordon-ca.mobileconfig") | ||
| 41 | - | _, _ = w.Write(mc) | 38 | + | _, _ = w.Write(h.mobileconfig) |
| 42 | } | 39 | } | ||
| 43 | 40 | ||||
| 44 | // ServeOnboardingPage serves the CA trust onboarding HTML page. | 41 | // ServeOnboardingPage serves the CA trust onboarding HTML page. | ||
M internal/adapters/out/pki/ca.go +1 -5
| @@ -44,7 +44,6 @@ type CA struct { | |||
| 44 | 44 | rootCert *x509.Certificate | |
| 45 | 45 | rootKey crypto.Signer | |
| 46 | 46 | rootPEM []byte | |
| 47 | - | rootDER []byte | |
| 48 | 47 | ||
| 49 | 48 | mu sync.RWMutex | |
| 50 | 49 | interCert *x509.Certificate | |
| @@ -82,7 +81,7 @@ func NewCA(dataDir string, log zerowrap.Logger) (*CA, error) { | |||
| 82 | 81 | func (ca *CA) RootCertificate() []byte { return ca.rootPEM } | |
| 83 | 82 | ||
| 84 | 83 | // RootCertificateDER returns the root CA certificate in DER format. | |
| 85 | - | func (ca *CA) RootCertificateDER() []byte { return ca.rootDER } | |
| 84 | + | func (ca *CA) RootCertificateDER() []byte { return ca.rootCert.Raw } | |
| 86 | 85 | ||
| 87 | 86 | // RootFingerprint returns the SHA-256 fingerprint of the root CA cert | |
| 88 | 87 | // formatted as colon-separated hex. | |
| @@ -138,7 +137,6 @@ func (ca *CA) loadOrGenerateRoot() error { | |||
| 138 | 137 | ca.rootCert = cert | |
| 139 | 138 | ca.rootKey = key | |
| 140 | 139 | ca.rootPEM = certPEM | |
| 141 | - | ca.rootDER = cert.Raw | |
| 142 | 140 | ca.log.Info().Str("cn", cert.Subject.CommonName).Msg("loaded existing root CA") | |
| 143 | 141 | return nil | |
| 144 | 142 | } | |
| @@ -197,7 +195,6 @@ func (ca *CA) loadOrGenerateRoot() error { | |||
| 197 | 195 | ca.rootCert = cert | |
| 198 | 196 | ca.rootKey = key | |
| 199 | 197 | ca.rootPEM = certPEM | |
| 200 | - | ca.rootDER = certDER | |
| 201 | 198 | ca.log.Info().Str("cn", cert.Subject.CommonName).Msg("generated new root CA") | |
| 202 | 199 | return nil | |
| 203 | 200 | } | |
| @@ -331,7 +328,6 @@ func (ca *CA) IssueCertificate(domain string) (*tls.Certificate, error) { | |||
| 331 | 328 | ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, | |
| 332 | 329 | } | |
| 333 | 330 | ||
| 334 | - | // AIA extension: URL where clients can fetch the CA cert. | |
| 335 | 331 | if ca.aiaURL != "" { | |
| 336 | 332 | template.IssuingCertificateURL = []string{ca.aiaURL} | |
| 337 | 333 | } | |
| @@ -44,7 +44,6 @@ type CA struct { | |||||
| 44 | rootCert *x509.Certificate | 44 | rootCert *x509.Certificate | ||
| 45 | rootKey crypto.Signer | 45 | rootKey crypto.Signer | ||
| 46 | rootPEM []byte | 46 | rootPEM []byte | ||
| 47 | - | rootDER []byte | |||
| 48 | 47 | ||||
| 49 | mu sync.RWMutex | 48 | mu sync.RWMutex | ||
| 50 | interCert *x509.Certificate | 49 | interCert *x509.Certificate | ||
| @@ -82,7 +81,7 @@ func NewCA(dataDir string, log zerowrap.Logger) (*CA, error) { | |||||
| 82 | func (ca *CA) RootCertificate() []byte { return ca.rootPEM } | 81 | func (ca *CA) RootCertificate() []byte { return ca.rootPEM } | ||
| 83 | 82 | ||||
| 84 | // RootCertificateDER returns the root CA certificate in DER format. | 83 | // RootCertificateDER returns the root CA certificate in DER format. | ||
| 85 | - | func (ca *CA) RootCertificateDER() []byte { return ca.rootDER } | 84 | + | func (ca *CA) RootCertificateDER() []byte { return ca.rootCert.Raw } |
| 86 | 85 | ||||
| 87 | // RootFingerprint returns the SHA-256 fingerprint of the root CA cert | 86 | // RootFingerprint returns the SHA-256 fingerprint of the root CA cert | ||
| 88 | // formatted as colon-separated hex. | 87 | // formatted as colon-separated hex. | ||
| @@ -138,7 +137,6 @@ func (ca *CA) loadOrGenerateRoot() error { | |||||
| 138 | ca.rootCert = cert | 137 | ca.rootCert = cert | ||
| 139 | ca.rootKey = key | 138 | ca.rootKey = key | ||
| 140 | ca.rootPEM = certPEM | 139 | ca.rootPEM = certPEM | ||
| 141 | - | ca.rootDER = cert.Raw | |||
| 142 | ca.log.Info().Str("cn", cert.Subject.CommonName).Msg("loaded existing root CA") | 140 | ca.log.Info().Str("cn", cert.Subject.CommonName).Msg("loaded existing root CA") | ||
| 143 | return nil | 141 | return nil | ||
| 144 | } | 142 | } | ||
| @@ -197,7 +195,6 @@ func (ca *CA) loadOrGenerateRoot() error { | |||||
| 197 | ca.rootCert = cert | 195 | ca.rootCert = cert | ||
| 198 | ca.rootKey = key | 196 | ca.rootKey = key | ||
| 199 | ca.rootPEM = certPEM | 197 | ca.rootPEM = certPEM | ||
| 200 | - | ca.rootDER = certDER | |||
| 201 | ca.log.Info().Str("cn", cert.Subject.CommonName).Msg("generated new root CA") | 198 | ca.log.Info().Str("cn", cert.Subject.CommonName).Msg("generated new root CA") | ||
| 202 | return nil | 199 | return nil | ||
| 203 | } | 200 | } | ||
| @@ -331,7 +328,6 @@ func (ca *CA) IssueCertificate(domain string) (*tls.Certificate, error) { | |||||
| 331 | ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, | 328 | ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, | ||
| 332 | } | 329 | } | ||
| 333 | 330 | ||||
| 334 | - | // AIA extension: URL where clients can fetch the CA cert. | |||
| 335 | if ca.aiaURL != "" { | 331 | if ca.aiaURL != "" { | ||
| 336 | template.IssuingCertificateURL = []string{ca.aiaURL} | 332 | template.IssuingCertificateURL = []string{ca.aiaURL} | ||
| 337 | } | 333 | } | ||
M internal/usecase/pki/service.go +2 -7
| @@ -78,7 +78,6 @@ func (s *Service) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, | |||
| 78 | 78 | return nil, fmt.Errorf("no SNI in ClientHello") | |
| 79 | 79 | } | |
| 80 | 80 | ||
| 81 | - | // Check cache | |
| 82 | 81 | if entry, ok := s.cache.Load(domain); ok { | |
| 83 | 82 | cached := entry.(*cachedCert) | |
| 84 | 83 | if time.Now().Before(cached.expiresAt) { | |
| @@ -87,12 +86,10 @@ func (s *Service) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, | |||
| 87 | 86 | s.cache.Delete(domain) | |
| 88 | 87 | } | |
| 89 | 88 | ||
| 90 | - | // Check allowlist | |
| 91 | 89 | if !s.isDomainAllowed(hello.Context(), domain) { | |
| 92 | 90 | return nil, fmt.Errorf("domain %q not in route table", domain) | |
| 93 | 91 | } | |
| 94 | 92 | ||
| 95 | - | // Issue new cert | |
| 96 | 93 | cert, err := s.ca.IssueCertificate(domain) | |
| 97 | 94 | if err != nil { | |
| 98 | 95 | s.log.Error().Err(err).Str("domain", domain).Msg("failed to issue leaf cert") | |
| @@ -114,10 +111,8 @@ func (s *Service) isDomainAllowed(ctx context.Context, domain string) bool { | |||
| 114 | 111 | return true | |
| 115 | 112 | } | |
| 116 | 113 | } | |
| 117 | - | for d := range s.routes.GetExternalRoutes() { | |
| 118 | - | if d == domain { | |
| 119 | - | return true | |
| 120 | - | } | |
| 114 | + | if _, ok := s.routes.GetExternalRoutes()[domain]; ok { | |
| 115 | + | return true | |
| 121 | 116 | } | |
| 122 | 117 | return false | |
| 123 | 118 | } | |
| @@ -78,7 +78,6 @@ func (s *Service) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, | |||||
| 78 | return nil, fmt.Errorf("no SNI in ClientHello") | 78 | return nil, fmt.Errorf("no SNI in ClientHello") | ||
| 79 | } | 79 | } | ||
| 80 | 80 | ||||
| 81 | - | // Check cache | |||
| 82 | if entry, ok := s.cache.Load(domain); ok { | 81 | if entry, ok := s.cache.Load(domain); ok { | ||
| 83 | cached := entry.(*cachedCert) | 82 | cached := entry.(*cachedCert) | ||
| 84 | if time.Now().Before(cached.expiresAt) { | 83 | if time.Now().Before(cached.expiresAt) { | ||
| @@ -87,12 +86,10 @@ func (s *Service) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, | |||||
| 87 | s.cache.Delete(domain) | 86 | s.cache.Delete(domain) | ||
| 88 | } | 87 | } | ||
| 89 | 88 | ||||
| 90 | - | // Check allowlist | |||
| 91 | if !s.isDomainAllowed(hello.Context(), domain) { | 89 | if !s.isDomainAllowed(hello.Context(), domain) { | ||
| 92 | return nil, fmt.Errorf("domain %q not in route table", domain) | 90 | return nil, fmt.Errorf("domain %q not in route table", domain) | ||
| 93 | } | 91 | } | ||
| 94 | 92 | ||||
| 95 | - | // Issue new cert | |||
| 96 | cert, err := s.ca.IssueCertificate(domain) | 93 | cert, err := s.ca.IssueCertificate(domain) | ||
| 97 | if err != nil { | 94 | if err != nil { | ||
| 98 | s.log.Error().Err(err).Str("domain", domain).Msg("failed to issue leaf cert") | 95 | s.log.Error().Err(err).Str("domain", domain).Msg("failed to issue leaf cert") | ||
| @@ -114,10 +111,8 @@ func (s *Service) isDomainAllowed(ctx context.Context, domain string) bool { | |||||
| 114 | return true | 111 | return true | ||
| 115 | } | 112 | } | ||
| 116 | } | 113 | } | ||
| 117 | - | for d := range s.routes.GetExternalRoutes() { | 114 | + | if _, ok := s.routes.GetExternalRoutes()[domain]; ok { |
| 118 | - | if d == domain { | 115 | + | return true |
| 119 | - | return true | |||
| 120 | - | } | |||
| 121 | } | 116 | } | ||
| 122 | return false | 117 | return false | ||
| 123 | } | 118 | } | ||