Make key names unique by adding a UUID (spiffe#5058)

Signed-off-by: Matteo Kamm <[email protected]>

Author: InverseIntegral

Diff for: pkg/server/plugin/keymanager/hashicorpvault/hashicorp_vault.go

@@ -36,6 +36,7 @@ func builtin(p *Plugin) catalog.BuiltIn {
 }
 
 type keyEntry struct {
+	KeyName   string
 	PublicKey *keymanagerv1.PublicKey
 }
 
@@ -123,10 +124,11 @@ type Plugin struct {
 	keymanagerv1.UnsafeKeyManagerServer
 	configv1.UnsafeConfigServer
 
-	logger   hclog.Logger
-	serverID string
-	mu       sync.RWMutex
-	entries  map[string]keyEntry
+	logger     hclog.Logger
+	serverID   string
+	mu         sync.RWMutex
+	entries    map[string]keyEntry
+	entriesMtx sync.RWMutex
 
 	authMethod AuthMethod
 	cc         *ClientConfig
@@ -316,7 +318,7 @@ func (p *Plugin) GenerateKey(ctx context.Context, req *keymanagerv1.GenerateKeyR
 		return nil, err
 	}
 
-	p.entries[spireKeyID] = *newKeyEntry
+	p.setKeyEntry(spireKeyID, *newKeyEntry)
 
 	return &keymanagerv1.GenerateKeyResponse{
 		PublicKey: newKeyEntry.PublicKey,
@@ -351,7 +353,7 @@ func (p *Plugin) SignData(ctx context.Context, req *keymanagerv1.SignDataRequest
 		}
 	}
 
-	signature, err := p.vc.SignData(ctx, req.KeyId, req.Data, hashAlgo, signingAlgo)
+	signature, err := p.vc.SignData(ctx, keyEntry.KeyName, req.Data, hashAlgo, signingAlgo)
 	if err != nil {
 		return nil, err
 	}
@@ -451,12 +453,17 @@ func (p *Plugin) createKey(ctx context.Context, spireKeyID string, keyType keyma
 		return nil, err
 	}
 
-	err = p.vc.CreateKey(ctx, spireKeyID, *kt)
+	keyName, err := p.generateKeyName(spireKeyID)
 	if err != nil {
 		return nil, err
 	}
 
-	return p.vc.getKeyEntry(ctx, spireKeyID)
+	err = p.vc.CreateKey(ctx, keyName, *kt)
+	if err != nil {
+		return nil, err
+	}
+
+	return p.vc.getKeyEntry(ctx, keyName)
 }
 
 func convertToTransitKeyType(keyType keymanagerv1.KeyType) (*TransitKeyType, error) {
@@ -502,6 +509,8 @@ func makeFingerprint(pkixData []byte) string {
 
 func (p *Plugin) setCache(keyEntries []*keyEntry) {
 	// clean previous cache
+	p.entriesMtx.Lock()
+	defer p.entriesMtx.Unlock()
 	p.entries = make(map[string]keyEntry)
 
 	// add results to cache
@@ -511,6 +520,23 @@ func (p *Plugin) setCache(keyEntries []*keyEntry) {
 	}
 }
 
+// setKeyEntry adds the entry to the cache that matches the provided SPIRE Key ID
+func (p *Plugin) setKeyEntry(keyID string, ke keyEntry) {
+	p.entriesMtx.Lock()
+	defer p.entriesMtx.Unlock()
+
+	p.entries[keyID] = ke
+}
+
+// getKeyEntry gets the entry from the cache that matches the provided SPIRE Key ID
+func (p *Plugin) getKeyEntry(keyID string) (ke keyEntry, ok bool) {
+	p.entriesMtx.RLock()
+	defer p.entriesMtx.RUnlock()
+
+	ke, ok = p.entries[keyID]
+	return ke, ok
+}
+
 // 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

Diff for: pkg/server/plugin/keymanager/hashicorpvault/vault_client.go

@@ -375,13 +375,13 @@ const (
 
 // CreateKey creates a new key in the specified transit secret engine
 // See: https://developer.hashicorp.com/vault/api-docs/secret/transit#create-key
-func (c *Client) CreateKey(ctx context.Context, spireKeyID string, keyType TransitKeyType) error {
+func (c *Client) CreateKey(ctx context.Context, keyName string, keyType TransitKeyType) error {
 	arguments := map[string]interface{}{
 		"type":       keyType,
 		"exportable": "false", // SPIRE keys are never exportable
 	}
 
-	_, err := c.vaultClient.Logical().WriteWithContext(ctx, fmt.Sprintf("/%s/keys/%s", c.clientParams.TransitEnginePath, spireKeyID), arguments)
+	_, err := c.vaultClient.Logical().WriteWithContext(ctx, fmt.Sprintf("/%s/keys/%s", c.clientParams.TransitEnginePath, keyName), arguments)
 	if err != nil {
 		return status.Errorf(codes.Internal, "failed to create transit engine key: %v", err)
 	}
@@ -391,7 +391,7 @@ func (c *Client) CreateKey(ctx context.Context, spireKeyID string, keyType Trans
 
 // SignData signs the data using the transit engine key with the provided spire key id.
 // See: https://developer.hashicorp.com/vault/api-docs/secret/transit#sign-data
-func (c *Client) SignData(ctx context.Context, spireKeyID string, data []byte, hashAlgo TransitHashAlgorithm, signatureAlgo TransitSignatureAlgorithm) ([]byte, error) {
+func (c *Client) SignData(ctx context.Context, keyName string, data []byte, hashAlgo TransitHashAlgorithm, signatureAlgo TransitSignatureAlgorithm) ([]byte, error) {
 	encodedData := base64.StdEncoding.EncodeToString(data)
 
 	body := map[string]interface{}{
@@ -401,7 +401,7 @@ func (c *Client) SignData(ctx context.Context, spireKeyID string, data []byte, h
 		"prehashed":             "true",
 	}
 
-	sigResp, err := c.vaultClient.Logical().WriteWithContext(ctx, fmt.Sprintf("/%s/sign/%s/%s", c.clientParams.TransitEnginePath, spireKeyID, hashAlgo), body)
+	sigResp, err := c.vaultClient.Logical().WriteWithContext(ctx, fmt.Sprintf("/%s/sign/%s/%s", c.clientParams.TransitEnginePath, keyName, hashAlgo), body)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "transit engine sign call failed: %v", err)
 	}
@@ -449,18 +449,18 @@ func (c *Client) GetKeys(ctx context.Context) ([]*keyEntry, error) {
 		return nil, status.Errorf(codes.Internal, "transit engine list keys call was successful but keys are missing")
 	}
 
-	keyIds, ok := keys.([]interface{})
+	keyNames, ok := keys.([]interface{})
 	if !ok {
-		return nil, status.Errorf(codes.Internal, "expected keys data type %T but got %T", keyIds, keys)
+		return nil, status.Errorf(codes.Internal, "expected keys data type %T but got %T", keyNames, keys)
 	}
 
-	for _, keyId := range keyIds {
-		keyIdStr, ok := keyId.(string)
+	for _, keyName := range keyNames {
+		keyNameStr, ok := keyName.(string)
 		if !ok {
-			return nil, status.Errorf(codes.Internal, "expected key id data type %T but got %T", keyIdStr, keyId)
+			return nil, status.Errorf(codes.Internal, "expected key id data type %T but got %T", keyNameStr, keyName)
 		}
 
-		keyEntry, err := c.getKeyEntry(ctx, keyIdStr)
+		keyEntry, err := c.getKeyEntry(ctx, keyNameStr)
 		if err != nil {
 			return nil, err
 		}
@@ -471,9 +471,14 @@ func (c *Client) GetKeys(ctx context.Context) ([]*keyEntry, error) {
 	return keyEntries, nil
 }
 
-// getKeyEntry gets the transit engine key with the specified spire key id and converts it into a key entry.
-func (c *Client) getKeyEntry(ctx context.Context, spireKeyID string) (*keyEntry, error) {
-	keyData, err := c.getKey(ctx, spireKeyID)
+// getKeyEntry gets the transit engine key with the specified key name and converts it into a key entry.
+func (c *Client) getKeyEntry(ctx context.Context, keyName string) (*keyEntry, error) {
+	spireKeyID, ok := spireKeyIDFromKeyName(keyName)
+	if !ok {
+		return nil, status.Errorf(codes.Internal, "unable to get SPIRE key ID from key %s", keyName)
+	}
+
+	keyData, err := c.getKey(ctx, keyName)
 	if err != nil {
 		return nil, err
 	}
@@ -519,6 +524,7 @@ func (c *Client) getKeyEntry(ctx context.Context, spireKeyID string) (*keyEntry,
 	}
 
 	return &keyEntry{
+		KeyName: keyName,
 		PublicKey: &keymanagerv1.PublicKey{
 			Id:          spireKeyID,
 			Type:        keyType,
@@ -530,8 +536,8 @@ func (c *Client) getKeyEntry(ctx context.Context, spireKeyID string) (*keyEntry,
 
 // getKey returns a specific key from the transit engine.
 // See: https://developer.hashicorp.com/vault/api-docs/secret/transit#read-key
-func (c *Client) getKey(ctx context.Context, spireKeyID string) (map[string]interface{}, error) {
-	res, err := c.vaultClient.Logical().ReadWithContext(ctx, fmt.Sprintf("/%s/keys/%s", c.clientParams.TransitEnginePath, spireKeyID))
+func (c *Client) getKey(ctx context.Context, keyName string) (map[string]interface{}, error) {
+	res, err := c.vaultClient.Logical().ReadWithContext(ctx, fmt.Sprintf("/%s/keys/%s", c.clientParams.TransitEnginePath, keyName))
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to get transit engine key: %v", err)
 	}
@@ -558,3 +564,17 @@ func (c *Client) getKey(ctx context.Context, spireKeyID string) (map[string]inte
 
 	return currentKeyMap, nil
 }
+
+// spireKeyIDFromKeyName parses a Key Vault key name to get the
+// SPIRE Key ID. This Key ID is used in the Server KeyManager interface.
+func spireKeyIDFromKeyName(keyName string) (string, bool) {
+	// A key name would have the format <UUID>-<SPIRE-KEY-ID>.
+	// first we find the position where the SPIRE Key ID starts.
+	spireKeyIDIndex := 37 // 36 is the UUID length plus one '-' separator
+	if spireKeyIDIndex >= len(keyName) {
+		// The index is out of range.
+		return "", false
+	}
+	spireKeyID := keyName[spireKeyIDIndex:]
+	return spireKeyID, true
+}

Diff for: pkg/server/plugin/keymanager/hashicorpvault/vault_client_test.go

@@ -913,7 +913,7 @@ func TestGetKeyEntry(t *testing.T) {
 	client, err := cc.NewAuthenticatedClient(CERT, renewCh)
 	require.NoError(t, err)
 
-	resp, err := client.getKeyEntry(context.Background(), "x509-CA-A")
+	resp, err := client.getKeyEntry(context.Background(), "ab748227-3a10-40cc-87fd-2a5321aa638d-x509-CA-A")
 	require.NoError(t, err)
 
 	block, _ := pem.Decode([]byte("-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEV57LFbIQZzyZ2YcKZfB9mGWkUhJv\niRzIZOqV4wRHoUOZjMuhBMR2WviEsy65TYpcBjreAc6pbneiyhlTwPvgmw==\n-----END PUBLIC KEY-----\n"))
@@ -951,7 +951,7 @@ func TestGetKeyEntryErrorFromEndpoint(t *testing.T) {
 	client, err := cc.NewAuthenticatedClient(CERT, renewCh)
 	require.NoError(t, err)
 
-	resp, err := client.getKeyEntry(context.Background(), "x509-CA-A")
+	resp, err := client.getKeyEntry(context.Background(), "ab748227-3a10-40cc-87fd-2a5321aa638d-x509-CA-A")
 	spiretest.RequireGRPCStatusHasPrefix(t, err, codes.Internal, "failed to get transit engine key: Error making API request.")
 	require.Empty(t, resp)
 }

Diff for: pkg/server/plugin/keymanager/hashicorpvault/vault_fake_test.go

@@ -403,7 +403,7 @@ k8s_auth {
   "lease_duration": 0,
   "data": {
     "keys": [
-      "x509-CA-A"
+      "ab748227-3a10-40cc-87fd-2a5321aa638d-x509-CA-A"
     ]
   },
   "wrap_info": null,
@@ -447,7 +447,7 @@ k8s_auth {
     "min_available_version": 0,
     "min_decryption_version": 1,
     "min_encryption_version": 0,
-    "name": "x509-CA-A",
+    "name": "ab748227-3a10-40cc-87fd-2a5321aa638d-x509-CA-A",
     "supports_decryption": false,
     "supports_derivation": false,
     "supports_encryption": false,
@@ -482,7 +482,7 @@ k8s_auth {
     "min_available_version": 0,
     "min_decryption_version": 1,
     "min_encryption_version": 0,
-    "name": "x509-CA-A",
+    "name": "ab748227-3a10-40cc-87fd-2a5321aa638d-x509-CA-A",
     "supports_decryption": false,
     "supports_derivation": false,
     "supports_encryption": false,
@@ -517,7 +517,7 @@ k8s_auth {
     "min_available_version": 0,
     "min_decryption_version": 1,
     "min_encryption_version": 0,
-    "name": "x509-CA-A",
+    "name": "ab748227-3a10-40cc-87fd-2a5321aa638d-x509-CA-A",
     "supports_decryption": true,
     "supports_derivation": false,
     "supports_encryption": true,
@@ -552,7 +552,7 @@ k8s_auth {
     "min_available_version": 0,
     "min_decryption_version": 1,
     "min_encryption_version": 0,
-    "name": "x509-CA-A",
+    "name": "ab748227-3a10-40cc-87fd-2a5321aa638d-x509-CA-A",
     "supports_decryption": true,
     "supports_derivation": false,
     "supports_encryption": true,
@@ -587,7 +587,7 @@ k8s_auth {
     "min_available_version": 0,
     "min_decryption_version": 1,
     "min_encryption_version": 0,
-    "name": "x509-CA-A",
+    "name": "ab748227-3a10-40cc-87fd-2a5321aa638d-x509-CA-A",
     "supports_decryption": true,
     "supports_derivation": false,
     "supports_encryption": true,