rename encryption.Key.ID() to Version() & return structured version data (#19706)

* add Version to mounted key config

* rename encryption.Key.ID() to Version() & return structured version data

* update external services & accounts to use Key.Version()

* add TODO for json schema generation of KeyVersion.Type

* fix string conversion lint error

internal/database/external_accounts.go

@@ -475,10 +475,11 @@ func MaybeEncrypt(ctx context.Context, key encryption.Key, data string) (maybeEn
 			return "", "", err
 		}
 		data = string(encrypted)
-		keyIdent, err = key.ID(ctx)
+		version, err := key.Version(ctx)
 		if err != nil {
 			return "", "", err
 		}
+		keyIdent = version.JSON()
 	}
 
 	return data, keyIdent, nil

internal/database/external_services.go

@@ -567,8 +567,8 @@ func (e *ExternalServiceStore) Create(ctx context.Context, confGet func() *conf.
 func (e *ExternalServiceStore) maybeEncryptConfig(ctx context.Context, config string) (string, string, error) {
 	// encrypt the config before writing if we have a key configured
 	var (
-		keyID string
-		key   = keyring.Default().ExternalServiceKey
+		keyVersion string
+		key        = keyring.Default().ExternalServiceKey
 	)
 	if e.key != nil {
 		key = e.key
@@ -579,12 +579,13 @@ func (e *ExternalServiceStore) maybeEncryptConfig(ctx context.Context, config st
 			return "", "", err
 		}
 		config = string(encrypted)
-		keyID, err = key.ID(ctx)
+		version, err := key.Version(ctx)
 		if err != nil {
 			return "", "", err
 		}
+		keyVersion = version.JSON()
 	}
-	return config, keyID, nil
+	return config, keyVersion, nil
 }
 
 func (e *ExternalServiceStore) maybeDecryptConfig(ctx context.Context, config string, keyID string) (string, error) {

internal/database/external_services_test.go

@@ -1476,6 +1476,6 @@ func (k testKey) Decrypt(ctx context.Context, ciphertext []byte) (*encryption.Se
 	return &s, err
 }
 
-func (k testKey) ID(ctx context.Context) (string, error) {
-	return "testkey", nil
+func (k testKey) Version(ctx context.Context) (encryption.KeyVersion, error) {
+	return encryption.KeyVersion{Type: "testkey"}, nil
 }

internal/database/oob_migrate.go

@@ -82,10 +82,11 @@ func (m *ExternalServiceConfigMigrator) Up(ctx context.Context) (err error) {
 			return err
 		}
 
-		keyIdent, err := key.ID(ctx)
+		version, err := key.Version(ctx)
 		if err != nil {
 			return err
 		}
+		keyIdent := version.JSON()
 
 		// ensure encryption round-trip is valid with keyIdent
 		decrypted, err := key.Decrypt(ctx, encryptedCfg)
@@ -231,11 +232,13 @@ func (m *ExternalAccountsMigrator) Up(ctx context.Context) (err error) {
 		return nil
 	}
 
-	keyIdent, err := key.ID(ctx)
+	version, err := key.Version(ctx)
 	if err != nil {
 		return err
 	}
 
+	keyIdent := version.JSON()
+
 	tx, err := m.store.Transact(ctx)
 	if err != nil {
 		return err

internal/database/oob_migrate_test.go

@@ -168,8 +168,8 @@ func TestExternalServiceConfigMigrator(t *testing.T) {
 				t.Fatalf("decrypted config is different from the original one")
 			}
 
-			if id, _ := key.ID(ctx); keyID != id {
-				t.Fatalf("wrong encryption_key_id, want %s, got %s", id, keyID)
+			if version, _ := key.Version(ctx); keyID != version.JSON() {
+				t.Fatalf("wrong encryption_key_id, want %s, got %s", version.JSON(), keyID)
 			}
 
 			i++
@@ -285,8 +285,8 @@ func (k invalidKey) Decrypt(ctx context.Context, ciphertext []byte) (*encryption
 	return &s, nil
 }
 
-func (k invalidKey) ID(ctx context.Context) (string, error) {
-	return "invalidKey", nil
+func (k invalidKey) Version(ctx context.Context) (encryption.KeyVersion, error) {
+	return encryption.KeyVersion{Type: "invalidkey"}, nil
 }
 
 func TestExternalAccountsMigrator(t *testing.T) {
@@ -452,8 +452,8 @@ func TestExternalAccountsMigrator(t *testing.T) {
 				t.Fatalf("decrypted data is different from the original one")
 			}
 
-			if id, _ := key.ID(ctx); keyID != id {
-				t.Fatalf("wrong encryption_key_id, want %s, got %s", id, keyID)
+			if version, _ := key.Version(ctx); keyID != version.JSON() {
+				t.Fatalf("wrong encryption_key_id, want %s, got %s", version.JSON(), keyID)
 			}
 
 			i++

internal/encryption/cloudkms/cloud_kms.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
-	"fmt"
 	"hash/crc32"
 	"strings"
 
@@ -25,7 +24,7 @@ func NewKey(ctx context.Context, keyName string) (encryption.Key, error) {
 		name:   keyName,
 		client: client,
 	}
-	_, err = k.ID(ctx)
+	_, err = k.Version(ctx)
 	return k, err
 }
 
@@ -34,16 +33,20 @@ type Key struct {
 	client *kms.KeyManagementClient
 }
 
-func (k *Key) ID(ctx context.Context) (string, error) {
+func (k *Key) Version(ctx context.Context) (encryption.KeyVersion, error) {
 	key, err := k.client.GetCryptoKey(ctx, &kmspb.GetCryptoKeyRequest{
 		Name: k.name,
 	})
 	if err != nil {
-		return "", errors.Wrap(err, "getting key ident")
+		return encryption.KeyVersion{}, errors.Wrap(err, "getting key version")
 	}
 	// return the primary key version name, as that will include which key
 	// revision is currently in use
-	return fmt.Sprintf("cloudkms:%s", key.Primary.Name), nil
+	return encryption.KeyVersion{
+		Type:    "cloudkms",
+		Version: key.Primary.Name,
+		Name:    key.Name,
+	}, nil
 }
 
 // Decrypt a secret, it must have been encrypted with the same Key

internal/encryption/key.go

@@ -1,15 +1,30 @@
 package encryption
 
-import "context"
+import (
+	"context"
+	"encoding/json"
+)
 
 // Key combines the Encrypter & Decrypter interfaces.
 type Key interface {
 	Encrypter
 	Decrypter
 
-	// ID returns an identifier string containing anything to concretely identify
+	// Version returns info containing to concretely identify
 	// the underlying key, eg: key type, name, & version.
-	ID(ctx context.Context) (string, error)
+	Version(ctx context.Context) (KeyVersion, error)
+}
+
+type KeyVersion struct {
+	// TODO: generate this as an enum from JSONSchema
+	Type    string
+	Name    string
+	Version string
+}
+
+func (v KeyVersion) JSON() string {
+	buf, _ := json.Marshal(v)
+	return string(buf)
 }
 
 // Encrypter is anything that can encrypt a value

internal/encryption/mounted/key.go

@@ -13,21 +13,57 @@ import (
 	"os"
 	"strings"
 
+	"github.com/sourcegraph/sourcegraph/schema"
+
 	"github.com/cockroachdb/errors"
 
 	"github.com/sourcegraph/sourcegraph/internal/encryption"
-	"github.com/sourcegraph/sourcegraph/schema"
 )
 
+func NewKey(ctx context.Context, k schema.MountedEncryptionKey) (*Key, error) {
+	var secret []byte
+	if k.EnvVarName != "" && k.Filepath == "" {
+		secret = []byte(os.Getenv(k.EnvVarName))
+
+	} else if k.Filepath != "" && k.EnvVarName == "" {
+		keyBytes, err := os.ReadFile(k.Filepath)
+		if err != nil {
+			return nil, errors.Errorf("error reading secret file for %q: %v", k.Keyname, err)
+		}
+		secret = keyBytes
+	} else {
+		// Either the user has set none of EnvVarName or Filepath or both in their config. Either way we return an error.
+		return nil, errors.Errorf(
+			"must use only one of EnvVarName and Filepath, EnvVarName: %q, Filepath: %q",
+			k.EnvVarName, k.Filepath,
+		)
+	}
+
+	if len(secret) != 32 {
+		return nil, fmt.Errorf("invalid key length: %d, expected 32 bytes", len(secret))
+	}
+
+	return &Key{
+		keyname: k.Keyname,
+		version: k.Version,
+		secret:  secret,
+	}, nil
+}
+
 // Key is an encryption.Key implementation that uses AES GCM encryption, using a
 // secret loaded either from an env var or a file
 type Key struct {
 	keyname string
 	secret  []byte
+	version string
 }
 
-func (k *Key) ID(ctx context.Context) (string, error) {
-	return k.keyname, nil
+func (k *Key) Version(ctx context.Context) (encryption.KeyVersion, error) {
+	return encryption.KeyVersion{
+		Type:    "mounted",
+		Name:    k.keyname,
+		Version: k.version,
+	}, nil
 }
 
 func (k *Key) Encrypt(ctx context.Context, plaintext []byte) ([]byte, error) {
@@ -103,35 +139,6 @@ func (k *Key) Decrypt(ctx context.Context, ciphertext []byte) (*encryption.Secre
 	return &s, nil
 }
 
-func NewKey(ctx context.Context, k schema.MountedEncryptionKey) (*Key, error) {
-	var secret []byte
-	if k.EnvVarName != "" && k.Filepath == "" {
-		secret = []byte(os.Getenv(k.EnvVarName))
-
-	} else if k.Filepath != "" && k.EnvVarName == "" {
-		keyBytes, err := os.ReadFile(k.Filepath)
-		if err != nil {
-			return nil, errors.Errorf("error reading secret file for %q: %v", k.Keyname, err)
-		}
-		secret = keyBytes
-	} else {
-		// Either the user has set none of EnvVarName or Filepath or both in their config. Either way we return an error.
-		return nil, errors.Errorf(
-			"must use only one of EnvVarName and Filepath, EnvVarName: %q, Filepath: %q",
-			k.EnvVarName, k.Filepath,
-		)
-	}
-
-	if len(secret) != 32 {
-		return nil, fmt.Errorf("invalid key length: %d, expected 32 bytes", len(secret))
-	}
-
-	return &Key{
-		keyname: k.Keyname,
-		secret:  secret,
-	}, nil
-}
-
 type encryptedValue struct {
 	KeyName    string
 	Ciphertext []byte

internal/encryption/noop.go

@@ -1,13 +1,19 @@
 package encryption
 
-import "context"
+import (
+	"context"
+)
 
 var _ Key = &NoopKey{}
 
 type NoopKey struct{}
 
-func (k *NoopKey) ID(ctx context.Context) (string, error) {
-	return "no-op", nil
+func (k *NoopKey) Version(ctx context.Context) (KeyVersion, error) {
+	return KeyVersion{
+		Type:    "noop",
+		Name:    "noop",
+		Version: "",
+	}, nil
 }
 
 func (k *NoopKey) Encrypt(ctx context.Context, plaintext []byte) ([]byte, error) {

schema/schema.go

@@ -839,6 +839,7 @@ type MountedEncryptionKey struct {
 	Filepath   string `json:"filepath,omitempty"`
 	Keyname    string `json:"keyname"`
 	Type       string `json:"type"`
+	Version    string `json:"version,omitempty"`
 }
 
 // NoOpEncryptionKey description: This encryption key is a no op, leaving your data in plaintext (not recommended).

schema/site.schema.json

@@ -1405,6 +1405,9 @@
         },
         "envVarName": {
           "type": "string"
+        },
+        "version": {
+          "type": "string"
         }
       }
     },