diff --git a/CHANGELOG.md b/CHANGELOG.md index 6be77a0be..e96bfe642 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## [Unreleased] + +### Changed + +- Minimum Go version is now go1.25.8, following our support policy. +- SPIFFE TLS config hooks now use `VerifyConnection` instead of `VerifyPeerCertificate`, so authorization also runs on resumed TLS sessions. Callers that previously extended a returned config by wrapping `VerifyPeerCertificate` must now extend `VerifyConnection` using manual chaining. + + ## [2.6.0] - 2025-08-21 ### Changed diff --git a/Makefile b/Makefile index 6275fe9c1..f65d1e115 100644 --- a/Makefile +++ b/Makefile @@ -74,7 +74,7 @@ protoc_gen_go_grpc_base_dir := $(build_dir)/protoc-gen-go-grpc protoc_gen_go_grpc_dir := $(protoc_gen_go_grpc_base_dir)/$(protoc_gen_go_grpc_version)-go$(go_version) protoc_gen_go_grpc_bin := $(protoc_gen_go_grpc_dir)/protoc-gen-go-grpc -golangci_lint_version = v2.0.2 +golangci_lint_version = v2.11.4 golangci_lint_dir = $(build_dir)/golangci_lint/$(golangci_lint_version) golangci_lint_bin = $(golangci_lint_dir)/golangci-lint @@ -85,17 +85,17 @@ apiprotos := \ # Toolchain ############################################################################# -go_version_full := 1.24.6 +go_version_full := 1.25.8 go_version := $(go_version_full:.0=) go_dir := $(build_dir)/go/$(go_version) ifeq ($(os1),windows) go_bin_dir = $(go_dir)/go/bin - go_url = https://storage.googleapis.com/golang/go$(go_version).$(os1)-$(arch2).zip + go_url = https://dl.google.com/go/go$(go_version_full).$(os1)-$(arch2).zip exe=".exe" else go_bin_dir = $(go_dir)/bin - go_url = https://storage.googleapis.com/golang/go$(go_version).$(os1)-$(arch2).tar.gz + go_url = https://dl.google.com/go/go$(go_version_full).$(os1)-$(arch2).tar.gz exe= endif diff --git a/examples/spiffe-jwt-using-proxy/proxy/main.go b/examples/spiffe-jwt-using-proxy/proxy/main.go index 54c8e06d3..d983a7016 100644 --- a/examples/spiffe-jwt-using-proxy/proxy/main.go +++ b/examples/spiffe-jwt-using-proxy/proxy/main.go @@ -72,7 +72,7 @@ func run(ctx context.Context) error { func handler(p *httputil.ReverseProxy) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { - log.Printf("%s %s", r.Method, r.URL) + log.Printf("%q %q", r.Method, r.URL.String()) //nolint:gosec // intentional request logging; values are quoted to avoid log forging p.ServeHTTP(w, r) } } diff --git a/go.mod b/go.mod index 7902f62f2..dc23c03c4 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/spiffe/go-spiffe/v2 -go 1.24.0 +go 1.25.8 require ( github.com/Microsoft/go-winio v0.6.2 diff --git a/spiffetls/spiffetls_test.go b/spiffetls/spiffetls_test.go index 0512acd2b..d6d290c86 100644 --- a/spiffetls/spiffetls_test.go +++ b/spiffetls/spiffetls_test.go @@ -496,6 +496,11 @@ func setupTestEnv(t *testing.T) (*testEnv, func()) { // Create custom workload API sources for the server wlCtx, wlCancel := context.WithTimeout(context.Background(), time.Second*5) + defer func() { + if testEnv.err != nil { + wlCancel() + } + }() testEnv.wlCancel = wlCancel testEnv.wlAPIClientA, testEnv.err = workloadapi.New(wlCtx, workloadapi.WithAddr(testEnv.wlAPIServerA.Addr())) if testEnv.err != nil { diff --git a/spiffetls/tlsconfig/config.go b/spiffetls/tlsconfig/config.go index 0331fc198..241619e47 100644 --- a/spiffetls/tlsconfig/config.go +++ b/spiffetls/tlsconfig/config.go @@ -17,13 +17,40 @@ func TLSClientConfig(bundle x509bundle.Source, authorizer Authorizer, opts ...Op } // HookTLSClientConfig sets up the TLS configuration to verify and authorize -// the server X509-SVID. If there is an existing callback set for -// VerifyPeerCertificate it will be wrapped by this package and invoked -// after SPIFFE authentication has completed. +// the server X509-SVID. +// +// SPIFFE authentication uses VerifyConnection, so it runs on every handshake, +// including resumed sessions. VerifyPeerCertificate is not used for SPIFFE +// authentication. +// +// If config.VerifyPeerCertificate or config.VerifyConnection is already set +// when this function is called, those callbacks are preserved and invoked +// after SPIFFE authentication succeeds, first VerifyPeerCertificate, then +// VerifyConnection. +// +// Callers that previously extended the returned config by wrapping +// VerifyPeerCertificate must now extend VerifyConnection instead. Manual +// chaining is recommended: +// +// prev := config.VerifyConnection +// config.VerifyConnection = func(cs tls.ConnectionState) error { +// if prev != nil { +// if err := prev(cs); err != nil { +// return err +// } +// } +// // additional post-SPIFFE checks +// return nil +// } +// +// Note: opts is accepted for API consistency with related Hook functions but +// has no effect in this function. func HookTLSClientConfig(config *tls.Config, bundle x509bundle.Source, authorizer Authorizer, opts ...Option) { + prevVerifyPeerCertificate := config.VerifyPeerCertificate + prevVerifyConnection := config.VerifyConnection resetAuthFields(config) config.InsecureSkipVerify = true - config.VerifyPeerCertificate = WrapVerifyPeerCertificate(config.VerifyPeerCertificate, bundle, authorizer, opts...) + config.VerifyConnection = wrapVerifyConnection(prevVerifyPeerCertificate, prevVerifyConnection, bundle, authorizer) } // A Option changes the defaults used to by mTLS ClientConfig functions. @@ -64,14 +91,27 @@ func MTLSClientConfig(svid x509svid.Source, bundle x509bundle.Source, authorizer } // HookMTLSClientConfig sets up the TLS configuration to present an X509-SVID -// to the server and verify and authorize the server X509-SVID. If there is an -// existing callback set for VerifyPeerCertificate it will be wrapped by -// this package and invoked after SPIFFE authentication has completed. +// to the server and verify and authorize the server X509-SVID. +// +// SPIFFE authentication uses VerifyConnection, so it runs on every handshake, +// including resumed sessions. VerifyPeerCertificate is not used for SPIFFE +// authentication. +// +// If config.VerifyPeerCertificate or config.VerifyConnection is already set +// when this function is called, those callbacks are preserved and invoked +// after SPIFFE authentication succeeds, first VerifyPeerCertificate, then +// VerifyConnection. +// +// Callers that previously extended the returned config by wrapping +// VerifyPeerCertificate must now extend VerifyConnection instead. See +// HookTLSClientConfig for an example. func HookMTLSClientConfig(config *tls.Config, svid x509svid.Source, bundle x509bundle.Source, authorizer Authorizer, opts ...Option) { + prevVerifyPeerCertificate := config.VerifyPeerCertificate + prevVerifyConnection := config.VerifyConnection resetAuthFields(config) config.GetClientCertificate = GetClientCertificate(svid, opts...) config.InsecureSkipVerify = true - config.VerifyPeerCertificate = WrapVerifyPeerCertificate(config.VerifyPeerCertificate, bundle, authorizer, opts...) + config.VerifyConnection = wrapVerifyConnection(prevVerifyPeerCertificate, prevVerifyConnection, bundle, authorizer) } // MTLSWebClientConfig returns a TLS configuration which presents an X509-SVID @@ -116,15 +156,27 @@ func MTLSServerConfig(svid x509svid.Source, bundle x509bundle.Source, authorizer } // HookMTLSServerConfig sets up the TLS configuration to present an X509-SVID -// to the client and require, verify, and authorize the client X509-SVID. If -// there is an existing callback set for VerifyPeerCertificate it will be -// wrapped by this package and invoked after SPIFFE authentication has -// completed. +// to the client and require, verify, and authorize the client X509-SVID. +// +// SPIFFE authentication uses VerifyConnection, so it runs on every handshake, +// including resumed sessions. VerifyPeerCertificate is not used for SPIFFE +// authentication. +// +// If config.VerifyPeerCertificate or config.VerifyConnection is already set +// when this function is called, those callbacks are preserved and invoked +// after SPIFFE authentication succeeds, first VerifyPeerCertificate, then +// VerifyConnection. +// +// Callers that previously extended the returned config by wrapping +// VerifyPeerCertificate must now extend VerifyConnection instead. See +// HookTLSClientConfig for an example. func HookMTLSServerConfig(config *tls.Config, svid x509svid.Source, bundle x509bundle.Source, authorizer Authorizer, opts ...Option) { + prevVerifyPeerCertificate := config.VerifyPeerCertificate + prevVerifyConnection := config.VerifyConnection resetAuthFields(config) config.ClientAuth = tls.RequireAnyClientCert config.GetCertificate = GetCertificate(svid, opts...) - config.VerifyPeerCertificate = WrapVerifyPeerCertificate(config.VerifyPeerCertificate, bundle, authorizer, opts...) + config.VerifyConnection = wrapVerifyConnection(prevVerifyPeerCertificate, prevVerifyConnection, bundle, authorizer) } // MTLSWebServerConfig returns a TLS configuration which presents a web @@ -136,16 +188,29 @@ func MTLSWebServerConfig(cert *tls.Certificate, bundle x509bundle.Source, author return config } -// HookMTLSWebServerConfig sets up the TLS configuration to presents a web +// HookMTLSWebServerConfig sets up the TLS configuration to present a web // server certificate to the client and require, verify, and authorize client -// X509-SVIDs. If there is an existing callback set for VerifyPeerCertificate -// it will be wrapped by this package and invoked after SPIFFE -// authentication has completed. +// X509-SVIDs. +// +// SPIFFE authentication uses VerifyConnection, so it runs on every handshake, +// including resumed sessions. VerifyPeerCertificate is not used for SPIFFE +// authentication. +// +// If config.VerifyPeerCertificate or config.VerifyConnection is already set +// when this function is called, those callbacks are preserved and invoked +// after SPIFFE authentication succeeds, first VerifyPeerCertificate, then +// VerifyConnection. +// +// Callers that previously extended the returned config by wrapping +// VerifyPeerCertificate must now extend VerifyConnection instead. See +// HookTLSClientConfig for an example. func HookMTLSWebServerConfig(config *tls.Config, cert *tls.Certificate, bundle x509bundle.Source, authorizer Authorizer, opts ...Option) { + prevVerifyPeerCertificate := config.VerifyPeerCertificate + prevVerifyConnection := config.VerifyConnection resetAuthFields(config) config.ClientAuth = tls.RequireAnyClientCert config.Certificates = []tls.Certificate{*cert} - config.VerifyPeerCertificate = WrapVerifyPeerCertificate(config.VerifyPeerCertificate, bundle, authorizer, opts...) + config.VerifyConnection = wrapVerifyConnection(prevVerifyPeerCertificate, prevVerifyConnection, bundle, authorizer) } // GetCertificate returns a GetCertificate callback for tls.Config. It uses the @@ -204,6 +269,44 @@ func WrapVerifyPeerCertificate(wrapped func([][]byte, [][]*x509.Certificate) err } } +func wrapVerifyConnection( + prevVerifyPeerCertificate func([][]byte, [][]*x509.Certificate) error, + prevVerifyConnection func(tls.ConnectionState) error, + bundle x509bundle.Source, + authorizer Authorizer, +) func(tls.ConnectionState) error { + return func(cs tls.ConnectionState) error { + rawCerts := rawPeerCertificates(cs.PeerCertificates) + id, certs, err := x509svid.ParseAndVerify(rawCerts, bundle) + if err != nil { + return err + } + + if err := authorizer(id, certs); err != nil { + return err + } + + if prevVerifyPeerCertificate != nil { + if err := prevVerifyPeerCertificate(rawCerts, certs); err != nil { + return err + } + } + + if prevVerifyConnection != nil { + return prevVerifyConnection(cs) + } + return nil + } +} + +func rawPeerCertificates(certs []*x509.Certificate) [][]byte { + raw := make([][]byte, 0, len(certs)) + for _, cert := range certs { + raw = append(raw, cert.Raw) + } + return raw +} + func getTLSCertificate(svid x509svid.Source, trace Trace) (*tls.Certificate, error) { var traceVal interface{} if trace.GetCertificate != nil { @@ -252,4 +355,6 @@ func resetAuthFields(config *tls.Config) { config.InsecureSkipVerify = false config.NameToCertificate = nil //nolint:staticcheck // setting to nil is OK config.RootCAs = nil + config.VerifyPeerCertificate = nil + config.VerifyConnection = nil } diff --git a/spiffetls/tlsconfig/config_test.go b/spiffetls/tlsconfig/config_test.go index 814c28aee..f5e7c9cba 100644 --- a/spiffetls/tlsconfig/config_test.go +++ b/spiffetls/tlsconfig/config_test.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "strings" + "sync/atomic" "testing" "time" @@ -43,7 +44,8 @@ func TestTLSClientConfig(t *testing.T) { assert.True(t, config.InsecureSkipVerify) assert.Nil(t, config.NameToCertificate) //nolint:staticcheck // setting to nil is OK assert.Nil(t, config.RootCAs) - assert.NotNil(t, config.VerifyPeerCertificate) + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) } func TestHookTLSClientConfig(t *testing.T) { @@ -61,7 +63,8 @@ func TestHookTLSClientConfig(t *testing.T) { assert.True(t, config.InsecureSkipVerify) assert.Nil(t, config.NameToCertificate) //nolint:staticcheck // setting to nil is OK assert.Nil(t, config.RootCAs) - assert.NotNil(t, config.VerifyPeerCertificate) + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) assertUnrelatedFieldsUntouched(t, base, config) } @@ -81,7 +84,8 @@ func TestMTLSClientConfig(t *testing.T) { assert.True(t, config.InsecureSkipVerify) assert.Nil(t, config.NameToCertificate) //nolint:staticcheck // setting to nil is OK assert.Nil(t, config.RootCAs) - assert.NotNil(t, config.VerifyPeerCertificate) + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) } func TestHookMTLSClientConfig(t *testing.T) { @@ -102,7 +106,8 @@ func TestHookMTLSClientConfig(t *testing.T) { assert.True(t, config.InsecureSkipVerify) assert.Nil(t, config.NameToCertificate) //nolint:staticcheck // setting to nil is OK assert.Nil(t, config.RootCAs) - assert.NotNil(t, config.VerifyPeerCertificate) + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) assertUnrelatedFieldsUntouched(t, base, config) } @@ -199,7 +204,8 @@ func TestMTLSServerConfig(t *testing.T) { assert.False(t, config.InsecureSkipVerify) assert.Nil(t, config.NameToCertificate) //nolint:staticcheck // setting to nil is OK assert.Nil(t, config.RootCAs) - assert.NotNil(t, config.VerifyPeerCertificate) + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) } func TestHookMTLSServerConfig(t *testing.T) { @@ -220,7 +226,8 @@ func TestHookMTLSServerConfig(t *testing.T) { assert.False(t, config.InsecureSkipVerify) assert.Nil(t, config.NameToCertificate) //nolint:staticcheck // setting to nil is OK assert.Nil(t, config.RootCAs) - assert.NotNil(t, config.VerifyPeerCertificate) + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) assertUnrelatedFieldsUntouched(t, base, config) } @@ -238,7 +245,8 @@ func TestMTLSWebServerConfig(t *testing.T) { assert.False(t, config.InsecureSkipVerify) assert.Nil(t, config.NameToCertificate) //nolint:staticcheck // setting to nil is OK assert.Nil(t, config.RootCAs) - assert.NotNil(t, config.VerifyPeerCertificate) + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) } func TestHookMTLSWebServerConfig(t *testing.T) { @@ -257,7 +265,8 @@ func TestHookMTLSWebServerConfig(t *testing.T) { assert.False(t, config.InsecureSkipVerify) assert.Nil(t, config.NameToCertificate) //nolint:staticcheck // setting to nil is OK assert.Nil(t, config.RootCAs) - assert.NotNil(t, config.VerifyPeerCertificate) + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) assertUnrelatedFieldsUntouched(t, base, config) } @@ -512,6 +521,117 @@ func TestWrapVerifyPeerCertificate(t *testing.T) { } } +// TestHookInputCallbacksPreserved verifies that VerifyPeerCertificate and +// VerifyConnection set on the input config are captured and invoked after +// SPIFFE authentication succeeds for Hook functions that install SPIFFE auth +// via VerifyConnection. +func TestHookInputCallbacksPreserved(t *testing.T) { + td := spiffeid.RequireTrustDomainFromString("domain1.test") + ca := test.NewCA(t, td) + bundle := ca.X509Bundle() + svid := ca.CreateX509SVID(spiffeid.RequireFromPath(td, "/host")) + + var vpcCalls, vcCalls atomic.Int32 + + inputVPC := func([][]byte, [][]*x509.Certificate) error { //nolint:unparam // always nil by design in this test + vpcCalls.Add(1) + return nil + } + inputVC := func(tls.ConnectionState) error { //nolint:unparam // always nil by design in this test + vcCalls.Add(1) + return nil + } + + cs := tls.ConnectionState{PeerCertificates: svid.Certificates} + + reset := func() *tls.Config { + vpcCalls.Store(0) + vcCalls.Store(0) + return &tls.Config{ + VerifyPeerCertificate: inputVPC, + VerifyConnection: inputVC, + } + } + + assertBothCalled := func(t *testing.T, config *tls.Config) { + t.Helper() + assert.Nil(t, config.VerifyPeerCertificate) + assert.NotNil(t, config.VerifyConnection) + require.NoError(t, config.VerifyConnection(cs)) + assert.EqualValues(t, 1, vpcCalls.Load()) + assert.EqualValues(t, 1, vcCalls.Load()) + } + + t.Run("HookTLSClientConfig", func(t *testing.T) { + config := reset() + tlsconfig.HookTLSClientConfig(config, bundle, tlsconfig.AuthorizeAny()) + assertBothCalled(t, config) + }) + + t.Run("HookMTLSClientConfig", func(t *testing.T) { + config := reset() + tlsconfig.HookMTLSClientConfig(config, svid, bundle, tlsconfig.AuthorizeAny()) + assertBothCalled(t, config) + }) + + t.Run("HookMTLSServerConfig", func(t *testing.T) { + config := reset() + tlsconfig.HookMTLSServerConfig(config, svid, bundle, tlsconfig.AuthorizeAny()) + assertBothCalled(t, config) + }) + + t.Run("HookMTLSWebServerConfig", func(t *testing.T) { + tlsCert := &tls.Certificate{Certificate: [][]byte{[]byte("body")}} + config := reset() + tlsconfig.HookMTLSWebServerConfig(config, tlsCert, bundle, tlsconfig.AuthorizeAny()) + assertBothCalled(t, config) + }) + + t.Run("only VerifyPeerCertificate set", func(t *testing.T) { + vpcCalls.Store(0) + config := &tls.Config{VerifyPeerCertificate: inputVPC} //nolint:gosec // G123: intentionally testing input-only VPC before Hook installs VerifyConnection + tlsconfig.HookTLSClientConfig(config, bundle, tlsconfig.AuthorizeAny()) + require.NoError(t, config.VerifyConnection(cs)) + assert.EqualValues(t, 1, vpcCalls.Load()) + }) + + t.Run("only VerifyConnection set", func(t *testing.T) { + vcCalls.Store(0) + config := &tls.Config{VerifyConnection: inputVC} + tlsconfig.HookTLSClientConfig(config, bundle, tlsconfig.AuthorizeAny()) + require.NoError(t, config.VerifyConnection(cs)) + assert.EqualValues(t, 1, vcCalls.Load()) + }) +} + +// TestHookInputCallbackErrorStopsChain verifies that if the preserved +// VerifyPeerCertificate callback returns an error, the preserved +// VerifyConnection callback is not invoked. +func TestHookInputCallbackErrorStopsChain(t *testing.T) { + td := spiffeid.RequireTrustDomainFromString("domain1.test") + ca := test.NewCA(t, td) + bundle := ca.X509Bundle() + svid := ca.CreateX509SVID(spiffeid.RequireFromPath(td, "/host")) + + var vcCalls atomic.Int32 + + config := &tls.Config{ + VerifyPeerCertificate: func([][]byte, [][]*x509.Certificate) error { + return errors.New("vpc rejected") + }, + VerifyConnection: func(tls.ConnectionState) error { + vcCalls.Add(1) + return nil + }, + } + + tlsconfig.HookTLSClientConfig(config, bundle, tlsconfig.AuthorizeAny()) + cs := tls.ConnectionState{PeerCertificates: svid.Certificates} + err := config.VerifyConnection(cs) + require.EqualError(t, err, "vpc rejected") + assert.EqualValues(t, 0, vcCalls.Load()) +} + func TestTLSHandshake(t *testing.T) { td := spiffeid.RequireTrustDomainFromString("domain1.test") ca1 := test.NewCA(t, td) diff --git a/workloadapi/addr_posix_test.go b/workloadapi/addr_posix_test.go index 134ed4f77..fbcd847c0 100644 --- a/workloadapi/addr_posix_test.go +++ b/workloadapi/addr_posix_test.go @@ -22,7 +22,7 @@ func validateAddressCasesOS() []validateAddressCase { err: "workload endpoint unix socket URI must not include a fragment", }, { - addr: "unix://john:doe@foo/path", + addr: "unix://john@foo/path", err: "workload endpoint unix socket URI must not include user info", }, { diff --git a/workloadapi/addr_test.go b/workloadapi/addr_test.go index 42a6bc5aa..413bff051 100644 --- a/workloadapi/addr_test.go +++ b/workloadapi/addr_test.go @@ -58,7 +58,7 @@ func TestValidateAddress(t *testing.T) { err: "workload endpoint tcp socket URI must not include a fragment", }, { - addr: "tcp://john:doe@1.2.3.4:5/path", + addr: "tcp://john@1.2.3.4:5/path", err: "workload endpoint tcp socket URI must not include user info", }, {