security(globalsign): remove InsecureSkipVerify and pin CA pool (H-5)

The GlobalSign Atlas HVCA connector previously used InsecureSkipVerify:true
on its mTLS TLS config, disabling server certificate validation and
defeating the purpose of the client-side mTLS handshake. This was a
CWE-295 Improper Certificate Validation vulnerability silently degrading
trust on every production call to GlobalSign's signing API.

Remediation (per H-5 audit finding, Lens 4.4):

- Remove InsecureSkipVerify from all three http.Client construction sites
  (ValidateConfig, getHTTPClient, and legacy initialisation path).
- Introduce buildServerTLSConfig() helper that constructs tls.Config with
  MinVersion: tls.VersionTLS12 (addresses adjacent L-1 recommendation).
- New optional config field `server_ca_path` (env:
  CERTCTL_GLOBALSIGN_SERVER_CA_PATH). When unset the connector trusts the
  system root CA bundle (correct default for GlobalSign's publicly-trusted
  HVCA endpoints). When set the bundle is loaded via x509.NewCertPool() +
  AppendCertsFromPEM, and only those roots are trusted (supports private
  HVCA deployments and defence-in-depth root pinning).
- Error wrapping chain: "failed to read server CA bundle at %s" and
  "no valid PEM certificates found in server CA bundle at %s" surface
  config problems at ValidateConfig time instead of silently failing at
  request time.

Docs, config, service env-seed, and GUI issuer type definition updated to
expose the new field. Tests: 9 dead `InsecureSkipVerify: true` client
TLSClientConfig blocks (no-ops against httptest.NewServer plain-HTTP)
replaced with bare http.Client; new TestGlobalSign_ServerTLSConfig covers
pinned-CA trust, untrusted-server rejection, missing-file and invalid-PEM
error paths.

Verification:
- go build ./... clean
- go vet ./... clean
- go test -race ./internal/connector/issuer/globalsign/... ./internal/config/... ./internal/service/... ok
- go test ./... (excluding testcontainers-gated repo layer) ok
- golangci-lint run ./... 0 issues
- govulncheck ./... 0 reachable vulns
- Per-layer coverage: service 68.7% (≥55), handler 83.6% (≥60), domain 82.0% (≥40), middleware 63.8% (≥30)
- globalsign package coverage: 75.9%
- Invariant sweep: 0 InsecureSkipVerify references remain in globalsign
  package (only a test-file comment documenting the removal).
This commit is contained in:
shankar0123
2026-04-17 01:40:58 +00:00
parent 119986fa7e
commit 6315ef102a
6 changed files with 257 additions and 67 deletions
+14 -10
View File
@@ -577,17 +577,21 @@ func (s *IssuerService) buildEnvVarSeeds(cfg *config.Config) []*domain.Issuer {
// Conditional: GlobalSign — only seed if API URL and API key are set
if cfg.GlobalSign.APIUrl != "" && cfg.GlobalSign.APIKey != "" {
globalSignConfig := map[string]interface{}{
"api_url": cfg.GlobalSign.APIUrl,
"api_key": cfg.GlobalSign.APIKey,
"api_secret": cfg.GlobalSign.APISecret,
"client_cert_path": cfg.GlobalSign.ClientCertPath,
"client_key_path": cfg.GlobalSign.ClientKeyPath,
}
if cfg.GlobalSign.ServerCAPath != "" {
globalSignConfig["server_ca_path"] = cfg.GlobalSign.ServerCAPath
}
seeds = append(seeds, &domain.Issuer{
ID: "iss-globalsign",
Name: "GlobalSign Atlas",
Type: domain.IssuerTypeGlobalSign,
Config: mustJSON(map[string]interface{}{
"api_url": cfg.GlobalSign.APIUrl,
"api_key": cfg.GlobalSign.APIKey,
"api_secret": cfg.GlobalSign.APISecret,
"client_cert_path": cfg.GlobalSign.ClientCertPath,
"client_key_path": cfg.GlobalSign.ClientKeyPath,
}),
ID: "iss-globalsign",
Name: "GlobalSign Atlas",
Type: domain.IssuerTypeGlobalSign,
Config: mustJSON(globalSignConfig),
Enabled: true,
Source: "env",
CreatedAt: now,