From 2ad05bb665d76ad01564429ff16543c57c060f61 Mon Sep 17 00:00:00 2001 From: "David Muto (pseudomuto)" Date: Sat, 25 Jul 2026 10:29:14 -0400 Subject: [PATCH] [creds]: Split transport credentials into role-typed resolvers The creds package was a shallow catalog: five near-parallel constructors, two loaders, and a wide set of validator methods. The decision of which credential a TLS config actually wants was re-derived at three call sites (proxy dialing, server listening, and config validation), which could drift; worse, the outbound validation rules (cert and key must be set together, a client cert requires a CA) lived only in config validation while the runtime trusted that validation had already run. Replace the catalog with two role-typed constructors, NewDialer and NewListener, built from options (Insecure, WithCA, WithCertificate). Each resolves the TLS mode and cross-field legality once, and Validate, DialOption, and ServerOption all run the same checks, so validation and runtime can no longer disagree. Secure is now the default: only Insecure() yields plaintext, and an empty client config verifies against the system root pool rather than silently downgrading (the public-CA upstream case, e.g. Temporal Cloud). Certificate material is parsed once per Dialer and reused across per-request dials. Migrate config, proxy, and server to the new constructors, removing the hand-rolled decision trees and the per-upstream loader plumbing. --- .gitignore | 1 + e2e/harness_test.go | 41 +-- internal/config/config_test.go | 4 +- internal/config/listen.go | 96 +++--- internal/config/listen_test.go | 95 +----- internal/config/upstream_test.go | 4 +- internal/proxy/fx.go | 66 +--- internal/proxy/server.go | 8 +- internal/server/fx.go | 19 +- internal/server/server.go | 2 +- internal/transport/creds/dialer.go | 94 ++++++ internal/transport/creds/dialer_test.go | 214 +++++++++++++ internal/transport/creds/doc.go | 21 +- internal/transport/creds/insecure.go | 42 --- internal/transport/creds/insecure_test.go | 39 --- internal/transport/creds/listener.go | 88 +++++ internal/transport/creds/listener_test.go | 170 ++++++++++ internal/transport/creds/material.go | 66 ++++ internal/transport/creds/mtls.go | 204 ------------ internal/transport/creds/mtls_test.go | 302 ------------------ internal/transport/creds/options.go | 94 ++++++ internal/transport/creds/resolve.go | 51 +++ internal/transport/creds/tls.go | 185 ----------- internal/transport/creds/tls_test.go | 238 -------------- internal/transport/creds/validate.go | 93 ++++++ pkg/testutil/certs.go | 37 ++- pkg/testutil/certs_test.go | 23 ++ .../validation/certs/certs.go | 71 ++-- .../validation/certs/certs_test.go | 141 ++++---- pkg/validation/certs/doc.go | 6 + 30 files changed, 1102 insertions(+), 1413 deletions(-) create mode 100644 internal/transport/creds/dialer.go create mode 100644 internal/transport/creds/dialer_test.go delete mode 100644 internal/transport/creds/insecure.go delete mode 100644 internal/transport/creds/insecure_test.go create mode 100644 internal/transport/creds/listener.go create mode 100644 internal/transport/creds/listener_test.go create mode 100644 internal/transport/creds/material.go delete mode 100644 internal/transport/creds/mtls.go delete mode 100644 internal/transport/creds/mtls_test.go create mode 100644 internal/transport/creds/options.go create mode 100644 internal/transport/creds/resolve.go delete mode 100644 internal/transport/creds/tls.go delete mode 100644 internal/transport/creds/tls_test.go create mode 100644 internal/transport/creds/validate.go rename internal/transport/creds/validation.go => pkg/validation/certs/certs.go (75%) rename internal/transport/creds/validation_test.go => pkg/validation/certs/certs_test.go (68%) create mode 100644 pkg/validation/certs/doc.go diff --git a/.gitignore b/.gitignore index c1131a8..1198189 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ CLAUDE.local.md # dev things dev/certs/ +docs/ # build things ./proxy diff --git a/e2e/harness_test.go b/e2e/harness_test.go index 6d28ffc..14b90ce 100644 --- a/e2e/harness_test.go +++ b/e2e/harness_test.go @@ -2,17 +2,10 @@ package e2e import ( "context" - "crypto/rand" - "crypto/rsa" "crypto/tls" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" - "math/big" "net" "sync" "testing" - "time" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" @@ -158,7 +151,7 @@ func newFakeTLSUpstream(t *testing.T) fakeTLSUpstream { go func() { _ = fakeUpstream.Serve(lis) }() t.Cleanup(fakeUpstream.Stop) - clientCertFile, clientKeyFile := generateMatchingRSAKeyPair(t) + clientCertFile, clientKeyFile := testutil.GenerateRSACert(t) return fakeTLSUpstream{ svc: svc, @@ -214,35 +207,3 @@ func dialUnix(t *testing.T, upstream string) *grpc.ClientConn { require.NoError(t, err) return conn } - -// generateMatchingRSAKeyPair writes a self-signed RSA-2048 certificate and its -// matching private key to a fresh [testing.T.TempDir] and returns the paths. -// Unlike [testutil.RSACert], the private key is retained and written out, so -// the pair loads via [tls.LoadX509KeyPair] for use as a real TLS client -// identity. -func generateMatchingRSAKeyPair(t *testing.T) (certFile, keyFile string) { - t.Helper() - - dir := t.TempDir() - - key, err := rsa.GenerateKey(rand.Reader, 2048) - require.NoError(t, err) - - tmpl := &x509.Certificate{ - SerialNumber: big.NewInt(1), - Subject: pkix.Name{CommonName: "proxy-client"}, - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - KeyUsage: x509.KeyUsageDigitalSignature, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, - } - - der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) - require.NoError(t, err) - - certFile = testutil.WriteFile(t, dir, "client-cert.pem", pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})) - keyFile = testutil.WriteFile(t, dir, "client-key.pem", - pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)})) - - return certFile, keyFile -} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 6e87e1d..400f1b4 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -209,13 +209,12 @@ func TestConfig_Validate(t *testing.T) { cfg: &config.Config{ Listen: config.ListenConfig{ HostPort: ":8080", - TLS: &config.TLSConfig{}, // empty -> creds.TLS PEM read failures + TLS: &config.TLSConfig{}, // empty -> "a server certificate is required" }, Upstreams: validUpstreams, }, wantTuples: [][2]string{ {"tls", "cert"}, - {"tls", "key"}, }, }, { @@ -230,7 +229,6 @@ func TestConfig_Validate(t *testing.T) { wantTuples: [][2]string{ {"", "hostPort"}, {"tls", "cert"}, - {"tls", "key"}, }, }, { diff --git a/internal/config/listen.go b/internal/config/listen.go index 2787b81..5459271 100644 --- a/internal/config/listen.go +++ b/internal/config/listen.go @@ -24,14 +24,6 @@ type ( Key string `yaml:"key"` // PEM-encoded private key ServerName string `yaml:"serverName"` // Optional SNI override } - - // caTrustAnchor validates a certificate used as an outbound trust anchor - // (client-side TLS with no client certificate presented). The anchor may be - // either a CA or a pinned self-signed leaf. It is checked for expiry, a secure - // signature algorithm, and sufficient key size. - caTrustAnchor struct { - caFile string - } ) // Validate checks the host:port and, when present, the TLS configuration. @@ -46,64 +38,50 @@ func (l *ListenConfig) Validate() error { ) } -// Validate checks the configured certificate material: mutual TLS when a CA is -// set, otherwise server-only TLS. -func (t *TLSConfig) Validate() error { - return validation.Validate( - "", - validation.WhenRules( - func() bool { return t.CA != "" }, - validation.Nested("", t.mtlsCreds()), - ), - validation.WhenRules( - func() bool { return t.CA == "" }, - validation.Nested("", creds.NewServerTLS(t.Cert, t.Key)), - ), - ) +// Listener resolves the inbound (server) credential for this TLS block. A nil +// receiver yields an insecure listener. +func (t *TLSConfig) Listener() *creds.Listener { + return creds.NewListener(t.credsOptions()...) +} + +// Dialer resolves the outbound (client) credential for this TLS block. A nil +// receiver yields an insecure dialer. +func (t *TLSConfig) Dialer() *creds.Dialer { + return creds.NewDialer(t.credsOptions()...) } -// mtlsCreds builds the mutual-TLS credential described by the config: the CA -// verifies the peer and the client key pair is the presented certificate. It is -// shared by the inbound listener validation and the outbound upstream validation -// so the construction lives in one place. -func (t *TLSConfig) mtlsCreds() *creds.MTLS { - return creds.NewMTLS(t.CA, t.Cert, t.Key, creds.MTLSOptions{ServerName: t.ServerName}) +// Validate checks the inbound (listener) TLS material. It delegates to the +// resolved server credential, which owns the mode decision (server TLS vs mutual +// TLS) and the certificate file checks. +func (t *TLSConfig) Validate() error { + return t.Listener().Validate() } // validateOutbound validates the config as client-side TLS used to dial an -// upstream. A client certificate (cert+key) selects mutual TLS and requires a -// CA; a CA alone verifies the upstream against a private trust anchor while -// presenting no client certificate; neither means client TLS against the system -// root pool. Callers must invoke this only when the receiver is non-nil. +// upstream. It delegates to the resolved client credential, which owns the mode +// decision (system-root, custom-CA, or mutual TLS) and the legality and file +// checks. Callers must invoke this only when the receiver is non-nil. func (t *TLSConfig) validateOutbound() validation.Errors { - hasCert := t.Cert != "" || t.Key != "" + return validation.Nested("tls", t.Dialer())() +} - switch { - case (t.Cert == "") != (t.Key == ""): - return validation.Errors{{Subject: "tls", Message: "cert and key must be set together"}} - case hasCert && t.CA == "": - return validation.Errors{{Subject: "tls", Field: "ca", Message: "is required when a client certificate is set"}} - case hasCert: - return validation.Nested("tls", t.mtlsCreds())() - case t.CA != "": - return validation.Nested("tls", caTrustAnchor{caFile: t.CA})() - default: - return nil +// credsOptions maps the TLS block onto a set of creds options shared by both +// roles; the caller picks the role via creds.NewListener or creds.NewDialer. A +// nil block is plaintext. A present block with no CA and no client certificate +// resolves to system-root TLS for a client. +func (t *TLSConfig) credsOptions() []creds.Option { + if t == nil { + return []creds.Option{creds.Insecure()} } -} -// Validate checks that the trust anchor is not expired and is signed with a -// strong algorithm and a sufficiently large key. -func (c caTrustAnchor) Validate() error { - return validation.Validate( - "", - validation.Field("ca", c.caFile, func(path string) error { - return creds.ValidatePEMFile( - path, - creds.CertificateNotExpired(), - creds.UsesSecureCertificateAlgorithm(), - creds.HasSufficientKeySize(), - ) - }), - ) + var opts []creds.Option + if t.CA != "" { + opts = append(opts, creds.WithCA(t.CA)) + } + + if t.Cert != "" || t.Key != "" { + opts = append(opts, creds.WithCertificate(t.Cert, t.Key)) + } + + return opts } diff --git a/internal/config/listen_test.go b/internal/config/listen_test.go index d26c794..96c354e 100644 --- a/internal/config/listen_test.go +++ b/internal/config/listen_test.go @@ -48,97 +48,12 @@ func TestListenConfig_Validate(t *testing.T) { } } -// TestListenConfig_Validate_TLS_RouteSelection exercises the branch in -// TLSConfig.Validate that picks creds.TLS when CA is empty and creds.MTLS -// when CA is set. The deep cert-content checks live in creds' own tests; we -// fingerprint the routing decision via the (Subject, Field) tuples that -// surface for missing files, which is stable across both routes. -func TestListenConfig_Validate_TLS_RouteSelection(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - tls *config.TLSConfig - // Each entry is the (Subject, Field) tuple expected on one Error. - // Messages are intentionally not pinned: creds errors include the - // host's filesystem-error wording which differs across OSes. - wantTuples [][2]string - }{ - { - name: "empty TLS routes through creds.TLS (CA empty)", - tls: &config.TLSConfig{}, - wantTuples: [][2]string{ - {"tls", "cert"}, // creds.TLS PEM read failure on cert - {"tls", "key"}, // creds.TLS PEM read failure on key - }, - }, - { - name: "missing-file paths route through creds.TLS (CA empty)", - tls: &config.TLSConfig{ - Cert: "/missing/cert.pem", - Key: "/missing/key.pem", - }, - wantTuples: [][2]string{ - {"tls", "cert"}, // creds.TLS PEM read failure on cert - {"tls", "key"}, // creds.TLS PEM read failure on key (no "ca") - }, - }, - { - name: "CA set routes through creds.MTLS", - tls: &config.TLSConfig{ - Cert: "/missing/cert.pem", - Key: "/missing/key.pem", - CA: "/missing/ca.pem", - }, - wantTuples: [][2]string{ - {"tls", "cert"}, // creds.MTLS PEM read failure on cert - {"tls", "key"}, // creds.MTLS PEM read failure on key - {"tls", "ca"}, // creds.MTLS PEM read failure on ca - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - cfg := &config.ListenConfig{HostPort: ":8080", TLS: tt.tls} - err := cfg.Validate() - require.Error(t, err) - - var errs validation.Errors - require.True(t, errors.As(err, &errs)) - - got := make([][2]string, len(errs)) - for i, e := range errs { - got[i] = [2]string{e.Subject, e.Field} - } - require.ElementsMatch(t, tt.wantTuples, got) - }) - } -} - func TestTLSConfig_Validate(t *testing.T) { t.Parallel() - // Called standalone (not via Nested), entries keep the empty Subject - // that TLSConfig.Validate sets. An empty struct routes through the - // CA=="" branch (creds.TLS), so we expect cert + key PEM read failures. - tls := &config.TLSConfig{} - err := tls.Validate() - require.Error(t, err) - - var errs validation.Errors - require.True(t, errors.As(err, &errs)) - require.Len(t, errs, 2) - - for _, e := range errs { - require.Empty(t, e.Subject, "standalone Validate leaves Subject empty") - } - - fields := make([]string, len(errs)) - for i, e := range errs { - fields[i] = e.Field - } - require.ElementsMatch(t, []string{"cert", "key"}, fields) + // An empty inbound TLS block resolves to server TLS but supplies no + // certificate, so validation reports a single legality failure. The + // per-mode certificate-content checks live in the creds package tests. + err := (&config.TLSConfig{}).Validate() + require.ErrorContains(t, err, "a server certificate is required") } diff --git a/internal/config/upstream_test.go b/internal/config/upstream_test.go index 76d0176..158eca6 100644 --- a/internal/config/upstream_test.go +++ b/internal/config/upstream_test.go @@ -556,7 +556,7 @@ func TestUpstreamOutboundTLS(t *testing.T) { _, certFile, _ := testutil.GenerateMTLSCerts(t) return &config.TLSConfig{Cert: certFile} }, - wantErr: "cert and key must be set together", + wantErr: "certificate and key must be set together", }, { name: "key without cert is rejected", @@ -564,7 +564,7 @@ func TestUpstreamOutboundTLS(t *testing.T) { _, _, keyFile := testutil.GenerateMTLSCerts(t) return &config.TLSConfig{Key: keyFile} }, - wantErr: "cert and key must be set together", + wantErr: "certificate and key must be set together", }, { name: "client cert without CA is rejected", diff --git a/internal/proxy/fx.go b/internal/proxy/fx.go index 97af4ed..65d8152 100644 --- a/internal/proxy/fx.go +++ b/internal/proxy/fx.go @@ -12,7 +12,6 @@ import ( "github.com/temporalio/temporal-proxy/internal/config" "github.com/temporalio/temporal-proxy/internal/protoutil" "github.com/temporalio/temporal-proxy/internal/transport/connect" - "github.com/temporalio/temporal-proxy/internal/transport/creds" "github.com/temporalio/temporal-proxy/pkg/crypto" "github.com/temporalio/temporal-proxy/pkg/logger" ) @@ -145,41 +144,27 @@ type ProxyParams struct { // request-independent dial options (namespace translation and outbound // credentials). func upstreamResolver(upstream *config.Upstream, opts []grpc.DialOption) (connect.Resolver, error) { + // One Dialer per upstream owns the TLS-mode decision and parses its + // certificate material once, so a templated upstream reuses it across every + // per-request dial (only the rendered server name varies). + dialer := upstream.Listen.TLS.Dialer() + if upstream.IsTemplated() { translator := func(s string) string { return s } if upstream.Namespaces.Rules.Configured() { translator = upstream.Namespaces.Rules.Remote } - // Share one loader across the per-request credentials so a templated - // upstream reads and parses its TLS material once rather than on every - // request. Mutual TLS (a configured client certificate) uses a CertLoader - // for the client key pair and CA; CA-only client TLS uses a CAPoolLoader - // for the trust anchor. The system-root client-TLS and insecure paths have - // no files to load, so both loaders stay nil. - var ( - loader *creds.CertLoader - caLoader *creds.CAPoolLoader - ) - if tls := upstream.Listen.TLS; tls != nil { - switch { - case tls.Cert != "" || tls.Key != "": - loader = creds.NewCertLoader(tls.CA, tls.Cert, tls.Key) - case tls.CA != "": - caLoader = creds.NewCAPoolLoader(tls.CA) - } - } - return NewDynamicResolver( upstream, WithRemoteNamespacer(translator), WithOptionsFactory(func(data RouteData) ([]grpc.DialOption, error) { - creds, err := upstreamCreds(upstream, data.ResolvedServerName, loader, caLoader).DialOption() + cred, err := dialer.DialOption(data.ResolvedServerName) if err != nil { return nil, err } - return append(slices.Clone(opts), creds), nil + return append(slices.Clone(opts), cred), nil }), ) } @@ -189,43 +174,10 @@ func upstreamResolver(upstream *config.Upstream, opts []grpc.DialOption) (connec serverName = upstream.Listen.TLS.ServerName } - creds, err := upstreamCreds(upstream, serverName, nil, nil).DialOption() + cred, err := dialer.DialOption(serverName) if err != nil { return nil, fmt.Errorf("failed to build credentials for upstream %q: %w", upstream.Name, err) } - return connect.StaticResolver(upstream.Listen.HostPort, append(slices.Clone(opts), creds)...), nil -} - -// upstreamCreds derives the credentials used to dial the upstream frontend from -// the upstream TLS configuration. A configured client certificate (cert+key) -// selects mutual TLS, which verifies the upstream against the configured CA. -// Without a client certificate the proxy uses client-side TLS: a custom root -// pool when a CA is set, otherwise the system root pool. No TLS at all is -// insecure. serverName overrides the SNI/certificate-verification name; it is -// the upstream's static configured ServerName, or a per-request value rendered -// from a templated ServerName. loader and caLoader, when non-nil, supply the -// pre-parsed TLS material (mutual-TLS key pair plus CA, and CA-only trust -// anchor respectively) so it is loaded once and reused across the per-request -// credentials of a templated upstream; pass nil for both on a fixed-address -// upstream. At most one is used, selected by the same TLS mode as the returned -// credential. -func upstreamCreds(upstream *config.Upstream, serverName string, loader *creds.CertLoader, caLoader *creds.CAPoolLoader) Credentials { - tls := upstream.Listen.TLS - if tls == nil { - return creds.NewInsecure() - } - - if tls.Cert != "" || tls.Key != "" { - return creds.NewMTLS(tls.CA, tls.Cert, tls.Key, creds.MTLSOptions{ - ServerName: serverName, - Loader: loader, - }) - } - - if tls.CA != "" { - return creds.NewClientTLSWithCA(tls.CA, serverName, caLoader) - } - - return creds.NewClientTLS(serverName) + return connect.StaticResolver(upstream.Listen.HostPort, append(slices.Clone(opts), cred)...), nil } diff --git a/internal/proxy/server.go b/internal/proxy/server.go index b8e11c4..2f7301e 100644 --- a/internal/proxy/server.go +++ b/internal/proxy/server.go @@ -18,12 +18,6 @@ import ( ) type ( - // Credentials produces the [grpc.DialOption] used to secure the outbound - // connection to the upstream Temporal frontend. - Credentials interface { - DialOption() (grpc.DialOption, error) - } - // Server proxies the Temporal WorkflowService. It re-serves an upstream // frontend on a local unix socket, letting local workers connect without TLS // while the upstream hop stays secured. The upstream connection(s) it @@ -59,7 +53,7 @@ func New(hostPort string, cc grpc.ClientConnInterface, opts ...Option) (*Server, svr, err := server.New( // NB: Hosting on local unix port, no need for TLS here. - server.WithCredentials(creds.NewInsecure()), + server.WithCredentials(creds.NewListener(creds.Insecure())), server.WithLogger(pops.logger), server.WithService(func(sr grpc.ServiceRegistrar) { workflowservice.RegisterWorkflowServiceServer(sr, wfs) diff --git a/internal/server/fx.go b/internal/server/fx.go index 0a03cc7..66683f7 100644 --- a/internal/server/fx.go +++ b/internal/server/fx.go @@ -12,7 +12,6 @@ import ( "github.com/temporalio/temporal-proxy/internal/auth" "github.com/temporalio/temporal-proxy/internal/config" "github.com/temporalio/temporal-proxy/internal/metrics" - "github.com/temporalio/temporal-proxy/internal/transport/creds" "github.com/temporalio/temporal-proxy/pkg/logger" ) @@ -109,21 +108,5 @@ type ServerParams struct { } func (p *ServerParams) creds() Credentials { - tls := p.Config.Listen.TLS - if tls == nil { - return creds.NewInsecure() - } - - if tls.CA != "" { - return creds.NewMTLS( - tls.CA, - tls.Cert, - tls.Key, - creds.MTLSOptions{ - ServerName: tls.ServerName, - }, - ) - } - - return creds.NewServerTLS(tls.Cert, tls.Key) + return p.Config.Listen.TLS.Listener() } diff --git a/internal/server/server.go b/internal/server/server.go index 0fc36d8..d15de64 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -65,7 +65,7 @@ type ( // logger. func New(sopts ...Option) (*Server, error) { opts := &options{ - creds: creds.NewInsecure(), + creds: creds.NewListener(creds.Insecure()), healthCheck: defaultHealthCheck(), logger: logger.Default(), } diff --git a/internal/transport/creds/dialer.go b/internal/transport/creds/dialer.go new file mode 100644 index 0000000..50ff937 --- /dev/null +++ b/internal/transport/creds/dialer.go @@ -0,0 +1,94 @@ +package creds + +import ( + "crypto/tls" + "fmt" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" +) + +// Dialer is a client-side credential resolved from a set of [Option]s. It owns +// the outbound TLS-mode decision (insecure, system-root TLS, custom-CA TLS, or +// mutual TLS) and builds the corresponding +// [google.golang.org/grpc.DialOption]. Certificate and CA material is parsed +// once and reused, so a single Dialer backs every per-request dial of a +// templated upstream. +type Dialer struct { + opts *options + mode Mode + material *material +} + +// NewDialer builds a client-side credential from opts. With no material options +// it verifies the peer against the system root pool. Construction performs no +// file I/O and never fails; illegal combinations and file problems surface at +// Validate and DialOption. +func NewDialer(opts ...Option) *Dialer { + o := newOptions(opts) + mode := resolveClient(o) + + return &Dialer{opts: o, mode: mode, material: &material{opts: o, mode: mode}} +} + +// Mode reports the resolved outbound TLS mode. +func (d *Dialer) Mode() Mode { + return d.mode +} + +// Validate checks the credential at configuration time without dialing. It +// first checks the cross-field legality of the configuration, then (for modes +// that reference files) reads and inspects the certificate material. +func (d *Dialer) Validate() error { + if err := d.opts.validateClient(); err != nil { + return err + } + + return validateMaterial(d.opts, d.mode) +} + +// DialOption returns the [grpc.DialOption] for outbound connections. serverName +// sets the SNI and hostname-verification name (empty uses the dial target's +// host). The certificate and CA material is parsed on the first call and reused +// on subsequent calls, so only serverName varies per call. The same legality +// guard as Validate runs first, so an illegal configuration fails here rather +// than dialing with the wrong mode. +func (d *Dialer) DialOption(serverName string) (grpc.DialOption, error) { + if err := d.opts.validateClient(); err != nil { + return nil, err + } + + switch d.mode { + case ModeInsecure: + return grpc.WithTransportCredentials(insecure.NewCredentials()), nil + case ModeSystemTLS: + return grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{ + ServerName: serverName, + MinVersion: minTLSVersion, + })), nil + case ModeCustomCA: + if err := d.material.load(); err != nil { + return nil, err + } + + return grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{ + ServerName: serverName, + MinVersion: minTLSVersion, + RootCAs: d.material.caPool, + })), nil + case ModeMutualTLS: + if err := d.material.load(); err != nil { + return nil, err + } + + return grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{ + Certificates: []tls.Certificate{d.material.cert}, + RootCAs: d.material.caPool, + ServerName: serverName, + MinVersion: minTLSVersion, + })), nil + default: + return nil, fmt.Errorf("creds: unreachable dialer mode %d", d.mode) + } +} diff --git a/internal/transport/creds/dialer_test.go b/internal/transport/creds/dialer_test.go new file mode 100644 index 0000000..95af9ac --- /dev/null +++ b/internal/transport/creds/dialer_test.go @@ -0,0 +1,214 @@ +package creds_test + +import ( + "os" + "sync" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/temporalio/temporal-proxy/internal/transport/creds" + "github.com/temporalio/temporal-proxy/pkg/testutil" +) + +func TestDialer_Mode(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + opts []creds.Option + want creds.Mode + }{ + { + name: "explicit insecure", + opts: []creds.Option{creds.Insecure()}, + want: creds.ModeInsecure, + }, + { + name: "no material verifies against system roots", + opts: nil, + want: creds.ModeSystemTLS, + }, + { + name: "CA only verifies against a private anchor", + opts: []creds.Option{creds.WithCA("ca.pem")}, + want: creds.ModeCustomCA, + }, + { + name: "client certificate is mutual TLS", + opts: []creds.Option{creds.WithCA("ca.pem"), creds.WithCertificate("cert.pem", "key.pem")}, + want: creds.ModeMutualTLS, + }, + { + name: "client certificate without CA still resolves as mutual TLS", + opts: []creds.Option{creds.WithCertificate("cert.pem", "key.pem")}, + want: creds.ModeMutualTLS, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tc.want, creds.NewDialer(tc.opts...).Mode()) + }) + } +} + +func TestDialer_Validate(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + opts []creds.Option + wantErr string + }{ + { + name: "certificate without key", + opts: []creds.Option{creds.WithCA("ca.pem"), creds.WithCertificate("cert.pem", "")}, + wantErr: "certificate and key must be set together", + }, + { + name: "key without certificate", + opts: []creds.Option{creds.WithCA("ca.pem"), creds.WithCertificate("", "key.pem")}, + wantErr: "certificate and key must be set together", + }, + { + name: "client certificate requires a CA", + opts: []creds.Option{creds.WithCertificate("cert.pem", "key.pem")}, + wantErr: "certificate authority is required when a client certificate is set", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.ErrorContains(t, creds.NewDialer(tc.opts...).Validate(), tc.wantErr) + }) + } + + t.Run("insecure", func(t *testing.T) { + t.Parallel() + require.NoError(t, creds.NewDialer(creds.Insecure()).Validate()) + }) +} + +func TestDialer_Validate_Files(t *testing.T) { + t.Parallel() + + t.Run("mutual TLS with valid material", func(t *testing.T) { + t.Parallel() + ca, cert, key := testutil.GenerateMTLSCerts(t) + err := creds.NewDialer(creds.WithCA(ca), creds.WithCertificate(cert, key)).Validate() + require.NoError(t, err) + }) + + t.Run("system roots needs no files", func(t *testing.T) { + t.Parallel() + require.NoError(t, creds.NewDialer().Validate()) + }) + + t.Run("custom CA accepts a pinned non-CA anchor", func(t *testing.T) { + t.Parallel() + // The leaf cert is not a CA. A client trust anchor may be a pinned + // self-signed leaf, so custom-CA validation must NOT require IsCA. + _, leaf, _ := testutil.GenerateMTLSCerts(t) + require.NoError(t, creds.NewDialer(creds.WithCA(leaf)).Validate()) + }) + + t.Run("mutual TLS rejects a non-CA used as the CA", func(t *testing.T) { + t.Parallel() + // Same leaf, but in mutual TLS the CA must actually be a CA. + _, leaf, key := testutil.GenerateMTLSCerts(t) + err := creds.NewDialer(creds.WithCA(leaf), creds.WithCertificate(leaf, key)).Validate() + require.ErrorContains(t, err, "not a CA") + }) +} + +func TestDialer_DialOption(t *testing.T) { + t.Parallel() + + t.Run("insecure", func(t *testing.T) { + t.Parallel() + opt, err := creds.NewDialer(creds.Insecure()).DialOption("") + require.NoError(t, err) + require.NotNil(t, opt) + }) + + t.Run("system roots", func(t *testing.T) { + t.Parallel() + opt, err := creds.NewDialer().DialOption("upstream.example.com") + require.NoError(t, err) + require.NotNil(t, opt) + }) + + t.Run("custom CA", func(t *testing.T) { + t.Parallel() + ca, _, _ := testutil.GenerateMTLSCerts(t) + opt, err := creds.NewDialer(creds.WithCA(ca)).DialOption("upstream.example.com") + require.NoError(t, err) + require.NotNil(t, opt) + }) + + t.Run("mutual TLS", func(t *testing.T) { + t.Parallel() + ca, cert, key := testutil.GenerateMTLSCerts(t) + opt, err := creds.NewDialer(creds.WithCA(ca), creds.WithCertificate(cert, key)).DialOption("upstream.example.com") + require.NoError(t, err) + require.NotNil(t, opt) + }) + + t.Run("illegal configuration fails without dialing", func(t *testing.T) { + t.Parallel() + // Client certificate with no CA: the same legality guard as Validate. + ca, cert, key := testutil.GenerateMTLSCerts(t) + _ = ca + _, err := creds.NewDialer(creds.WithCertificate(cert, key)).DialOption("x") + require.ErrorContains(t, err, "certificate authority is required") + }) +} + +func TestDialer_DialOption_ParsesMaterialOnce(t *testing.T) { + t.Parallel() + + ca, cert, key := testutil.GenerateMTLSCerts(t) + d := creds.NewDialer(creds.WithCA(ca), creds.WithCertificate(cert, key)) + + // First call parses and caches the certificate/CA material. + _, err := d.DialOption("first") + require.NoError(t, err) + + // Remove the files. A Dialer that reused its cached material does not need + // them again; a Dialer that re-reads per call would now fail. + require.NoError(t, os.Remove(cert)) + require.NoError(t, os.Remove(key)) + require.NoError(t, os.Remove(ca)) + + _, err = d.DialOption("second") + require.NoError(t, err) +} + +func TestDialer_DialOption_ConcurrentReuse(t *testing.T) { + t.Parallel() + + ca, cert, key := testutil.GenerateMTLSCerts(t) + d := creds.NewDialer(creds.WithCA(ca), creds.WithCertificate(cert, key)) + + const n = 20 + errs := make(chan error, n) + var wg sync.WaitGroup + for i := range n { + wg.Add(1) + go func(i int) { + defer wg.Done() + _, err := d.DialOption("host") + errs <- err + _ = i + }(i) + } + wg.Wait() + close(errs) + + for err := range errs { + require.NoError(t, err) + } +} diff --git a/internal/transport/creds/doc.go b/internal/transport/creds/doc.go index 74d45e9..edcbdbd 100644 --- a/internal/transport/creds/doc.go +++ b/internal/transport/creds/doc.go @@ -1,10 +1,17 @@ -// Package creds provides gRPC transport credential implementations for use -// with Temporal proxy connections. +// Package creds resolves TLS transport credentials for Temporal proxy +// connections from a small set of options. // -// Each credential type exposes two methods: +// A [Dialer] secures outbound (client) connections and a [Listener] secures +// inbound (server) connections. Both are built from the same options +// ([Insecure], [WithCA], [WithCertificate]) but interpret them per role: for a +// client a CA is a trust anchor and a certificate is presented to the upstream; +// for a server a CA requires and verifies client certificates. Each constructor +// resolves the TLS [Mode] and the cross-field legality of the configuration +// once, so validation, dialing, and serving cannot disagree. // -// - DialOption returns a [google.golang.org/grpc.DialOption] for configuring -// outbound (client) connections. -// - ServerOption returns a [google.golang.org/grpc.ServerOption] for -// configuring inbound (server) connections. +// Security is the default: only [Insecure] yields a plaintext credential, and an +// accidentally-empty client credential verifies the peer against the system root +// pool rather than silently downgrading. Construction performs no file I/O; +// certificate material is read and parsed lazily (and once) when a credential is +// validated or used. package creds diff --git a/internal/transport/creds/insecure.go b/internal/transport/creds/insecure.go deleted file mode 100644 index d8d6d29..0000000 --- a/internal/transport/creds/insecure.go +++ /dev/null @@ -1,42 +0,0 @@ -package creds - -import ( - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" -) - -// Insecure disables transport security on both client and server -// connections. No encryption or certificate verification is performed. -// -// Use this only in trusted environments (e.g. local development, loopback -// connections) where TLS is handled at another layer such as a service mesh. -type Insecure struct{} - -// NewInsecure returns an [Insecure] that disables transport security. -func NewInsecure() *Insecure { - return new(Insecure) -} - -// DialOption returns a [grpc.DialOption] that disables transport security for -// outbound connections. -func (c *Insecure) DialOption() (grpc.DialOption, error) { - return grpc.WithTransportCredentials(insecure.NewCredentials()), nil -} - -// ServerOption returns a [grpc.ServerOption] that disables transport security -// for inbound connections. -func (c *Insecure) ServerOption() (grpc.ServerOption, error) { - return grpc.Creds(insecure.NewCredentials()), nil -} - -// Validate reports any configuration problems with this credential. Insecure -// has no certificates or keys to inspect, so it always succeeds. -func (c *Insecure) Validate() error { - return nil -} - -// Encrypted reports whether the transport is encrypted. Insecure disables -// transport security entirely, so it always returns false. -func (c *Insecure) Encrypted() bool { - return false -} diff --git a/internal/transport/creds/insecure_test.go b/internal/transport/creds/insecure_test.go deleted file mode 100644 index af01a0a..0000000 --- a/internal/transport/creds/insecure_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package creds_test - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/temporalio/temporal-proxy/internal/transport/creds" -) - -func TestInsecure_DialOption(t *testing.T) { - t.Parallel() - - opt, err := creds.NewInsecure().DialOption() - require.NoError(t, err) - require.NotNil(t, opt) -} - -func TestInsecure_ServerOption(t *testing.T) { - t.Parallel() - - opt, err := creds.NewInsecure().ServerOption() - require.NoError(t, err) - require.NotNil(t, opt) -} - -func TestInsecure_Validate(t *testing.T) { - t.Parallel() - - // Insecure has no certificate material to inspect; Validate is a no-op. - require.NoError(t, creds.NewInsecure().Validate()) -} - -func TestInsecure_Encrypted(t *testing.T) { - t.Parallel() - - // Insecure disables transport security, so it is never secure. - require.False(t, creds.NewInsecure().Encrypted()) -} diff --git a/internal/transport/creds/listener.go b/internal/transport/creds/listener.go new file mode 100644 index 0000000..c5bd6ce --- /dev/null +++ b/internal/transport/creds/listener.go @@ -0,0 +1,88 @@ +package creds + +import ( + "crypto/tls" + "fmt" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" +) + +// Listener is a server-side credential resolved from a set of [Option]s. It +// owns the inbound TLS-mode decision (insecure, server TLS, or mutual TLS) and +// builds the corresponding [google.golang.org/grpc.ServerOption]. +type Listener struct { + opts *options + mode Mode +} + +// NewListener builds a server-side credential from opts. With no material +// options it is a server-TLS listener that requires its own certificate. +// Construction performs no file I/O and never fails; illegal combinations and +// file problems surface at Validate and ServerOption. +func NewListener(opts ...Option) *Listener { + o := newOptions(opts) + + return &Listener{opts: o, mode: resolveServer(o)} +} + +// Mode reports the resolved inbound TLS mode. +func (l *Listener) Mode() Mode { + return l.mode +} + +// Validate checks the credential at configuration time without binding. It +// first checks the cross-field legality of the configuration, then (for modes +// that reference files) reads and inspects the certificate material. +func (l *Listener) Validate() error { + if err := l.opts.validateServer(); err != nil { + return err + } + + return validateMaterial(l.opts, l.mode) +} + +// ServerOption returns the [grpc.ServerOption] for inbound connections. The same +// legality guard as Validate runs first. Server TLS presents the configured +// certificate; mutual TLS additionally requires and verifies client +// certificates against the configured CA. Both require at least TLS 1.2 and +// restrict TLS 1.2 sessions to the preferred AES-GCM cipher suites. +func (l *Listener) ServerOption() (grpc.ServerOption, error) { + if err := l.opts.validateServer(); err != nil { + return nil, err + } + + if l.mode == ModeInsecure { + return grpc.Creds(insecure.NewCredentials()), nil + } + + cert, err := tls.LoadX509KeyPair(l.opts.cert, l.opts.key) + if err != nil { + return nil, fmt.Errorf("failed to load server key pair: %w", err) + } + + cfg := &tls.Config{ + Certificates: []tls.Certificate{cert}, + MinVersion: minTLSVersion, + CipherSuites: preferredCipherSuites, + } + + if l.mode == ModeMutualTLS { + pool, err := loadCAPool(l.opts.ca) + if err != nil { + return nil, err + } + + cfg.ClientAuth = tls.RequireAndVerifyClientCert + cfg.ClientCAs = pool + } + + return grpc.Creds(credentials.NewTLS(cfg)), nil +} + +// Encrypted reports whether the inbound transport is encrypted. Only the +// insecure mode is unencrypted. +func (l *Listener) Encrypted() bool { + return l.mode != ModeInsecure +} diff --git a/internal/transport/creds/listener_test.go b/internal/transport/creds/listener_test.go new file mode 100644 index 0000000..5efa9c6 --- /dev/null +++ b/internal/transport/creds/listener_test.go @@ -0,0 +1,170 @@ +package creds_test + +import ( + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/temporalio/temporal-proxy/internal/transport/creds" + "github.com/temporalio/temporal-proxy/pkg/testutil" +) + +func TestListener_Mode(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + opts []creds.Option + want creds.Mode + }{ + { + name: "explicit insecure", + opts: []creds.Option{creds.Insecure()}, + want: creds.ModeInsecure, + }, + { + name: "no material requires own certificate (server TLS)", + opts: nil, + want: creds.ModeServerTLS, + }, + { + name: "certificate only is server TLS", + opts: []creds.Option{creds.WithCertificate("cert.pem", "key.pem")}, + want: creds.ModeServerTLS, + }, + { + name: "CA present requires client certs (mutual TLS)", + opts: []creds.Option{creds.WithCA("ca.pem"), creds.WithCertificate("cert.pem", "key.pem")}, + want: creds.ModeMutualTLS, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.Equal(t, tc.want, creds.NewListener(tc.opts...).Mode()) + }) + } +} + +func TestListener_Validate(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + opts []creds.Option + wantErr string + }{ + { + name: "server TLS requires a certificate", + opts: nil, + wantErr: "a server certificate is required", + }, + { + name: "mutual TLS requires the server's own certificate", + opts: []creds.Option{creds.WithCA("ca.pem")}, + wantErr: "a server certificate is required", + }, + { + name: "certificate without key", + opts: []creds.Option{creds.WithCertificate("cert.pem", "")}, + wantErr: "certificate and key must be set together", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.ErrorContains(t, creds.NewListener(tc.opts...).Validate(), tc.wantErr) + }) + } + + t.Run("insecure", func(t *testing.T) { + t.Parallel() + require.NoError(t, creds.NewListener(creds.Insecure()).Validate()) + }) +} + +func TestListener_Validate_Files(t *testing.T) { + t.Parallel() + + t.Run("server TLS with valid ECDSA leaf", func(t *testing.T) { + cert := testutil.ECDSACert(t, &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + }) + + _, keyFile := testutil.GenerateSelfSignedCert(t) + certFile := testutil.WriteFile(t, t.TempDir(), "cert.crt", cert) + + require.NoError(t, creds.NewListener(creds.WithCertificate(certFile, keyFile)).Validate()) + }) + + t.Run("server TLS with valid RSA leaf", func(t *testing.T) { + t.Parallel() + _, cert, key := testutil.GenerateMTLSCerts(t) + require.NoError(t, creds.NewListener(creds.WithCertificate(cert, key)).Validate()) + }) + + t.Run("mutual TLS with valid material", func(t *testing.T) { + t.Parallel() + ca, cert, key := testutil.GenerateMTLSCerts(t) + require.NoError(t, creds.NewListener(creds.WithCA(ca), creds.WithCertificate(cert, key)).Validate()) + }) + + t.Run("missing certificate file surfaces a read error", func(t *testing.T) { + t.Parallel() + _, _, key := testutil.GenerateMTLSCerts(t) + err := creds.NewListener(creds.WithCertificate("/no/such/cert.pem", key)).Validate() + require.ErrorContains(t, err, "failed to read") + }) +} + +func TestListener_ServerOption(t *testing.T) { + t.Parallel() + + t.Run("insecure", func(t *testing.T) { + t.Parallel() + opt, err := creds.NewListener(creds.Insecure()).ServerOption() + require.NoError(t, err) + require.NotNil(t, opt) + }) + + t.Run("server TLS", func(t *testing.T) { + t.Parallel() + _, cert, key := testutil.GenerateMTLSCerts(t) + opt, err := creds.NewListener(creds.WithCertificate(cert, key)).ServerOption() + require.NoError(t, err) + require.NotNil(t, opt) + }) + + t.Run("mutual TLS", func(t *testing.T) { + t.Parallel() + ca, cert, key := testutil.GenerateMTLSCerts(t) + opt, err := creds.NewListener(creds.WithCA(ca), creds.WithCertificate(cert, key)).ServerOption() + require.NoError(t, err) + require.NotNil(t, opt) + }) + + t.Run("missing certificate is illegal", func(t *testing.T) { + t.Parallel() + _, err := creds.NewListener().ServerOption() + require.ErrorContains(t, err, "a server certificate is required") + }) +} + +func TestListener_Encrypted(t *testing.T) { + t.Parallel() + + _, cert, key := testutil.GenerateMTLSCerts(t) + + require.False(t, creds.NewListener(creds.Insecure()).Encrypted()) + require.True(t, creds.NewListener(creds.WithCertificate(cert, key)).Encrypted()) + require.True(t, creds.NewListener(creds.WithCA(cert), creds.WithCertificate(cert, key)).Encrypted()) +} diff --git a/internal/transport/creds/material.go b/internal/transport/creds/material.go new file mode 100644 index 0000000..24803e4 --- /dev/null +++ b/internal/transport/creds/material.go @@ -0,0 +1,66 @@ +package creds + +import ( + "crypto/tls" + "crypto/x509" + "fmt" + "os" + "sync" +) + +// material reads and parses the certificate and CA files a mode references +// exactly once, caching the result (or the error) for reuse across every +// DialOption call of a single Dialer. It is safe for concurrent use: the parsed +// certificate and CA pool are immutable after the first load. A rotated file on +// disk is not picked up until the process restarts, matching how a fixed-address +// upstream loads its material once at startup. +type material struct { + opts *options + mode Mode + + once sync.Once + cert tls.Certificate + caPool *x509.CertPool + err error +} + +func (m *material) load() error { + m.once.Do(func() { + switch m.mode { + case ModeMutualTLS: + cert, err := tls.LoadX509KeyPair(m.opts.cert, m.opts.key) + if err != nil { + m.err = fmt.Errorf("failed to load client key pair: %w", err) + return + } + + pool, err := loadCAPool(m.opts.ca) + if err != nil { + m.err = err + return + } + + m.cert, m.caPool = cert, pool + case ModeCustomCA: + m.caPool, m.err = loadCAPool(m.opts.ca) + } + }) + + return m.err +} + +// loadCAPool reads a PEM-encoded CA certificate file and returns a cert pool +// containing it. +func loadCAPool(path string) (*x509.CertPool, error) { + caBytes, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("failed to load CA certificate: %w", err) + } + + pool := x509.NewCertPool() + if !pool.AppendCertsFromPEM(caBytes) { + return nil, fmt.Errorf("failed to parse CA file: %s", path) + } + + return pool, nil +} diff --git a/internal/transport/creds/mtls.go b/internal/transport/creds/mtls.go deleted file mode 100644 index e267d04..0000000 --- a/internal/transport/creds/mtls.go +++ /dev/null @@ -1,204 +0,0 @@ -package creds - -import ( - "crypto/tls" - "crypto/x509" - "fmt" - "os" - "sync" - - "google.golang.org/grpc" - "google.golang.org/grpc/credentials" - - "github.com/temporalio/temporal-proxy/pkg/validation" -) - -type ( - // MTLS enables mutual TLS on gRPC connections. Both the client and server - // present certificates, and each side verifies the other against a shared CA. - // Minimum TLS version is 1.2. - MTLS struct { - caFile string - certFile string - keyFile string - serverName string - loader *CertLoader - } - - // MTLSOptions holds optional configuration for [MTLS] connections. - MTLSOptions struct { - // ServerName overrides the server name used to verify the server's - // certificate hostname. Useful when the server's certificate CN does not - // match its dial address. - ServerName string - - // Loader, when set, supplies the client certificate and CA pool used by - // [MTLS.DialOption]. Share one loader across the short-lived MTLS values - // built per request for a templated upstream so the certificate and CA - // files are read and parsed once rather than on every request. When nil, - // the MTLS loads (and caches) its own material on first use. - Loader *CertLoader - } - - // CertLoader reads and parses a client certificate/key pair and a CA - // certificate pool once, caching the result for reuse across [MTLS] values. - // It is safe for concurrent use: the parsed material is immutable after the - // first load, so a cached CA pool and certificate may be shared by many - // dial options. A rotated certificate on disk is not picked up until the - // process restarts, matching how a fixed-address upstream loads its material - // once at startup. - CertLoader struct { - caFile string - certFile string - keyFile string - - once sync.Once - cert tls.Certificate - caPool *x509.CertPool - err error - } -) - -// NewMTLS returns an [MTLS] credential that loads the CA certificate from -// caFile and the client certificate/key pair from certFile and keyFile. opts -// provides additional configuration; a zero-value [MTLSOptions] is valid. -func NewMTLS(caFile, certFile, keyFile string, opts MTLSOptions) *MTLS { - loader := opts.Loader - if loader == nil { - loader = NewCertLoader(caFile, certFile, keyFile) - } - - return &MTLS{ - caFile: caFile, - certFile: certFile, - keyFile: keyFile, - serverName: opts.ServerName, - loader: loader, - } -} - -// NewCertLoader returns a [CertLoader] that loads the CA certificate from caFile -// and the client certificate/key pair from certFile and keyFile on first use. -func NewCertLoader(caFile, certFile, keyFile string) *CertLoader { - return &CertLoader{caFile: caFile, certFile: certFile, keyFile: keyFile} -} - -// DialOption returns a [grpc.DialOption] that configures the outbound -// connection with mutual TLS. The client presents its certificate/key pair and -// verifies the server against the CA pool loaded from caFile. -func (c *MTLS) DialOption() (grpc.DialOption, error) { - cert, ca, err := c.loader.load() - if err != nil { - return nil, err - } - - return grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{ - Certificates: []tls.Certificate{cert}, - MinVersion: minTLSVersion, - RootCAs: ca, - ServerName: c.serverName, - })), nil -} - -// ServerOption returns a [grpc.ServerOption] that configures the server to -// require and verify client certificates against the CA pool loaded from -// caFile. The server uses AES-256-GCM and AES-128-GCM cipher suites and -// requires at least TLS 1.2. -func (c *MTLS) ServerOption() (grpc.ServerOption, error) { - cert, err := tls.LoadX509KeyPair(c.certFile, c.keyFile) - if err != nil { - return nil, fmt.Errorf("failed to load server key pair: %w", err) - } - - ca, err := loadCAPool(c.caFile) - if err != nil { - return nil, err - } - - return grpc.Creds(credentials.NewTLS(&tls.Config{ - Certificates: []tls.Certificate{cert}, - ClientAuth: tls.RequireAndVerifyClientCert, - ClientCAs: ca, - MinVersion: minTLSVersion, - CipherSuites: preferredCipherSuites, - })), nil -} - -// Validate checks both the leaf certificate and the CA certificate for -// configuration problems. The leaf must be unexpired, signed with a strong -// algorithm, use a key type compatible with [preferredCipherSuites], and have a -// sufficiently large key; the CA must be unexpired, have the CA basic -// constraint set, be signed with a strong algorithm, and have a sufficiently -// large key. Both files are always checked; failures are collected into a -// single [validation.Errors] so callers see every problem in one call. -func (c *MTLS) Validate() error { - return validation.Validate( - "", - validation.Field("cert", c.certFile, func(path string) error { - return ValidatePEMFile( - path, - CertificateNotExpired(), - UsesSecureCertificateAlgorithm(preferredCipherSuites...), - HasSufficientKeySize(), - ) - }), - validation.Field("key", c.keyFile, ValidatePEMKeyFile), - validation.Field("ca", c.caFile, func(path string) error { - return ValidatePEMFile( - path, - CertificateNotExpired(), - IsCACertificate(), - UsesSecureCertificateAlgorithm(), - HasSufficientKeySize(), - ) - }), - ) -} - -// Encrypted reports whether the transport is encrypted. mTLS always encrypts the -// transport; InsecureSkipVerify weakens peer verification but does not disable -// encryption, so it returns true. -func (c *MTLS) Encrypted() bool { - return true -} - -// loadCAPool reads a PEM-encoded CA certificate file and returns a cert pool -// containing it. -func loadCAPool(path string) (*x509.CertPool, error) { - caBytes, err := os.ReadFile(path) - if err != nil { - return nil, fmt.Errorf("failed to load CA certificate: %w", err) - } - - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(caBytes) { - return nil, fmt.Errorf("failed to parse CA file: %s", path) - } - - return pool, nil -} - -// load reads and parses the client certificate/key pair and CA pool on the first -// call, caching the result (or the error) for every subsequent call. The -// returned certificate and CA pool are immutable and safe to share across dial -// options and goroutines. -func (l *CertLoader) load() (tls.Certificate, *x509.CertPool, error) { - l.once.Do(func() { - cert, err := tls.LoadX509KeyPair(l.certFile, l.keyFile) - if err != nil { - l.err = fmt.Errorf("failed to load client key pair: %w", err) - return - } - - ca, err := loadCAPool(l.caFile) - if err != nil { - l.err = err - return - } - - l.cert = cert - l.caPool = ca - }) - - return l.cert, l.caPool, l.err -} diff --git a/internal/transport/creds/mtls_test.go b/internal/transport/creds/mtls_test.go deleted file mode 100644 index 35b3018..0000000 --- a/internal/transport/creds/mtls_test.go +++ /dev/null @@ -1,302 +0,0 @@ -package creds_test - -import ( - "crypto/x509" - "crypto/x509/pkix" - "math/big" - "os" - "path/filepath" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/temporalio/temporal-proxy/internal/transport/creds" - "github.com/temporalio/temporal-proxy/pkg/testutil" - "github.com/temporalio/temporal-proxy/pkg/validation" -) - -func TestMTLS_DialOption(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - setup func(t *testing.T) (caFile, certFile, keyFile string) - wantErr string - }{ - { - name: "success", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - return testutil.GenerateMTLSCerts(t) - }, - }, - { - name: "missing cert file", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - caFile, _, keyFile := testutil.GenerateMTLSCerts(t) - return caFile, filepath.Join(t.TempDir(), "missing.pem"), keyFile - }, - wantErr: "failed to load client key pair", - }, - { - name: "missing key file", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - caFile, certFile, _ := testutil.GenerateMTLSCerts(t) - return caFile, certFile, filepath.Join(t.TempDir(), "missing.pem") - }, - wantErr: "failed to load client key pair", - }, - { - name: "missing CA file", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - _, certFile, keyFile := testutil.GenerateMTLSCerts(t) - return filepath.Join(t.TempDir(), "missing.pem"), certFile, keyFile - }, - wantErr: "failed to load CA certificate", - }, - { - name: "invalid CA PEM", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - dir := t.TempDir() - _, certFile, keyFile := testutil.GenerateMTLSCerts(t) - caFile := testutil.WriteFile(t, dir, "ca.pem", []byte("not pem")) - return caFile, certFile, keyFile - }, - wantErr: "failed to parse CA file", - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - caFile, certFile, keyFile := tc.setup(t) - opt, err := creds.NewMTLS(caFile, certFile, keyFile, creds.MTLSOptions{}).DialOption() - if tc.wantErr != "" { - require.ErrorContains(t, err, tc.wantErr) - require.Nil(t, opt) - return - } - - require.NoError(t, err) - require.NotNil(t, opt) - }) - } -} - -func TestMTLS_Options(t *testing.T) { - t.Parallel() - - t.Run("ServerName is propagated", func(t *testing.T) { - t.Parallel() - - caFile, certFile, keyFile := testutil.GenerateMTLSCerts(t) - opt, err := creds.NewMTLS(caFile, certFile, keyFile, creds.MTLSOptions{ - ServerName: "example.com", - }).DialOption() - require.NoError(t, err) - require.NotNil(t, opt) - }) -} - -func TestMTLS_ServerOption(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - setup func(t *testing.T) (caFile, certFile, keyFile string) - wantErr string - }{ - { - name: "success", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - return testutil.GenerateMTLSCerts(t) - }, - }, - { - name: "missing cert file", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - caFile, _, keyFile := testutil.GenerateMTLSCerts(t) - return caFile, filepath.Join(t.TempDir(), "missing.pem"), keyFile - }, - wantErr: "failed to load server key pair", - }, - { - name: "missing key file", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - caFile, certFile, _ := testutil.GenerateMTLSCerts(t) - return caFile, certFile, filepath.Join(t.TempDir(), "missing.pem") - }, - wantErr: "failed to load server key pair", - }, - { - name: "missing CA file", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - _, certFile, keyFile := testutil.GenerateMTLSCerts(t) - return filepath.Join(t.TempDir(), "missing.pem"), certFile, keyFile - }, - wantErr: "failed to load CA certificate", - }, - { - name: "invalid CA PEM", - setup: func(t *testing.T) (string, string, string) { - t.Helper() - dir := t.TempDir() - _, certFile, keyFile := testutil.GenerateMTLSCerts(t) - caFile := testutil.WriteFile(t, dir, "ca.pem", []byte("not pem")) - return caFile, certFile, keyFile - }, - wantErr: "failed to parse CA file", - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - caFile, certFile, keyFile := tc.setup(t) - opt, err := creds.NewMTLS(caFile, certFile, keyFile, creds.MTLSOptions{}).ServerOption() - if tc.wantErr != "" { - require.ErrorContains(t, err, tc.wantErr) - require.Nil(t, opt) - return - } - - require.NoError(t, err) - require.NotNil(t, opt) - }) - } -} - -func TestMTLS_Validate(t *testing.T) { - t.Parallel() - - validLeaf := func(t *testing.T) string { - t.Helper() - return writePEMFile(t, testutil.RSACert(t, validTemplate())) - } - - validCA := func(t *testing.T) string { - t.Helper() - return writePEMFile(t, testutil.RSACert(t, caTemplate())) - } - - t.Run("valid leaf and CA pass", func(t *testing.T) { - t.Parallel() - - err := creds.NewMTLS(validCA(t), validLeaf(t), validKey(t), creds.MTLSOptions{}).Validate() - require.NoError(t, err) - }) - - t.Run("missing leaf cert file", func(t *testing.T) { - t.Parallel() - - err := creds.NewMTLS(validCA(t), filepath.Join(t.TempDir(), "missing.pem"), "", creds.MTLSOptions{}).Validate() - require.ErrorContains(t, err, "failed to read PEM file") - }) - - t.Run("missing CA file", func(t *testing.T) { - t.Parallel() - - err := creds.NewMTLS(filepath.Join(t.TempDir(), "missing.pem"), validLeaf(t), "", creds.MTLSOptions{}).Validate() - require.ErrorContains(t, err, "failed to read PEM file") - }) - - t.Run("non-CA cert in CA slot fails", func(t *testing.T) { - t.Parallel() - - // validTemplate has IsCA=false; using it as the CA file should fail the - // IsCACertificate validator. - nonCAFile := writePEMFile(t, testutil.RSACert(t, validTemplate())) - err := creds.NewMTLS(nonCAFile, validLeaf(t), "", creds.MTLSOptions{}).Validate() - require.ErrorContains(t, err, "not a CA") - }) - - t.Run("leaf and CA failures are both reported", func(t *testing.T) { - t.Parallel() - - // Validate runs both checks and combines failures into a single - // validation.Errors, so a bad leaf does not hide a bad CA. - expiredLeaf := writePEMFile(t, testutil.RSACert(t, &x509.Certificate{ - SerialNumber: big.NewInt(7), - Subject: pkix.Name{CommonName: "expired-leaf"}, - NotBefore: time.Now().Add(-2 * time.Hour), - NotAfter: time.Now().Add(-time.Hour), - })) - nonCAFile := writePEMFile(t, testutil.RSACert(t, validTemplate())) - - // Pass a valid key file so the leaf/CA failures aren't joined by a - // third key-file error; this test's intent is leaf+CA aggregation. - err := creds.NewMTLS(nonCAFile, expiredLeaf, validKey(t), creds.MTLSOptions{}).Validate() - require.ErrorContains(t, err, "expired-leaf") - require.ErrorContains(t, err, "not a CA") - - var verrs validation.Errors - require.ErrorAs(t, err, &verrs) - require.Len(t, verrs, 2) - }) -} - -func TestMTLS_Encrypted(t *testing.T) { - t.Parallel() - require.True(t, creds.NewMTLS("", "", "", creds.MTLSOptions{}).Encrypted()) -} - -func TestCertLoader(t *testing.T) { - t.Parallel() - - t.Run("shared loader reads material once", func(t *testing.T) { - t.Parallel() - - caFile, certFile, keyFile := testutil.GenerateMTLSCerts(t) - loader := creds.NewCertLoader(caFile, certFile, keyFile) - - // Simulate two per-request credentials for the same upstream sharing the - // loader; only ServerName differs. - opt, err := creds.NewMTLS(caFile, certFile, keyFile, creds.MTLSOptions{ - ServerName: "first.example.com", - Loader: loader, - }).DialOption() - require.NoError(t, err) - require.NotNil(t, opt) - - // Remove the CA file. A re-read would fail at the CA step, so a successful - // second DialOption proves the loader cached the parsed material. - require.NoError(t, os.Remove(caFile)) - - opt, err = creds.NewMTLS(caFile, certFile, keyFile, creds.MTLSOptions{ - ServerName: "second.example.com", - Loader: loader, - }).DialOption() - require.NoError(t, err) - require.NotNil(t, opt) - }) - - t.Run("load error is cached", func(t *testing.T) { - t.Parallel() - - _, certFile, keyFile := testutil.GenerateMTLSCerts(t) - missingCA := filepath.Join(t.TempDir(), "missing.pem") - loader := creds.NewCertLoader(missingCA, certFile, keyFile) - - m := creds.NewMTLS(missingCA, certFile, keyFile, creds.MTLSOptions{Loader: loader}) - - opt, err := m.DialOption() - require.ErrorContains(t, err, "failed to load CA certificate") - require.Nil(t, opt) - - // The cached error is returned again without another disk read. - opt, err = m.DialOption() - require.ErrorContains(t, err, "failed to load CA certificate") - require.Nil(t, opt) - }) -} diff --git a/internal/transport/creds/options.go b/internal/transport/creds/options.go new file mode 100644 index 0000000..3d67ab4 --- /dev/null +++ b/internal/transport/creds/options.go @@ -0,0 +1,94 @@ +package creds + +import "github.com/temporalio/temporal-proxy/pkg/validation" + +var errKeyAndCertRequired = validation.Error{Field: "cert", Message: "certificate and key must be set together"} + +type ( + // Option configures the TLS material for a [Dialer] or [Listener]. + Option interface { + apply(*options) + } + + options struct { + insecure bool + ca string + cert string + key string + } + + optFunc func(*options) +) + +func (f optFunc) apply(o *options) { f(o) } + +// Insecure disables transport security. It is the only way to obtain a +// plaintext credential: security is the default, so an accidentally-empty +// credential fails toward TLS rather than silently downgrading. Use it +// deliberately, for example on the local loopback socket. +func Insecure() Option { + return optFunc(func(o *options) { o.insecure = true }) +} + +// WithCA verifies the peer against the CA (or pinned trust anchor) in caFile. +func WithCA(caFile string) Option { + return optFunc(func(o *options) { o.ca = caFile }) +} + +// WithCertificate presents the certFile/keyFile key pair. Both must be set +// together; that pairing is enforced when the credential is validated or used. +func WithCertificate(certFile, keyFile string) Option { + return optFunc(func(o *options) { + o.cert = certFile + o.key = keyFile + }) +} + +// validateClient reports cross-field problems with an outbound configuration +// before any file is read: a certificate and key must be supplied together, and +// a client certificate requires a CA to verify the peer against. An insecure or +// material-free (system-root) configuration is always legal. +func (o *options) validateClient() error { + if o.insecure { + return nil + } + + if (o.cert == "") != (o.key == "") { + return errKeyAndCertRequired + } + + if (o.cert != "" || o.key != "") && o.ca == "" { + return validation.Error{Field: "ca", Message: "certificate authority is required when a client certificate is set"} + } + + return nil +} + +// validateServer reports cross-field problems with an inbound configuration +// before any file is read: a listener always presents its own certificate, so a +// certificate and key are both required (and must be supplied together). An +// insecure configuration is always legal. +func (o *options) validateServer() error { + if o.insecure { + return nil + } + + if (o.cert == "") != (o.key == "") { + return errKeyAndCertRequired + } + + if o.cert == "" || o.key == "" { + return validation.Error{Field: "cert", Message: "a server certificate is required"} + } + + return nil +} + +func newOptions(opts []Option) *options { + o := new(options) + for _, opt := range opts { + opt.apply(o) + } + + return o +} diff --git a/internal/transport/creds/resolve.go b/internal/transport/creds/resolve.go new file mode 100644 index 0000000..5616b07 --- /dev/null +++ b/internal/transport/creds/resolve.go @@ -0,0 +1,51 @@ +package creds + +// Mode is the transport-security decision derived once from the configured +// material and the role. It is exposed for logging and tests; callers never +// branch on the raw file paths themselves. +type Mode int + +const ( + // ModeInsecure disables transport security. + ModeInsecure Mode = iota + // ModeServerTLS presents a server certificate; clients are not required to + // present one. + ModeServerTLS + // ModeSystemTLS verifies the peer against the system root pool (client side). + ModeSystemTLS + // ModeCustomCA verifies the peer against a private CA or pinned anchor + // (client side). + ModeCustomCA + // ModeMutualTLS presents a certificate and verifies the peer's certificate. + ModeMutualTLS +) + +// resolveServer computes the inbound (listener) mode for o. A CA requires and +// verifies client certificates (mutual TLS); otherwise the listener presents +// its own certificate (server TLS). +func resolveServer(o *options) Mode { + switch { + case o.insecure: + return ModeInsecure + case o.ca != "": + return ModeMutualTLS + default: + return ModeServerTLS + } +} + +// resolveClient computes the outbound (dialer) mode for o. A client certificate +// selects mutual TLS; a CA alone verifies the peer against a private anchor; +// with no material the peer is verified against the system root pool. +func resolveClient(o *options) Mode { + switch { + case o.insecure: + return ModeInsecure + case o.cert != "" || o.key != "": + return ModeMutualTLS + case o.ca != "": + return ModeCustomCA + default: + return ModeSystemTLS + } +} diff --git a/internal/transport/creds/tls.go b/internal/transport/creds/tls.go deleted file mode 100644 index 665edb8..0000000 --- a/internal/transport/creds/tls.go +++ /dev/null @@ -1,185 +0,0 @@ -package creds - -import ( - "crypto/tls" - "crypto/x509" - "fmt" - "sync" - - "google.golang.org/grpc" - "google.golang.org/grpc/credentials" - - "github.com/temporalio/temporal-proxy/pkg/validation" -) - -// minTLSVersion is the minimum TLS version accepted on both client and server -// connections. TLS 1.2 is the lowest version still considered secure for -// production workloads. -const minTLSVersion = tls.VersionTLS12 - -// preferredCipherSuites lists the only TLS 1.2 cipher suites accepted on -// server connections. All use ECDHE for forward secrecy and AES-GCM for -// authenticated encryption; both RSA and ECDSA leaf keys are supported so the -// proxy accepts identity material from either family. TLS 1.3 cipher suites -// are not controlled by this field and are always negotiated by the Go -// runtime. -var preferredCipherSuites = []uint16{ - tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, -} - -type ( - // TLS enables one-way (server-side) TLS on gRPC connections. The server - // presents a certificate to the client; the client is not required to present - // one. Minimum TLS version is 1.2. - TLS struct { - certFile string - keyFile string - serverName string - caFile string - caLoader *CAPoolLoader - } - - // CAPoolLoader reads and parses a CA certificate pool once, caching the result - // for reuse across the [TLS] values built per request for a templated - // upstream. It is safe for concurrent use: the parsed pool is immutable after - // the first load, so it may be shared by many dial options. A rotated CA on - // disk is not picked up until the process restarts, matching how a - // fixed-address upstream loads its material once at startup. This is the - // CA-only analogue of [CertLoader], which also caches a client key pair for - // mutual TLS. - CAPoolLoader struct { - caFile string - - once sync.Once - pool *x509.CertPool - err error - } -) - -// NewClientTLS returns a [TLS] credential suitable for outbound (client-side) -// connections. The server's certificate is verified against the system root CA -// pool; no client certificate is presented. Use [NewMTLS] when mutual -// authentication is required. serverName overrides the name used for SNI and -// certificate verification; when empty it defaults to the dial target's host. -func NewClientTLS(serverName string) *TLS { - return &TLS{serverName: serverName} -} - -// NewClientTLSWithCA returns a [TLS] credential for outbound (client-side) -// connections that verifies the server's certificate against the CA loaded from -// caFile instead of the system root pool. No client certificate is presented; -// use [NewMTLS] when the upstream requires mutual authentication. serverName -// overrides the name used for SNI and verification; when empty it defaults to -// the dial target's host. -// -// loader, when non-nil, supplies the CA pool so it is read and parsed once and -// reused across the per-request credentials of a templated upstream; pass nil -// for a fixed-address upstream, in which case the credential loads (and caches) -// its own pool on first use. -func NewClientTLSWithCA(caFile, serverName string, loader *CAPoolLoader) *TLS { - if loader == nil { - loader = NewCAPoolLoader(caFile) - } - - return &TLS{caFile: caFile, serverName: serverName, caLoader: loader} -} - -// NewServerTLS returns a [TLS] credential that loads the server certificate and -// private key from certFile and keyFile respectively. -func NewServerTLS(certFile, keyFile string) *TLS { - return &TLS{ - certFile: certFile, - keyFile: keyFile, - } -} - -// NewCAPoolLoader returns a [CAPoolLoader] that reads and parses the CA -// certificate pool from caFile on first use. -func NewCAPoolLoader(caFile string) *CAPoolLoader { - return &CAPoolLoader{caFile: caFile} -} - -// DialOption returns a [grpc.DialOption] that configures the outbound -// connection with TLS. When no custom CA is configured, the server's certificate -// is verified against the system root CA pool. When a custom CA was configured -// via [NewClientTLSWithCA], the server's certificate is verified against that -// custom CA pool instead. The configured server name (when non-empty) is used for -// SNI and verification; otherwise the dial target's host is used. No client -// certificate is presented; use [MTLS] when mutual authentication is required. -func (c *TLS) DialOption() (grpc.DialOption, error) { - cfg := &tls.Config{ - ServerName: c.serverName, - MinVersion: minTLSVersion, - } - - if c.caFile != "" { - pool, err := c.caLoader.load() - if err != nil { - return nil, err - } - - cfg.RootCAs = pool - } - - return grpc.WithTransportCredentials(credentials.NewTLS(cfg)), nil -} - -// ServerOption returns a [grpc.ServerOption] that configures the server with -// TLS using the certificate and key provided at construction. The server -// requires at least TLS 1.2 and restricts TLS 1.2 sessions to AES-GCM cipher -// suites with ECDHE key exchange. -func (c *TLS) ServerOption() (grpc.ServerOption, error) { - cert, err := tls.LoadX509KeyPair(c.certFile, c.keyFile) - if err != nil { - return nil, fmt.Errorf("failed to load server key pair: %w", err) - } - - cfg := &tls.Config{ - Certificates: []tls.Certificate{cert}, - MinVersion: minTLSVersion, - CipherSuites: preferredCipherSuites, - } - - return grpc.Creds(credentials.NewTLS(cfg)), nil -} - -// Validate reads the configured certificate file and returns a [validation.Errors] -// describing any problems found: an expired or not-yet-valid certificate, a weak -// signature algorithm, or a public key type incompatible with [preferredCipherSuites]. -// Client-mode [TLS] credentials (those constructed via [NewClientTLS]) have no -// certFile and will fail with a read error; callers should only invoke Validate -// on server-mode credentials. -func (c *TLS) Validate() error { - return validation.Validate( - "", - validation.Field("cert", c.certFile, func(path string) error { - return ValidatePEMFile( - path, - CertificateNotExpired(), - UsesSecureCertificateAlgorithm(preferredCipherSuites...), - HasSufficientKeySize(), - ) - }), - validation.Field("key", c.keyFile, ValidatePEMKeyFile), - ) -} - -// Encrypted reports whether the transport is encrypted. TLS always encrypts the -// transport, so it returns true. -func (c *TLS) Encrypted() bool { - return true -} - -// load reads and parses the CA pool on the first call, caching the result (or -// the error) for every subsequent call. The returned pool is immutable and safe -// to share across dial options and goroutines. -func (l *CAPoolLoader) load() (*x509.CertPool, error) { - l.once.Do(func() { - l.pool, l.err = loadCAPool(l.caFile) - }) - - return l.pool, l.err -} diff --git a/internal/transport/creds/tls_test.go b/internal/transport/creds/tls_test.go deleted file mode 100644 index e1f0cf8..0000000 --- a/internal/transport/creds/tls_test.go +++ /dev/null @@ -1,238 +0,0 @@ -package creds_test - -import ( - "crypto/x509" - "crypto/x509/pkix" - "math/big" - "os" - "path/filepath" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/temporalio/temporal-proxy/internal/transport/creds" - "github.com/temporalio/temporal-proxy/pkg/testutil" -) - -func TestTLS_DialOption(t *testing.T) { - t.Parallel() - - t.Run("without a server name", func(t *testing.T) { - t.Parallel() - - // DialOption does not load any files; file paths are irrelevant. - opt, err := creds.NewClientTLS("").DialOption() - require.NoError(t, err) - require.NotNil(t, opt) - }) - - t.Run("with a server name", func(t *testing.T) { - t.Parallel() - - opt, err := creds.NewClientTLS("example.com").DialOption() - require.NoError(t, err) - require.NotNil(t, opt) - }) -} - -func TestTLS_ServerOption(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - setup func(t *testing.T) (certFile, keyFile string) - wantErr string - }{ - { - name: "success", - setup: func(t *testing.T) (string, string) { - t.Helper() - return testutil.GenerateSelfSignedCert(t) - }, - }, - { - name: "missing cert file", - setup: func(t *testing.T) (string, string) { - t.Helper() - _, keyFile := testutil.GenerateSelfSignedCert(t) - return filepath.Join(t.TempDir(), "missing.pem"), keyFile - }, - wantErr: "failed to load server key pair", - }, - { - name: "missing key file", - setup: func(t *testing.T) (string, string) { - t.Helper() - certFile, _ := testutil.GenerateSelfSignedCert(t) - return certFile, filepath.Join(t.TempDir(), "missing.pem") - }, - wantErr: "failed to load server key pair", - }, - { - name: "invalid cert content", - setup: func(t *testing.T) (string, string) { - t.Helper() - dir := t.TempDir() - certFile := testutil.WriteFile(t, dir, "cert.pem", []byte("not a cert")) - keyFile := testutil.WriteFile(t, dir, "key.pem", []byte("not a key")) - return certFile, keyFile - }, - wantErr: "failed to load server key pair", - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - certFile, keyFile := tc.setup(t) - opt, err := creds.NewServerTLS(certFile, keyFile).ServerOption() - if tc.wantErr != "" { - require.ErrorContains(t, err, tc.wantErr) - require.Nil(t, opt) - return - } - - require.NoError(t, err) - require.NotNil(t, opt) - }) - } -} - -func TestTLS_Validate(t *testing.T) { - t.Parallel() - - t.Run("valid RSA cert passes", func(t *testing.T) { - t.Parallel() - - // Validate only checks that the key file exists and is PEM; it does - // not verify cert/key matching (LoadX509KeyPair does that at runtime). - certFile := writePEMFile(t, testutil.RSACert(t, validTemplate())) - require.NoError(t, creds.NewServerTLS(certFile, validKey(t)).Validate()) - }) - - t.Run("missing cert file", func(t *testing.T) { - t.Parallel() - - err := creds.NewServerTLS(filepath.Join(t.TempDir(), "missing.pem"), "").Validate() - require.ErrorContains(t, err, "failed to read PEM file") - }) - - t.Run("expired cert fails", func(t *testing.T) { - t.Parallel() - - expired := testutil.RSACert(t, &x509.Certificate{ - SerialNumber: big.NewInt(1), - Subject: pkix.Name{CommonName: "expired"}, - NotBefore: time.Now().Add(-2 * time.Hour), - NotAfter: time.Now().Add(-time.Hour), - }) - certFile := writePEMFile(t, expired) - - err := creds.NewServerTLS(certFile, "").Validate() - require.ErrorContains(t, err, "expired") - }) - - t.Run("ECDSA cert accepted", func(t *testing.T) { - t.Parallel() - - // preferredCipherSuites includes ECDHE_ECDSA suites so - // ECDSA identity material validates successfully. - certFile := writePEMFile(t, testutil.ECDSACert(t, validTemplate())) - require.NoError(t, creds.NewServerTLS(certFile, validKey(t)).Validate()) - }) - - t.Run("client TLS has no certFile and fails to read", func(t *testing.T) { - t.Parallel() - - // NewClientTLS leaves certFile empty; Validate is documented as - // server-mode-only, but we still want to confirm it surfaces a clear - // IO error rather than panicking. - err := creds.NewClientTLS("").Validate() - require.ErrorContains(t, err, "failed to read PEM file") - }) -} - -func TestTLS_DialOption_WithCA(t *testing.T) { - t.Parallel() - - t.Run("valid CA builds a dial option", func(t *testing.T) { - t.Parallel() - - caFile, _, _ := testutil.GenerateMTLSCerts(t) - - opt, err := creds.NewClientTLSWithCA(caFile, "localhost", nil).DialOption() - require.NoError(t, err) - require.NotNil(t, opt) - }) - - t.Run("missing CA file is an error", func(t *testing.T) { - t.Parallel() - - missing := filepath.Join(t.TempDir(), "nope.pem") - - _, err := creds.NewClientTLSWithCA(missing, "localhost", nil).DialOption() - require.ErrorContains(t, err, "failed to load CA certificate") - }) - - t.Run("unparseable CA file is an error", func(t *testing.T) { - t.Parallel() - - bad := testutil.WriteFile(t, t.TempDir(), "ca.pem", []byte("not a pem")) - - _, err := creds.NewClientTLSWithCA(bad, "localhost", nil).DialOption() - require.ErrorContains(t, err, "failed to parse CA file") - }) -} - -func TestTLS_Encrypted(t *testing.T) { - t.Parallel() - - // TLS encrypts the transport in both client and server modes. - require.True(t, creds.NewClientTLS("").Encrypted()) - require.True(t, creds.NewServerTLS("", "").Encrypted()) -} - -func TestCAPoolLoader(t *testing.T) { - t.Parallel() - - t.Run("shared loader reads the CA once", func(t *testing.T) { - t.Parallel() - - caFile, _, _ := testutil.GenerateMTLSCerts(t) - loader := creds.NewCAPoolLoader(caFile) - - // Simulate two per-request credentials for the same templated upstream - // sharing the loader; only ServerName differs. - opt, err := creds.NewClientTLSWithCA(caFile, "first.example.com", loader).DialOption() - require.NoError(t, err) - require.NotNil(t, opt) - - // Remove the CA file. A re-read would fail, so a successful second - // DialOption proves the loader cached the parsed pool. - require.NoError(t, os.Remove(caFile)) - - opt, err = creds.NewClientTLSWithCA(caFile, "second.example.com", loader).DialOption() - require.NoError(t, err) - require.NotNil(t, opt) - }) - - t.Run("load error is cached", func(t *testing.T) { - t.Parallel() - - missing := filepath.Join(t.TempDir(), "missing.pem") - loader := creds.NewCAPoolLoader(missing) - - c := creds.NewClientTLSWithCA(missing, "localhost", loader) - - opt, err := c.DialOption() - require.ErrorContains(t, err, "failed to load CA certificate") - require.Nil(t, opt) - - // The cached error is returned again without another disk read. - opt, err = c.DialOption() - require.ErrorContains(t, err, "failed to load CA certificate") - require.Nil(t, opt) - }) -} diff --git a/internal/transport/creds/validate.go b/internal/transport/creds/validate.go new file mode 100644 index 0000000..0834b61 --- /dev/null +++ b/internal/transport/creds/validate.go @@ -0,0 +1,93 @@ +package creds + +import ( + "crypto/tls" + + "github.com/temporalio/temporal-proxy/pkg/validation" + "github.com/temporalio/temporal-proxy/pkg/validation/certs" +) + +// minTLSVersion is the minimum TLS version accepted on both client and server +// connections. TLS 1.2 is the lowest version still considered secure for +// production workloads. +const minTLSVersion = tls.VersionTLS12 + +// preferredCipherSuites lists the only TLS 1.2 cipher suites accepted on +// server connections. All use ECDHE for forward secrecy and AES-GCM for +// authenticated encryption; both RSA and ECDSA leaf keys are supported so the +// proxy accepts identity material from either family. TLS 1.3 cipher suites +// are not controlled by this field and are always negotiated by the Go +// runtime. +var preferredCipherSuites = []uint16{ + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, +} + +// validateMaterial reads and inspects the certificate files a mode references. +// It runs after legality, so cert/key pairing and CA presence are already +// guaranteed for the mode in question. Modes with no files (insecure, system +// roots) validate trivially. +func validateMaterial(o *options, mode Mode) error { + switch mode { + case ModeCustomCA: + // A client trust anchor may be a pinned self-signed leaf, so it is not + // required to carry the CA basic constraint. + return validation.Validate( + "", + validation.Field("ca", o.ca, trustAnchorChecks), + ) + case ModeServerTLS: + return validation.Validate( + "", + validation.Field("cert", o.cert, leafChecks), + validation.Field("key", o.key, certs.ValidatePEMKeyFile), + ) + case ModeMutualTLS: + return validation.Validate( + "", + validation.Field("cert", o.cert, leafChecks), + validation.Field("key", o.key, certs.ValidatePEMKeyFile), + validation.Field("ca", o.ca, caChecks), + ) + default: // ModeInsecure, ModeSystemTLS + return nil + } +} + +// leafChecks validates a presented certificate: unexpired, signed with a strong +// algorithm, using a key type compatible with the allowed cipher suites, and of +// a sufficient size. +func leafChecks(path string) error { + return certs.ValidatePEMFile( + path, + certs.NotExpired(), + certs.SecureAlgorithm(preferredCipherSuites...), + certs.SufficientKeySize(), + ) +} + +// caChecks validates a certificate authority: unexpired, an actual CA, signed +// with a strong algorithm, and of a sufficient size. +func caChecks(path string) error { + return certs.ValidatePEMFile( + path, + certs.NotExpired(), + certs.IsCA(), + certs.SecureAlgorithm(), + certs.SufficientKeySize(), + ) +} + +// trustAnchorChecks validates a client-side trust anchor: unexpired, signed with +// a strong algorithm, and of a sufficient size. Unlike caChecks it does not +// require the CA basic constraint, so a pinned self-signed leaf is accepted. +func trustAnchorChecks(path string) error { + return certs.ValidatePEMFile( + path, + certs.NotExpired(), + certs.SecureAlgorithm(), + certs.SufficientKeySize(), + ) +} diff --git a/pkg/testutil/certs.go b/pkg/testutil/certs.go index e233c16..4952599 100644 --- a/pkg/testutil/certs.go +++ b/pkg/testutil/certs.go @@ -83,10 +83,45 @@ func GenerateSelfSignedCert(t *testing.T) (certFile, keyFile string) { return certFile, keyFile } +// GenerateRSACert writes a self-signed RSA-2048 certificate and its matching +// PKCS#1 private key to a fresh [testing.T.TempDir] and returns the paths. It is +// the RSA counterpart to [GenerateSelfSignedCert]: the key is retained and +// written out, so the pair loads via [crypto/tls.LoadX509KeyPair] as a real TLS +// identity. The certificate advertises CN "localhost" with DNSNames=["localhost"] +// and both server- and client-auth extended key usages, so it serves as either a +// server certificate or a client identity. It is valid for one hour. +func GenerateRSACert(t *testing.T) (certFile, keyFile string) { + t.Helper() + + dir := t.TempDir() + + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "localhost"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, + DNSNames: []string{"localhost"}, + } + + certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + require.NoError(t, err) + + certFile = WriteFile(t, dir, "cert.pem", pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})) + keyFile = WriteFile(t, dir, "key.pem", + pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)})) + + return certFile, keyFile +} + // GenerateMTLSCerts writes a self-signed ECDSA P-256 CA certificate plus an // RSA-2048 leaf certificate signed by that CA (with its matching key) to a // fresh [testing.T.TempDir] and returns the three paths. The leaf is RSA -// because creds.MTLS.Validate checks the leaf's key type against an RSA-only +// because credential validation checks the leaf's key type against an RSA-only // cipher suite allowlist; an ECDSA leaf fails that check even though it // verifies fine against the CA. The leaf advertises CN "localhost" with // DNSNames=["localhost"]; both certificates are valid for one hour. Use this diff --git a/pkg/testutil/certs_test.go b/pkg/testutil/certs_test.go index abbab46..3cfdf00 100644 --- a/pkg/testutil/certs_test.go +++ b/pkg/testutil/certs_test.go @@ -75,6 +75,29 @@ func TestGenerateSelfSignedCert(t *testing.T) { require.WithinDuration(t, time.Now(), cert.NotAfter, 2*time.Hour) } +func TestGenerateRSACert(t *testing.T) { + t.Parallel() + + certFile, keyFile := testutil.GenerateRSACert(t) + + // Both files should load as a tls.Certificate pair, verifying the key + // matches the cert and is usable as a real TLS identity. + pair, err := tls.LoadX509KeyPair(certFile, keyFile) + require.NoError(t, err) + require.NotEmpty(t, pair.Certificate) + + certPEM, err := os.ReadFile(certFile) + require.NoError(t, err) + + cert := parseSingleCert(t, certPEM) + require.IsType(t, &rsa.PublicKey{}, cert.PublicKey) + require.Equal(t, 2048, cert.PublicKey.(*rsa.PublicKey).Size()*8) + + // Usable as either a server or a client identity. + require.Contains(t, cert.ExtKeyUsage, x509.ExtKeyUsageServerAuth) + require.Contains(t, cert.ExtKeyUsage, x509.ExtKeyUsageClientAuth) +} + func TestGenerateMTLSCerts(t *testing.T) { t.Parallel() diff --git a/internal/transport/creds/validation.go b/pkg/validation/certs/certs.go similarity index 75% rename from internal/transport/creds/validation.go rename to pkg/validation/certs/certs.go index cf787eb..7e2afa1 100644 --- a/internal/transport/creds/validation.go +++ b/pkg/validation/certs/certs.go @@ -1,4 +1,4 @@ -package creds +package certs import ( "crypto/ecdsa" @@ -17,11 +17,11 @@ import ( const ( // minRSAKeyBits is the smallest RSA modulus size accepted by - // HasSufficientKeySize. 2048 bits is the current NIST-recommended minimum. + // SufficientKeySize. 2048 bits is the current NIST-recommended minimum. minRSAKeyBits = 2048 // minECDSAKeyBits is the smallest ECDSA curve size accepted by - // HasSufficientKeySize. 256 bits (e.g. P-256) is the recommended minimum. + // SufficientKeySize. 256 bits (e.g. P-256) is the recommended minimum. minECDSAKeyBits = 256 ) @@ -58,20 +58,22 @@ var ( } ) -// Validator inspects a single parsed certificate and returns a -// validation.Error describing any failure, or nil if the certificate is valid. -type Validator validation.Check[*x509.Certificate] +// Check inspects a single parsed certificate and returns a [validation.Error] +// describing any failure, or nil if the certificate is valid. It is an alias for +// the underlying [validation.Check] so callers can compose plain functions with +// the built-in checks in this package. +type Check = validation.Check[*x509.Certificate] -// ValidatePEMFile reads path and forwards its contents to ValidatePEM. A read +// ValidatePEMFile reads path and forwards its contents to [ValidatePEM]. A read // failure is returned wrapped so callers can distinguish IO problems from // validation failures; the wrapped os.ReadFile error already includes the path. -func ValidatePEMFile(path string, validators ...Validator) error { +func ValidatePEMFile(path string, checks ...Check) error { data, err := os.ReadFile(path) if err != nil { return fmt.Errorf("failed to read PEM file: %w", err) } - return ValidatePEM(data, validators...) + return ValidatePEM(data, checks...) } // ValidatePEMKeyFile reads path and verifies it contains at least one PEM @@ -101,12 +103,12 @@ func ValidatePEMKeyFile(path string) error { return errors.New("no PRIVATE KEY block found in PEM data") } -// ValidatePEM parses all CERTIFICATE blocks from pemData and runs each -// validator against every parsed certificate, collecting all failures into an -// Errors value. Returns nil immediately when no validators are provided. -func ValidatePEM(pemData []byte, validators ...Validator) error { - // With no validators there is nothing to check; skip PEM parsing entirely. - if len(validators) == 0 { +// ValidatePEM parses all CERTIFICATE blocks from pemData and runs each check +// against every parsed certificate, collecting all failures into an +// [validation.Errors] value. Returns nil immediately when no checks are provided. +func ValidatePEM(pemData []byte, checks ...Check) error { + // With no checks there is nothing to verify; skip PEM parsing entirely. + if len(checks) == 0 { return nil } @@ -117,8 +119,8 @@ func ValidatePEM(pemData []byte, validators ...Validator) error { var errs validation.Errors for _, cert := range parsed { - for _, v := range validators { - if vErr := v(cert); vErr != nil { + for _, check := range checks { + if vErr := check(cert); vErr != nil { if ve, ok := errors.AsType[validation.Error](vErr); ok { errs = append(errs, ve) continue @@ -140,10 +142,9 @@ func ValidatePEM(pemData []byte, validators ...Validator) error { return nil } -// CertificateNotExpired returns a CertificateValidator that rejects -// certificates whose NotBefore is in the future or whose NotAfter is in the -// past. -func CertificateNotExpired() Validator { +// NotExpired returns a [Check] that rejects certificates whose NotBefore is in +// the future or whose NotAfter is in the past. +func NotExpired() Check { return func(cert *x509.Certificate) error { now := time.Now() if now.Before(cert.NotBefore) { @@ -166,9 +167,9 @@ func CertificateNotExpired() Validator { } } -// IsCACertificate returns a CertificateValidator that rejects certificates -// that do not have the CA basic constraint set. -func IsCACertificate() Validator { +// IsCA returns a [Check] that rejects certificates that do not have the CA basic +// constraint set. +func IsCA() Check { return func(cert *x509.Certificate) error { if !cert.IsCA { return validation.Error{ @@ -182,11 +183,11 @@ func IsCACertificate() Validator { } } -// UsesSecureCertificateAlgorithm returns a CertificateValidator that rejects -// certificates signed with known-weak algorithms (SHA-1, MD5, MD2, DSA). When -// allowedSuites are provided it additionally rejects certificates whose public -// key type is incompatible with every suite in that list. -func UsesSecureCertificateAlgorithm(allowedSuites ...uint16) Validator { +// SecureAlgorithm returns a [Check] that rejects certificates signed with +// known-weak algorithms (SHA-1, MD5, MD2, DSA). When allowedSuites are provided +// it additionally rejects certificates whose public key type is incompatible +// with every suite in that list. +func SecureAlgorithm(allowedSuites ...uint16) Check { return func(cert *x509.Certificate) error { if weakAlgorithms[cert.SignatureAlgorithm] { return validation.Error{ @@ -224,12 +225,12 @@ func UsesSecureCertificateAlgorithm(allowedSuites ...uint16) Validator { } } -// HasSufficientKeySize returns a Validator that rejects certificates whose -// public key is below the recommended minimum size: RSA keys must be at least -// minRSAKeyBits and ECDSA keys at least minECDSAKeyBits. Key types other than -// RSA and ECDSA (e.g. Ed25519) are unsupported and rejected, since the proxy's -// allowed cipher suites cover only RSA and ECDSA keys. -func HasSufficientKeySize() Validator { +// SufficientKeySize returns a [Check] that rejects certificates whose public key +// is below the recommended minimum size: RSA keys must be at least 2048 bits and +// ECDSA keys at least 256 bits. Key types other than RSA and ECDSA (e.g. +// Ed25519) are unsupported and rejected, since the supported cipher suites cover +// only RSA and ECDSA keys. +func SufficientKeySize() Check { return func(cert *x509.Certificate) error { switch pub := cert.PublicKey.(type) { case *rsa.PublicKey: diff --git a/internal/transport/creds/validation_test.go b/pkg/validation/certs/certs_test.go similarity index 68% rename from internal/transport/creds/validation_test.go rename to pkg/validation/certs/certs_test.go index de4d5b5..97b4211 100644 --- a/internal/transport/creds/validation_test.go +++ b/pkg/validation/certs/certs_test.go @@ -1,4 +1,4 @@ -package creds_test +package certs_test import ( "crypto/ecdsa" @@ -17,17 +17,17 @@ import ( "github.com/stretchr/testify/require" - "github.com/temporalio/temporal-proxy/internal/transport/creds" "github.com/temporalio/temporal-proxy/pkg/testutil" "github.com/temporalio/temporal-proxy/pkg/validation" + "github.com/temporalio/temporal-proxy/pkg/validation/certs" ) -func TestValidatePEM_NoValidators(t *testing.T) { +func TestValidatePEM_NoChecks(t *testing.T) { t.Parallel() - // Invalid PEM: no validators means the function should return nil without - // even attempting to parse the data. - require.NoError(t, creds.ValidatePEM([]byte("not-pem-at-all"))) + // Invalid PEM: no checks means the function should return nil without even + // attempting to parse the data. + require.NoError(t, certs.ValidatePEM([]byte("not-pem-at-all"))) } func TestValidatePEMFile(t *testing.T) { @@ -36,20 +36,20 @@ func TestValidatePEMFile(t *testing.T) { t.Run("missing file", func(t *testing.T) { t.Parallel() - err := creds.ValidatePEMFile( + err := certs.ValidatePEMFile( "/tmp/definitely-not-a-real-cert.pem", - creds.CertificateNotExpired(), + certs.NotExpired(), ) require.Error(t, err) require.ErrorContains(t, err, "failed to read PEM file") }) - t.Run("missing file with no validators still errors", func(t *testing.T) { + t.Run("missing file with no checks still errors", func(t *testing.T) { t.Parallel() - // The IO read happens before the no-validators short-circuit, so a - // missing file should surface even without anything to validate. - err := creds.ValidatePEMFile("/tmp/definitely-not-a-real-cert.pem") + // The IO read happens before the no-checks short-circuit, so a missing + // file should surface even without anything to validate. + err := certs.ValidatePEMFile("/tmp/definitely-not-a-real-cert.pem") require.Error(t, err) require.ErrorContains(t, err, "failed to read PEM file") }) @@ -58,13 +58,13 @@ func TestValidatePEMFile(t *testing.T) { t.Parallel() path := writePEMFile(t, testutil.RSACert(t, validTemplate())) - require.NoError(t, creds.ValidatePEMFile( + require.NoError(t, certs.ValidatePEMFile( path, - creds.CertificateNotExpired(), + certs.NotExpired(), )) }) - t.Run("validator failure is propagated", func(t *testing.T) { + t.Run("check failure is propagated", func(t *testing.T) { t.Parallel() expired := testutil.RSACert(t, &x509.Certificate{ @@ -75,16 +75,16 @@ func TestValidatePEMFile(t *testing.T) { }) path := writePEMFile(t, expired) - err := creds.ValidatePEMFile(path, creds.CertificateNotExpired()) + err := certs.ValidatePEMFile(path, certs.NotExpired()) require.Error(t, err) require.ErrorContains(t, err, "expired") }) - t.Run("non-PEM contents are rejected when validators run", func(t *testing.T) { + t.Run("non-PEM contents are rejected when checks run", func(t *testing.T) { t.Parallel() path := writePEMFile(t, []byte("this is not a PEM block")) - err := creds.ValidatePEMFile(path, creds.CertificateNotExpired()) + err := certs.ValidatePEMFile(path, certs.NotExpired()) require.Error(t, err) require.ErrorContains(t, err, "no certificates found") }) @@ -93,7 +93,7 @@ func TestValidatePEMFile(t *testing.T) { func TestValidatePEM_InvalidPEM(t *testing.T) { t.Parallel() - require.Error(t, creds.ValidatePEM([]byte("not-pem"), creds.CertificateNotExpired())) + require.Error(t, certs.ValidatePEM([]byte("not-pem"), certs.NotExpired())) } func TestValidatePEM_MultipleCerts(t *testing.T) { @@ -108,7 +108,7 @@ func TestValidatePEM_MultipleCerts(t *testing.T) { }) combined := append(valid, expired...) - err := creds.ValidatePEM(combined, creds.CertificateNotExpired()) + err := certs.ValidatePEM(combined, certs.NotExpired()) require.Error(t, err) var verr validation.Errors @@ -117,7 +117,7 @@ func TestValidatePEM_MultipleCerts(t *testing.T) { require.Equal(t, "expired", verr[0].Subject) } -func TestCertificateNotExpired(t *testing.T) { +func TestNotExpired(t *testing.T) { t.Parallel() tests := []struct { @@ -164,7 +164,7 @@ func TestCertificateNotExpired(t *testing.T) { t.Parallel() certPEM := testutil.RSACert(t, tt.tmpl) - err := creds.ValidatePEM(certPEM, creds.CertificateNotExpired()) + err := certs.ValidatePEM(certPEM, certs.NotExpired()) if tt.wantErr { require.Error(t, err) require.Contains(t, err.Error(), tt.errMsg) @@ -175,27 +175,27 @@ func TestCertificateNotExpired(t *testing.T) { } } -func TestIsCACertificate(t *testing.T) { +func TestIsCA(t *testing.T) { t.Parallel() t.Run("CA cert passes", func(t *testing.T) { t.Parallel() certPEM := testutil.RSACert(t, caTemplate()) - require.NoError(t, creds.ValidatePEM(certPEM, creds.IsCACertificate())) + require.NoError(t, certs.ValidatePEM(certPEM, certs.IsCA())) }) t.Run("non-CA cert fails", func(t *testing.T) { t.Parallel() certPEM := testutil.RSACert(t, validTemplate()) - err := creds.ValidatePEM(certPEM, creds.IsCACertificate()) + err := certs.ValidatePEM(certPEM, certs.IsCA()) require.Error(t, err) require.Contains(t, err.Error(), "not a CA") }) } -func TestUsesSecureCertificateAlgorithm(t *testing.T) { +func TestSecureAlgorithm(t *testing.T) { t.Parallel() rsaSuites := []uint16{ @@ -207,21 +207,21 @@ func TestUsesSecureCertificateAlgorithm(t *testing.T) { t.Parallel() certPEM := testutil.RSACert(t, validTemplate()) - require.NoError(t, creds.ValidatePEM(certPEM, creds.UsesSecureCertificateAlgorithm())) + require.NoError(t, certs.ValidatePEM(certPEM, certs.SecureAlgorithm())) }) t.Run("ECDSA cert with no suites passes", func(t *testing.T) { t.Parallel() certPEM := testutil.ECDSACert(t, validTemplate()) - require.NoError(t, creds.ValidatePEM(certPEM, creds.UsesSecureCertificateAlgorithm())) + require.NoError(t, certs.ValidatePEM(certPEM, certs.SecureAlgorithm())) }) t.Run("RSA cert with RSA suite passes", func(t *testing.T) { t.Parallel() certPEM := testutil.RSACert(t, validTemplate()) - require.NoError(t, creds.ValidatePEM(certPEM, creds.UsesSecureCertificateAlgorithm(rsaSuites...))) + require.NoError(t, certs.ValidatePEM(certPEM, certs.SecureAlgorithm(rsaSuites...))) }) t.Run("ECDSA cert incompatible with RSA-only suites fails", func(t *testing.T) { @@ -233,7 +233,7 @@ func TestUsesSecureCertificateAlgorithm(t *testing.T) { } certPEM := testutil.ECDSACert(t, validTemplate()) - err := creds.ValidatePEM(certPEM, creds.UsesSecureCertificateAlgorithm(rsaOnlySuites...)) + err := certs.ValidatePEM(certPEM, certs.SecureAlgorithm(rsaOnlySuites...)) require.Error(t, err) require.Contains(t, err.Error(), "key type") }) @@ -241,61 +241,36 @@ func TestUsesSecureCertificateAlgorithm(t *testing.T) { t.Run("unknown suites skip key type check", func(t *testing.T) { t.Parallel() - // 0xFFFF is intentionally not in suiteKeyTypes, so no key type constraint applies. + // 0xFFFF is intentionally not a known suite, so no key type constraint applies. certPEM := testutil.ECDSACert(t, validTemplate()) - require.NoError(t, creds.ValidatePEM(certPEM, creds.UsesSecureCertificateAlgorithm(0xFFFF))) + require.NoError(t, certs.ValidatePEM(certPEM, certs.SecureAlgorithm(0xFFFF))) }) } -func TestUsesSecureCertificateAlgorithm_WeakAlgorithm(t *testing.T) { +func TestSecureAlgorithm_WeakAlgorithm(t *testing.T) { t.Parallel() - // SHA1WithRSA is produced by openssl, but Go's x509.CreateCertificate does - // not expose SHA-1 signing for RSA directly. We construct and parse a - // certificate DER manually to simulate a weak-algorithm cert. + // Go rejects SHA-1 signing by default, so we build a well-formed modern cert + // and patch its SignatureAlgorithm to a weak value after parsing to exercise + // the rejection path without relying on Go producing a SHA-1 signature. key, err := rsa.GenerateKey(rand.Reader, 2048) require.NoError(t, err) - tmpl := &x509.Certificate{ - SerialNumber: big.NewInt(1), - Subject: pkix.Name{CommonName: "sha1cert"}, - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - SignatureAlgorithm: x509.SHA1WithRSA, - } - - // Go 1.21+ rejects SHA1 signing by default; we test the validator on a - // well-formed modern cert but swap the SignatureAlgorithm field after - // parsing to simulate the weak-algorithm case without relying on Go - // producing a SHA1 signature. + tmpl := validTemplate() der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) - // CreateCertificate may error with SHA1 — if so, construct the cert object directly. - if err != nil { - // Build a parsed cert with a weak algorithm set manually. - goodTmpl := validTemplate() - der, err = x509.CreateCertificate(rand.Reader, goodTmpl, goodTmpl, &key.PublicKey, key) - require.NoError(t, err) - - cert, parseErr := x509.ParseCertificate(der) - require.NoError(t, parseErr) + require.NoError(t, err) - // Patch the signature algorithm to be weak. - cert.SignatureAlgorithm = x509.SHA1WithRSA + cert, err := x509.ParseCertificate(der) + require.NoError(t, err) - v := creds.UsesSecureCertificateAlgorithm() - err = v(cert) - require.Error(t, err) - require.Contains(t, err.Error(), "weak signature algorithm") - return - } + cert.SignatureAlgorithm = x509.SHA1WithRSA - certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) - err = creds.ValidatePEM(certPEM, creds.UsesSecureCertificateAlgorithm()) + err = certs.SecureAlgorithm()(cert) require.Error(t, err) require.Contains(t, err.Error(), "weak signature algorithm") } -func TestHasSufficientKeySize(t *testing.T) { +func TestSufficientKeySize(t *testing.T) { t.Parallel() tests := []struct { @@ -348,7 +323,7 @@ func TestHasSufficientKeySize(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := creds.ValidatePEM(tt.certPEM(t), creds.HasSufficientKeySize()) + err := certs.ValidatePEM(tt.certPEM(t), certs.SufficientKeySize()) if tt.wantErr { require.Error(t, err) require.Contains(t, err.Error(), tt.errMsg) @@ -365,13 +340,13 @@ func TestValidatePEMKeyFile(t *testing.T) { t.Run("valid private key file", func(t *testing.T) { t.Parallel() - require.NoError(t, creds.ValidatePEMKeyFile(validKey(t))) + require.NoError(t, certs.ValidatePEMKeyFile(validKey(t))) }) t.Run("missing file", func(t *testing.T) { t.Parallel() - err := creds.ValidatePEMKeyFile("/tmp/definitely-not-a-real-key.pem") + err := certs.ValidatePEMKeyFile("/tmp/definitely-not-a-real-key.pem") require.Error(t, err) require.ErrorContains(t, err, "failed to read PEM key file") }) @@ -379,7 +354,7 @@ func TestValidatePEMKeyFile(t *testing.T) { t.Run("empty path", func(t *testing.T) { t.Parallel() - err := creds.ValidatePEMKeyFile("") + err := certs.ValidatePEMKeyFile("") require.Error(t, err) require.ErrorContains(t, err, "failed to read PEM key file") }) @@ -388,7 +363,7 @@ func TestValidatePEMKeyFile(t *testing.T) { t.Parallel() path := testutil.WriteFile(t, t.TempDir(), "key.pem", []byte("not pem at all")) - err := creds.ValidatePEMKeyFile(path) + err := certs.ValidatePEMKeyFile(path) require.Error(t, err) require.ErrorContains(t, err, "no PRIVATE KEY block") }) @@ -398,7 +373,7 @@ func TestValidatePEMKeyFile(t *testing.T) { // A CERTIFICATE block is valid PEM but not a private key. path := writePEMFile(t, testutil.RSACert(t, validTemplate())) - err := creds.ValidatePEMKeyFile(path) + err := certs.ValidatePEMKeyFile(path) require.Error(t, err) require.ErrorContains(t, err, "no PRIVATE KEY block") }) @@ -406,8 +381,8 @@ func TestValidatePEMKeyFile(t *testing.T) { t.Run("PEM with mixed blocks accepts first PRIVATE KEY", func(t *testing.T) { t.Parallel() - // A CERTIFICATE then a PRIVATE KEY block: the loop should keep - // scanning past the cert and accept the key. + // A CERTIFICATE then a PRIVATE KEY block: the loop should keep scanning + // past the cert and accept the key. certPEM := testutil.RSACert(t, validTemplate()) _, keyFile := testutil.GenerateSelfSignedCert(t) @@ -415,7 +390,7 @@ func TestValidatePEMKeyFile(t *testing.T) { mixed := append(append([]byte{}, certPEM...), keyPEM...) path := testutil.WriteFile(t, t.TempDir(), "mixed.pem", mixed) - require.NoError(t, creds.ValidatePEMKeyFile(path)) + require.NoError(t, certs.ValidatePEMKeyFile(path)) }) } @@ -431,10 +406,10 @@ func writePEMFile(t *testing.T, data []byte) string { return testutil.WriteFile(t, t.TempDir(), "cert.pem", data) } -// validKey returns a path to a PEM-encoded private key file. The key -// contents are not parsed by ValidatePEMKeyFile, so any PEM block typed -// *PRIVATE KEY satisfies the check; this helper reuses the ECDSA key -// produced by testutil to avoid duplicating key-generation code. +// validKey returns a path to a PEM-encoded private key file. The key contents +// are not parsed by ValidatePEMKeyFile, so any PEM block typed *PRIVATE KEY +// satisfies the check; this helper reuses the ECDSA key produced by testutil to +// avoid duplicating key-generation code. func validKey(t *testing.T) string { t.Helper() _, keyFile := testutil.GenerateSelfSignedCert(t) @@ -473,8 +448,8 @@ func ecdsaCert(t *testing.T, curve elliptic.Curve) []byte { } // ed25519Cert generates a self-signed Ed25519 certificate and returns its PEM -// encoding. Ed25519 is unsupported by HasSufficientKeySize, so this exercises -// the rejection path for key types other than RSA and ECDSA. +// encoding. Ed25519 is unsupported by SufficientKeySize, so this exercises the +// rejection path for key types other than RSA and ECDSA. func ed25519Cert(t *testing.T) []byte { t.Helper() diff --git a/pkg/validation/certs/doc.go b/pkg/validation/certs/doc.go new file mode 100644 index 0000000..ee28f39 --- /dev/null +++ b/pkg/validation/certs/doc.go @@ -0,0 +1,6 @@ +// Package certs provides reusable [validation.Check] building blocks for +// inspecting X.509 certificates and PEM material: expiry, CA basic constraint, +// signature-algorithm and key-type strength, and key size. ValidatePEM and its +// file variants parse PEM data and run a set of checks against every certificate +// they contain, aggregating failures into a [validation.Errors]. +package certs