From 459533674ffab5d272c25a91b81069543e36f720 Mon Sep 17 00:00:00 2001 From: Tao Guo Date: Tue, 29 Jul 2025 22:52:06 +1000 Subject: [PATCH 1/2] Make sure SPIFFE ID is verified against workload's config --- README.md | 4 +- auth/spiffe.go | 73 +++++++++++++++--------- auth/spiffe_test.go | 133 ++++++++++++++++++++++++++------------------ config.yaml.sample | 5 +- proxy/proxy.go | 7 ++- 5 files changed, 134 insertions(+), 88 deletions(-) diff --git a/README.md b/README.md index 7625339..e53395f 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ workloads: authentication: type: "spiffe" config: - trust_domain: "spiffe://company.com/" + spiffe_ids: ["spiffe://company.com/prod.company"] endpoint: "unix:///tmp/spire-agent/public/api.sock" - workload_id: "staging" @@ -166,7 +166,7 @@ authentication: authentication: type: "spiffe" config: - trust_domain: "spiffe://company.com/" + spiffe_ids: ["spiffe://company.com/prod.company"] endpoint: "unix:///tmp/spire-agent/public/api.sock" audiences: ["temporal_cloud_proxy"] ``` diff --git a/auth/spiffe.go b/auth/spiffe.go index e2398d3..b06367b 100644 --- a/auth/spiffe.go +++ b/auth/spiffe.go @@ -10,10 +10,10 @@ import ( ) type SpiffeAuthenticator struct { - TrustDomain string `yaml:"trust_domain"` - Audiences []string `yaml:"audiences"` - Endpoint string `yaml:"endpoint"` - jwtSource *workloadapi.JWTSource + SpiffeIDs []string `yaml:"spiffe_ids"` + Audiences []string `yaml:"audiences"` + Endpoint string `yaml:"endpoint"` + jwtSource *workloadapi.JWTSource } func (s *SpiffeAuthenticator) Type() string { @@ -21,11 +21,17 @@ func (s *SpiffeAuthenticator) Type() string { } func (s *SpiffeAuthenticator) Init(ctx context.Context, config map[string]interface{}) error { - trustDomain, ok := config["trust_domain"].(string) - if !ok { - return fmt.Errorf("trust_domain is required") + spiffeIDsRaw, ok := config["spiffe_ids"].([]interface{}) + if !ok || len(spiffeIDsRaw) == 0 { + return fmt.Errorf("spiffe_ids is required") + } + for _, id := range spiffeIDsRaw { + spiffeID, ok := id.(string) + if !ok { + return fmt.Errorf("spiffe_ids must contain only strings") + } + s.SpiffeIDs = append(s.SpiffeIDs, spiffeID) } - s.TrustDomain = trustDomain endpoint, ok := config["endpoint"].(string) if !ok { @@ -53,14 +59,11 @@ func (s *SpiffeAuthenticator) Init(ctx context.Context, config map[string]interf } func (s *SpiffeAuthenticator) Authenticate(ctx context.Context, credentials interface{}) (*AuthenticationResult, error) { - token, ok := credentials.(string) - if !ok { - return nil, fmt.Errorf("credentials must be a string token") + token, err := s.extractToken(credentials) + if err != nil { + return nil, err } - const prefix = "Bearer " - token = strings.TrimPrefix(token, prefix) - svid, err := jwtsvid.ParseAndValidate(token, s.jwtSource, s.Audiences) if err != nil { return &AuthenticationResult{ @@ -68,23 +71,39 @@ func (s *SpiffeAuthenticator) Authenticate(ctx context.Context, credentials inte }, fmt.Errorf("invalid token: %w", err) } - claims := make(map[string]interface{}) - for k, v := range svid.Claims { - claims[k] = v + return s.validateSVID(svid) +} + +func (s *SpiffeAuthenticator) extractToken(credentials interface{}) (string, error) { + token, ok := credentials.(string) + if !ok { + return "", fmt.Errorf("credentials must be a string token") } - // TODO should be clearer on what is the trust domain / path / etc - if !strings.HasPrefix(svid.ID.String(), s.TrustDomain) { - return &AuthenticationResult{ - Authenticated: false, - }, fmt.Errorf("invalid trust domain and/or subject: %v", svid.ID.String()) + const prefix = "Bearer " + return strings.TrimPrefix(token, prefix), nil +} + +func (s *SpiffeAuthenticator) validateSVID(svid *jwtsvid.SVID) (*AuthenticationResult, error) { + for _, allowedID := range s.SpiffeIDs { + if svid.ID.String() == allowedID { + claims := make(map[string]interface{}) + for k, v := range svid.Claims { + claims[k] = v + } + + return &AuthenticationResult{ + Authenticated: true, + Subject: svid.ID.String(), + Claims: claims, + Expiration: svid.Expiry, + }, nil + } } + return &AuthenticationResult{ - Authenticated: true, - Subject: svid.ID.String(), - Claims: claims, - Expiration: svid.Expiry, - }, nil + Authenticated: false, + }, fmt.Errorf("invalid SPIFFE ID: %s not in allowed list %v", svid.ID.String(), s.SpiffeIDs) } func (s *SpiffeAuthenticator) Close() error { diff --git a/auth/spiffe_test.go b/auth/spiffe_test.go index 2a6f408..c1a404d 100644 --- a/auth/spiffe_test.go +++ b/auth/spiffe_test.go @@ -41,60 +41,62 @@ func TestSpiffeAuthenticator_Init(t *testing.T) { { name: "valid configuration with all fields", config: map[string]interface{}{ - "trust_domain": "example.org", - "endpoint": "unix:///tmp/spire-agent/public/api.sock", - "audiences": []interface{}{"service1", "service2"}, + "spiffe_ids": []string{"spiffe://example.org/service"}, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", + "audiences": []interface{}{"service1", "service2"}, }, expectError: false, expectedAuth: &SpiffeAuthenticator{ - TrustDomain: "example.org", - Endpoint: "unix:///tmp/spire-agent/public/api.sock", - Audiences: []string{"service1", "service2"}, + SpiffeIDs: []string{"spiffe://example.org/service"}, + Endpoint: "unix:///tmp/spire-agent/public/api.sock", + Audiences: []string{"service1", "service2"}, }, }, { name: "valid configuration without audiences", config: map[string]interface{}{ - "trust_domain": "example.org", - "endpoint": "unix:///tmp/spire-agent/public/api.sock", + "spiffe_ids": []string{"spiffe://example.org/service"}, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", }, expectError: false, expectedAuth: &SpiffeAuthenticator{ - TrustDomain: "example.org", - Endpoint: "unix:///tmp/spire-agent/public/api.sock", - Audiences: nil, + SpiffeIDs: []string{"spiffe://example.org/service"}, + Endpoint: "unix:///tmp/spire-agent/public/api.sock", + Audiences: nil, }, }, { - name: "missing trust_domain", + name: "missing spiffe_ids", config: map[string]interface{}{ "endpoint": "unix:///tmp/spire-agent/public/api.sock", }, expectError: true, - errorContains: "trust_domain is required", + errorContains: "spiffe_ids is required", }, { - name: "missing endpoint", + name: "invalid spiffe_ids type", config: map[string]interface{}{ - "trust_domain": "example.org", + "spiffe_ids": 123, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", }, expectError: true, - errorContains: "endpoint is required", + errorContains: "spiffe_ids is required", }, + { - name: "invalid trust_domain type", + name: "missing endpoint", config: map[string]interface{}{ - "trust_domain": 123, - "endpoint": "unix:///tmp/spire-agent/public/api.sock", + "spiffe_ids": []string{"spiffe://example.org/service"}, }, expectError: true, - errorContains: "trust_domain is required", + errorContains: "endpoint is required", }, + { name: "invalid endpoint type", config: map[string]interface{}{ - "trust_domain": "example.org", - "endpoint": 123, + "spiffe_ids": []string{"spiffe://example.org/service"}, + "endpoint": 123, }, expectError: true, errorContains: "endpoint is required", @@ -102,15 +104,15 @@ func TestSpiffeAuthenticator_Init(t *testing.T) { { name: "mixed audience types", config: map[string]interface{}{ - "trust_domain": "example.org", - "endpoint": "unix:///tmp/spire-agent/public/api.sock", - "audiences": []interface{}{"service1", 123, "service2"}, + "spiffe_ids": []string{"spiffe://example.org/service"}, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", + "audiences": []interface{}{"service1", 123, "service2"}, }, expectError: false, expectedAuth: &SpiffeAuthenticator{ - TrustDomain: "example.org", - Endpoint: "unix:///tmp/spire-agent/public/api.sock", - Audiences: []string{"service1", "service2"}, // non-string audiences are filtered out + SpiffeIDs: []string{"spiffe://example.org/service"}, + Endpoint: "unix:///tmp/spire-agent/public/api.sock", + Audiences: []string{"service1", "service2"}, // non-string audiences are filtered out }, }, } @@ -136,7 +138,7 @@ func TestSpiffeAuthenticator_Init(t *testing.T) { } else { assert.NoError(t, err) if tt.expectedAuth != nil { - assert.Equal(t, tt.expectedAuth.TrustDomain, auth.TrustDomain) + assert.Equal(t, tt.expectedAuth.SpiffeIDs, auth.SpiffeIDs) assert.Equal(t, tt.expectedAuth.Endpoint, auth.Endpoint) assert.Equal(t, tt.expectedAuth.Audiences, auth.Audiences) } @@ -149,7 +151,7 @@ func TestSpiffeAuthenticator_Authenticate(t *testing.T) { tests := []struct { name string credentials interface{} - trustDomain string + spiffeIDs []string audiences []string setupMock func() *MockJWTSource expectError bool @@ -160,7 +162,7 @@ func TestSpiffeAuthenticator_Authenticate(t *testing.T) { { name: "successful authentication with valid token", credentials: "valid.jwt.token", - trustDomain: "spiffe://example.org", + spiffeIDs: []string{"spiffe://example.org/service"}, audiences: []string{"service1"}, setupMock: func() *MockJWTSource { // This test would require mocking jwtsvid.ParseAndValidate @@ -172,7 +174,7 @@ func TestSpiffeAuthenticator_Authenticate(t *testing.T) { { name: "successful authentication with Bearer prefix", credentials: "Bearer valid.jwt.token", - trustDomain: "spiffe://example.org", + spiffeIDs: []string{"spiffe://example.org/service"}, audiences: []string{"service1"}, setupMock: func() *MockJWTSource { return nil @@ -182,7 +184,7 @@ func TestSpiffeAuthenticator_Authenticate(t *testing.T) { { name: "invalid credentials type", credentials: 123, - trustDomain: "spiffe://example.org", + spiffeIDs: []string{"spiffe://example.org/service"}, audiences: []string{"service1"}, expectError: true, errorContains: "credentials must be a string token", @@ -190,7 +192,7 @@ func TestSpiffeAuthenticator_Authenticate(t *testing.T) { { name: "empty token", credentials: "", - trustDomain: "spiffe://example.org", + spiffeIDs: []string{"spiffe://example.org/service"}, audiences: []string{"service1"}, expectError: true, errorContains: "invalid token", @@ -198,7 +200,7 @@ func TestSpiffeAuthenticator_Authenticate(t *testing.T) { { name: "nil credentials", credentials: nil, - trustDomain: "spiffe://example.org", + spiffeIDs: []string{"spiffe://example.org/service"}, audiences: []string{"service1"}, expectError: true, errorContains: "credentials must be a string token", @@ -208,8 +210,8 @@ func TestSpiffeAuthenticator_Authenticate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { auth := &SpiffeAuthenticator{ - TrustDomain: tt.trustDomain, - Audiences: tt.audiences, + SpiffeIDs: tt.spiffeIDs, + Audiences: tt.audiences, } ctx := context.Background() @@ -353,11 +355,11 @@ func TestSpiffeAuthenticator_TokenParsing(t *testing.T) { // Helper method to test configuration parsing without JWT source creation func (s *SpiffeAuthenticator) initConfig(ctx context.Context, config map[string]interface{}) error { - trustDomain, ok := config["trust_domain"].(string) - if !ok { - return errors.New("trust_domain is required") + spiffeIDs, ok := config["spiffe_ids"].([]string) + if !ok || len(spiffeIDs) == 0 { + return errors.New("spiffe_ids is required") } - s.TrustDomain = trustDomain + s.SpiffeIDs = spiffeIDs endpoint, ok := config["endpoint"].(string) if !ok { @@ -389,9 +391,9 @@ func TestSpiffeAuthenticator_Integration(t *testing.T) { ctx := context.Background() config := map[string]interface{}{ - "trust_domain": "example.org", - "endpoint": "unix:///tmp/spire-agent/public/api.sock", - "audiences": []interface{}{"test-service"}, + "id": "spiffe://example.org/service", + "endpoint": "unix:///tmp/spire-agent/public/api.sock", + "audiences": []interface{}{"test-service"}, } err := auth.Init(ctx, config) @@ -419,34 +421,57 @@ func TestSpiffeAuthenticator_ConfigurationEdgeCases(t *testing.T) { { name: "nil config values", config: map[string]interface{}{ - "trust_domain": nil, - "endpoint": nil, + "endpoint": nil, }, expectError: true, }, { name: "empty string values", config: map[string]interface{}{ - "trust_domain": "", - "endpoint": "", + "spiffe_ids": []string{""}, + "endpoint": "", }, expectError: false, // Empty strings are valid, just not useful }, { name: "audiences as empty slice", config: map[string]interface{}{ - "trust_domain": "example.org", - "endpoint": "unix:///tmp/spire-agent/public/api.sock", - "audiences": []interface{}{}, + "spiffe_ids": []string{"spiffe://example.org/service"}, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", + "audiences": []interface{}{}, }, expectError: false, }, { name: "audiences as nil", config: map[string]interface{}{ - "trust_domain": "example.org", - "endpoint": "unix:///tmp/spire-agent/public/api.sock", - "audiences": nil, + "spiffe_ids": []string{"spiffe://example.org/service"}, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", + "audiences": nil, + }, + expectError: false, + }, + { + name: "empty spiffe_ids slice", + config: map[string]interface{}{ + "spiffe_ids": []string{}, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", + }, + expectError: true, + }, + { + name: "nil spiffe_ids", + config: map[string]interface{}{ + "spiffe_ids": nil, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", + }, + expectError: true, + }, + { + name: "multiple spiffe_ids", + config: map[string]interface{}{ + "spiffe_ids": []string{"spiffe://example.org/service1", "spiffe://example.org/service2"}, + "endpoint": "unix:///tmp/spire-agent/public/api.sock", }, expectError: false, }, diff --git a/config.yaml.sample b/config.yaml.sample index 4cea565..4ad5dc8 100644 --- a/config.yaml.sample +++ b/config.yaml.sample @@ -35,7 +35,8 @@ workloads: # authentication: # spiffe authentication example # type: "spiffe" # config: -# trust_domain: "spiffe://example.org/" +# spiffe_ids: +# - "spiffe://example.org/my.workload.1" # endpoint: "unix:///tmp/spire-agent/public/api.sock" # audiences: # - "temporal_cloud_proxy" @@ -44,4 +45,4 @@ workloads: # config: # jwks-url: "http://localhost:8200/v1/identity/oidc/.well-known/keys" # audiences: -# - "temporal_cloud_proxy" \ No newline at end of file +# - "temporal_cloud_proxy" diff --git a/proxy/proxy.go b/proxy/proxy.go index 3824c54..e5b4fec 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -5,13 +5,14 @@ import ( "crypto/tls" "errors" "fmt" + "os" + "sync" + "time" + "github.com/temporal-sa/temporal-cloud-proxy/auth" "github.com/temporal-sa/temporal-cloud-proxy/codec" "github.com/temporal-sa/temporal-cloud-proxy/config" "github.com/temporal-sa/temporal-cloud-proxy/metrics" - "os" - "sync" - "time" "go.opentelemetry.io/otel/attribute" "go.temporal.io/sdk/converter" From 1e0a1c901b7710a6b7e53b2f61e9837135e4f514 Mon Sep 17 00:00:00 2001 From: Brendan Myers Date: Tue, 5 Aug 2025 07:04:10 +0800 Subject: [PATCH 2/2] fix: validate spiffe audience config --- auth/spiffe.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/auth/spiffe.go b/auth/spiffe.go index b06367b..dc0e1e6 100644 --- a/auth/spiffe.go +++ b/auth/spiffe.go @@ -39,12 +39,16 @@ func (s *SpiffeAuthenticator) Init(ctx context.Context, config map[string]interf } s.Endpoint = endpoint - if audiencesRaw, ok := config["audiences"].([]interface{}); ok { - for _, a := range audiencesRaw { - if audience, ok := a.(string); ok { - s.Audiences = append(s.Audiences, audience) - } + audiencesRaw, ok := config["audiences"].([]interface{}) + if !ok || len(audiencesRaw) == 0 { + return fmt.Errorf("audiences is required") + } + for _, a := range audiencesRaw { + audience, ok := a.(string) + if !ok { + return fmt.Errorf("audiences must contain only strings") } + s.Audiences = append(s.Audiences, audience) } clientOptions := workloadapi.WithClientOptions(workloadapi.WithAddr(s.Endpoint))