diff --git a/.changelog/15669.txt b/.changelog/15669.txt new file mode 100644 index 000000000..2f83aebca --- /dev/null +++ b/.changelog/15669.txt @@ -0,0 +1,3 @@ +```release-note:improvement +connect: ensure all vault connect CA tests use limited privilege tokens +``` diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 5801160a5..252b2f876 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -7095,7 +7095,12 @@ func TestAgentConnectCALeafCert_Vault_doesNotChurnLeafCertsAtIdle(t *testing.T) t.Parallel() testVault := ca.NewTestVaultServer(t) - defer testVault.Stop() + + vaultToken := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) a := StartTestAgent(t, TestAgent{Overrides: fmt.Sprintf(` connect { @@ -7108,7 +7113,7 @@ func TestAgentConnectCALeafCert_Vault_doesNotChurnLeafCertsAtIdle(t *testing.T) intermediate_pki_path = "pki-intermediate/" } } - `, testVault.Addr, testVault.RootToken)}) + `, testVault.Addr, vaultToken)}) defer a.Shutdown() testrpc.WaitForTestAgent(t, a.RPC, "dc1") testrpc.WaitForActiveCARoot(t, a.RPC, "dc1", nil) diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index fea9aab45..9845eea74 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -375,6 +375,30 @@ func testCrossSignProviders(t *testing.T, provider1, provider2 Provider) { } } +func testCrossSignProvidersShouldFail(t *testing.T, provider1, provider2 Provider) { + t.Helper() + + // Get the root from the new provider to be cross-signed. + root, err := provider2.GenerateRoot() + require.NoError(t, err) + + newRoot, err := connect.ParseCert(root.PEM) + require.NoError(t, err) + requireNotEncoded(t, newRoot.SubjectKeyId) + requireNotEncoded(t, newRoot.AuthorityKeyId) + + newInterPEM, err := provider2.ActiveIntermediate() + require.NoError(t, err) + newIntermediate, err := connect.ParseCert(newInterPEM) + require.NoError(t, err) + requireNotEncoded(t, newIntermediate.SubjectKeyId) + requireNotEncoded(t, newIntermediate.AuthorityKeyId) + + // Have provider1 cross sign our new root cert. + _, err = provider1.CrossSignCA(newRoot) + require.Error(t, err) +} + func TestConsulProvider_SignIntermediate(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index d42c9037c..8c3bc3e4c 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -885,14 +885,12 @@ func makePathHelper(namespace, path string) string { func (v *VaultProvider) readNamespaced(namespace string, resource string) (*vaultapi.Secret, error) { defer v.setNamespace(namespace)() - result, err := v.client.Logical().Read(resource) - return result, err + return v.client.Logical().Read(resource) } func (v *VaultProvider) writeNamespaced(namespace string, resource string, data map[string]interface{}) (*vaultapi.Secret, error) { defer v.setNamespace(namespace)() - result, err := v.client.Logical().Write(resource, data) - return result, err + return v.client.Logical().Write(resource, data) } func (v *VaultProvider) setNamespace(namespace string) func() { diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 13326150e..3c5a1f44d 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -20,32 +20,36 @@ import ( "github.com/hashicorp/consul/sdk/testutil/retry" ) -const pkiTestPolicy = ` +const pkiTestPolicyBase = ` path "sys/mounts" { capabilities = ["read"] } -path "sys/mounts/pki-root" +path "sys/mounts/%[1]s" { capabilities = ["create", "read", "update", "delete", "list"] } -path "sys/mounts/pki-intermediate" +path "sys/mounts/%[2]s" { capabilities = ["create", "read", "update", "delete", "list"] } -path "sys/mounts/pki-intermediate/tune" +path "sys/mounts/%[2]s/tune" { capabilities = ["update"] } -path "pki-root/*" +path "%[1]s/*" { capabilities = ["create", "read", "update", "delete", "list"] } -path "pki-intermediate/*" +path "%[2]s/*" { capabilities = ["create", "read", "update", "delete", "list"] }` +func pkiTestPolicy(rootPath, intermediatePath string) string { + return fmt.Sprintf(pkiTestPolicyBase, rootPath, intermediatePath) +} + func TestVaultCAProvider_ParseVaultCAConfig(t *testing.T) { cases := map[string]struct { rawConfig map[string]interface{} @@ -157,12 +161,15 @@ func TestVaultCAProvider_Configure(t *testing.T) { testcases := []struct { name string - rawConfig map[string]interface{} + rawConfig map[string]any expectedValue func(t *testing.T, v *VaultProvider) }{ { - name: "DefaultConfig", - rawConfig: map[string]interface{}{}, + name: "DefaultConfig", + rawConfig: map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }, expectedValue: func(t *testing.T, v *VaultProvider) { headers := v.client.Headers() require.Equal(t, "", headers.Get(vaultconst.NamespaceHeaderName)) @@ -171,8 +178,12 @@ func TestVaultCAProvider_Configure(t *testing.T) { }, }, { - name: "TestConfigWithNamespace", - rawConfig: map[string]interface{}{"namespace": "ns1"}, + name: "TestConfigWithNamespace", + rawConfig: map[string]any{ + "namespace": "ns1", + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }, expectedValue: func(t *testing.T, v *VaultProvider) { h := v.client.Headers() @@ -183,8 +194,16 @@ func TestVaultCAProvider_Configure(t *testing.T) { for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - provider, _ := testVaultProviderWithConfig(t, true, testcase.rawConfig) + testVault := NewTestVaultServer(t) + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, true, testVault.Addr, token, testcase.rawConfig) testcase.expectedValue(t, provider) }) } @@ -193,11 +212,23 @@ func TestVaultCAProvider_Configure(t *testing.T) { } func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) { - SkipIfVaultNotPresent(t) - provider, testVault := testVaultProviderWithConfig(t, false, nil) - defer testVault.Stop() + t.Parallel() + + testVault := NewTestVaultServer(t) + + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, false, testVault.Addr, token, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) cert, err := provider.ActiveIntermediate() require.Empty(t, cert) @@ -205,12 +236,11 @@ func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) { } func TestVaultCAProvider_RenewToken(t *testing.T) { - SkipIfVaultNotPresent(t) - testVault, err := runTestVault(t) - require.NoError(t, err) - testVault.WaitUntilReady(t) + t.Parallel() + + testVault := NewTestVaultServer(t) // Create a token with a short TTL to be renewed by the provider. ttl := 1 * time.Second @@ -221,8 +251,10 @@ func TestVaultCAProvider_RenewToken(t *testing.T) { require.NoError(t, err) providerToken := secret.Auth.ClientToken - _, err = createVaultProvider(t, true, testVault.Addr, providerToken, nil) - require.NoError(t, err) + _ = createVaultProvider(t, true, testVault.Addr, providerToken, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) // Check the last renewal time. secret, err = testVault.client.Auth().Token().Lookup(providerToken) @@ -241,12 +273,11 @@ func TestVaultCAProvider_RenewToken(t *testing.T) { } func TestVaultCAProvider_RenewTokenStopWatcherOnConfigure(t *testing.T) { - SkipIfVaultNotPresent(t) - testVault, err := runTestVault(t) - require.NoError(t, err) - testVault.WaitUntilReady(t) + t.Parallel() + + testVault := NewTestVaultServer(t) // Create a token with a short TTL to be renewed by the provider. ttl := 1 * time.Second @@ -257,8 +288,10 @@ func TestVaultCAProvider_RenewTokenStopWatcherOnConfigure(t *testing.T) { require.NoError(t, err) providerToken := secret.Auth.ClientToken - provider, err := createVaultProvider(t, true, testVault.Addr, providerToken, nil) - require.NoError(t, err) + provider := createVaultProvider(t, true, testVault.Addr, providerToken, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) var gotStopped = uint32(0) provider.stopWatcher = func() { @@ -280,63 +313,48 @@ func TestVaultCAProvider_RenewTokenStopWatcherOnConfigure(t *testing.T) { require.Greater(r, lastRenewal, firstRenewal) }) - providerConfig := vaultProviderConfig(t, testVault.Addr, providerToken, nil) + providerConfig := vaultProviderConfig(t, testVault.Addr, providerToken, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) require.NoError(t, provider.Configure(providerConfig)) require.Equal(t, uint32(1), atomic.LoadUint32(&gotStopped)) } func TestVaultCAProvider_Bootstrap(t *testing.T) { - SkipIfVaultNotPresent(t) - providerWDefaultRootCertTtl, testvault1 := testVaultProviderWithConfig(t, true, map[string]interface{}{ - "LeafCertTTL": "1h", - }) - defer testvault1.Stop() - client1 := testvault1.client + t.Parallel() - providerCustomRootCertTtl, testvault2 := testVaultProviderWithConfig(t, true, map[string]interface{}{ - "LeafCertTTL": "1h", - "RootCertTTL": "8761h", - }) - defer testvault2.Stop() - client2 := testvault2.client - - cases := []struct { - certFunc func() (string, error) + type testcase struct { + name string + caConfig map[string]any + certFunc func(*VaultProvider) (string, error) backendPath string rootCaCreation bool - provider *VaultProvider - client *vaultapi.Client expectedRootCertTTL string - }{ - { - certFunc: func() (string, error) { - root, err := providerWDefaultRootCertTtl.GenerateRoot() - return root.PEM, err - }, - backendPath: "pki-root/", - rootCaCreation: true, - client: client1, - provider: providerWDefaultRootCertTtl, - expectedRootCertTTL: structs.DefaultRootCertTTL, - }, - { - certFunc: providerCustomRootCertTtl.ActiveIntermediate, - backendPath: "pki-intermediate/", - rootCaCreation: false, - provider: providerCustomRootCertTtl, - client: client2, - expectedRootCertTTL: "8761h", - }, } - // Verify the root and intermediate certs match the ones in the vault backends - for _, tc := range cases { - provider := tc.provider - client := tc.client - cert, err := tc.certFunc() + run := func(t *testing.T, tc testcase) { + t.Parallel() + + tc.caConfig["RootPKIPath"] = "pki-root/" + tc.caConfig["IntermediatePKIPath"] = "pki-intermediate/" + + testVault := NewTestVaultServer(t) + + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, true, testVault.Addr, token, tc.caConfig) + client := testVault.client + + cert, err := tc.certFunc(provider) require.NoError(t, err) req := client.NewRequest("GET", "/v1/"+tc.backendPath+"ca/pem") resp, err := client.RawRequest(req) @@ -361,6 +379,42 @@ func TestVaultCAProvider_Bootstrap(t *testing.T) { require.WithinDuration(t, expectedNotAfter, parsed.NotAfter, 10*time.Minute, "expected parsed cert ttl to be the same as the value configured") } } + + cases := []testcase{ + { + name: "default-root-cert-ttl", + caConfig: map[string]any{ + "LeafCertTTL": "1h", + }, + certFunc: func(provider *VaultProvider) (string, error) { + root, err := provider.GenerateRoot() + return root.PEM, err + }, + backendPath: "pki-root/", + rootCaCreation: true, + expectedRootCertTTL: structs.DefaultRootCertTTL, + }, + { + name: "custom-root-cert-ttl", + caConfig: map[string]any{ + "LeafCertTTL": "1h", + "RootCertTTL": "8761h", + }, + certFunc: func(provider *VaultProvider) (string, error) { + return provider.ActiveIntermediate() + }, + backendPath: "pki-intermediate/", + rootCaCreation: false, + expectedRootCertTTL: "8761h", + }, + } + + // Verify the root and intermediate certs match the ones in the vault backends + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + run(t, tc) + }) + } } func assertCorrectKeyType(t *testing.T, want, certPEM string) { @@ -380,178 +434,272 @@ func assertCorrectKeyType(t *testing.T, want, certPEM string) { } func TestVaultCAProvider_SignLeaf(t *testing.T) { - SkipIfVaultNotPresent(t) + t.Parallel() + + run := func(t *testing.T, tc KeyTestCase) { + t.Parallel() + + testVault := NewTestVaultServer(t) + + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, true, testVault.Addr, token, map[string]any{ + "LeafCertTTL": "1h", + "PrivateKeyType": tc.KeyType, + "PrivateKeyBits": tc.KeyBits, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) + + spiffeService := &connect.SpiffeIDService{ + Host: "node1", + Namespace: "default", + Datacenter: "dc1", + Service: "foo", + } + + root, err := provider.GenerateRoot() + require.NoError(t, err) + rootPEM := root.PEM + assertCorrectKeyType(t, tc.KeyType, rootPEM) + + intPEM, err := provider.ActiveIntermediate() + require.NoError(t, err) + assertCorrectKeyType(t, tc.KeyType, intPEM) + + // Generate a leaf cert for the service. + var firstSerial uint64 + { + raw, _ := connect.TestCSR(t, spiffeService) + + csr, err := connect.ParseCSR(raw) + require.NoError(t, err) + + cert, err := provider.Sign(csr) + require.NoError(t, err) + + parsed, err := connect.ParseCert(cert) + require.NoError(t, err) + require.Equal(t, parsed.URIs[0], spiffeService.URI()) + firstSerial = parsed.SerialNumber.Uint64() + + // Ensure the cert is valid now and expires within the correct limit. + now := time.Now() + require.True(t, parsed.NotAfter.Sub(now) < time.Hour) + require.True(t, parsed.NotBefore.Before(now)) + + // Make sure we can validate the cert as expected. + require.NoError(t, connect.ValidateLeaf(rootPEM, cert, []string{intPEM})) + requireTrailingNewline(t, cert) + } + + // Generate a new cert for another service and make sure + // the serial number is unique. + spiffeService.Service = "bar" + { + raw, _ := connect.TestCSR(t, spiffeService) + + csr, err := connect.ParseCSR(raw) + require.NoError(t, err) + + cert, err := provider.Sign(csr) + require.NoError(t, err) + + parsed, err := connect.ParseCert(cert) + require.NoError(t, err) + require.Equal(t, parsed.URIs[0], spiffeService.URI()) + require.NotEqual(t, firstSerial, parsed.SerialNumber.Uint64()) + + // Ensure the cert is valid now and expires within the correct limit. + require.True(t, time.Until(parsed.NotAfter) < time.Hour) + require.True(t, parsed.NotBefore.Before(time.Now())) + + // Make sure we can validate the cert as expected. + require.NoError(t, connect.ValidateLeaf(rootPEM, cert, []string{intPEM})) + } + } + for _, tc := range KeyTestCases { - tc := tc t.Run(tc.Desc, func(t *testing.T) { - provider, testVault := testVaultProviderWithConfig(t, true, map[string]interface{}{ - "LeafCertTTL": "1h", - "PrivateKeyType": tc.KeyType, - "PrivateKeyBits": tc.KeyBits, - }) - defer testVault.Stop() - - spiffeService := &connect.SpiffeIDService{ - Host: "node1", - Namespace: "default", - Datacenter: "dc1", - Service: "foo", - } - - root, err := provider.GenerateRoot() - require.NoError(t, err) - rootPEM := root.PEM - assertCorrectKeyType(t, tc.KeyType, rootPEM) - - intPEM, err := provider.ActiveIntermediate() - require.NoError(t, err) - assertCorrectKeyType(t, tc.KeyType, intPEM) - - // Generate a leaf cert for the service. - var firstSerial uint64 - { - raw, _ := connect.TestCSR(t, spiffeService) - - csr, err := connect.ParseCSR(raw) - require.NoError(t, err) - - cert, err := provider.Sign(csr) - require.NoError(t, err) - - parsed, err := connect.ParseCert(cert) - require.NoError(t, err) - require.Equal(t, parsed.URIs[0], spiffeService.URI()) - firstSerial = parsed.SerialNumber.Uint64() - - // Ensure the cert is valid now and expires within the correct limit. - now := time.Now() - require.True(t, parsed.NotAfter.Sub(now) < time.Hour) - require.True(t, parsed.NotBefore.Before(now)) - - // Make sure we can validate the cert as expected. - require.NoError(t, connect.ValidateLeaf(rootPEM, cert, []string{intPEM})) - requireTrailingNewline(t, cert) - } - - // Generate a new cert for another service and make sure - // the serial number is unique. - spiffeService.Service = "bar" - { - raw, _ := connect.TestCSR(t, spiffeService) - - csr, err := connect.ParseCSR(raw) - require.NoError(t, err) - - cert, err := provider.Sign(csr) - require.NoError(t, err) - - parsed, err := connect.ParseCert(cert) - require.NoError(t, err) - require.Equal(t, parsed.URIs[0], spiffeService.URI()) - require.NotEqual(t, firstSerial, parsed.SerialNumber.Uint64()) - - // Ensure the cert is valid now and expires within the correct limit. - require.True(t, time.Until(parsed.NotAfter) < time.Hour) - require.True(t, parsed.NotBefore.Before(time.Now())) - - // Make sure we can validate the cert as expected. - require.NoError(t, connect.ValidateLeaf(rootPEM, cert, []string{intPEM})) - } + run(t, tc) }) } } func TestVaultCAProvider_CrossSignCA(t *testing.T) { - SkipIfVaultNotPresent(t) + t.Parallel() + tests := CASigningKeyTypeCases() + run := func(t *testing.T, tc CASigningKeyTypes, withSudo, expectFailure bool) { + t.Parallel() + + if tc.SigningKeyType != tc.CSRKeyType { + // TODO: uncomment since the bug is closed + // See https://github.com/hashicorp/vault/issues/7709 + t.Skip("Vault doesn't support cross-signing different key types yet.") + } + + testVault1 := NewTestVaultServer(t) + + attr1 := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + WithSudo: withSudo, + } + token1 := CreateVaultTokenWithAttrs(t, testVault1.client, attr1) + + provider1 := createVaultProvider(t, true, testVault1.Addr, token1, map[string]any{ + "LeafCertTTL": "1h", + "PrivateKeyType": tc.SigningKeyType, + "PrivateKeyBits": tc.SigningKeyBits, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) + + testutil.RunStep(t, "init", func(t *testing.T) { + root, err := provider1.GenerateRoot() + require.NoError(t, err) + assertCorrectKeyType(t, tc.SigningKeyType, root.PEM) + + intPEM, err := provider1.ActiveIntermediate() + require.NoError(t, err) + assertCorrectKeyType(t, tc.SigningKeyType, intPEM) + }) + + testVault2 := NewTestVaultServer(t) + + attr2 := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + WithSudo: false, // irrelevant for the new CA provider + } + token2 := CreateVaultTokenWithAttrs(t, testVault2.client, attr2) + + provider2 := createVaultProvider(t, true, testVault2.Addr, token2, map[string]any{ + "LeafCertTTL": "1h", + "PrivateKeyType": tc.CSRKeyType, + "PrivateKeyBits": tc.CSRKeyBits, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) + + testutil.RunStep(t, "swap", func(t *testing.T) { + root, err := provider2.GenerateRoot() + require.NoError(t, err) + assertCorrectKeyType(t, tc.CSRKeyType, root.PEM) + + intPEM, err := provider2.ActiveIntermediate() + require.NoError(t, err) + assertCorrectKeyType(t, tc.CSRKeyType, intPEM) + + if expectFailure { + testCrossSignProvidersShouldFail(t, provider1, provider2) + } else { + testCrossSignProviders(t, provider1, provider2) + } + }) + } + for _, tc := range tests { - tc := tc t.Run(tc.Desc, func(t *testing.T) { - - if tc.SigningKeyType != tc.CSRKeyType { - // See https://github.com/hashicorp/vault/issues/7709 - t.Skip("Vault doesn't support cross-signing different key types yet.") - } - provider1, testVault1 := testVaultProviderWithConfig(t, true, map[string]interface{}{ - "LeafCertTTL": "1h", - "PrivateKeyType": tc.SigningKeyType, - "PrivateKeyBits": tc.SigningKeyBits, + t.Run("without sudo", func(t *testing.T) { + run(t, tc, false, true) }) - defer testVault1.Stop() - - { - root, err := provider1.GenerateRoot() - require.NoError(t, err) - assertCorrectKeyType(t, tc.SigningKeyType, root.PEM) - - intPEM, err := provider1.ActiveIntermediate() - require.NoError(t, err) - assertCorrectKeyType(t, tc.SigningKeyType, intPEM) - } - - provider2, testVault2 := testVaultProviderWithConfig(t, true, map[string]interface{}{ - "LeafCertTTL": "1h", - "PrivateKeyType": tc.CSRKeyType, - "PrivateKeyBits": tc.CSRKeyBits, + t.Run("with sudo", func(t *testing.T) { + run(t, tc, true, false) }) - defer testVault2.Stop() - - { - root, err := provider2.GenerateRoot() - require.NoError(t, err) - assertCorrectKeyType(t, tc.CSRKeyType, root.PEM) - - intPEM, err := provider2.ActiveIntermediate() - require.NoError(t, err) - assertCorrectKeyType(t, tc.CSRKeyType, intPEM) - } - - testCrossSignProviders(t, provider1, provider2) }) } } func TestVaultProvider_SignIntermediate(t *testing.T) { - SkipIfVaultNotPresent(t) + t.Parallel() + tests := CASigningKeyTypeCases() + run := func(t *testing.T, tc CASigningKeyTypes) { + t.Parallel() + + testVault1 := NewTestVaultServer(t) + + attr1 := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token1 := CreateVaultTokenWithAttrs(t, testVault1.client, attr1) + + provider1 := createVaultProvider(t, true, testVault1.Addr, token1, map[string]any{ + "LeafCertTTL": "1h", + "PrivateKeyType": tc.SigningKeyType, + "PrivateKeyBits": tc.SigningKeyBits, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) + + testVault2 := NewTestVaultServer(t) + + attr2 := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token2 := CreateVaultTokenWithAttrs(t, testVault2.client, attr2) + + provider2 := createVaultProvider(t, false, testVault2.Addr, token2, map[string]any{ + "LeafCertTTL": "1h", + "PrivateKeyType": tc.CSRKeyType, + "PrivateKeyBits": tc.CSRKeyBits, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) + + testSignIntermediateCrossDC(t, provider1, provider2) + } + for _, tc := range tests { - tc := tc t.Run(tc.Desc, func(t *testing.T) { - provider1, testVault1 := testVaultProviderWithConfig(t, true, map[string]interface{}{ - "LeafCertTTL": "1h", - "PrivateKeyType": tc.SigningKeyType, - "PrivateKeyBits": tc.SigningKeyBits, - }) - defer testVault1.Stop() - - provider2, testVault2 := testVaultProviderWithConfig(t, false, map[string]interface{}{ - "LeafCertTTL": "1h", - "PrivateKeyType": tc.CSRKeyType, - "PrivateKeyBits": tc.CSRKeyBits, - }) - defer testVault2.Stop() - - testSignIntermediateCrossDC(t, provider1, provider2) + run(t, tc) }) } } func TestVaultProvider_SignIntermediateConsul(t *testing.T) { - SkipIfVaultNotPresent(t) + t.Parallel() + // primary = Vault, secondary = Consul t.Run("pri=vault,sec=consul", func(t *testing.T) { - provider1, testVault1 := testVaultProviderWithConfig(t, true, nil) - defer testVault1.Stop() + t.Parallel() + + testVault1 := NewTestVaultServer(t) + + attr1 := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token1 := CreateVaultTokenWithAttrs(t, testVault1.client, attr1) + + provider1 := createVaultProvider(t, true, testVault1.Addr, token1, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) conf := testConsulCAConfig() delegate := newMockDelegate(t, conf) @@ -566,6 +714,8 @@ func TestVaultProvider_SignIntermediateConsul(t *testing.T) { // primary = Consul, secondary = Vault t.Run("pri=consul,sec=vault", func(t *testing.T) { + t.Parallel() + conf := testConsulCAConfig() delegate := newMockDelegate(t, conf) provider1 := TestConsulProvider(t, delegate) @@ -578,29 +728,46 @@ func TestVaultProvider_SignIntermediateConsul(t *testing.T) { intermediateCertTTL := getIntermediateCertTTL(t, conf) leafCertTTL := intermediateCertTTL - 4*time.Hour - overrideConf := map[string]interface{}{ - "LeafCertTTL": []uint8(leafCertTTL.String()), + overrideConf := map[string]any{ + "LeafCertTTL": []uint8(leafCertTTL.String()), + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", } - provider2, testVault2 := testVaultProviderWithConfig(t, false, overrideConf) - defer testVault2.Stop() + testVault2 := NewTestVaultServer(t) + + attr2 := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token2 := CreateVaultTokenWithAttrs(t, testVault2.client, attr2) + + provider2 := createVaultProvider(t, false, testVault2.Addr, token2, overrideConf) testSignIntermediateCrossDC(t, provider1, provider2) }) } func TestVaultProvider_Cleanup(t *testing.T) { - SkipIfVaultNotPresent(t) - testVault, err := runTestVault(t) - require.NoError(t, err) + t.Parallel() - testVault.WaitUntilReady(t) + testVault := NewTestVaultServer(t) t.Run("provider-change", func(t *testing.T) { - provider, err := createVaultProvider(t, true, testVault.Addr, testVault.RootToken, nil) - require.NoError(t, err) + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, true, testVault.Addr, token, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) // ensure that the intermediate PKI mount exists mounts, err := provider.client.Sys().ListMounts() @@ -617,8 +784,17 @@ func TestVaultProvider_Cleanup(t *testing.T) { }) t.Run("pki-path-change", func(t *testing.T) { - provider, err := createVaultProvider(t, true, testVault.Addr, testVault.RootToken, nil) - require.NoError(t, err) + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, true, testVault.Addr, token, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) // ensure that the intermediate PKI mount exists mounts, err := provider.client.Sys().ListMounts() @@ -626,12 +802,12 @@ func TestVaultProvider_Cleanup(t *testing.T) { require.Contains(t, mounts, provider.config.IntermediatePKIPath) // call cleanup with an intermediate pki path change - this should cause removal of the mount - require.NoError(t, provider.Cleanup(false, map[string]interface{}{ + require.NoError(t, provider.Cleanup(false, map[string]any{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": token, "RootPKIPath": "pki-root/", // - "IntermediatePKIPath": "pki-intermediate2/", + "IntermediatePKIPath": "pki-intermediate-2/", // Tests duration parsing after msgpack type mangling during raft apply. "LeafCertTTL": []uint8("72h"), })) @@ -643,8 +819,17 @@ func TestVaultProvider_Cleanup(t *testing.T) { }) t.Run("pki-path-unchanged", func(t *testing.T) { - provider, err := createVaultProvider(t, true, testVault.Addr, testVault.RootToken, nil) - require.NoError(t, err) + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, true, testVault.Addr, token, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) // ensure that the intermediate PKI mount exists mounts, err := provider.client.Sys().ListMounts() @@ -652,9 +837,9 @@ func TestVaultProvider_Cleanup(t *testing.T) { require.Contains(t, mounts, provider.config.IntermediatePKIPath) // call cleanup with no config changes - this should not cause removal of the intermediate pki path - require.NoError(t, provider.Cleanup(false, map[string]interface{}{ + require.NoError(t, provider.Cleanup(false, map[string]any{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": token, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", // Tests duration parsing after msgpack type mangling during raft apply. @@ -669,7 +854,6 @@ func TestVaultProvider_Cleanup(t *testing.T) { } func TestVaultProvider_ConfigureWithAuthMethod(t *testing.T) { - SkipIfVaultNotPresent(t) cases := []struct { @@ -723,12 +907,12 @@ func TestVaultProvider_ConfigureWithAuthMethod(t *testing.T) { err := testVault.Client().Sys().EnableAuthWithOptions(c.authMethodType, &vaultapi.EnableAuthOptions{Type: c.authMethodType}) require.NoError(t, err) - err = testVault.Client().Sys().PutPolicy("pki", pkiTestPolicy) + err = testVault.Client().Sys().PutPolicy("pki", pkiTestPolicy("pki-root", "pki-intermediate")) require.NoError(t, err) authMethodConf := c.configureAuthMethodFunc(t, testVault.Client()) - conf := map[string]interface{}{ + conf := map[string]any{ "Address": testVault.Addr, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", @@ -752,12 +936,13 @@ func TestVaultProvider_ConfigureWithAuthMethod(t *testing.T) { } func TestVaultProvider_RotateAuthMethodToken(t *testing.T) { - SkipIfVaultNotPresent(t) + t.Parallel() + testVault := NewTestVaultServer(t) - err := testVault.Client().Sys().PutPolicy("pki", pkiTestPolicy) + err := testVault.Client().Sys().PutPolicy("pki", pkiTestPolicy("pki-root", "pki-intermediate")) require.NoError(t, err) err = testVault.Client().Sys().EnableAuthWithOptions("approle", &vaultapi.EnableAuthOptions{Type: "approle"}) @@ -778,7 +963,7 @@ func TestVaultProvider_RotateAuthMethodToken(t *testing.T) { require.NoError(t, err) secretID := resp.Data["secret_id"] - conf := map[string]interface{}{ + conf := map[string]any{ "Address": testVault.Addr, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", @@ -814,6 +999,8 @@ func TestVaultProvider_RotateAuthMethodToken(t *testing.T) { func TestVaultProvider_ReconfigureIntermediateTTL(t *testing.T) { SkipIfVaultNotPresent(t) + t.Parallel() + // Set up a standard policy without any sys/mounts/pki-intermediate/tune permissions. policy := ` path "sys/mounts" @@ -849,7 +1036,7 @@ func TestVaultProvider_ReconfigureIntermediateTTL(t *testing.T) { providerToken := secret.Auth.ClientToken makeProviderConfWithTTL := func(ttl string) ProviderConfig { - conf := map[string]interface{}{ + conf := map[string]any{ "Address": testVault.Addr, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", @@ -906,7 +1093,21 @@ func TestVaultProvider_ReconfigureIntermediateTTL(t *testing.T) { func TestVaultCAProvider_GenerateIntermediate(t *testing.T) { SkipIfVaultNotPresent(t) - provider, _ := testVaultProviderWithConfig(t, true, nil) + t.Parallel() + + testVault := NewTestVaultServer(t) + + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, true, testVault.Addr, token, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) orig, err := provider.ActiveIntermediate() require.NoError(t, err) @@ -927,6 +1128,8 @@ func TestVaultCAProvider_GenerateIntermediate(t *testing.T) { func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) { SkipIfVaultNotPresent(t) + t.Parallel() + // Primary DC will be a consul provider. conf := testConsulCAConfig() delegate := newMockDelegate(t, conf) @@ -940,8 +1143,19 @@ func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) { intermediateCertTTL := getIntermediateCertTTL(t, conf) leafCertTTL := intermediateCertTTL - 4*time.Hour - provider, _ := testVaultProviderWithConfig(t, false, map[string]any{ - "LeafCertTTL": []uint8(leafCertTTL.String()), + testVault := NewTestVaultServer(t) + + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) + + provider := createVaultProvider(t, false, testVault.Addr, token, map[string]any{ + "LeafCertTTL": []uint8(leafCertTTL.String()), + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", }) var origIntermediate string @@ -995,7 +1209,6 @@ func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) { } func TestVaultCAProvider_VaultManaged(t *testing.T) { - SkipIfVaultNotPresent(t) const vaultManagedPKIPolicy = ` @@ -1020,12 +1233,7 @@ path "auth/token/lookup-self" { } ` - testVault, err := runTestVault(t) - if err != nil { - t.Fatalf("err: %v", err) - } - - testVault.WaitUntilReady(t) + testVault := NewTestVaultServer(t) client := testVault.Client() @@ -1039,7 +1247,7 @@ path "auth/token/lookup-self" { MaxLeaseTTL: "12m", }, })) - _, err = client.Logical().Write("pki-root/root/generate/internal", map[string]interface{}{ + _, err := client.Logical().Write("pki-root/root/generate/internal", map[string]interface{}{ "common_name": "testconsul", }) require.NoError(t, err) @@ -1063,20 +1271,17 @@ path "auth/token/lookup-self" { providerToken := secret.Auth.ClientToken // We want to test the provider.Configure() step - _, err = createVaultProvider(t, true, testVault.Addr, providerToken, nil) - require.NoError(t, err) + + _ = createVaultProvider(t, true, testVault.Addr, providerToken, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) } func TestVaultCAProvider_ConsulManaged(t *testing.T) { - SkipIfVaultNotPresent(t) - testVault, err := runTestVault(t) - if err != nil { - t.Fatalf("err: %v", err) - } - - testVault.WaitUntilReady(t) + testVault := NewTestVaultServer(t) client := testVault.Client() @@ -1086,7 +1291,7 @@ func TestVaultCAProvider_ConsulManaged(t *testing.T) { // be responsible for mounting root and intermediate PKI // Generate a policy and token for the VaultProvider to use - require.NoError(t, client.Sys().PutPolicy("consul-ca", pkiTestPolicy)) + require.NoError(t, client.Sys().PutPolicy("consul-ca", pkiTestPolicy("pki-root", "pki-intermediate"))) tcr := &vaultapi.TokenCreateRequest{ Policies: []string{"consul-ca"}, } @@ -1095,8 +1300,11 @@ func TestVaultCAProvider_ConsulManaged(t *testing.T) { providerToken := secret.Auth.ClientToken // We want to test the provider.Configure() step - _, err = createVaultProvider(t, true, testVault.Addr, providerToken, nil) - require.NoError(t, err) + + _ = createVaultProvider(t, true, testVault.Addr, providerToken, map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }) } func getIntermediateCertTTL(t *testing.T, caConf *structs.CAConfiguration) time.Duration { @@ -1118,23 +1326,8 @@ func getIntermediateCertTTL(t *testing.T, caConf *structs.CAConfiguration) time. return dur } -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) - } - - testVault.WaitUntilReady(t) - - provider, err := createVaultProvider(t, isPrimary, testVault.Addr, testVault.RootToken, rawConf) - if err != nil { - testVault.Stop() - t.Fatalf("err: %v", err) - } - return provider, testVault -} - -func createVaultProvider(t *testing.T, isPrimary bool, addr, token string, rawConf map[string]interface{}) (*VaultProvider, error) { +func createVaultProvider(t *testing.T, isPrimary bool, addr, token string, rawConf map[string]any) *VaultProvider { + t.Helper() cfg := vaultProviderConfig(t, addr, token, rawConf) provider := NewVaultProvider(hclog.New(nil)) @@ -1153,18 +1346,34 @@ func createVaultProvider(t *testing.T, isPrimary bool, addr, token string, rawCo require.NoError(t, err) } - return provider, nil + return provider } -func vaultProviderConfig(t *testing.T, addr, token string, rawConf map[string]interface{}) ProviderConfig { - conf := map[string]interface{}{ - "Address": addr, - "Token": token, - "RootPKIPath": "pki-root/", - "IntermediatePKIPath": "pki-intermediate/", +func vaultProviderConfig(t *testing.T, addr, token string, rawConf map[string]any) ProviderConfig { + t.Helper() + require.NotEmpty(t, rawConf, "config map is required with at least %q and %q set", + "RootPKIPath", + "IntermediatePKIPath") + + conf := map[string]any{ + "Address": addr, + "Token": token, // Tests duration parsing after msgpack type mangling during raft apply. "LeafCertTTL": []uint8("72h"), } + + hasRequired := false + if rawConf != nil { + _, ok1 := rawConf["RootPKIPath"] + _, ok2 := rawConf["IntermediatePKIPath"] + hasRequired = ok1 && ok2 + } + if !hasRequired { + t.Fatalf("The caller must provide both %q and %q config settings to avoid an incidental collision", + "RootPKIPath", + "IntermediatePKIPath") + } + for k, v := range rawConf { conf[k] = v } diff --git a/agent/connect/ca/testing.go b/agent/connect/ca/testing.go index 5bd8c9908..ee5bb473c 100644 --- a/agent/connect/ca/testing.go +++ b/agent/connect/ca/testing.go @@ -10,8 +10,10 @@ import ( "sync" "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-uuid" vaultapi "github.com/hashicorp/vault/api" "github.com/mitchellh/go-testing-interface" + "github.com/stretchr/testify/require" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/sdk/freeport" @@ -27,11 +29,7 @@ import ( // these types of old CA key. // - SignIntermediate muse bt able to sign all the types of secondary // intermediate CA key with all these types of primary CA key -var KeyTestCases = []struct { - Desc string - KeyType string - KeyBits int -}{ +var KeyTestCases = []KeyTestCase{ { Desc: "Default Key Type (EC 256)", KeyType: connect.DefaultPrivateKeyType, @@ -44,6 +42,12 @@ var KeyTestCases = []struct { }, } +type KeyTestCase struct { + Desc string + KeyType string + KeyBits int +} + // CASigningKeyTypes is a struct with params for tests that sign one CA CSR with // another CA key. type CASigningKeyTypes struct { @@ -106,17 +110,6 @@ func SkipIfVaultNotPresent(t testing.T) { } 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" @@ -124,7 +117,7 @@ func runTestVault(t testing.T) (*TestVaultServer, error) { path, err := exec.LookPath(vaultBinaryName) if err != nil || path == "" { - return nil, fmt.Errorf("%q not found on $PATH", vaultBinaryName) + t.Fatalf("%q not found on $PATH", vaultBinaryName) } ports := freeport.GetN(t, 2) @@ -138,9 +131,7 @@ func runTestVault(t testing.T) (*TestVaultServer, error) { client, err := vaultapi.NewClient(&vaultapi.Config{ Address: "http://" + clientAddr, }) - if err != nil { - return nil, err - } + require.NoError(t, err) client.SetToken(token) args := []string{ @@ -157,9 +148,7 @@ func runTestVault(t testing.T) (*TestVaultServer, error) { cmd := exec.Command(vaultBinaryName, args...) cmd.Stdout = io.Discard cmd.Stderr = io.Discard - if err := cmd.Start(); err != nil { - return nil, err - } + require.NoError(t, cmd.Start()) testVault := &TestVaultServer{ RootToken: token, @@ -173,7 +162,9 @@ func runTestVault(t testing.T) (*TestVaultServer, error) { } }) - return testVault, nil + testVault.WaitUntilReady(t) + + return testVault } type TestVaultServer struct { @@ -224,6 +215,9 @@ func (v *TestVaultServer) Stop() error { // 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 { + if strings.Contains(err.Error(), "exec: Wait was already called") { + return nil + } return err } return nil @@ -238,3 +232,127 @@ func requireTrailingNewline(t testing.T, leafPEM string) { t.Fatalf("cert do not end with a new line") } } + +// The zero value implies unprivileged. +type VaultTokenAttributes struct { + RootPath, IntermediatePath string + + ConsulManaged bool + VaultManaged bool + WithSudo bool + + CustomRules string +} + +func (a *VaultTokenAttributes) DisplayName() string { + switch { + case a == nil: + return "unprivileged" + case a.CustomRules != "": + return "custom" + case a.ConsulManaged: + return "consul-managed" + case a.VaultManaged: + return "vault-managed" + default: + return "unprivileged" + } +} + +func (a *VaultTokenAttributes) Rules(t testing.T) string { + switch { + case a == nil: + return "" + + case a.CustomRules != "": + return a.CustomRules + + case a.RootPath == "": + t.Fatal("missing required RootPath") + return "" // dead code + + case a.IntermediatePath == "": + t.Fatal("missing required IntermediatePath") + return "" // dead code + + case a.ConsulManaged: + // Consul Managed PKI Mounts + rules := fmt.Sprintf(` +path "sys/mounts" { + capabilities = [ "read" ] +} + +path "sys/mounts/%[1]s" { + capabilities = [ "create", "read", "update", "delete", "list" ] +} + +path "sys/mounts/%[2]s" { + capabilities = [ "create", "read", "update", "delete", "list" ] +} + +# Needed for Consul 1.11+ +path "sys/mounts/%[2]s/tune" { + capabilities = [ "update" ] +} + +# vault token renewal +path "auth/token/renew-self" { + capabilities = [ "update" ] +} +path "auth/token/lookup-self" { + capabilities = [ "read" ] +} + +path "%[1]s/*" { + capabilities = [ "create", "read", "update", "delete", "list" ] +} + +path "%[2]s/*" { + capabilities = [ "create", "read", "update", "delete", "list" ] +} +`, a.RootPath, a.IntermediatePath) + + if a.WithSudo { + rules += fmt.Sprintf(` + +path "%[1]s/root/sign-self-issued" { + capabilities = [ "sudo", "update" ] +} +`, a.RootPath) + } + + return rules + + case a.VaultManaged: + // Vault-managed PKI root. + t.Fatal("TODO: implement this and use it in tests") + return "" + + default: + // zero value + return "" + } +} + +func CreateVaultTokenWithAttrs(t testing.T, client *vaultapi.Client, attr *VaultTokenAttributes) string { + policyName, err := uuid.GenerateUUID() + require.NoError(t, err) + + rules := attr.Rules(t) + + token := createVaultTokenAndPolicy(t, client, policyName, rules) + // t.Logf("created vault token with scope %q: %s", attr.DisplayName(), token) + return token +} + +func createVaultTokenAndPolicy(t testing.T, client *vaultapi.Client, policyName, policyRules string) string { + require.NoError(t, client.Sys().PutPolicy(policyName, policyRules)) + + renew := true + tok, err := client.Auth().Token().Create(&vaultapi.TokenCreateRequest{ + Policies: []string{policyName}, + Renewable: &renew, + }) + require.NoError(t, err) + return tok.Auth.ClientToken +} diff --git a/agent/consul/acl_server.go b/agent/consul/acl_server.go index 6fdfa1dc8..cc7e3a766 100644 --- a/agent/consul/acl_server.go +++ b/agent/consul/acl_server.go @@ -87,7 +87,7 @@ func (s *Server) checkBindingRuleUUID(id string) (bool, error) { } func (s *Server) InPrimaryDatacenter() bool { - return s.config.PrimaryDatacenter == "" || s.config.Datacenter == s.config.PrimaryDatacenter + return s.config.InPrimaryDatacenter() } func (s *Server) LocalTokensEnabled() bool { diff --git a/agent/consul/config.go b/agent/consul/config.go index 85cddbb2a..72bd9b396 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -415,6 +415,10 @@ type Config struct { *EnterpriseConfig } +func (c *Config) InPrimaryDatacenter() bool { + return c.PrimaryDatacenter == "" || c.Datacenter == c.PrimaryDatacenter +} + // CheckProtocolVersion validates the protocol version. func (c *Config) CheckProtocolVersion() error { if c.ProtocolVersion < ProtocolVersionMin { diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go index ee7d34d4d..8e0cd6d37 100644 --- a/agent/consul/connect_ca_endpoint_test.go +++ b/agent/consul/connect_ca_endpoint_test.go @@ -9,11 +9,10 @@ import ( "testing" "time" + msgpackrpc "github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - msgpackrpc "github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/connect" ca "github.com/hashicorp/consul/agent/connect/ca" @@ -560,10 +559,23 @@ func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) { testVault := ca.NewTestVaultServer(t) - newConfig := func(keyType string, keyBits int) map[string]interface{} { - return map[string]interface{}{ + token1 := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-primary", + ConsulManaged: true, + WithSudo: true, + }) + + token2 := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) + + newConfig := func(token string, keyType string, keyBits int) map[string]any { + return map[string]any{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": token, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", "PrivateKeyType": keyType, @@ -574,7 +586,7 @@ func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) { _, s1 := testServerWithConfig(t, func(c *Config) { c.CAConfig = &structs.CAConfiguration{ Provider: "vault", - Config: newConfig(connect.DefaultPrivateKeyType, connect.DefaultPrivateKeyBits), + Config: newConfig(token1, connect.DefaultPrivateKeyType, connect.DefaultPrivateKeyBits), } }) testrpc.WaitForTestAgent(t, s1.RPC, "dc1") @@ -592,7 +604,7 @@ func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) { configFn: func() *structs.CAConfiguration { return &structs.CAConfiguration{ Provider: "vault", - Config: newConfig("rsa", 4096), + Config: newConfig(token2, "rsa", 4096), ForceWithoutCrossSigning: true, } }, @@ -602,7 +614,7 @@ func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) { configFn: func() *structs.CAConfiguration { return &structs.CAConfiguration{ Provider: "vault", - Config: newConfig("rsa", 2048), + Config: newConfig(token2, "rsa", 2048), ForceWithoutCrossSigning: true, } }, @@ -613,7 +625,7 @@ func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) { configFn: func() *structs.CAConfiguration { return &structs.CAConfiguration{ Provider: "vault", - Config: newConfig("ec", 256), + Config: newConfig(token2, "ec", 256), ForceWithoutCrossSigning: true, } }, @@ -624,7 +636,7 @@ func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) { configFn: func() *structs.CAConfiguration { return &structs.CAConfiguration{ Provider: "vault", - Config: newConfig("rsa", 4096), + Config: newConfig(token2, "rsa", 4096), ForceWithoutCrossSigning: true, } }, @@ -632,7 +644,7 @@ func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) { } for _, tc := range testSteps { - t.Run(tc.name, func(t *testing.T) { + testutil.RunStep(t, tc.name, func(t *testing.T) { args := &structs.CARequest{ Datacenter: "dc1", Config: tc.configFn(), diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 848976da1..008289c94 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -16,14 +16,13 @@ import ( "golang.org/x/time/rate" "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/lib/semaphore" - "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/lib/routine" + "github.com/hashicorp/consul/lib/semaphore" ) type caState string @@ -1069,7 +1068,7 @@ func (c *CAManager) secondaryRequestNewSigningCert(provider ca.Provider, newActi } if err := setLeafSigningCert(newActiveRoot, intermediatePEM); err != nil { - return err + return fmt.Errorf("Failed to set the leaf signing cert to the intermediate: %w", err) } c.logger.Info("received new intermediate certificate from primary datacenter") @@ -1117,15 +1116,13 @@ func pruneExpiredIntermediates(caRoot *structs.CARoot) error { // runRenewIntermediate periodically attempts to renew the intermediate cert. func (c *CAManager) runRenewIntermediate(ctx context.Context) error { - isPrimary := c.serverConf.Datacenter == c.serverConf.PrimaryDatacenter - for { select { case <-ctx.Done(): return nil case <-time.After(structs.IntermediateCertRenewInterval): retryLoopBackoffAbortOnSuccess(ctx, func() error { - return c.RenewIntermediate(ctx, isPrimary) + return c.RenewIntermediate(ctx) }, func(err error) { c.logger.Error("error renewing intermediate certs", "routine", intermediateCertRenewWatchRoutineName, @@ -1139,7 +1136,15 @@ func (c *CAManager) runRenewIntermediate(ctx context.Context) error { // RenewIntermediate checks the intermediate cert for // expiration. If more than half the time a cert is valid has passed, // it will try to renew it. -func (c *CAManager) RenewIntermediate(ctx context.Context, isPrimary bool) error { +func (c *CAManager) RenewIntermediate(ctx context.Context) error { + return c.renewIntermediate(ctx, false) +} + +func (c *CAManager) renewIntermediateNow(ctx context.Context) error { + return c.renewIntermediate(ctx, true) +} + +func (c *CAManager) renewIntermediate(ctx context.Context, forceNow bool) error { // Grab the 'lock' right away so the provider/config can't be changed out while we check // the intermediate. if _, err := c.setState(caStateRenewIntermediate, true); err != nil { @@ -1147,6 +1152,8 @@ func (c *CAManager) RenewIntermediate(ctx context.Context, isPrimary bool) error } defer c.setState(caStateInitialized, false) + isPrimary := c.serverConf.InPrimaryDatacenter() + provider, _ := c.getCAProvider() if provider == nil { // this happens when leadership is being revoked and this go routine will be stopped @@ -1184,8 +1191,10 @@ func (c *CAManager) RenewIntermediate(ctx context.Context, isPrimary bool) error return fmt.Errorf("error parsing active intermediate cert: %v", err) } - if lessThanHalfTimePassed(c.timeNow(), intermediateCert.NotBefore, intermediateCert.NotAfter) { - return nil + if !forceNow { + if lessThanHalfTimePassed(c.timeNow(), intermediateCert.NotBefore, intermediateCert.NotAfter) { + return nil + } } // Enough time has passed, go ahead with getting a new intermediate. @@ -1193,19 +1202,27 @@ func (c *CAManager) RenewIntermediate(ctx context.Context, isPrimary bool) error if !isPrimary { renewalFunc = c.secondaryRequestNewSigningCert } - errCh := make(chan error, 1) - go func() { - errCh <- renewalFunc(provider, activeRoot) - }() - // Wait for the renewal func to return or for the context to be canceled. - select { - case <-ctx.Done(): - return ctx.Err() - case err := <-errCh: + if forceNow { + err := renewalFunc(provider, activeRoot) if err != nil { return err } + } else { + errCh := make(chan error, 1) + go func() { + errCh <- renewalFunc(provider, activeRoot) + }() + + // Wait for the renewal func to return or for the context to be canceled. + select { + case <-ctx.Done(): + return ctx.Err() + case err := <-errCh: + if err != nil { + return err + } + } } if err := c.persistNewRootAndConfig(provider, activeRoot, nil); err != nil { diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index adc065627..ec23cc543 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -16,14 +16,13 @@ import ( "testing" "time" + msgpackrpc "github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc" + "github.com/hashicorp/consul-net-rpc/net/rpc" vaultapi "github.com/hashicorp/vault/api" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc" - msgpackrpc "github.com/hashicorp/consul-net-rpc/net-rpc-msgpackrpc" - "github.com/hashicorp/consul-net-rpc/net/rpc" - "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/connect" ca "github.com/hashicorp/consul/agent/connect/ca" @@ -48,12 +47,24 @@ func TestCAManager_Initialize_Vault_Secondary_SharedVault(t *testing.T) { vault := ca.NewTestVaultServer(t) + primaryVaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-primary", + ConsulManaged: true, + }) + + secondaryVaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-secondary", + ConsulManaged: true, + }) + _, serverDC1 := testServerWithConfig(t, func(c *Config) { c.CAConfig = &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": primaryVaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-primary/", }, @@ -81,7 +92,7 @@ func TestCAManager_Initialize_Vault_Secondary_SharedVault(t *testing.T) { Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": secondaryVaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-secondary/", }, @@ -393,7 +404,7 @@ func TestCAManager_UpdateConfigWhileRenewIntermediate(t *testing.T) { // happen in the expected order. errCh := make(chan error) go func() { - errCh <- manager.RenewIntermediate(context.TODO(), false) + errCh <- manager.RenewIntermediate(context.TODO()) }() waitForCh(t, delegate.callbackCh, "provider/GenerateIntermediateCSR") @@ -570,7 +581,14 @@ func TestCAManager_Initialize_Logging(t *testing.T) { func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) { ca.SkipIfVaultNotPresent(t) + vault := ca.NewTestVaultServer(t) + vaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + WithSudo: true, + }) _, s1 := testServerWithConfig(t, func(c *Config) { c.PrimaryDatacenter = "dc1" @@ -578,7 +596,7 @@ func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) { Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": vaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", }, @@ -599,12 +617,18 @@ func TestCAManager_UpdateConfiguration_Vault_Primary(t *testing.T) { require.NoError(t, err) require.Equal(t, connect.HexString(cert.SubjectKeyId), origRoot.SigningKeyID) + vaultToken2 := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root-2", + IntermediatePath: "pki-intermediate-2", + ConsulManaged: true, + }) + err = s1.caManager.UpdateConfiguration(&structs.CARequest{ Config: &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": vaultToken2, "RootPKIPath": "pki-root-2/", "IntermediatePKIPath": "pki-intermediate-2/", }, @@ -636,12 +660,18 @@ func TestCAManager_Initialize_Vault_WithIntermediateAsPrimaryCA(t *testing.T) { meshRootPath := "pki-root" primaryCert := setupPrimaryCA(t, vclient, meshRootPath, "") + primaryVaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: meshRootPath, + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) + _, s1 := testServerWithConfig(t, func(c *Config) { c.CAConfig = &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": primaryVaultToken, "RootPKIPath": meshRootPath, "IntermediatePKIPath": "pki-intermediate/", }, @@ -665,6 +695,12 @@ func TestCAManager_Initialize_Vault_WithIntermediateAsPrimaryCA(t *testing.T) { // TODO: renew primary leaf signing cert // TODO: rotate root + secondaryVaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: meshRootPath, + IntermediatePath: "pki-secondary", + ConsulManaged: true, + }) + testutil.RunStep(t, "run secondary DC", func(t *testing.T) { _, sDC2 := testServerWithConfig(t, func(c *Config) { c.Datacenter = "dc2" @@ -673,7 +709,7 @@ func TestCAManager_Initialize_Vault_WithIntermediateAsPrimaryCA(t *testing.T) { Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": secondaryVaultToken, "RootPKIPath": meshRootPath, "IntermediatePKIPath": "pki-secondary/", }, @@ -704,12 +740,18 @@ func TestCAManager_Verify_Vault_NoChangeToSecondaryConfig(t *testing.T) { vault := ca.NewTestVaultServer(t) + primaryVaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) + _, sDC1 := testServerWithConfig(t, func(c *Config) { c.CAConfig = &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": primaryVaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", }, @@ -718,6 +760,12 @@ func TestCAManager_Verify_Vault_NoChangeToSecondaryConfig(t *testing.T) { defer sDC1.Shutdown() testrpc.WaitForActiveCARoot(t, sDC1.RPC, "dc1", nil) + secondaryVaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate-2", + ConsulManaged: true, + }) + _, sDC2 := testServerWithConfig(t, func(c *Config) { c.Datacenter = "dc2" c.PrimaryDatacenter = "dc1" @@ -725,7 +773,7 @@ func TestCAManager_Verify_Vault_NoChangeToSecondaryConfig(t *testing.T) { Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": secondaryVaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate-2/", }, @@ -740,7 +788,7 @@ func TestCAManager_Verify_Vault_NoChangeToSecondaryConfig(t *testing.T) { err := msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", &structs.DCSpecificRequest{}, &configBefore) require.NoError(t, err) - renewLeafSigningCert(t, sDC1.caManager, sDC1.caManager.primaryRenewIntermediate) + require.NoError(t, sDC1.caManager.renewIntermediateNow(context.Background())) // Give the secondary some time to notice the update time.Sleep(100 * time.Millisecond) @@ -778,37 +826,62 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { vault := ca.NewTestVaultServer(t) vclient := vault.Client() + rootPEM := generateExternalRootCA(t, vclient) primaryCAPath := "pki-primary" primaryCert := setupPrimaryCA(t, vclient, primaryCAPath, rootPEM) + primaryVaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: primaryCAPath, + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + WithSudo: true, + }) + _, serverDC1 := testServerWithConfig(t, func(c *Config) { c.CAConfig = &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": primaryVaultToken, "RootPKIPath": primaryCAPath, "IntermediatePKIPath": "pki-intermediate/", }, } }) testrpc.WaitForTestAgent(t, serverDC1.RPC, "dc1") + testrpc.WaitForActiveCARoot(t, serverDC1.RPC, "dc1", nil) - var origLeaf string + var ( + origLeaf string + primaryLeafSigningCert string + ) roots := structs.IndexedCARoots{} testutil.RunStep(t, "verify primary DC", func(t *testing.T) { codec := rpcClient(t, serverDC1) err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) - require.Len(t, roots.Roots, 1) - require.Equal(t, primaryCert, roots.Roots[0].RootCert) + + // Verify CA trust heirarchy is expected. + require.Len(t, roots.Roots, 1, "should have one because there's no provider rotation yet") + require.Equal(t, primaryCert, roots.Roots[0].RootCert, "should be the offline root") require.Contains(t, roots.Roots[0].RootCert, rootPEM) + require.Len(t, roots.Roots[0].IntermediateCerts, 1, "should just have the primary's intermediate") + + active := roots.Active() leafCert := getLeafCert(t, codec, roots.TrustDomain, "dc1") - verifyLeafCert(t, roots.Active(), leafCert) + verifyLeafCert(t, active, leafCert) + origLeaf = leafCert + primaryLeafSigningCert = serverDC1.caManager.getLeafSigningCertFromRoot(active) + }) + + secondaryVaultToken := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: "should-be-ignored", + IntermediatePath: "pki-secondary", + ConsulManaged: true, }) _, serverDC2 := testServerWithConfig(t, func(c *Config) { @@ -818,72 +891,91 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": secondaryVaultToken, "RootPKIPath": "should-be-ignored", "IntermediatePKIPath": "pki-secondary/", }, } }) + joinWAN(t, serverDC2, serverDC1) + testrpc.WaitForActiveCARoot(t, serverDC2.RPC, "dc2", nil) - var origLeafSecondary string + var ( + origLeafSecondary string + secondaryLeafSigningCert string + ) testutil.RunStep(t, "start secondary DC", func(t *testing.T) { - joinWAN(t, serverDC2, serverDC1) - testrpc.WaitForActiveCARoot(t, serverDC2.RPC, "dc2", nil) - codec := rpcClient(t, serverDC2) roots = structs.IndexedCARoots{} err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) - require.Len(t, roots.Roots, 1) + + require.Len(t, roots.Roots, 1, "should have one because there's no provider rotation yet") + require.Equal(t, primaryCert, roots.Roots[0].RootCert, "should be the offline root") + require.Contains(t, roots.Roots[0].RootCert, rootPEM) + require.Len(t, roots.Roots[0].IntermediateCerts, 2, "should have the primary's intermediate and our own") + + active := roots.Active() leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc2") verifyLeafCert(t, roots.Roots[0], leafPEM) + origLeafSecondary = leafPEM + secondaryLeafSigningCert = serverDC2.caManager.getLeafSigningCertFromRoot(active) }) testutil.RunStep(t, "renew leaf signing CA in primary", func(t *testing.T) { - previous := serverDC1.caManager.getLeafSigningCertFromRoot(roots.Active()) - - renewLeafSigningCert(t, serverDC1.caManager, serverDC1.caManager.primaryRenewIntermediate) + require.NoError(t, serverDC1.caManager.renewIntermediateNow(context.Background())) codec := rpcClient(t, serverDC1) roots = structs.IndexedCARoots{} err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) - require.Len(t, roots.Roots, 1) - require.Len(t, roots.Roots[0].IntermediateCerts, 2) + + require.Len(t, roots.Roots, 1, "should have one because there's no provider rotation yet") + require.Equal(t, primaryCert, roots.Roots[0].RootCert, "should be the offline root") + require.Contains(t, roots.Roots[0].RootCert, rootPEM) + require.Len(t, roots.Roots[0].IntermediateCerts, 2, "we renewed, so we have our old primary and our new primary intermediate") + + active := roots.Active() newCert := serverDC1.caManager.getLeafSigningCertFromRoot(roots.Active()) - require.NotEqual(t, previous, newCert) + require.NotEqual(t, primaryLeafSigningCert, newCert) + primaryLeafSigningCert = newCert leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc1") - verifyLeafCert(t, roots.Roots[0], leafPEM) + verifyLeafCert(t, active, leafPEM) // original certs from old signing cert should still verify - verifyLeafCert(t, roots.Roots[0], origLeaf) + verifyLeafCert(t, active, origLeaf) }) + var oldSecondaryData *structs.CARoot testutil.RunStep(t, "renew leaf signing CA in secondary", func(t *testing.T) { - previous := serverDC2.caManager.getLeafSigningCertFromRoot(roots.Active()) - - renewLeafSigningCert(t, serverDC2.caManager, serverDC2.caManager.secondaryRequestNewSigningCert) + require.NoError(t, serverDC2.caManager.renewIntermediateNow(context.Background())) codec := rpcClient(t, serverDC2) roots = structs.IndexedCARoots{} err := msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) - require.Len(t, roots.Roots, 1) - // one intermediate from primary, two from secondary - require.Len(t, roots.Roots[0].IntermediateCerts, 3) - newCert := serverDC1.caManager.getLeafSigningCertFromRoot(roots.Active()) - require.NotEqual(t, previous, newCert) + require.Len(t, roots.Roots, 1, "should have one because there's no provider rotation yet") + require.Equal(t, primaryCert, roots.Roots[0].RootCert, "should be the offline root") + require.Contains(t, roots.Roots[0].RootCert, rootPEM) + require.Len(t, roots.Roots[0].IntermediateCerts, 3, "one intermediate from primary, two from secondary") + + active := roots.Active() + oldSecondaryData = active + + newCert := serverDC2.caManager.getLeafSigningCertFromRoot(active) + require.NotEqual(t, secondaryLeafSigningCert, newCert) + secondaryLeafSigningCert = newCert leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc2") - verifyLeafCert(t, roots.Roots[0], leafPEM) + verifyLeafCert(t, active, leafPEM) // original certs from old signing cert should still verify - verifyLeafCert(t, roots.Roots[0], origLeaf) + verifyLeafCert(t, active, origLeaf) }) testutil.RunStep(t, "rotate root by changing the provider", func(t *testing.T) { @@ -902,20 +994,47 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { roots = structs.IndexedCARoots{} err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", &structs.DCSpecificRequest{}, &roots) require.NoError(t, err) - require.Len(t, roots.Roots, 2) + + require.Len(t, roots.Roots, 2, "two because we rotated the provider") + active := roots.Active() - require.Len(t, active.IntermediateCerts, 1) + require.NotEqual(t, primaryCert, active.RootCert, "should NOT be the offline root, because we switched") + require.NotContains(t, active.RootCert, rootPEM) + require.Len(t, active.IntermediateCerts, 1, "only one new intermediate in the primary") leafPEM := getLeafCert(t, codec, roots.TrustDomain, "dc1") verifyLeafCert(t, roots.Active(), leafPEM) + // wait for secondary to witness it + codec2 := rpcClient(t, serverDC2) + retry.Run(t, func(r *retry.R) { + var reply structs.IndexedCARoots + err := msgpackrpc.CallWithCodec(codec2, "ConnectCA.Roots", &structs.DCSpecificRequest{ + Datacenter: "dc2", + }, &reply) + require.NoError(r, err) + + require.Len(r, reply.Roots, 2, "primary provider rotated, so secondary gets rekeyed") + + active := reply.Active() + require.NotNil(r, active) + + if oldSecondaryData.ID == reply.ActiveRootID { + r.Fatal("wait; did not witness primary root rotation yet") + } + + newCert := serverDC2.caManager.getLeafSigningCertFromRoot(roots.Active()) + require.NotEqual(r, secondaryLeafSigningCert, newCert) + secondaryLeafSigningCert = newCert + }) + // original certs from old root cert should still verify verifyLeafCertWithRoots(t, roots, origLeaf) // original certs from secondary should still verify rootsSecondary := structs.IndexedCARoots{} r := &structs.DCSpecificRequest{Datacenter: "dc2"} - err = msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", r, &rootsSecondary) + err = msgpackrpc.CallWithCodec(codec2, "ConnectCA.Roots", r, &rootsSecondary) require.NoError(t, err) verifyLeafCertWithRoots(t, rootsSecondary, origLeafSecondary) }) @@ -923,6 +1042,12 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { testutil.RunStep(t, "rotate to a different external root", func(t *testing.T) { setupPrimaryCA(t, vclient, "pki-primary-2/", rootPEM) + primaryVaultToken2 := ca.CreateVaultTokenWithAttrs(t, vault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-primary-2", + IntermediatePath: "pki-intermediate-2", + ConsulManaged: true, + }) + codec := rpcClient(t, serverDC1) req := &structs.CARequest{ Op: structs.CAOpSetConfig, @@ -930,7 +1055,7 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { Provider: "vault", Config: map[string]interface{}{ "Address": vault.Addr, - "Token": vault.RootToken, + "Token": primaryVaultToken2, "RootPKIPath": "pki-primary-2/", "IntermediatePKIPath": "pki-intermediate-2/", }, @@ -963,28 +1088,6 @@ func TestCAManager_Initialize_Vault_WithExternalTrustedCA(t *testing.T) { }) } -// renewLeafSigningCert mimics RenewIntermediate. This is unfortunate, but -// necessary for now as there is no easy way to invoke that logic unconditionally. -// Currently, it requires patching values and polling for the operation to -// complete, which adds a lot of distractions to a test case. -// With this function we can instead unconditionally rotate the leaf signing cert -// synchronously. -func renewLeafSigningCert(t *testing.T, manager *CAManager, fn func(ca.Provider, *structs.CARoot) error) { - t.Helper() - provider, _ := manager.getCAProvider() - - store := manager.delegate.State() - _, root, err := store.CARootActive(nil) - require.NoError(t, err) - - activeRoot := root.Clone() - err = fn(provider, activeRoot) - require.NoError(t, err) - err = manager.persistNewRootAndConfig(provider, activeRoot, nil) - require.NoError(t, err) - manager.setCAProvider(provider, activeRoot) -} - func generateExternalRootCA(t *testing.T, client *vaultapi.Client) string { t.Helper() err := client.Sys().Mount("corp", &vaultapi.MountInput{ @@ -1046,6 +1149,7 @@ func setupPrimaryCA(t *testing.T, client *vaultapi.Client, path string, rootPEM "certificate": buf.String(), }) require.NoError(t, err, "failed to set signed intermediate") + // TODO: also fix issuers here? return lib.EnsureTrailingNewline(buf.String()) } diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index 2d7e06f04..bfa1fefc2 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -328,13 +328,19 @@ func TestCAManager_RenewIntermediate_Vault_Primary(t *testing.T) { testVault := ca.NewTestVaultServer(t) + vaultToken := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) + _, s1 := testServerWithConfig(t, func(c *Config) { c.PrimaryDatacenter = "dc1" c.CAConfig = &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": vaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", "LeafCertTTL": "2s", @@ -700,7 +706,6 @@ func TestCAManager_Initialize_Vault_KeepOldRoots_Primary(t *testing.T) { t.Parallel() testVault := ca.NewTestVaultServer(t) - defer testVault.Stop() dir1pre, s1pre := testServer(t) defer os.RemoveAll(dir1pre) @@ -710,12 +715,18 @@ func TestCAManager_Initialize_Vault_KeepOldRoots_Primary(t *testing.T) { testrpc.WaitForLeader(t, s1pre.RPC, "dc1") + vaultToken := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) + // Update the CA config to use Vault - this should force the generation of a new root cert. vaultCAConf := &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": vaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", }, @@ -765,7 +776,12 @@ func TestCAManager_Initialize_Vault_FixesSigningKeyID_Primary(t *testing.T) { t.Parallel() testVault := ca.NewTestVaultServer(t) - defer testVault.Stop() + + vaultToken := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) dir1pre, s1pre := testServerWithConfig(t, func(c *Config) { c.Build = "1.6.0" @@ -774,7 +790,7 @@ func TestCAManager_Initialize_Vault_FixesSigningKeyID_Primary(t *testing.T) { Provider: "vault", Config: map[string]interface{}{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": vaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", }, @@ -1545,13 +1561,19 @@ func TestCAManager_Initialize_Vault_BadCAConfigDoesNotPreventLeaderEstablishment require.Empty(t, rootsList.Roots) require.Nil(t, activeRoot) + goodVaultToken := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) + // Now that the leader is up and we have verified that there are no roots / CA init failed, // verify that we can reconfigure away from the bad configuration. newConfig := &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": goodVaultToken, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", }, @@ -1675,7 +1697,13 @@ func TestConnectCA_ConfigurationSet_Vault_ForceWithoutCrossSigning(t *testing.T) ca.SkipIfVaultNotPresent(t) testVault := ca.NewTestVaultServer(t) - defer testVault.Stop() + + vaultToken1 := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + WithSudo: true, + }) _, s1 := testServerWithConfig(t, func(c *Config) { c.Build = "1.9.1" @@ -1684,7 +1712,7 @@ func TestConnectCA_ConfigurationSet_Vault_ForceWithoutCrossSigning(t *testing.T) Provider: "vault", Config: map[string]interface{}{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": vaultToken1, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", }, @@ -1705,13 +1733,19 @@ func TestConnectCA_ConfigurationSet_Vault_ForceWithoutCrossSigning(t *testing.T) require.Len(t, rootList.Roots, 1) oldRoot := rootList.Roots[0] + vaultToken2 := ca.CreateVaultTokenWithAttrs(t, testVault.Client(), &ca.VaultTokenAttributes{ + RootPath: "pki-root-2", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + }) + // Update the provider config to use a new PKI path, which should // cause a rotation. newConfig := &structs.CAConfiguration{ Provider: "vault", Config: map[string]interface{}{ "Address": testVault.Addr, - "Token": testVault.RootToken, + "Token": vaultToken2, "RootPKIPath": "pki-root-2/", "IntermediatePKIPath": "pki-intermediate/", },