diff --git a/internal/web/service/setting_mtls.go b/internal/web/service/setting_mtls.go index d62812cd6..367197bb8 100644 --- a/internal/web/service/setting_mtls.go +++ b/internal/web/service/setting_mtls.go @@ -194,7 +194,7 @@ func (s *SettingService) NodeMtlsClientCAPool() (*x509.CertPool, error) { } certs, err := parseCertificateBundlePEM([]byte(caPem)) if err != nil { - return nil, fmt.Errorf("nodeMtlsClientCAPem is not a valid certificate bundle: %w", err) + return nil, fmt.Errorf("%w: %w", ErrNodeMtlsTrustBundleInvalid, err) } pool := x509.NewCertPool() for _, cert := range certs { @@ -203,6 +203,10 @@ func (s *SettingService) NodeMtlsClientCAPool() (*x509.CertPool, error) { return pool, nil } +// ErrNodeMtlsTrustBundleInvalid separates a stored bundle that will not parse +// from a settings read that failed, which callers report differently. +var ErrNodeMtlsTrustBundleInvalid = errors.New("nodeMtlsClientCAPem is not a valid certificate bundle") + // parseCertificateBundlePEM avoids AppendCertsFromPEM because that helper can // silently accept a bundle after parsing only its first certificate. func parseCertificateBundlePEM(bundle []byte) ([]*x509.Certificate, error) { diff --git a/internal/web/service/setting_mtls_bundle_test.go b/internal/web/service/setting_mtls_bundle_test.go index c1dface5f..3526725db 100644 --- a/internal/web/service/setting_mtls_bundle_test.go +++ b/internal/web/service/setting_mtls_bundle_test.go @@ -1,6 +1,7 @@ package service import ( + "errors" "strings" "testing" @@ -72,3 +73,21 @@ func TestNodeMtlsClientCAPoolRejectsPartiallyValidBundle(t *testing.T) { t.Fatalf("NodeMtlsClientCAPool() = %v, error = %v, want %q", pool, err, want) } } + +// The boot path tells the operator whether the bundle itself is unusable or the +// settings read failed, so the parse failure has to carry a matchable cause. +func TestNodeMtlsClientCAPoolTagsAnInvalidBundle(t *testing.T) { + s := setupSettingMtlsDB(t) + + if err := s.setString("nodeMtlsClientCAPem", "-----BEGIN CERTIFICATE-----\nnot base64\n-----END CERTIFICATE-----\n"); err != nil { + t.Fatalf("setString: %v", err) + } + + pool, err := s.NodeMtlsClientCAPool() + if pool != nil { + t.Fatalf("NodeMtlsClientCAPool() returned a pool built from an unusable bundle") + } + if !errors.Is(err, ErrNodeMtlsTrustBundleInvalid) { + t.Fatalf("NodeMtlsClientCAPool() error = %v, want it to wrap ErrNodeMtlsTrustBundleInvalid", err) + } +} diff --git a/internal/web/web.go b/internal/web/web.go index d726039af..015d4f915 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -6,6 +6,7 @@ import ( "context" "crypto/tls" "embed" + "errors" "fmt" "io" "io/fs" @@ -626,9 +627,14 @@ func (s *Server) start(restartXray bool, startTgBot bool) (err error) { // Opt-in node mTLS: when a trust CA is configured, request and verify // client certs (VerifyClientCertIfGiven keeps browsers working). With // no CA the listener is unchanged. - if pool, perr := s.settingService.NodeMtlsClientCAPool(); perr != nil { - logger.Warning("node mTLS: failed to build client CA trust pool:", perr) - } else if pool != nil { + pool, perr := s.settingService.NodeMtlsClientCAPool() + switch { + case errors.Is(perr, service.ErrNodeMtlsTrustBundleInvalid): + logger.Error("Node mTLS is configured but its trust bundle will not parse, so client certificates are not accepted:", perr) + case perr != nil: + logger.Error("Node mTLS trust bundle could not be read, so client certificates are not accepted:", perr) + } + if pool != nil { applyNodeMtls(c, pool) logger.Info("Node mTLS enabled: verifying client certificates for the node API") }