From 426a99227e93199b7d989ec4a4cde4e0d76f81f2 Mon Sep 17 00:00:00 2001 From: Matteo Kamm Date: Sun, 1 Dec 2024 16:03:12 +0100 Subject: [PATCH] Introduce unique server id for HA setup (#5058) Signed-off-by: Matteo Kamm --- ...lugin_server_keymanager_hashicorp_vault.md | 41 +++++++--- .../hashicorpvault/hashicorp_vault.go | 79 ++++++++++++++++++- .../hashicorpvault/hashicorp_vault_test.go | 34 +++++++- .../hashicorpvault/vault_fake_test.go | 20 +++-- 4 files changed, 150 insertions(+), 24 deletions(-) diff --git a/doc/plugin_server_keymanager_hashicorp_vault.md b/doc/plugin_server_keymanager_hashicorp_vault.md index 4db7927949..78414c3a0f 100644 --- a/doc/plugin_server_keymanager_hashicorp_vault.md +++ b/doc/plugin_server_keymanager_hashicorp_vault.md @@ -7,17 +7,19 @@ SVIDs as needed. The plugin accepts the following configuration options: -| key | type | required | description | default | -|:---------------------|:-------|:---------|:---------------------------------------------------------------------------------------------------------|:---------------------| -| vault_addr | string | | The URL of the Vault server. (e.g., ) | `${VAULT_ADDR}` | -| namespace | string | | Name of the Vault namespace. This is only available in the Vault Enterprise. | `${VAULT_NAMESPACE}` | -| transit_engine_path | string | | Path of the transit engine that stores the keys. | transit | -| ca_cert_path | string | | Path to a CA certificate file used to verify the Vault server certificate. Only PEM format is supported. | `${VAULT_CACERT}` | -| insecure_skip_verify | bool | | If true, vault client accepts any server certificates. Should only be used for test environments. | false | -| cert_auth | struct | | Configuration for the Client Certificate authentication method | | -| token_auth | struct | | Configuration for the Token authentication method | | -| approle_auth | struct | | Configuration for the AppRole authentication method | | -| k8s_auth | struct | | Configuration for the Kubernetes authentication method | | +| key | type | required | description | default | +|:---------------------|:-------|:--------------------------------------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------|:---------------------| +| key_identifier_file | string | Required if key_identifier_value is not set | A file path location where information about generated keys will be persisted. See "[Management of keys](#management-of-keys)" for more information. | "" | +| key_identifier_value | string | Required if key_identifier_file is not set | A static identifier for the SPIRE server instance (used instead of `key_identifier_file`). | "" | +| vault_addr | string | | The URL of the Vault server. (e.g., ) | `${VAULT_ADDR}` | +| namespace | string | | Name of the Vault namespace. This is only available in the Vault Enterprise. | `${VAULT_NAMESPACE}` | +| transit_engine_path | string | | Path of the transit engine that stores the keys. | transit | +| ca_cert_path | string | | Path to a CA certificate file used to verify the Vault server certificate. Only PEM format is supported. | `${VAULT_CACERT}` | +| insecure_skip_verify | bool | | If true, vault client accepts any server certificates. Should only be used for test environments. | false | +| cert_auth | struct | | Configuration for the Client Certificate authentication method | | +| token_auth | struct | | Configuration for the Token authentication method | | +| approle_auth | struct | | Configuration for the AppRole authentication method | | +| k8s_auth | struct | | Configuration for the Kubernetes authentication method | | The plugin supports **Client Certificate**, **Token** and **AppRole** authentication methods. @@ -160,3 +162,20 @@ path "pki/root/sign-intermediate" { } } ``` + +### Management of keys + +The plugin needs a way to identify the specific server instance where it's +running. For that, either the `key_identifier_file` or `key_identifier_value` +setting must be used. Setting a _Key Identifier File_ instructs the plugin to +manage the identifier of the server automatically, storing the server ID in the +specified file. This method should be appropriate for most situations. +If a _Key Identifier File_ is configured and the file is not found during server +startup, the file is recreated with a new auto-generated server ID. +Consequently, if the file is lost, the plugin will not be able to identify keys +that it has previously managed and will recreate new keys on demand. + +If you need more control over the identifier that's used for the server, the +`key_identifier_value` setting can be used to specify a +static identifier for the server instance. This setting is appropriate in situations +where a key identifier file can't be persisted. diff --git a/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go b/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go index e8df07314f..6ce5562dc1 100644 --- a/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go +++ b/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go @@ -7,11 +7,13 @@ import ( "errors" "fmt" "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/gofrs/uuid/v5" "github.com/hashicorp/go-hclog" "github.com/hashicorp/hcl" keymanagerv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/plugin/server/keymanager/v1" configv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/service/common/config/v1" "github.com/spiffe/spire/pkg/common/catalog" + "github.com/spiffe/spire/pkg/common/diskutil" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "os" @@ -51,6 +53,9 @@ type Config struct { // TransitEnginePath specifies the path to the transit engine to perform key operations. TransitEnginePath string `hcl:"transit_engine_path" json:"transit_engine_path"` + KeyIdentifierFile string `hcl:"key_identifier_file" json:"key_identifier_file"` + KeyIdentifierValue string `hcl:"key_identifier_value" json:"key_identifier_value"` + // If true, vault client accepts any server certificates. // It should be used only test environment so on. InsecureSkipVerify bool `hcl:"insecure_skip_verify" json:"insecure_skip_verify"` @@ -118,9 +123,10 @@ type Plugin struct { keymanagerv1.UnsafeKeyManagerServer configv1.UnsafeConfigServer - logger hclog.Logger - mu sync.RWMutex - entries map[string]keyEntry + logger hclog.Logger + serverID string + mu sync.RWMutex + entries map[string]keyEntry authMethod AuthMethod cc *ClientConfig @@ -156,6 +162,17 @@ func (p *Plugin) Configure(ctx context.Context, req *configv1.ConfigureRequest) return nil, status.Errorf(codes.InvalidArgument, "unable to decode configuration: %v", err) } + serverID := config.KeyIdentifierValue + if serverID == "" { + var err error + + if serverID, err = getOrCreateServerID(config.KeyIdentifierFile); err != nil { + return nil, err + } + } + + p.logger.Debug("Loaded server id", "server_id", serverID) + if config.InsecureSkipVerify { p.logger.Warn("TLS verification of Vault certificates is skipped. This is only recommended for test environments.") } @@ -178,6 +195,7 @@ func (p *Plugin) Configure(ctx context.Context, req *configv1.ConfigureRequest) p.authMethod = am p.cc = vcConfig + p.serverID = serverID if p.vc == nil { err := p.genVaultClient() @@ -492,3 +510,58 @@ func (p *Plugin) setCache(keyEntries []*keyEntry) { p.logger.Debug("Key loaded", "key_id", e.PublicKey.Id, "key_type", e.PublicKey.Type) } } + +// generateKeyName returns a new identifier to be used as a key name. +// The returned name has the form: - +// where UUID is a new randomly generated UUID and SPIRE-KEY-ID is provided +// through the spireKeyID parameter. +func (p *Plugin) generateKeyName(spireKeyID string) (keyName string, err error) { + uniqueID, err := generateUniqueID() + if err != nil { + return "", err + } + + return fmt.Sprintf("%s-%s", uniqueID, spireKeyID), nil +} + +// generateUniqueID returns a randomly generated UUID. +func generateUniqueID() (id string, err error) { + u, err := uuid.NewV4() + if err != nil { + return "", status.Errorf(codes.Internal, "could not create a randomly generated UUID: %v", err) + } + + return u.String(), nil +} + +func getOrCreateServerID(idPath string) (string, error) { + data, err := os.ReadFile(idPath) + switch { + case errors.Is(err, os.ErrNotExist): + return createServerID(idPath) + case err != nil: + return "", status.Errorf(codes.Internal, "failed to read server ID from path: %v", err) + } + + serverID, err := uuid.FromString(string(data)) + if err != nil { + return "", status.Errorf(codes.Internal, "failed to parse server ID from path: %v", err) + } + return serverID.String(), nil +} + +// createServerID creates a randomly generated UUID to be used as a server ID +// and stores it in the specified idPath. +func createServerID(idPath string) (string, error) { + id, err := generateUniqueID() + if err != nil { + return "", status.Errorf(codes.Internal, "failed to generate ID for server: %v", err) + } + + err = diskutil.WritePrivateFile(idPath, []byte(id)) + if err != nil { + return "", status.Errorf(codes.Internal, "failed to persist server ID on path: %v", err) + } + + return id, nil +} diff --git a/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault_test.go b/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault_test.go index 5260ac47c5..b06dfc413b 100644 --- a/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault_test.go +++ b/pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault_test.go @@ -8,6 +8,8 @@ import ( "github.com/spiffe/spire/pkg/server/plugin/keymanager" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" + "os" + "path/filepath" "testing" "text/template" @@ -17,6 +19,11 @@ import ( "github.com/spiffe/spire/test/spiretest" ) +const ( + validServerID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + validServerIDFile = "test-server-id" +) + func TestPluginConfigure(t *testing.T) { for _, tt := range []struct { name string @@ -198,8 +205,9 @@ func TestPluginConfigure(t *testing.T) { if tt.plainConfig != "" { plainConfig = tt.plainConfig } else { - plainConfig = getTestConfigureRequest(t, fmt.Sprintf("https://%v/", addr), tt.configTmpl) + plainConfig = getTestConfigureRequest(t, fmt.Sprintf("https://%v/", addr), createKeyIdentifierFile(t), tt.configTmpl) } + plugintest.Load(t, builtin(p), nil, plugintest.CaptureConfigureError(&err), plugintest.Configure(plainConfig), @@ -481,6 +489,7 @@ func TestPluginGenerateKey(t *testing.T) { plugintest.CoreConfig(catalog.CoreConfig{TrustDomain: spiffeid.RequireTrustDomainFromString("example.org")}), } if tt.config != nil { + tt.config.KeyIdentifierFile = createKeyIdentifierFile(t) tt.config.VaultAddr = fmt.Sprintf("https://%s", addr) cp, err := p.genClientParams(tt.authMethod, tt.config) require.NoError(t, err) @@ -667,7 +676,7 @@ func TestPluginGetKey(t *testing.T) { p := New() options := []plugintest.Option{ plugintest.CaptureConfigureError(&err), - plugintest.Configure(getTestConfigureRequest(t, fmt.Sprintf("https://%v/", addr), tt.configTmpl)), + plugintest.Configure(getTestConfigureRequest(t, fmt.Sprintf("https://%v/", addr), createKeyIdentifierFile(t), tt.configTmpl)), plugintest.CoreConfig(catalog.CoreConfig{ TrustDomain: spiffeid.RequireTrustDomainFromString("example.org"), }), @@ -704,11 +713,17 @@ func TestPluginGetKey(t *testing.T) { // TODO: Should the Sign function also be tested? -func getTestConfigureRequest(t *testing.T, addr string, tpl string) string { +func getTestConfigureRequest(t *testing.T, addr string, keyIdentifierFile string, tpl string) string { templ, err := template.New("plugin config").Parse(tpl) require.NoError(t, err) - cp := &struct{ Addr string }{Addr: addr} + cp := &struct { + Addr string + KeyIdentifierFile string + }{ + Addr: addr, + KeyIdentifierFile: keyIdentifierFile, + } var c bytes.Buffer err = templ.Execute(&c, cp) @@ -758,3 +773,14 @@ func setupFakeVaultServer() *FakeVaultServerConfig { fakeVaultServer.RenewResponse = []byte(testRenewResponse) return fakeVaultServer } + +func createKeyIdentifierFile(t *testing.T) string { + tempDir := t.TempDir() + tempFilePath := filepath.ToSlash(filepath.Join(tempDir, validServerIDFile)) + err := os.WriteFile(tempFilePath, []byte(validServerID), 0o600) + if err != nil { + t.Error(err) + } + + return tempFilePath +} diff --git a/pkg/server/plugin/keymanager/hashicorpvault/vault_fake_test.go b/pkg/server/plugin/keymanager/hashicorpvault/vault_fake_test.go index 6806cf0c7b..c63121cb0c 100644 --- a/pkg/server/plugin/keymanager/hashicorpvault/vault_fake_test.go +++ b/pkg/server/plugin/keymanager/hashicorpvault/vault_fake_test.go @@ -23,6 +23,7 @@ const ( var ( testTokenAuthConfigTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" token_auth { @@ -30,11 +31,13 @@ token_auth { }` testTokenAuthConfigWithEnvTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" token_auth {}` testCertAuthConfigTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" cert_auth { @@ -45,6 +48,7 @@ cert_auth { }` testCertAuthConfigWithEnvTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" cert_auth { @@ -52,6 +56,7 @@ cert_auth { }` testAppRoleAuthConfigTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" approle_auth { @@ -61,6 +66,7 @@ approle_auth { }` testAppRoleAuthConfigWithEnvTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" approle_auth { @@ -68,6 +74,7 @@ approle_auth { }` testK8sAuthConfigTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" k8s_auth { @@ -77,6 +84,7 @@ k8s_auth { }` testMultipleAuthConfigsTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" cert_auth {} @@ -87,13 +95,8 @@ approle_auth { approle_secret_id = "test-approle-secret-id" }` - testConfigWithVaultAddrEnvTpl = ` -ca_cert_path = "testdata/root-cert.pem" -token_auth { - token = "test-token" -}` - testConfigWithTransitEnginePathTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" transit_engine_path = "test-path" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" @@ -102,6 +105,7 @@ token_auth { }` testConfigWithTransitEnginePathEnvTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" token_auth { @@ -109,6 +113,7 @@ token_auth { }` testNamespaceConfigTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" namespace = "test-ns" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" @@ -117,6 +122,7 @@ token_auth { }` testNamespaceEnvTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" token_auth { @@ -124,6 +130,7 @@ token_auth { }` testK8sAuthNoRoleNameTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" k8s_auth { @@ -132,6 +139,7 @@ k8s_auth { }` testK8sAuthNoTokenPathTpl = ` +key_identifier_file = "{{ .KeyIdentifierFile }}" vault_addr = "{{ .Addr }}" ca_cert_path = "testdata/root-cert.pem" k8s_auth {