From 6a671db2b2e69f75d09bb8ad9ddedec564ffb102 Mon Sep 17 00:00:00 2001 From: Anton Kaymakchi Date: Fri, 29 Apr 2022 22:06:06 +0100 Subject: [PATCH 1/3] Add posibility to disable SPIFFE cert validation per envoy instance Signed-off-by: Anton Kaymakchi --- pkg/agent/endpoints/sdsv3/handler.go | 35 +++++-- pkg/agent/endpoints/sdsv3/handler_test.go | 121 ++++++++++++++++++++++ 2 files changed, 147 insertions(+), 9 deletions(-) diff --git a/pkg/agent/endpoints/sdsv3/handler.go b/pkg/agent/endpoints/sdsv3/handler.go index c6901942bd..83eb467642 100644 --- a/pkg/agent/endpoints/sdsv3/handler.go +++ b/pkg/agent/endpoints/sdsv3/handler.go @@ -28,6 +28,10 @@ import ( "google.golang.org/protobuf/types/known/anypb" ) +const ( + disableSPIFFECertValidationKey = "disable_spiffe_cert_validation" +) + type Attestor interface { Attest(ctx context.Context) ([]*common.Selector, error) } @@ -249,15 +253,9 @@ func (h *Handler) buildResponse(versionInfo string, req *discovery_v3.DiscoveryR } returnAllEntries := len(names) == 0 - // Use RootCA as default, but replace with SPIFFE auth when Envoy version is at least v1.18.0 - var builder validationContextBuilder - if !h.c.DisableSPIFFECertValidation && supportsSPIFFEAuthExtension(req) { - builder, err = newSpiffeBuilder(upd.Bundle, upd.FederatedBundles) - if err != nil { - return nil, err - } - } else { - builder = newRootCABuilder(upd.Bundle, upd.FederatedBundles) + builder, err := h.getValidationContextBuilder(req, upd) + if err != nil { + return nil, err } // TODO: verify the type url @@ -340,6 +338,14 @@ type validationContextBuilder interface { buildAll(resourceName string) (*any.Any, error) } +func (h *Handler) getValidationContextBuilder(req *discovery_v3.DiscoveryRequest, upd *cache.WorkloadUpdate) (validationContextBuilder, error) { + if !h.isDisabledSPIFFECertValidation(req) && supportsSPIFFEAuthExtension(req) { + return newSpiffeBuilder(upd.Bundle, upd.FederatedBundles) + } + + return newRootCABuilder(upd.Bundle, upd.FederatedBundles), nil +} + type rootCABuilder struct { bundles map[string]*bundleutil.Bundle } @@ -500,6 +506,17 @@ func supportsSPIFFEAuthExtension(req *discovery_v3.DiscoveryRequest) bool { return false } +func (h *Handler) isDisabledSPIFFECertValidation(req *discovery_v3.DiscoveryRequest) bool { + if h.c.DisableSPIFFECertValidation { + return true + } + + fields := req.Node.GetMetadata().GetFields() + v, ok := fields[disableSPIFFECertValidationKey] + + return ok && (v.GetBoolValue() || v.GetStringValue() == "true") +} + func buildTLSCertificate(identity cache.Identity, defaultSVIDName string) (*anypb.Any, error) { name := identity.Entry.SpiffeId if defaultSVIDName != "" { diff --git a/pkg/agent/endpoints/sdsv3/handler_test.go b/pkg/agent/endpoints/sdsv3/handler_test.go index 525b5ab5f0..d9a2dbef90 100644 --- a/pkg/agent/endpoints/sdsv3/handler_test.go +++ b/pkg/agent/endpoints/sdsv3/handler_test.go @@ -32,6 +32,7 @@ import ( "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" "google.golang.org/protobuf/types/known/anypb" + "google.golang.org/protobuf/types/known/structpb" ) var ( @@ -471,6 +472,66 @@ func TestStreamSecrets(t *testing.T) { }, expectSecrets: []*tls_v3.Secret{tdValidationContext3}, }, + { + name: "Disable custom validation per instance", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"default"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + Metadata: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + disableSPIFFECertValidationKey: structpb.NewBoolValue(true), + }, + }, + }, + }, + expectSecrets: []*tls_v3.Secret{workloadTLSCertificate3}, + }, + { + name: "Disable SPIFFE cert validation per instance with string value", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"default"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + Metadata: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + disableSPIFFECertValidationKey: structpb.NewStringValue("true"), + }, + }, + }, + }, + expectSecrets: []*tls_v3.Secret{workloadTLSCertificate3}, + }, + { + name: "Disable SPIFFE cert validation set to false per instance", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"spiffe://domain.test"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + Metadata: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + disableSPIFFECertValidationKey: structpb.NewBoolValue(false), + }, + }, + }, + }, + expectSecrets: []*tls_v3.Secret{tdValidationContextSpiffeValidator}, + }, + { + name: "Disable SPIFFE cert validation set unknown string value", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"spiffe://domain.test"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + Metadata: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + disableSPIFFECertValidationKey: structpb.NewStringValue("test"), + }, + }, + }, + }, + expectSecrets: []*tls_v3.Secret{tdValidationContextSpiffeValidator}, + }, } { t.Run(tt.name, func(t *testing.T) { test := setupTestWithConfig(t, tt.config) @@ -854,6 +915,66 @@ func TestFetchSecrets(t *testing.T) { }, expectSecrets: []*tls_v3.Secret{tdValidationContext3}, }, + { + name: "Disable custom validation per instance", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"default"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + Metadata: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + disableSPIFFECertValidationKey: structpb.NewBoolValue(true), + }, + }, + }, + }, + expectSecrets: []*tls_v3.Secret{workloadTLSCertificate3}, + }, + { + name: "Disable SPIFFE cert validation per instance with string value", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"default"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + Metadata: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + disableSPIFFECertValidationKey: structpb.NewStringValue("true"), + }, + }, + }, + }, + expectSecrets: []*tls_v3.Secret{workloadTLSCertificate3}, + }, + { + name: "Disable SPIFFE cert validation set to false per instance", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"spiffe://domain.test"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + Metadata: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + disableSPIFFECertValidationKey: structpb.NewBoolValue(false), + }, + }, + }, + }, + expectSecrets: []*tls_v3.Secret{tdValidationContextSpiffeValidator}, + }, + { + name: "Disable SPIFFE cert validation set unknown string value", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"spiffe://domain.test"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + Metadata: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + disableSPIFFECertValidationKey: structpb.NewStringValue("test"), + }, + }, + }, + }, + expectSecrets: []*tls_v3.Secret{tdValidationContextSpiffeValidator}, + }, } { t.Run(tt.name, func(t *testing.T) { test := setupTestWithConfig(t, tt.config) From 79fd5d28d145c2aba0a5e5e6306988b842b52249 Mon Sep 17 00:00:00 2001 From: Anton Kaymakchi Date: Fri, 29 Apr 2022 22:16:13 +0100 Subject: [PATCH 2/3] Add some documentation Signed-off-by: Anton Kaymakchi --- doc/spire_agent.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/spire_agent.md b/doc/spire_agent.md index 64c1967e75..fc08f20c64 100644 --- a/doc/spire_agent.md +++ b/doc/spire_agent.md @@ -321,6 +321,8 @@ support for the [SPIFFE Certificate Validator](https://www.envoyproxy.io/docs/en extension, which is only available starting with Envoy 1.18. The default name is configurable (see `default_all_bundles_name` under [SDS Configuration](#sds-configuration). +SPIFFE Certificate Validator enabled by default and can be disabled by setting `disable_spiffe_cert_validation` to `true` in [SDS Configuration](#sds-configuration) for all instances or per instance in envoy's node metadata. + ## OpenShift Support The default security profile of [OpenShift](https://www.openshift.com/products/container-platform) forbids access to host level resources. A custom set of policies can be applied to enable the level of access needed by Spire to operate within OpenShift. From 4e740f00838793d2f4ecd9d751bed83cdd024a01 Mon Sep 17 00:00:00 2001 From: Anton Kaymakchi Date: Sat, 30 Apr 2022 12:42:02 +0100 Subject: [PATCH 3/3] Address PR comments Signed-off-by: Anton Kaymakchi --- doc/spire_agent.md | 2 +- pkg/agent/endpoints/sdsv3/handler.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/spire_agent.md b/doc/spire_agent.md index fc08f20c64..c90bfafde5 100644 --- a/doc/spire_agent.md +++ b/doc/spire_agent.md @@ -321,7 +321,7 @@ support for the [SPIFFE Certificate Validator](https://www.envoyproxy.io/docs/en extension, which is only available starting with Envoy 1.18. The default name is configurable (see `default_all_bundles_name` under [SDS Configuration](#sds-configuration). -SPIFFE Certificate Validator enabled by default and can be disabled by setting `disable_spiffe_cert_validation` to `true` in [SDS Configuration](#sds-configuration) for all instances or per instance in envoy's node metadata. +The [SPIFFE Certificate Validator](https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/tls_spiffe_validator_config.proto) configures Envoy to perform SPIFFE authentication. It is used by default but can be disabled by setting `disable_spiffe_cert_validation` to `true` in [SDS Configuration](#sds-configuration). Alternatively, to disable for an individual envoy instance, the `disable_spiffe_cert_validation` key can be configured and set to `true` in the Envoy node metadata. When used, Envoy will perform standard X.509 certificate chain validation. ## OpenShift Support diff --git a/pkg/agent/endpoints/sdsv3/handler.go b/pkg/agent/endpoints/sdsv3/handler.go index 83eb467642..d31f224c85 100644 --- a/pkg/agent/endpoints/sdsv3/handler.go +++ b/pkg/agent/endpoints/sdsv3/handler.go @@ -339,7 +339,7 @@ type validationContextBuilder interface { } func (h *Handler) getValidationContextBuilder(req *discovery_v3.DiscoveryRequest, upd *cache.WorkloadUpdate) (validationContextBuilder, error) { - if !h.isDisabledSPIFFECertValidation(req) && supportsSPIFFEAuthExtension(req) { + if !h.isSPIFFECertValidationDisabled(req) && supportsSPIFFEAuthExtension(req) { return newSpiffeBuilder(upd.Bundle, upd.FederatedBundles) } @@ -506,7 +506,7 @@ func supportsSPIFFEAuthExtension(req *discovery_v3.DiscoveryRequest) bool { return false } -func (h *Handler) isDisabledSPIFFECertValidation(req *discovery_v3.DiscoveryRequest) bool { +func (h *Handler) isSPIFFECertValidationDisabled(req *discovery_v3.DiscoveryRequest) bool { if h.c.DisableSPIFFECertValidation { return true }