Skip to content

Commit

Permalink
Introduce unique server id for HA setup (spiffe#5058)
Browse files Browse the repository at this point in the history 
Signed-off-by: Matteo Kamm <[email protected]>
  • Loading branch information
@InverseIntegral
InverseIntegral committed Dec 1, 2024
1 parent d8f67f7 commit 426a992
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 24 deletions.
41 changes: 30 additions & 11 deletions doc/plugin_server_keymanager_hashicorp_vault.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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., <https://vault.example.com:8443/>) | `${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., <https://vault.example.com:8443/>) | `${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.

Expand Down Expand Up @@ -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.
79 changes: 76 additions & 3 deletions pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"`
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.")
}
Expand All @@ -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()
Expand Down Expand Up @@ -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: <UUID>-<SPIRE-KEY-ID>
// 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
}
34 changes: 30 additions & 4 deletions pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"),
}),
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
20 changes: 14 additions & 6 deletions pkg/server/plugin/keymanager/hashicorpvault/vault_fake_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,21 @@ const (

var (
testTokenAuthConfigTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
token_auth {
token = "test-token"
}`

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 {
Expand All @@ -45,13 +48,15 @@ cert_auth {
}`

testCertAuthConfigWithEnvTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
cert_auth {
cert_auth_mount_point = "test-cert-auth"
}`

testAppRoleAuthConfigTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
approle_auth {
Expand All @@ -61,13 +66,15 @@ approle_auth {
}`

testAppRoleAuthConfigWithEnvTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
approle_auth {
approle_auth_mount_point = "test-approle-auth"
}`

testK8sAuthConfigTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
k8s_auth {
Expand All @@ -77,6 +84,7 @@ k8s_auth {
}`

testMultipleAuthConfigsTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
cert_auth {}
Expand All @@ -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"
Expand All @@ -102,13 +105,15 @@ token_auth {
}`

testConfigWithTransitEnginePathEnvTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
token_auth {
token = "test-token"
}`

testNamespaceConfigTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
namespace = "test-ns"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
Expand All @@ -117,13 +122,15 @@ token_auth {
}`

testNamespaceEnvTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
token_auth {
token = "test-token"
}`

testK8sAuthNoRoleNameTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
k8s_auth {
Expand All @@ -132,6 +139,7 @@ k8s_auth {
}`

testK8sAuthNoTokenPathTpl = `
key_identifier_file = "{{ .KeyIdentifierFile }}"
vault_addr = "{{ .Addr }}"
ca_cert_path = "testdata/root-cert.pem"
k8s_auth {
Expand Down

0 comments on commit 426a992

Please sign in to comment.