From 16c1f2a8b9350e5636069509128dd3af43ccfd54 Mon Sep 17 00:00:00 2001 From: Chris Thain Date: Wed, 11 Oct 2023 10:55:04 -0700 Subject: [PATCH] Manual backport of #19095 --- .changelog/19095.txt | 3 + agent/connect/ca/provider_vault.go | 141 +++++++++----- agent/connect/ca/provider_vault_test.go | 246 ++++++++++++++++++------ agent/connect/ca/testing.go | 32 ++- 4 files changed, 315 insertions(+), 107 deletions(-) create mode 100644 .changelog/19095.txt diff --git a/.changelog/19095.txt b/.changelog/19095.txt new file mode 100644 index 0000000000000..f18e19725ec21 --- /dev/null +++ b/.changelog/19095.txt @@ -0,0 +1,3 @@ +```release-note:bug +ca: ensure Vault CA provider respects Vault Enterprise namespace configuration. +``` diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index b61bd95d3f175..60627b5314a24 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + package ca import ( @@ -8,7 +11,6 @@ import ( "fmt" "io" "net/http" - "os" "strings" "time" @@ -96,7 +98,7 @@ func vaultTLSConfig(config *structs.VaultCAProviderConfig) *vaultapi.TLSConfig { // Configure sets up the provider using the given configuration. // Configure supports being called multiple times to re-configure the provider. func (v *VaultProvider) Configure(cfg ProviderConfig) error { - config, err := ParseVaultCAConfig(cfg.RawConfig) + config, err := ParseVaultCAConfig(cfg.RawConfig, v.isPrimary) if err != nil { return err } @@ -187,11 +189,11 @@ func (v *VaultProvider) Configure(cfg ProviderConfig) error { } func (v *VaultProvider) ValidateConfigUpdate(prevRaw, nextRaw map[string]interface{}) error { - prev, err := ParseVaultCAConfig(prevRaw) + prev, err := ParseVaultCAConfig(prevRaw, v.isPrimary) if err != nil { return fmt.Errorf("failed to parse existing CA config: %w", err) } - next, err := ParseVaultCAConfig(nextRaw) + next, err := ParseVaultCAConfig(nextRaw, v.isPrimary) if err != nil { return fmt.Errorf("failed to parse new CA config: %w", err) } @@ -274,10 +276,10 @@ func (v *VaultProvider) State() (map[string]string, error) { return nil, nil } -// GenerateRoot mounts and initializes a new root PKI backend if needed. -func (v *VaultProvider) GenerateRoot() (RootResult, error) { +// GenerateCAChain mounts and initializes a new root PKI backend if needed. +func (v *VaultProvider) GenerateCAChain() (string, error) { if !v.isPrimary { - return RootResult{}, fmt.Errorf("provider is not the root certificate authority") + return "", fmt.Errorf("provider is not the root certificate authority") } // Set up the root PKI backend if necessary. @@ -297,7 +299,7 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { }, }) if err != nil { - return RootResult{}, fmt.Errorf("failed to mount root CA backend: %w", err) + return "", fmt.Errorf("failed to mount root CA backend: %w", err) } // We want to initialize afterwards @@ -305,7 +307,7 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { case ErrBackendNotInitialized: uid, err := connect.CompactUID() if err != nil { - return RootResult{}, err + return "", err } resp, err := v.writeNamespaced(v.config.RootPKINamespace, v.config.RootPKIPath+"root/generate/internal", map[string]interface{}{ "common_name": connect.CACN("vault", uid, v.clusterID, v.isPrimary), @@ -314,23 +316,23 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { "key_bits": v.config.PrivateKeyBits, }) if err != nil { - return RootResult{}, fmt.Errorf("failed to initialize root CA: %w", err) + return "", fmt.Errorf("failed to initialize root CA: %w", err) } var ok bool rootPEM, ok = resp.Data["certificate"].(string) if !ok { - return RootResult{}, fmt.Errorf("unexpected response from Vault: %v", resp.Data["certificate"]) + return "", fmt.Errorf("unexpected response from Vault: %v", resp.Data["certificate"]) } default: if err != nil { - return RootResult{}, fmt.Errorf("unexpected error while setting root PKI backend: %w", err) + return "", fmt.Errorf("unexpected error while setting root PKI backend: %w", err) } } rootChain, err := v.getCAChain(v.config.RootPKINamespace, v.config.RootPKIPath) if err != nil { - return RootResult{}, err + return "", err } // Workaround for a bug in the Vault PKI API. @@ -339,7 +341,7 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) { rootChain = rootPEM } - return RootResult{PEM: rootChain}, nil + return rootChain, nil } // GenerateIntermediateCSR creates a private key and generates a CSR @@ -422,6 +424,9 @@ func (v *VaultProvider) setupIntermediatePKIPath() error { "require_cn": false, }) + // enable auto-tidy with tidy_expired_issuers + v.autotidyIssuers(v.config.IntermediatePKIPath) + return err } @@ -493,7 +498,7 @@ func (v *VaultProvider) SetIntermediate(intermediatePEM, rootPEM, keyId string) } // ActiveIntermediate returns the current intermediate certificate. -func (v *VaultProvider) ActiveIntermediate() (string, error) { +func (v *VaultProvider) ActiveLeafSigningCert() (string, error) { cert, err := v.getCA(v.config.IntermediatePKINamespace, v.config.IntermediatePKIPath) // This error is expected when calling initializeSecondaryCA for the @@ -513,7 +518,7 @@ func (v *VaultProvider) ActiveIntermediate() (string, error) { // because the endpoint only returns the raw PEM contents of the CA cert // and not the typical format of the secrets endpoints. func (v *VaultProvider) getCA(namespace, path string) (string, error) { - resp, err := v.client.WithNamespace(namespace).Logical().ReadRaw(path + "/ca/pem") + resp, err := v.client.WithNamespace(v.getNamespace(namespace)).Logical().ReadRaw(path + "/ca/pem") if resp != nil { defer resp.Body.Close() } @@ -539,7 +544,7 @@ func (v *VaultProvider) getCA(namespace, path string) (string, error) { // TODO: refactor to remove duplication with getCA func (v *VaultProvider) getCAChain(namespace, path string) (string, error) { - resp, err := v.client.WithNamespace(namespace).Logical().ReadRaw(path + "/ca_chain") + resp, err := v.client.WithNamespace(v.getNamespace(namespace)).Logical().ReadRaw(path + "/ca_chain") if resp != nil { defer resp.Body.Close() } @@ -559,10 +564,10 @@ func (v *VaultProvider) getCAChain(namespace, path string) (string, error) { return root, nil } -// GenerateIntermediate mounts the configured intermediate PKI backend if +// GenerateLeafSigningCert mounts the configured intermediate PKI backend if // necessary, then generates and signs a new CA CSR using the root PKI backend // and updates the intermediate backend to use that new certificate. -func (v *VaultProvider) GenerateIntermediate() (string, error) { +func (v *VaultProvider) GenerateLeafSigningCert() (string, error) { csr, keyId, err := v.generateIntermediateCSR() if err != nil { return "", err @@ -598,7 +603,7 @@ func (v *VaultProvider) GenerateIntermediate() (string, error) { } } - return v.ActiveIntermediate() + return v.ActiveLeafSigningCert() } // setDefaultIntermediateIssuer updates the default issuer for @@ -816,7 +821,7 @@ func (v *VaultProvider) Cleanup(providerTypeChange bool, otherConfig map[string] v.Stop() if !providerTypeChange { - newConfig, err := ParseVaultCAConfig(otherConfig) + newConfig, err := ParseVaultCAConfig(otherConfig, v.isPrimary) if err != nil { return err } @@ -844,34 +849,72 @@ func (v *VaultProvider) Stop() { v.stopWatcher() } -func (v *VaultProvider) PrimaryUsesIntermediate() {} - // We use raw path here func (v *VaultProvider) mountNamespaced(namespace, path string, mountInfo *vaultapi.MountInput) error { - return v.client.WithNamespace(namespace).Sys().Mount(path, mountInfo) + return v.client.WithNamespace(v.getNamespace(namespace)).Sys().Mount(path, mountInfo) } func (v *VaultProvider) tuneMountNamespaced(namespace, path string, mountConfig *vaultapi.MountConfigInput) error { - return v.client.WithNamespace(namespace).Sys().TuneMount(path, *mountConfig) + return v.client.WithNamespace(v.getNamespace(namespace)).Sys().TuneMount(path, *mountConfig) } func (v *VaultProvider) unmountNamespaced(namespace, path string) error { - return v.client.WithNamespace(namespace).Sys().Unmount(path) + return v.client.WithNamespace(v.getNamespace(namespace)).Sys().Unmount(path) } func (v *VaultProvider) readNamespaced(namespace string, resource string) (*vaultapi.Secret, error) { - return v.client.WithNamespace(namespace).Logical().Read(resource) + return v.client.WithNamespace(v.getNamespace(namespace)).Logical().Read(resource) } func (v *VaultProvider) writeNamespaced(namespace string, resource string, data map[string]interface{}) (*vaultapi.Secret, error) { - return v.client.WithNamespace(namespace).Logical().Write(resource, data) + return v.client.WithNamespace(v.getNamespace(namespace)).Logical().Write(resource, data) } func (v *VaultProvider) deleteNamespaced(namespace string, resource string) (*vaultapi.Secret, error) { - return v.client.WithNamespace(namespace).Logical().Delete(resource) + return v.client.WithNamespace(v.getNamespace(namespace)).Logical().Delete(resource) +} + +func (v *VaultProvider) getNamespace(namespace string) string { + if namespace != "" { + return namespace + } + return v.baseNamespace +} + +// autotidyIssuers sets Vault's auto-tidy to remove expired issuers +// Returns a boolean on success for testing (as there is no post-facto way of +// checking if it is set). Logs at info level on failure to set and why, +// returning the log message for test purposes as well. +func (v *VaultProvider) autotidyIssuers(path string) (bool, string) { + s, err := v.client.Logical().Write(path+"/config/auto-tidy", + map[string]interface{}{ + "enabled": true, + "tidy_expired_issuers": true, + }) + var errStr string + if err != nil { + errStr = err.Error() + switch { + case strings.Contains(errStr, "404"): + errStr = "vault versions < 1.12 don't support auto-tidy" + case strings.Contains(errStr, "400"): + errStr = "vault versions < 1.13 don't support the tidy_expired_issuers field" + case strings.Contains(errStr, "403"): + errStr = "permission denied on auto-tidy path in vault" + } + v.logger.Info("Unable to enable Vault's auto-tidy feature for expired issuers", "reason", errStr, "path", path) + } + // return values for tests + tidySet := false + if s != nil { + if tei, ok := s.Data["tidy_expired_issuers"]; ok { + tidySet, _ = tei.(bool) + } + } + return tidySet, errStr } -func ParseVaultCAConfig(raw map[string]interface{}) (*structs.VaultCAProviderConfig, error) { +func ParseVaultCAConfig(raw map[string]interface{}, isPrimary bool) (*structs.VaultCAProviderConfig, error) { config := structs.VaultCAProviderConfig{ CommonCAProviderConfig: defaultCommonConfig(), } @@ -902,10 +945,10 @@ func ParseVaultCAConfig(raw map[string]interface{}) (*structs.VaultCAProviderCon return nil, fmt.Errorf("only one of Vault token or Vault auth method can be provided, but not both") } - if config.RootPKIPath == "" { + if isPrimary && config.RootPKIPath == "" { return nil, fmt.Errorf("must provide a valid path to a root PKI backend") } - if !strings.HasSuffix(config.RootPKIPath, "/") { + if config.RootPKIPath != "" && !strings.HasSuffix(config.RootPKIPath, "/") { config.RootPKIPath += "/" } @@ -940,6 +983,14 @@ func vaultLogin(client *vaultapi.Client, authMethod *structs.VaultAuthMethod) (* return resp, nil } +// Note the authMethod's parameters (Params) is populated from a freeform map +// in the configuration where they could hardcode values to be passed directly +// to the `auth/*/login` endpoint. Each auth method's authentication code +// needs to handle two cases: +// - The legacy case (which should be deprecated) where the user has +// hardcoded login values directly (eg. a `jwt` string) +// - The case where they use the configuration option used in the +// vault agent's auth methods. func configureVaultAuthMethod(authMethod *structs.VaultAuthMethod) (VaultAuthenticator, error) { if authMethod.MountPath == "" { authMethod.MountPath = authMethod.Type @@ -949,20 +1000,18 @@ func configureVaultAuthMethod(authMethod *structs.VaultAuthMethod) (VaultAuthent switch authMethod.Type { case VaultAuthMethodTypeAWS: return NewAWSAuthClient(authMethod), nil + case VaultAuthMethodTypeAzure: + return NewAzureAuthClient(authMethod) case VaultAuthMethodTypeGCP: return NewGCPAuthClient(authMethod) + case VaultAuthMethodTypeJWT: + return NewJwtAuthClient(authMethod) + case VaultAuthMethodTypeAppRole: + return NewAppRoleAuthClient(authMethod) + case VaultAuthMethodTypeAliCloud: + return NewAliCloudAuthClient(authMethod) case VaultAuthMethodTypeKubernetes: - // For the Kubernetes Auth method, we will try to read the JWT token - // from the default service account file location if jwt was not provided. - if jwt, ok := authMethod.Params["jwt"]; !ok || jwt == "" { - serviceAccountToken, err := os.ReadFile(defaultK8SServiceAccountTokenPath) - if err != nil { - return nil, err - } - - authMethod.Params["jwt"] = string(serviceAccountToken) - } - return NewVaultAPIAuthClient(authMethod, loginPath), nil + return NewK8sAuthClient(authMethod) // These auth methods require a username for the login API path. case VaultAuthMethodTypeLDAP, VaultAuthMethodTypeUserpass, VaultAuthMethodTypeOkta, VaultAuthMethodTypeRadius: // Get username from the params. @@ -984,12 +1033,8 @@ func configureVaultAuthMethod(authMethod *structs.VaultAuthMethod) (VaultAuthent return nil, fmt.Errorf("'token' auth method is not supported via auth method configuration; " + "please provide the token with the 'token' parameter in the CA configuration") // The rest of the auth methods use auth/ login API path. - case VaultAuthMethodTypeAliCloud, - VaultAuthMethodTypeAppRole, - VaultAuthMethodTypeAzure, - VaultAuthMethodTypeCloudFoundry, + case VaultAuthMethodTypeCloudFoundry, VaultAuthMethodTypeGitHub, - VaultAuthMethodTypeJWT, VaultAuthMethodTypeKerberos, VaultAuthMethodTypeTLS: return NewVaultAPIAuthClient(authMethod, loginPath), nil diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index e9a89177506d8..ece7659d04aa5 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + package ca import ( @@ -5,6 +8,8 @@ import ( "encoding/json" "fmt" "io" + "strconv" + "strings" "sync/atomic" "testing" "time" @@ -56,6 +61,7 @@ func TestVaultCAProvider_ParseVaultCAConfig(t *testing.T) { cases := map[string]struct { rawConfig map[string]interface{} expConfig *structs.VaultCAProviderConfig + isPrimary bool expError string }{ "no token and no auth method provided": { @@ -66,15 +72,26 @@ func TestVaultCAProvider_ParseVaultCAConfig(t *testing.T) { rawConfig: map[string]interface{}{"Token": "test", "AuthMethod": map[string]interface{}{"Type": "test"}}, expError: "only one of Vault token or Vault auth method can be provided, but not both", }, - "no root PKI path": { - rawConfig: map[string]interface{}{"Token": "test"}, + "primary no root PKI path": { + rawConfig: map[string]interface{}{"Token": "test", "IntermediatePKIPath": "test"}, + isPrimary: true, expError: "must provide a valid path to a root PKI backend", }, + "secondary no root PKI path": { + rawConfig: map[string]interface{}{"Token": "test", "IntermediatePKIPath": "test"}, + isPrimary: false, + expConfig: &structs.VaultCAProviderConfig{ + CommonCAProviderConfig: defaultCommonConfig(), + Token: "test", + IntermediatePKIPath: "test/", + }, + }, "no root intermediate path": { rawConfig: map[string]interface{}{"Token": "test", "RootPKIPath": "test"}, expError: "must provide a valid path for the intermediate PKI backend", }, "adds a slash to RootPKIPath and IntermediatePKIPath": { + isPrimary: true, rawConfig: map[string]interface{}{"Token": "test", "RootPKIPath": "test", "IntermediatePKIPath": "test"}, expConfig: &structs.VaultCAProviderConfig{ CommonCAProviderConfig: defaultCommonConfig(), @@ -87,7 +104,7 @@ func TestVaultCAProvider_ParseVaultCAConfig(t *testing.T) { for name, c := range cases { t.Run(name, func(t *testing.T) { - config, err := ParseVaultCAConfig(c.rawConfig) + config, err := ParseVaultCAConfig(c.rawConfig, c.isPrimary) if c.expError != "" { require.EqualError(t, err, c.expError) } else { @@ -105,16 +122,16 @@ func TestVaultCAProvider_configureVaultAuthMethod(t *testing.T) { expError string hasLDG bool }{ - "alicloud": {expLoginPath: "auth/alicloud/login"}, - "approle": {expLoginPath: "auth/approle/login"}, + "alicloud": {expLoginPath: "auth/alicloud/login", params: map[string]any{"role": "test-role", "region": "test-region"}, hasLDG: true}, + "approle": {expLoginPath: "auth/approle/login", params: map[string]any{"role_id_file_path": "test-path"}, hasLDG: true}, "aws": {expLoginPath: "auth/aws/login", params: map[string]interface{}{"type": "iam"}, hasLDG: true}, - "azure": {expLoginPath: "auth/azure/login"}, + "azure": {expLoginPath: "auth/azure/login", params: map[string]interface{}{"role": "test-role", "resource": "test-resource"}, hasLDG: true}, "cf": {expLoginPath: "auth/cf/login"}, "github": {expLoginPath: "auth/github/login"}, "gcp": {expLoginPath: "auth/gcp/login", params: map[string]interface{}{"type": "iam", "role": "test-role"}}, - "jwt": {expLoginPath: "auth/jwt/login"}, + "jwt": {expLoginPath: "auth/jwt/login", params: map[string]any{"role": "test-role", "path": "test-path"}, hasLDG: true}, "kerberos": {expLoginPath: "auth/kerberos/login"}, - "kubernetes": {expLoginPath: "auth/kubernetes/login", params: map[string]interface{}{"jwt": "fake"}}, + "kubernetes": {expLoginPath: "auth/kubernetes/login", params: map[string]interface{}{"role": "test-role"}, hasLDG: true}, "ldap": {expLoginPath: "auth/ldap/login/foo", params: map[string]interface{}{"username": "foo"}}, "oci": {expLoginPath: "auth/oci/login/foo", params: map[string]interface{}{"role": "foo"}}, "okta": {expLoginPath: "auth/okta/login/foo", params: map[string]interface{}{"username": "foo"}}, @@ -243,7 +260,7 @@ func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) { "IntermediatePKIPath": "pki-intermediate/", }) - cert, err := provider.ActiveIntermediate() + cert, err := provider.ActiveLeafSigningCert() require.Empty(t, cert) require.NoError(t, err) } @@ -403,8 +420,8 @@ func TestVaultCAProvider_Bootstrap(t *testing.T) { "LeafCertTTL": "1h", }, certFunc: func(provider *VaultProvider) (string, error) { - root, err := provider.GenerateRoot() - return root.PEM, err + root, err := provider.GenerateCAChain() + return root, err }, backendPath: "pki-root/", rootCaCreation: true, @@ -417,7 +434,7 @@ func TestVaultCAProvider_Bootstrap(t *testing.T) { "RootCertTTL": "8761h", }, certFunc: func(provider *VaultProvider) (string, error) { - return provider.ActiveIntermediate() + return provider.ActiveLeafSigningCert() }, backendPath: "pki-intermediate/", rootCaCreation: false, @@ -481,12 +498,11 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { Service: "foo", } - root, err := provider.GenerateRoot() + rootPEM, err := provider.GenerateCAChain() require.NoError(t, err) - rootPEM := root.PEM assertCorrectKeyType(t, tc.KeyType, rootPEM) - intPEM, err := provider.ActiveIntermediate() + intPEM, err := provider.ActiveLeafSigningCert() require.NoError(t, err) assertCorrectKeyType(t, tc.KeyType, intPEM) @@ -559,12 +575,6 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) { 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{ @@ -584,11 +594,11 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) { }) testutil.RunStep(t, "init", func(t *testing.T) { - root, err := provider1.GenerateRoot() + rootPEM, err := provider1.GenerateCAChain() require.NoError(t, err) - assertCorrectKeyType(t, tc.SigningKeyType, root.PEM) + assertCorrectKeyType(t, tc.SigningKeyType, rootPEM) - intPEM, err := provider1.ActiveIntermediate() + intPEM, err := provider1.ActiveLeafSigningCert() require.NoError(t, err) assertCorrectKeyType(t, tc.SigningKeyType, intPEM) }) @@ -612,11 +622,11 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) { }) testutil.RunStep(t, "swap", func(t *testing.T) { - root, err := provider2.GenerateRoot() + rootPEM, err := provider2.GenerateCAChain() require.NoError(t, err) - assertCorrectKeyType(t, tc.CSRKeyType, root.PEM) + assertCorrectKeyType(t, tc.CSRKeyType, rootPEM) - intPEM, err := provider2.ActiveIntermediate() + intPEM, err := provider2.ActiveLeafSigningCert() require.NoError(t, err) assertCorrectKeyType(t, tc.CSRKeyType, intPEM) @@ -736,7 +746,7 @@ func TestVaultProvider_SignIntermediateConsul(t *testing.T) { delegate := newMockDelegate(t, conf) provider1 := TestConsulProvider(t, delegate) require.NoError(t, provider1.Configure(testProviderConfig(conf))) - _, err := provider1.GenerateRoot() + _, err := provider1.GenerateCAChain() require.NoError(t, err) // Ensure that we don't configure vault to try and mint leafs that @@ -1074,9 +1084,9 @@ func TestVaultProvider_ReconfigureIntermediateTTL(t *testing.T) { t.Cleanup(provider.Stop) err = provider.Configure(makeProviderConfWithTTL("222h")) require.NoError(t, err) - _, err = provider.GenerateRoot() + _, err = provider.GenerateCAChain() require.NoError(t, err) - _, err = provider.GenerateIntermediate() + _, err = provider.GenerateLeafSigningCert() require.NoError(t, err) // Attempt to update the ttl without permissions for the tune endpoint - shouldn't @@ -1125,23 +1135,23 @@ func TestVaultCAProvider_GenerateIntermediate(t *testing.T) { "IntermediatePKIPath": "pki-intermediate/", }) - orig, err := provider.ActiveIntermediate() + orig, err := provider.ActiveLeafSigningCert() require.NoError(t, err) // This test was created to ensure that our calls to Vault // returns a new Intermediate certificate and further calls - // to ActiveIntermediate return the same new cert. - new, err := provider.GenerateIntermediate() + // to ActiveLeafSigningCert return the same new cert. + newLeaf, err := provider.GenerateLeafSigningCert() require.NoError(t, err) - newActive, err := provider.ActiveIntermediate() + newActive, err := provider.ActiveLeafSigningCert() require.NoError(t, err) - require.Equal(t, new, newActive) - require.NotEqual(t, orig, new) + require.Equal(t, newLeaf, newActive) + require.NotEqual(t, orig, newLeaf) } -func TestVaultCAProvider_DeletePreviousIssuerAndKey(t *testing.T) { +func TestVaultCAProvider_AutoTidyExpiredIssuers(t *testing.T) { SkipIfVaultNotPresent(t) t.Parallel() @@ -1157,13 +1167,47 @@ func TestVaultCAProvider_DeletePreviousIssuerAndKey(t *testing.T) { "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", }) - res, err := testVault.Client().Logical().List("pki-intermediate/issuers") - require.NoError(t, err) - if res == nil { - t.Skip("Vault version < 1.11 does not have multi issuers functionality") + version := strings.Split(vaultTestVersion, ".") + require.Len(t, version, 3) + minorVersion, err := strconv.Atoi(version[1]) + require.NoError(t, err) + expIssSet, errStr := provider.autotidyIssuers("pki-intermediate/") + switch { + case minorVersion <= 11: + require.False(t, expIssSet) + require.Contains(t, errStr, "auto-tidy") + case minorVersion == 12: + require.False(t, expIssSet) + require.Contains(t, errStr, "tidy_expired_issuers") + default: // Consul 1.13+ + require.True(t, expIssSet) } + // check permission denied + expIssSet, errStr = provider.autotidyIssuers("pki-bad/") + require.False(t, expIssSet) + require.Contains(t, errStr, "permission denied") +} + +func TestVaultCAProvider_DeletePreviousIssuerAndKey(t *testing.T) { + SkipIfVaultNotPresent(t) + 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/", + }) + res, err := testVault.Client().Logical().List("pki-intermediate/issuers") + require.NoError(t, err) // Why 2 issuers? There is always an initial issuer that // gets created before we manage the lifecycle of issuers. // Since we're asserting that the number doesn't grow @@ -1178,7 +1222,7 @@ func TestVaultCAProvider_DeletePreviousIssuerAndKey(t *testing.T) { require.Len(t, res.Data["keys"], 1) for i := 0; i < 3; i++ { - _, err := provider.GenerateIntermediate() + _, err := provider.GenerateLeafSigningCert() require.NoError(t, err) res, err := testVault.Client().Logical().List("pki-intermediate/issuers") @@ -1201,7 +1245,7 @@ func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) { delegate := newMockDelegate(t, conf) primaryProvider := TestConsulProvider(t, delegate) require.NoError(t, primaryProvider.Configure(testProviderConfig(conf))) - _, err := primaryProvider.GenerateRoot() + _, err := primaryProvider.GenerateCAChain() require.NoError(t, err) // Ensure that we don't configure vault to try and mint leafs that @@ -1235,14 +1279,13 @@ func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) { // Sign the CSR with primaryProvider. intermediatePEM, err := primaryProvider.SignIntermediate(csr) require.NoError(t, err) - root, err := primaryProvider.GenerateRoot() + rootPEM, err := primaryProvider.GenerateCAChain() require.NoError(t, err) - rootPEM := root.PEM // Give the new intermediate to provider to use. require.NoError(t, provider.SetIntermediate(intermediatePEM, rootPEM, issuerID)) - origIntermediate, err = provider.ActiveIntermediate() + origIntermediate, err = provider.ActiveLeafSigningCert() require.NoError(t, err) }) @@ -1256,17 +1299,16 @@ func TestVaultCAProvider_GenerateIntermediate_inSecondary(t *testing.T) { // Sign the CSR with primaryProvider. intermediatePEM, err := primaryProvider.SignIntermediate(csr) require.NoError(t, err) - root, err := primaryProvider.GenerateRoot() + rootPEM, err := primaryProvider.GenerateCAChain() require.NoError(t, err) - rootPEM := root.PEM // Give the new intermediate to provider to use. require.NoError(t, provider.SetIntermediate(intermediatePEM, rootPEM, issuerID)) // This test was created to ensure that our calls to Vault // returns a new Intermediate certificate and further calls - // to ActiveIntermediate return the same new cert. - newActiveIntermediate, err := provider.ActiveIntermediate() + // to ActiveLeafSigningCert return the same new cert. + newActiveIntermediate, err := provider.ActiveLeafSigningCert() require.NoError(t, err) require.NotEqual(t, origIntermediate, newActiveIntermediate) @@ -1373,6 +1415,85 @@ func TestVaultCAProvider_ConsulManaged(t *testing.T) { }) } +func TestVaultCAProvider_EnterpriseNamespace(t *testing.T) { + SkipIfVaultNotPresent(t, vaultRequirements{Enterprise: true}) + t.Parallel() + + cases := map[string]struct { + namespaces map[string]string + }{ + "no configured namespaces": {}, + "only base namespace provided": {namespaces: map[string]string{"Namespace": "base-ns"}}, + "only root namespace provided": {namespaces: map[string]string{"RootPKINamespace": "root-pki-ns"}}, + "only intermediate namespace provided": {namespaces: map[string]string{"IntermediatePKINamespace": "int-pki-ns"}}, + "base and root namespace provided": { + namespaces: map[string]string{ + "Namespace": "base-ns", + "RootPKINamespace": "root-pki-ns", + }, + }, + "base and intermediate namespace provided": { + namespaces: map[string]string{ + "Namespace": "base-ns", + "IntermediatePKINamespace": "int-pki-ns", + }, + }, + "root and intermediate namespace provided": { + namespaces: map[string]string{ + "RootPKINamespace": "root-pki-ns", + "IntermediatePKINamespace": "int-pki-ns", + }, + }, + "all namespaces provided": { + namespaces: map[string]string{ + "Namespace": "base-ns", + "RootPKINamespace": "root-pki-ns", + "IntermediatePKINamespace": "int-pki-ns", + }, + }, + } + + for name, c := range cases { + c := c + t.Run(name, func(t *testing.T) { + t.Parallel() + + testVault := NewTestVaultServer(t) + token := "root" + + providerConfig := map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + } + for k, v := range c.namespaces { + providerConfig[k] = v + } + + if len(c.namespaces) > 0 { + // If explicit namespaces are provided, try to create the provider before any of the namespaces + // have been created. Verify that the provider fails to initialize. + provider, err := createVaultProviderE(t, true, testVault.Addr, token, providerConfig) + require.Error(t, err) + require.NotNil(t, provider) + } + + // Create the namespaces + client := testVault.Client() + client.SetToken(token) + + for _, ns := range c.namespaces { + _, err := client.Logical().Write(fmt.Sprintf("/sys/namespaces/%s", ns), map[string]any{}) + require.NoError(t, err) + } + + // Verify that once the namespaces have been created we are able to initialize the provider. + provider, err := createVaultProviderE(t, true, testVault.Addr, token, providerConfig) + require.NoError(t, err) + require.NotNil(t, provider) + }) + } +} + func getIntermediateCertTTL(t *testing.T, caConf *structs.CAConfiguration) time.Duration { t.Helper() @@ -1394,6 +1515,15 @@ func getIntermediateCertTTL(t *testing.T, caConf *structs.CAConfiguration) time. func createVaultProvider(t *testing.T, isPrimary bool, addr, token string, rawConf map[string]any) *VaultProvider { t.Helper() + + provider, err := createVaultProviderE(t, isPrimary, addr, token, rawConf) + require.NoError(t, err) + + return provider +} + +func createVaultProviderE(t *testing.T, isPrimary bool, addr, token string, rawConf map[string]any) (*VaultProvider, error) { + t.Helper() cfg := vaultProviderConfig(t, addr, token, rawConf) provider := NewVaultProvider(hclog.New(nil)) @@ -1404,15 +1534,19 @@ func createVaultProvider(t *testing.T, isPrimary bool, addr, token string, rawCo } t.Cleanup(provider.Stop) - require.NoError(t, provider.Configure(cfg)) + if err := provider.Configure(cfg); err != nil { + return provider, err + } if isPrimary { - _, err := provider.GenerateRoot() - require.NoError(t, err) - _, err = provider.GenerateIntermediate() - require.NoError(t, err) + if _, err := provider.GenerateCAChain(); err != nil { + return provider, err + } + if _, err := provider.GenerateLeafSigningCert(); err != nil { + return provider, err + } } - return provider + return provider, nil } func vaultProviderConfig(t *testing.T, addr, token string, rawConf map[string]any) ProviderConfig { diff --git a/agent/connect/ca/testing.go b/agent/connect/ca/testing.go index 450c13d6dd896..7386deea94944 100644 --- a/agent/connect/ca/testing.go +++ b/agent/connect/ca/testing.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + package ca import ( @@ -58,6 +61,10 @@ type CASigningKeyTypes struct { CSRKeyBits int } +type vaultRequirements struct { + Enterprise bool +} + // CASigningKeyTypeCases returns the cross-product of the important supported CA // key types for generating table tests for CA signing tests (CrossSignCA and // SignIntermediate). @@ -90,7 +97,7 @@ func TestConsulProvider(t testing.T, d ConsulProviderStateDelegate) *ConsulProvi // // These tests may be skipped in CI. They are run as part of a separate // integration test suite. -func SkipIfVaultNotPresent(t testing.T) { +func SkipIfVaultNotPresent(t testing.T, reqs ...vaultRequirements) { // Try to safeguard against tests that will never run in CI. // This substring should match the pattern used by the // test-connect-ca-providers CI job. @@ -107,6 +114,16 @@ func SkipIfVaultNotPresent(t testing.T) { if err != nil || path == "" { t.Skipf("%q not found on $PATH - download and install to run this test", vaultBinaryName) } + + // Check for any additional Vault requirements. + for _, r := range reqs { + if r.Enterprise { + ver := vaultVersion(t, vaultBinaryName) + if !strings.Contains(ver, "+ent") { + t.Skipf("%q is not a Vault Enterprise version", ver) + } + } + } } func NewTestVaultServer(t testing.T) *TestVaultServer { @@ -181,6 +198,7 @@ type TestVaultServer struct { } var printedVaultVersion sync.Once +var vaultTestVersion string func (v *TestVaultServer) Client() *vaultapi.Client { return v.client @@ -202,6 +220,7 @@ func (v *TestVaultServer) WaitUntilReady(t testing.T) { version = resp.Version }) printedVaultVersion.Do(func() { + vaultTestVersion = version fmt.Fprintf(os.Stderr, "[INFO] agent/connect/ca: testing with vault server version: %s\n", version) }) } @@ -234,8 +253,8 @@ func requireTrailingNewline(t testing.T, leafPEM string) { if len(leafPEM) == 0 { t.Fatalf("cert is empty") } - if '\n' != rune(leafPEM[len(leafPEM)-1]) { - t.Fatalf("cert do not end with a new line") + if rune(leafPEM[len(leafPEM)-1]) != '\n' { + t.Fatalf("cert does not end with a new line") } } @@ -362,3 +381,10 @@ func createVaultTokenAndPolicy(t testing.T, client *vaultapi.Client, policyName, require.NoError(t, err) return tok.Auth.ClientToken } + +func vaultVersion(t testing.T, vaultBinaryName string) string { + cmd := exec.Command(vaultBinaryName, []string{"version"}...) + output, err := cmd.Output() + require.NoError(t, err) + return string(output[:len(output)-1]) +}