From a8cc967a02faf99c412bf4837badeed832d683a6 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 9 Oct 2020 12:18:59 -0700 Subject: [PATCH] Merge pull request #8784 from hashicorp/renew-intermediate-primary connect: Enable renewing the intermediate cert in the primary DC --- .changelog/8784.txt | 3 + GNUmakefile | 2 + agent/connect/ca/provider.go | 7 + agent/connect/ca/provider_consul.go | 7 +- agent/connect/ca/provider_vault.go | 17 ++- agent/connect/ca/provider_vault_test.go | 163 ++---------------------- agent/connect/ca/testing.go | 158 +++++++++++++++++++++++ agent/consul/leader_connect.go | 60 ++++++++- agent/consul/leader_connect_test.go | 115 ++++++++++++++++- agent/consul/server.go | 2 +- 10 files changed, 368 insertions(+), 166 deletions(-) create mode 100644 .changelog/8784.txt diff --git a/.changelog/8784.txt b/.changelog/8784.txt new file mode 100644 index 0000000000..1809cfe72b --- /dev/null +++ b/.changelog/8784.txt @@ -0,0 +1,3 @@ +```release-note:bug +connect: Fixed an issue where the Vault intermediate was not renewed in the primary datacenter. +``` \ No newline at end of file diff --git a/GNUmakefile b/GNUmakefile index 9400e46de5..94d8dda667 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -357,6 +357,8 @@ test-connect-ca-providers: ifeq ("$(CIRCLECI)","true") # Run in CI gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report.xml" -- -cover -coverprofile=coverage.txt ./agent/connect/ca +# Run leader tests that require Vault + gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report-leader.xml" -- -cover -coverprofile=coverage-leader.txt -run TestLeader_Vault_ ./agent/consul else # Run locally @echo "Running /agent/connect/ca tests in verbose mode" diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index f9e637c419..17538ee6f4 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -16,6 +16,13 @@ import ( // on servers and CA provider. var ErrRateLimited = errors.New("operation rate limited by CA provider") +// PrimaryIntermediateProviders is a list of CA providers that make use use of an +// intermediate cert in the primary datacenter as well as the secondary. This is used +// when determining whether to run the intermediate renewal routine in the primary. +var PrimaryIntermediateProviders = map[string]struct{}{ + "vault": {}, +} + // ProviderConfig encapsulates all the data Consul passes to `Configure` on a // new provider instance. The provider must treat this as read-only and make // copies of any map or slice if it might modify them internally. diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index 0d677483ef..8ea4af7b46 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -21,14 +21,13 @@ import ( "github.com/hashicorp/go-hclog" ) -const ( - +var ( // NotBefore will be CertificateTimeDriftBuffer in the past to account for // time drift between different servers. CertificateTimeDriftBuffer = time.Minute -) -var ErrNotInitialized = errors.New("provider not initialized") + ErrNotInitialized = errors.New("provider not initialized") +) type ConsulProvider struct { Delegate ConsulProviderStateDelegate diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index d3a9ac7146..c74a2e4609 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "net/http" "strings" + "time" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" @@ -384,6 +385,7 @@ func (v *VaultProvider) GenerateIntermediate() (string, error) { "csr": csr, "use_csr_values": true, "format": "pem_bundle", + "ttl": v.config.IntermediateCertTTL.String(), }) if err != nil { return "", err @@ -456,6 +458,7 @@ func (v *VaultProvider) SignIntermediate(csr *x509.CertificateRequest) (string, "use_csr_values": true, "format": "pem_bundle", "max_path_length": 0, + "ttl": v.config.IntermediateCertTTL.String(), }) if err != nil { return "", err @@ -475,8 +478,20 @@ func (v *VaultProvider) SignIntermediate(csr *x509.CertificateRequest) (string, // CrossSignCA takes a CA certificate and cross-signs it to form a trust chain // back to our active root. func (v *VaultProvider) CrossSignCA(cert *x509.Certificate) (string, error) { + rootPEM, err := v.ActiveRoot() + if err != nil { + return "", err + } + rootCert, err := connect.ParseCert(rootPEM) + if err != nil { + return "", fmt.Errorf("error parsing root cert: %v", err) + } + if rootCert.NotAfter.Before(time.Now()) { + return "", fmt.Errorf("root certificate is expired") + } + var pemBuf bytes.Buffer - err := pem.Encode(&pemBuf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}) + err = pem.Encode(&pemBuf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}) if err != nil { return "", err } diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 3094cb092f..d14ceeefc4 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -5,15 +5,11 @@ import ( "encoding/json" "fmt" "io/ioutil" - "os" - "os/exec" - "sync" "testing" "time" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/sdk/freeport" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/go-hclog" vaultapi "github.com/hashicorp/vault/api" @@ -42,7 +38,7 @@ func TestVaultCAProvider_VaultTLSConfig(t *testing.T) { func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) provider, testVault := testVaultProviderWithConfig(t, false, nil) defer testVault.Stop() @@ -55,7 +51,7 @@ func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) { func TestVaultCAProvider_RenewToken(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) testVault, err := runTestVault(t) require.NoError(t, err) @@ -70,7 +66,7 @@ func TestVaultCAProvider_RenewToken(t *testing.T) { require.NoError(t, err) providerToken := secret.Auth.ClientToken - _, err = createVaultProvider(t, true, testVault.addr, providerToken, nil) + _, err = createVaultProvider(t, true, testVault.Addr, providerToken, nil) require.NoError(t, err) // Check the last renewal time. @@ -92,7 +88,7 @@ func TestVaultCAProvider_RenewToken(t *testing.T) { func TestVaultCAProvider_Bootstrap(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) provider, testVault := testVaultProvider(t) defer testVault.Stop() @@ -153,7 +149,7 @@ func assertCorrectKeyType(t *testing.T, want, certPEM string) { func TestVaultCAProvider_SignLeaf(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) for _, tc := range KeyTestCases { tc := tc @@ -237,7 +233,7 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { func TestVaultCAProvider_CrossSignCA(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) tests := CASigningKeyTypeCases() @@ -292,7 +288,7 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) { func TestVaultProvider_SignIntermediate(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) tests := CASigningKeyTypeCases() @@ -321,7 +317,7 @@ func TestVaultProvider_SignIntermediate(t *testing.T) { func TestVaultProvider_SignIntermediateConsul(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) // primary = Vault, secondary = Consul t.Run("pri=vault,sec=consul", func(t *testing.T) { @@ -382,11 +378,11 @@ func getIntermediateCertTTL(t *testing.T, caConf *structs.CAConfiguration) time. return dur } -func testVaultProvider(t *testing.T) (*VaultProvider, *testVaultServer) { +func testVaultProvider(t *testing.T) (*VaultProvider, *TestVaultServer) { return testVaultProviderWithConfig(t, true, nil) } -func testVaultProviderWithConfig(t *testing.T, isPrimary bool, rawConf map[string]interface{}) (*VaultProvider, *testVaultServer) { +func testVaultProviderWithConfig(t *testing.T, isPrimary bool, rawConf map[string]interface{}) (*VaultProvider, *TestVaultServer) { testVault, err := runTestVault(t) if err != nil { t.Fatalf("err: %v", err) @@ -394,7 +390,7 @@ func testVaultProviderWithConfig(t *testing.T, isPrimary bool, rawConf map[strin testVault.WaitUntilReady(t) - provider, err := createVaultProvider(t, isPrimary, testVault.addr, testVault.rootToken, rawConf) + provider, err := createVaultProvider(t, isPrimary, testVault.Addr, testVault.RootToken, rawConf) if err != nil { testVault.Stop() t.Fatalf("err: %v", err) @@ -443,140 +439,3 @@ func createVaultProvider(t *testing.T, isPrimary bool, addr, token string, rawCo return provider, nil } - -// skipIfVaultNotPresent skips the test if the vault binary is not in PATH. -// -// These tests may be skipped in CI. They are run as part of a separate -// integration test suite. -func skipIfVaultNotPresent(t *testing.T) { - vaultBinaryName := os.Getenv("VAULT_BINARY_NAME") - if vaultBinaryName == "" { - vaultBinaryName = "vault" - } - - path, err := exec.LookPath(vaultBinaryName) - if err != nil || path == "" { - t.Skipf("%q not found on $PATH - download and install to run this test", vaultBinaryName) - } -} - -func runTestVault(t *testing.T) (*testVaultServer, error) { - vaultBinaryName := os.Getenv("VAULT_BINARY_NAME") - if vaultBinaryName == "" { - vaultBinaryName = "vault" - } - - path, err := exec.LookPath(vaultBinaryName) - if err != nil || path == "" { - return nil, fmt.Errorf("%q not found on $PATH", vaultBinaryName) - } - - ports := freeport.MustTake(2) - returnPortsFn := func() { - freeport.Return(ports) - } - - var ( - clientAddr = fmt.Sprintf("127.0.0.1:%d", ports[0]) - clusterAddr = fmt.Sprintf("127.0.0.1:%d", ports[1]) - ) - - const token = "root" - - client, err := vaultapi.NewClient(&vaultapi.Config{ - Address: "http://" + clientAddr, - }) - if err != nil { - returnPortsFn() - return nil, err - } - client.SetToken(token) - - args := []string{ - "server", - "-dev", - "-dev-root-token-id", - token, - "-dev-listen-address", - clientAddr, - "-address", - clusterAddr, - } - - cmd := exec.Command(vaultBinaryName, args...) - cmd.Stdout = ioutil.Discard - cmd.Stderr = ioutil.Discard - if err := cmd.Start(); err != nil { - returnPortsFn() - return nil, err - } - - testVault := &testVaultServer{ - rootToken: token, - addr: "http://" + clientAddr, - cmd: cmd, - client: client, - returnPortsFn: returnPortsFn, - } - t.Cleanup(func() { - testVault.Stop() - }) - return testVault, nil -} - -type testVaultServer struct { - rootToken string - addr string - cmd *exec.Cmd - client *vaultapi.Client - - // returnPortsFn will put the ports claimed for the test back into the - returnPortsFn func() -} - -var printedVaultVersion sync.Once - -func (v *testVaultServer) WaitUntilReady(t *testing.T) { - var version string - retry.Run(t, func(r *retry.R) { - resp, err := v.client.Sys().Health() - if err != nil { - r.Fatalf("err: %v", err) - } - if !resp.Initialized { - r.Fatalf("vault server is not initialized") - } - if resp.Sealed { - r.Fatalf("vault server is sealed") - } - version = resp.Version - }) - printedVaultVersion.Do(func() { - fmt.Fprintf(os.Stderr, "[INFO] agent/connect/ca: testing with vault server version: %s\n", version) - }) -} - -func (v *testVaultServer) Stop() error { - // There was no process - if v.cmd == nil { - return nil - } - - if v.cmd.Process != nil { - if err := v.cmd.Process.Signal(os.Interrupt); err != nil { - return fmt.Errorf("failed to kill vault server: %v", err) - } - } - - // wait for the process to exit to be sure that the data dir can be - // deleted on all platforms. - if err := v.cmd.Wait(); err != nil { - return err - } - - if v.returnPortsFn != nil { - v.returnPortsFn() - } - - return nil -} diff --git a/agent/connect/ca/testing.go b/agent/connect/ca/testing.go index 9a637843e0..25533f8dd4 100644 --- a/agent/connect/ca/testing.go +++ b/agent/connect/ca/testing.go @@ -3,9 +3,15 @@ package ca import ( "fmt" "io/ioutil" + "os" + "os/exec" + "sync" "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/sdk/freeport" + "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/go-hclog" + vaultapi "github.com/hashicorp/vault/api" "github.com/mitchellh/go-testing-interface" ) @@ -76,3 +82,155 @@ func TestConsulProvider(t testing.T, d ConsulProviderStateDelegate) *ConsulProvi provider.SetLogger(logger) return provider } + +// SkipIfVaultNotPresent skips the test if the vault binary is not in PATH. +// +// These tests may be skipped in CI. They are run as part of a separate +// integration test suite. +func SkipIfVaultNotPresent(t testing.T) { + vaultBinaryName := os.Getenv("VAULT_BINARY_NAME") + if vaultBinaryName == "" { + vaultBinaryName = "vault" + } + + path, err := exec.LookPath(vaultBinaryName) + if err != nil || path == "" { + t.Skipf("%q not found on $PATH - download and install to run this test", vaultBinaryName) + } +} + +func NewTestVaultServer(t testing.T) *TestVaultServer { + testVault, err := runTestVault(t) + if err != nil { + t.Fatalf("err: %v", err) + } + + testVault.WaitUntilReady(t) + + return testVault +} + +func runTestVault(t testing.T) (*TestVaultServer, error) { + vaultBinaryName := os.Getenv("VAULT_BINARY_NAME") + if vaultBinaryName == "" { + vaultBinaryName = "vault" + } + + path, err := exec.LookPath(vaultBinaryName) + if err != nil || path == "" { + return nil, fmt.Errorf("%q not found on $PATH", vaultBinaryName) + } + + ports := freeport.MustTake(2) + returnPortsFn := func() { + freeport.Return(ports) + } + + var ( + clientAddr = fmt.Sprintf("127.0.0.1:%d", ports[0]) + clusterAddr = fmt.Sprintf("127.0.0.1:%d", ports[1]) + ) + + const token = "root" + + client, err := vaultapi.NewClient(&vaultapi.Config{ + Address: "http://" + clientAddr, + }) + if err != nil { + returnPortsFn() + return nil, err + } + client.SetToken(token) + + args := []string{ + "server", + "-dev", + "-dev-root-token-id", + token, + "-dev-listen-address", + clientAddr, + "-address", + clusterAddr, + } + + cmd := exec.Command(vaultBinaryName, args...) + cmd.Stdout = ioutil.Discard + cmd.Stderr = ioutil.Discard + if err := cmd.Start(); err != nil { + returnPortsFn() + return nil, err + } + + testVault := &TestVaultServer{ + RootToken: token, + Addr: "http://" + clientAddr, + cmd: cmd, + client: client, + returnPortsFn: returnPortsFn, + } + t.Cleanup(func() { + testVault.Stop() + }) + return testVault, nil +} + +type TestVaultServer struct { + RootToken string + Addr string + cmd *exec.Cmd + client *vaultapi.Client + + // returnPortsFn will put the ports claimed for the test back into the + returnPortsFn func() +} + +var printedVaultVersion sync.Once + +func (v *TestVaultServer) Client() *vaultapi.Client { + return v.client +} + +func (v *TestVaultServer) WaitUntilReady(t testing.T) { + var version string + retry.Run(t, func(r *retry.R) { + resp, err := v.client.Sys().Health() + if err != nil { + r.Fatalf("err: %v", err) + } + if !resp.Initialized { + r.Fatalf("vault server is not initialized") + } + if resp.Sealed { + r.Fatalf("vault server is sealed") + } + version = resp.Version + }) + printedVaultVersion.Do(func() { + fmt.Fprintf(os.Stderr, "[INFO] agent/connect/ca: testing with vault server version: %s\n", version) + }) +} + +func (v *TestVaultServer) Stop() error { + // There was no process + if v.cmd == nil { + return nil + } + + if v.cmd.Process != nil { + if err := v.cmd.Process.Signal(os.Interrupt); err != nil { + return fmt.Errorf("failed to kill vault server: %v", err) + } + } + + // wait for the process to exit to be sure that the data dir can be + // deleted on all platforms. + if err := v.cmd.Wait(); err != nil { + return err + } + + if v.returnPortsFn != nil { + v.returnPortsFn() + } + + return nil +} diff --git a/agent/consul/leader_connect.go b/agent/consul/leader_connect.go index fd621b4f6c..4739bd4254 100644 --- a/agent/consul/leader_connect.go +++ b/agent/consul/leader_connect.go @@ -510,6 +510,32 @@ func (s *Server) persistNewRoot(provider ca.Provider, newActiveRoot *structs.CAR return nil } +// getIntermediateCAPrimary regenerates the intermediate cert in the primary datacenter. +// This is only run for CAs that require an intermediary in the primary DC, such as Vault. +// This function is being called while holding caProviderReconfigurationLock +// which means it must never take that lock itself or call anything that does. +func (s *Server) getIntermediateCAPrimary(provider ca.Provider, newActiveRoot *structs.CARoot) error { + connectLogger := s.loggers.Named(logging.Connect) + // Generate and sign an intermediate cert using the root CA. + intermediatePEM, err := provider.GenerateIntermediate() + if err != nil { + return fmt.Errorf("error generating new intermediate cert: %v", err) + } + + intermediateCert, err := connect.ParseCert(intermediatePEM) + if err != nil { + return fmt.Errorf("error parsing intermediate cert: %v", err) + } + + // Append the new intermediate to our local active root entry. This is + // where the root representations start to diverge. + newActiveRoot.IntermediateCerts = append(newActiveRoot.IntermediateCerts, intermediatePEM) + newActiveRoot.SigningKeyID = connect.EncodeSigningKeyID(intermediateCert.SubjectKeyId) + + connectLogger.Info("generated new intermediate certificate for primary datacenter") + return nil +} + // getIntermediateCASigned is being called while holding caProviderReconfigurationLock // which means it must never take that lock itself or call anything that does. func (s *Server) getIntermediateCASigned(provider ca.Provider, newActiveRoot *structs.CARoot) error { @@ -558,9 +584,9 @@ func (s *Server) startConnectLeader() { if s.config.ConnectEnabled && s.config.Datacenter != s.config.PrimaryDatacenter { s.leaderRoutineManager.Start(secondaryCARootWatchRoutineName, s.secondaryCARootWatch) s.leaderRoutineManager.Start(intentionReplicationRoutineName, s.replicateIntentions) - s.leaderRoutineManager.Start(secondaryCertRenewWatchRoutineName, s.secondaryIntermediateCertRenewalWatch) } + s.leaderRoutineManager.Start(intermediateCertRenewWatchRoutineName, s.intermediateCertRenewalWatch) s.leaderRoutineManager.Start(caRootPruningRoutineName, s.runCARootPruning) } @@ -568,6 +594,7 @@ func (s *Server) startConnectLeader() { func (s *Server) stopConnectLeader() { s.leaderRoutineManager.Stop(secondaryCARootWatchRoutineName) s.leaderRoutineManager.Stop(intentionReplicationRoutineName) + s.leaderRoutineManager.Stop(intermediateCertRenewWatchRoutineName) s.leaderRoutineManager.Stop(caRootPruningRoutineName) // If the provider implements NeedsStop, we call Stop to perform any shutdown actions. @@ -650,11 +677,12 @@ func (s *Server) pruneCARoots() error { return nil } -// secondaryIntermediateCertRenewalWatch checks the intermediate cert for +// intermediateCertRenewalWatch checks the intermediate cert for // expiration. As soon as more than half the time a cert is valid has passed, // it will try to renew it. -func (s *Server) secondaryIntermediateCertRenewalWatch(ctx context.Context) error { +func (s *Server) intermediateCertRenewalWatch(ctx context.Context) error { connectLogger := s.loggers.Named(logging.Connect) + isPrimary := s.config.Datacenter == s.config.PrimaryDatacenter for { select { @@ -670,7 +698,8 @@ func (s *Server) secondaryIntermediateCertRenewalWatch(ctx context.Context) erro // this happens when leadership is being revoked and this go routine will be stopped return nil } - if !s.configuredSecondaryCA() { + // If this isn't the primary, make sure the CA has been initialized. + if !isPrimary && !s.configuredSecondaryCA() { return fmt.Errorf("secondary CA is not yet configured.") } @@ -680,13 +709,26 @@ func (s *Server) secondaryIntermediateCertRenewalWatch(ctx context.Context) erro return err } + // If this is the primary, check if this is a provider that uses an intermediate cert. If + // it isn't, we don't need to check for a renewal. + if isPrimary { + _, config, err := state.CAConfig(nil) + if err != nil { + return err + } + + if _, ok := ca.PrimaryIntermediateProviders[config.Provider]; !ok { + return nil + } + } + activeIntermediate, err := provider.ActiveIntermediate() if err != nil { return err } if activeIntermediate == "" { - return fmt.Errorf("secondary datacenter doesn't have an active intermediate.") + return fmt.Errorf("datacenter doesn't have an active intermediate.") } intermediateCert, err := connect.ParseCert(activeIntermediate) @@ -699,7 +741,11 @@ func (s *Server) secondaryIntermediateCertRenewalWatch(ctx context.Context) erro return nil } - if err := s.getIntermediateCASigned(provider, activeRoot); err != nil { + renewalFunc := s.getIntermediateCAPrimary + if !isPrimary { + renewalFunc = s.getIntermediateCASigned + } + if err := renewalFunc(provider, activeRoot); err != nil { return err } @@ -711,7 +757,7 @@ func (s *Server) secondaryIntermediateCertRenewalWatch(ctx context.Context) erro return nil }, func(err error) { connectLogger.Error("error renewing intermediate certs", - "routine", secondaryCertRenewWatchRoutineName, + "routine", intermediateCertRenewWatchRoutineName, "error", err, ) }) diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index cd720c6e1d..63060d71ae 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -13,7 +13,7 @@ import ( "time" "github.com/hashicorp/consul/agent/connect" - ca "github.com/hashicorp/consul/agent/connect/ca" + "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" tokenStore "github.com/hashicorp/consul/agent/token" @@ -181,6 +181,119 @@ func getCAProviderWithLock(s *Server) (ca.Provider, *structs.CARoot) { return s.getCAProvider() } +func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { + ca.SkipIfVaultNotPresent(t) + + // no parallel execution because we change globals + origInterval := structs.IntermediateCertRenewInterval + origMinTTL := structs.MinLeafCertTTL + origDriftBuffer := ca.CertificateTimeDriftBuffer + defer func() { + structs.IntermediateCertRenewInterval = origInterval + structs.MinLeafCertTTL = origMinTTL + ca.CertificateTimeDriftBuffer = origDriftBuffer + }() + + // Vault backdates certs by 30s by default. + ca.CertificateTimeDriftBuffer = 30 * time.Second + structs.IntermediateCertRenewInterval = time.Millisecond + structs.MinLeafCertTTL = time.Second + require := require.New(t) + + testVault := ca.NewTestVaultServer(t) + defer testVault.Stop() + + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.Build = "1.6.0" + c.PrimaryDatacenter = "dc1" + c.CAConfig = &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": testVault.Addr, + "Token": testVault.RootToken, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + "LeafCertTTL": "1s", + // The retry loop only retries for 7sec max and + // the ttl needs to be below so that it + // triggers definitely. + "IntermediateCertTTL": "5s", + }, + } + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + // Capture the current root. + var originalRoot *structs.CARoot + { + rootList, activeRoot, err := getTestRoots(s1, "dc1") + require.NoError(err) + require.Len(rootList.Roots, 1) + originalRoot = activeRoot + } + + // Get the original intermediate. + waitForActiveCARoot(t, s1, originalRoot) + provider, _ := getCAProviderWithLock(s1) + intermediatePEM, err := provider.ActiveIntermediate() + require.NoError(err) + _, err = connect.ParseCert(intermediatePEM) + require.NoError(err) + + // Wait for dc1's intermediate to be refreshed. + // It is possible that test fails when the blocking query doesn't return. + retry.Run(t, func(r *retry.R) { + provider, _ = getCAProviderWithLock(s1) + newIntermediatePEM, err := provider.ActiveIntermediate() + r.Check(err) + _, err = connect.ParseCert(intermediatePEM) + r.Check(err) + if newIntermediatePEM == intermediatePEM { + r.Fatal("not a renewed intermediate") + } + intermediatePEM = newIntermediatePEM + }) + require.NoError(err) + + // Get the root from dc1 and validate a chain of: + // dc1 leaf -> dc1 intermediate -> dc1 root + provider, caRoot := getCAProviderWithLock(s1) + + // Have the new intermediate sign a leaf cert and make sure the chain is correct. + spiffeService := &connect.SpiffeIDService{ + Host: "node1", + Namespace: "default", + Datacenter: "dc1", + Service: "foo", + } + raw, _ := connect.TestCSR(t, spiffeService) + + leafCsr, err := connect.ParseCSR(raw) + require.NoError(err) + + leafPEM, err := provider.Sign(leafCsr) + require.NoError(err) + + cert, err := connect.ParseCert(leafPEM) + require.NoError(err) + + // Check that the leaf signed by the new intermediate can be verified using the + // returned cert chain (signed intermediate + remote root). + intermediatePool := x509.NewCertPool() + intermediatePool.AppendCertsFromPEM([]byte(intermediatePEM)) + rootPool := x509.NewCertPool() + rootPool.AppendCertsFromPEM([]byte(caRoot.RootCert)) + + _, err = cert.Verify(x509.VerifyOptions{ + Intermediates: intermediatePool, + Roots: rootPool, + }) + require.NoError(err) +} + func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { // no parallel execution because we change globals origInterval := structs.IntermediateCertRenewInterval diff --git a/agent/consul/server.go b/agent/consul/server.go index f411147212..16d78f288c 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -108,7 +108,7 @@ const ( federationStatePruningRoutineName = "federation state pruning" intentionReplicationRoutineName = "intention replication" secondaryCARootWatchRoutineName = "secondary CA roots watch" - secondaryCertRenewWatchRoutineName = "secondary cert renew watch" + intermediateCertRenewWatchRoutineName = "intermediate cert renew watch" ) var (