From 7bc9facf99825822ee71dc046a2a7e036525b338 Mon Sep 17 00:00:00 2001 From: Anton Kaymakchi Date: Tue, 26 Apr 2022 16:42:23 +0100 Subject: [PATCH 1/4] Fix Makefile to work on M1 MacBook --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 0b6084b904..46fe6fbc87 100644 --- a/Makefile +++ b/Makefile @@ -84,6 +84,8 @@ ifeq ($(arch1),x86_64) arch2=amd64 else ifeq ($(arch1),aarch64) arch2=arm64 +else ifeq ($(arch1),arm64) +arch2=arm64 else $(error unsupported ARCH: $(arch1)) endif From 282adac9415498f78d96f44345fe5d7a571b29c8 Mon Sep 17 00:00:00 2001 From: Anton Kaymakchi Date: Tue, 26 Apr 2022 16:42:59 +0100 Subject: [PATCH 2/4] Add option to disable Envoy SDS custom validation --- cmd/spire-agent/cli/run/run.go | 15 +++++++++------ cmd/spire-agent/cli/run/run_test.go | 20 ++++++++++++++++++++ conf/agent/agent_full.conf | 3 +++ doc/spire_agent.md | 11 ++++++----- pkg/agent/agent.go | 1 + pkg/agent/config.go | 3 +++ pkg/agent/endpoints/config.go | 3 +++ pkg/agent/endpoints/endpoints.go | 11 ++++++----- pkg/agent/endpoints/endpoints_test.go | 2 ++ pkg/agent/endpoints/sdsv3/handler.go | 13 +++++++------ 10 files changed, 60 insertions(+), 22 deletions(-) diff --git a/cmd/spire-agent/cli/run/run.go b/cmd/spire-agent/cli/run/run.go index dd88598aee..3c18058daa 100644 --- a/cmd/spire-agent/cli/run/run.go +++ b/cmd/spire-agent/cli/run/run.go @@ -88,9 +88,10 @@ type agentConfig struct { } type sdsConfig struct { - DefaultSVIDName string `hcl:"default_svid_name"` - DefaultBundleName string `hcl:"default_bundle_name"` - DefaultAllBundlesName string `hcl:"default_all_bundles_name"` + DefaultSVIDName string `hcl:"default_svid_name"` + DefaultBundleName string `hcl:"default_bundle_name"` + DefaultAllBundlesName string `hcl:"default_all_bundles_name"` + DisableCustomValidation bool `hcl:"disable_custom_validation"` } type experimentalConfig struct { @@ -409,6 +410,7 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool) if ac.DefaultAllBundlesName == ac.DefaultBundleName { logger.Warn(`The "default_bundle_name" and "default_all_bundles_name" configurables have the same value. "default_all_bundles_name" will be ignored. Please configure distinct values or use the defaults. This will be a configuration error in a future release.`) } + ac.DisableCustomValidation = c.Agent.SDS.DisableCustomValidation err = setupTrustBundle(ac, c) if err != nil { @@ -556,9 +558,10 @@ func defaultConfig() *Config { LogFormat: log.DefaultFormat, SocketPath: common.DefaultSocketPath, SDS: sdsConfig{ - DefaultBundleName: defaultDefaultBundleName, - DefaultSVIDName: defaultDefaultSVIDName, - DefaultAllBundlesName: defaultDefaultAllBundlesName, + DefaultBundleName: defaultDefaultBundleName, + DefaultSVIDName: defaultDefaultSVIDName, + DefaultAllBundlesName: defaultDefaultAllBundlesName, + DisableCustomValidation: false, }, }, } diff --git a/cmd/spire-agent/cli/run/run_test.go b/cmd/spire-agent/cli/run/run_test.go index fb737deaa5..28bbfe1946 100644 --- a/cmd/spire-agent/cli/run/run_test.go +++ b/cmd/spire-agent/cli/run/run_test.go @@ -256,6 +256,26 @@ func TestMergeInput(t *testing.T) { require.Equal(t, "foo", c.Agent.SDS.DefaultAllBundlesName) }, }, + { + msg: "disable_custom_validation should default value of false", + fileInput: func(c *Config) {}, + cliInput: func(ac *agentConfig) {}, + test: func(t *testing.T, c *Config) { + require.Equal(t, false, c.Agent.SDS.DisableCustomValidation) + }, + }, + { + msg: "disable_custom_validation should be configurable by file", + fileInput: func(c *Config) { + c.Agent.SDS = sdsConfig{ + DisableCustomValidation: true, + } + }, + cliInput: func(ac *agentConfig) {}, + test: func(t *testing.T, c *Config) { + require.Equal(t, true, c.Agent.SDS.DisableCustomValidation) + }, + }, { msg: "insecure_bootstrap should be configurable by file", fileInput: func(c *Config) { diff --git a/conf/agent/agent_full.conf b/conf/agent/agent_full.conf index c803aa2e9c..e31dfd68b8 100644 --- a/conf/agent/agent_full.conf +++ b/conf/agent/agent_full.conf @@ -63,6 +63,9 @@ agent { # # all bundles (including federated bundles) with Envoy SDS. Cannot be used with # # Envoy releases prior to 1.18. # # default_all_bundles_name = "ALL" + + # # disable_custom_validation: disable Envoy SDS custom SPIFFE validation. Default: false + # # disable_custom_validation = false # } # allowed_foreign_jwt_claims: set a list of trusted claims to be returned when validating foreign JWTSVIDs diff --git a/doc/spire_agent.md b/doc/spire_agent.md index 68567a209d..cbde718097 100644 --- a/doc/spire_agent.md +++ b/doc/spire_agent.md @@ -75,11 +75,12 @@ Only one of these three options may be set at a time. ### SDS Configuration -| Configuration | Description | Default | -| -------------------------- | ------------------------------------------------------------------------------------------------ | ----------------- | -| `default_svid_name` | The TLS Certificate resource name to use for the default X509-SVID with Envoy SDS | default | -| `default_bundle_name` | The Validation Context resource name to use for the default X.509 bundle with Envoy SDS | ROOTCA | -| `default_all_bundles_name` | The Validation Context resource name to use for all bundles (including federated) with Envoy SDS | ALL | +| Configuration | Description | Default | +| --------------------------- | ------------------------------------------------------------------------------------------------ | ----------------- | +| `default_svid_name` | The TLS Certificate resource name to use for the default X509-SVID with Envoy SDS | default | +| `default_bundle_name` | The Validation Context resource name to use for the default X.509 bundle with Envoy SDS | ROOTCA | +| `default_all_bundles_name` | The Validation Context resource name to use for all bundles (including federated) with Envoy SDS | ALL | +| `disable_custom_validation` | Disable Envoy SDS custom validation | false | ### Profiling Names These are the available profiles that can be set in the `profiling_freq` configuration value: diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index dc20a12434..4be1a70541 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -249,6 +249,7 @@ func (a *Agent) newEndpoints(metrics telemetry.Metrics, mgr manager.Manager, att DefaultSVIDName: a.c.DefaultSVIDName, DefaultBundleName: a.c.DefaultBundleName, DefaultAllBundlesName: a.c.DefaultAllBundlesName, + DisableCustomValidation: a.c.DisableCustomValidation, AllowUnauthenticatedVerifiers: a.c.AllowUnauthenticatedVerifiers, AllowedForeignJWTClaims: a.c.AllowedForeignJWTClaims, TrustDomain: a.c.TrustDomain, diff --git a/pkg/agent/config.go b/pkg/agent/config.go index b3cdc7b8d5..4c8e13a175 100644 --- a/pkg/agent/config.go +++ b/pkg/agent/config.go @@ -28,6 +28,9 @@ type Config struct { // The Validation Context resource name to use for the default X.509 bundle with Envoy SDS DefaultBundleName string + // Disable custom Envoy SDS validator + DisableCustomValidation bool + // The TLS Certificate resource name to use for the default X509-SVID with Envoy SDS DefaultSVIDName string diff --git a/pkg/agent/endpoints/config.go b/pkg/agent/endpoints/config.go index 7b9473f656..f402438e3f 100644 --- a/pkg/agent/endpoints/config.go +++ b/pkg/agent/endpoints/config.go @@ -38,6 +38,9 @@ type Config struct { // The Validation Context resource name to use for the default X.509 bundle with Envoy SDS DefaultBundleName string + // Disable custom Envoy SDS validator + DisableCustomValidation bool + AllowUnauthenticatedVerifiers bool AllowedForeignJWTClaims []string diff --git a/pkg/agent/endpoints/endpoints.go b/pkg/agent/endpoints/endpoints.go index e5229fd6f6..098060ce34 100644 --- a/pkg/agent/endpoints/endpoints.go +++ b/pkg/agent/endpoints/endpoints.go @@ -81,11 +81,12 @@ func New(c Config) *Endpoints { }) sdsv3Server := c.newSDSv3Server(sdsv3.Config{ - Attestor: attestor, - Manager: c.Manager, - DefaultSVIDName: c.DefaultSVIDName, - DefaultBundleName: c.DefaultBundleName, - DefaultAllBundlesName: c.DefaultAllBundlesName, + Attestor: attestor, + Manager: c.Manager, + DefaultSVIDName: c.DefaultSVIDName, + DefaultBundleName: c.DefaultBundleName, + DefaultAllBundlesName: c.DefaultAllBundlesName, + DisableCustomValidation: c.DisableCustomValidation, }) healthServer := c.newHealthServer(healthv1.Config{ diff --git a/pkg/agent/endpoints/endpoints_test.go b/pkg/agent/endpoints/endpoints_test.go index 42981614aa..ba326dbfef 100644 --- a/pkg/agent/endpoints/endpoints_test.go +++ b/pkg/agent/endpoints/endpoints_test.go @@ -173,6 +173,7 @@ func TestEndpoints(t *testing.T) { DefaultSVIDName: "DefaultSVIDName", DefaultBundleName: "DefaultBundleName", DefaultAllBundlesName: "DefaultAllBundlesName", + DisableCustomValidation: true, AllowedForeignJWTClaims: tt.allowedClaims, // Assert the provided config and return a fake Workload API server @@ -206,6 +207,7 @@ func TestEndpoints(t *testing.T) { assert.Equal(t, "DefaultSVIDName", c.DefaultSVIDName) assert.Equal(t, "DefaultBundleName", c.DefaultBundleName) assert.Equal(t, "DefaultAllBundlesName", c.DefaultAllBundlesName) + assert.Equal(t, true, c.DisableCustomValidation) return FakeSDSv3Server{Attestor: attestor} }, diff --git a/pkg/agent/endpoints/sdsv3/handler.go b/pkg/agent/endpoints/sdsv3/handler.go index 997f3e8c1c..6b7226289c 100644 --- a/pkg/agent/endpoints/sdsv3/handler.go +++ b/pkg/agent/endpoints/sdsv3/handler.go @@ -38,11 +38,12 @@ type Manager interface { } type Config struct { - Attestor Attestor - Manager Manager - DefaultAllBundlesName string - DefaultBundleName string - DefaultSVIDName string + Attestor Attestor + Manager Manager + DefaultAllBundlesName string + DefaultBundleName string + DefaultSVIDName string + DisableCustomValidation bool } type Handler struct { @@ -250,7 +251,7 @@ func (h *Handler) buildResponse(versionInfo string, req *discovery_v3.DiscoveryR // Use RootCA as default, but replace with SPIFFE auth when Envoy version is at least v1.18.0 var builder validationContextBuilder - if supportsSPIFFEAuthExtension(req) { + if !h.c.DisableCustomValidation && supportsSPIFFEAuthExtension(req) { builder, err = newSpiffeBuilder(upd.Bundle, upd.FederatedBundles) if err != nil { return nil, err From 4696a3df5345f87db4ed8b5a8a6063e5f7fce444 Mon Sep 17 00:00:00 2001 From: Anton Kaymakchi Date: Tue, 26 Apr 2022 22:23:04 +0100 Subject: [PATCH 3/4] Update base version to pass integration tests --- pkg/common/version/version.go | 2 +- test/integration/suites/upgrade/versions.txt | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/common/version/version.go b/pkg/common/version/version.go index 091762d0ff..d9dd7b6402 100644 --- a/pkg/common/version/version.go +++ b/pkg/common/version/version.go @@ -8,7 +8,7 @@ const ( // IMPORTANT: When updating, make sure to reconcile the versions list that // is part of the upgrade integration test. See // test/integration/suites/upgrade/README.md for details. - Base = "1.1.3" + Base = "1.1.5" ) var ( diff --git a/test/integration/suites/upgrade/versions.txt b/test/integration/suites/upgrade/versions.txt index 59467b0e79..266d1686fa 100644 --- a/test/integration/suites/upgrade/versions.txt +++ b/test/integration/suites/upgrade/versions.txt @@ -4,3 +4,5 @@ 1.1.0 1.1.1 1.1.2 +1.1.3 +1.1.4 From a990696e1e8056298990962e17b6b8a422548f19 Mon Sep 17 00:00:00 2001 From: Anton Kaymakchi Date: Wed, 27 Apr 2022 17:20:18 +0100 Subject: [PATCH 4/4] Add some tests for disabled custom validation --- pkg/agent/endpoints/sdsv3/handler_test.go | 139 ++++++++++++++++++++-- 1 file changed, 129 insertions(+), 10 deletions(-) diff --git a/pkg/agent/endpoints/sdsv3/handler_test.go b/pkg/agent/endpoints/sdsv3/handler_test.go index babcac1f57..78e6c93d95 100644 --- a/pkg/agent/endpoints/sdsv3/handler_test.go +++ b/pkg/agent/endpoints/sdsv3/handler_test.go @@ -14,6 +14,7 @@ import ( discovery_v3 "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3" secret_v3 "github.com/envoyproxy/go-control-plane/envoy/service/secret/v3" envoy_type_v3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" + "github.com/imdario/mergo" "github.com/sirupsen/logrus/hooks/test" "github.com/spiffe/go-spiffe/v2/spiffeid" "github.com/spiffe/spire/pkg/agent/manager/cache" @@ -96,6 +97,19 @@ var ( }, } + tdValidationContext3 = &tls_v3.Secret{ + Name: "ALL", + Type: &tls_v3.Secret_ValidationContext{ + ValidationContext: &tls_v3.CertificateValidationContext{ + TrustedCa: &core_v3.DataSource{ + Specifier: &core_v3.DataSource_InlineBytes{ + InlineBytes: []byte("-----BEGIN CERTIFICATE-----\nQlVORExF\n-----END CERTIFICATE-----\n"), + }, + }, + }, + }, + } + fedBundle = bundleutil.BundleFromRootCA(spiffeid.RequireTrustDomainFromString("otherdomain.test"), &x509.Certificate{ Raw: []byte("FEDBUNDLE"), }) @@ -255,6 +269,7 @@ func TestStreamSecrets(t *testing.T) { for _, tt := range []struct { name string req *discovery_v3.DiscoveryRequest + config Config expectSecrets []*tls_v3.Secret expectCode codes.Code expectMsg string @@ -407,9 +422,57 @@ func TestStreamSecrets(t *testing.T) { expectCode: codes.InvalidArgument, expectMsg: "unable to retrieve all requested identities, missing map[spiffe://domain.test/WHATEVER:true]", }, + { + name: "Disable custom validation", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"default"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + }, + }, + config: Config{ + DefaultSVIDName: "default", + DefaultBundleName: "ROOTCA", + DefaultAllBundlesName: "ALL", + DisableCustomValidation: true, + }, + expectSecrets: []*tls_v3.Secret{workloadTLSCertificate3}, + }, + { + name: "Disable custom validation and set default bundle name to ALL", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"default"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + }, + }, + config: Config{ + DefaultSVIDName: "default", + DefaultBundleName: "ALL", + DefaultAllBundlesName: "ALL", + DisableCustomValidation: true, + }, + expectSecrets: []*tls_v3.Secret{workloadTLSCertificate3}, + }, + { + name: "Disable custom validation and set default bundle name to ALL", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"ALL"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + }, + }, + config: Config{ + DefaultSVIDName: "default", + DefaultBundleName: "ALL", + DefaultAllBundlesName: "ALL", + DisableCustomValidation: true, + }, + expectSecrets: []*tls_v3.Secret{tdValidationContext3}, + }, } { t.Run(tt.name, func(t *testing.T) { - test := setupTest(t) + test := setupTestWithConfig(t, tt.config) defer test.cleanup() stream, err := test.handler.StreamSecrets(context.Background()) @@ -627,6 +690,7 @@ func TestFetchSecrets(t *testing.T) { for _, tt := range []struct { name string req *discovery_v3.DiscoveryRequest + config Config expectSecrets []*tls_v3.Secret expectCode codes.Code expectMsg string @@ -741,9 +805,57 @@ func TestFetchSecrets(t *testing.T) { expectCode: codes.InvalidArgument, expectMsg: `unable to retrieve all requested identities, missing map[spiffe://domain.test/other:true]`, }, + { + name: "Disable custom validation", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"default"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + }, + }, + config: Config{ + DefaultSVIDName: "default", + DefaultBundleName: "ROOTCA", + DefaultAllBundlesName: "ALL", + DisableCustomValidation: true, + }, + expectSecrets: []*tls_v3.Secret{workloadTLSCertificate3}, + }, + { + name: "Disable custom validation and set default bundle name to ALL", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"default"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + }, + }, + config: Config{ + DefaultSVIDName: "default", + DefaultBundleName: "ALL", + DefaultAllBundlesName: "ALL", + DisableCustomValidation: true, + }, + expectSecrets: []*tls_v3.Secret{workloadTLSCertificate3}, + }, + { + name: "Disable custom validation and set default bundle name to ALL", + req: &discovery_v3.DiscoveryRequest{ + ResourceNames: []string{"ALL"}, + Node: &core_v3.Node{ + UserAgentVersionType: userAgentVersionTypeV18, + }, + }, + config: Config{ + DefaultSVIDName: "default", + DefaultBundleName: "ALL", + DefaultAllBundlesName: "ALL", + DisableCustomValidation: true, + }, + expectSecrets: []*tls_v3.Secret{tdValidationContext3}, + }, } { t.Run(tt.name, func(t *testing.T) { - test := setupTest(t) + test := setupTestWithConfig(t, tt.config) defer test.server.Stop() resp, err := test.handler.FetchSecrets(context.Background(), tt.req) @@ -773,15 +885,18 @@ func DeltaSecretsTest(t *testing.T) { require.Nil(t, resp) } -func setupTest(t *testing.T) *handlerTest { +func setupTestWithConfig(t *testing.T, c Config) *handlerTest { manager := NewFakeManager(t) - handler := New(Config{ - Attestor: FakeAttestor(workloadSelectors), - Manager: manager, - DefaultSVIDName: "default", - DefaultBundleName: "ROOTCA", - DefaultAllBundlesName: "ALL", - }) + defaultConfig := Config{ + Manager: manager, + Attestor: FakeAttestor(workloadSelectors), + DefaultSVIDName: "default", + DefaultBundleName: "ROOTCA", + DefaultAllBundlesName: "ALL", + DisableCustomValidation: false, + } + require.NoError(t, mergo.Merge(&c, defaultConfig)) + handler := New(c) received := make(chan struct{}) handler.hooks.received = received @@ -813,6 +928,10 @@ func setupTest(t *testing.T) *handlerTest { return test } +func setupTest(t *testing.T) *handlerTest { + return setupTestWithConfig(t, Config{}) +} + type handlerTest struct { t *testing.T