From 9e1e338c5b1c48fb676aec2129a3cce10d9e0d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=A0tibran=C3=BD?= Date: Wed, 12 Mar 2025 15:40:39 +0100 Subject: [PATCH] signingkeys: Change PrivateKey struct type to string, to match TEXT type of SQL column in signing_keys table. (#102016) --- pkg/services/signingkeys/signingkeys.go | 2 +- .../signingkeys/signingkeysimpl/service.go | 10 ++++------ .../signingkeys/signingkeysimpl/service_test.go | 7 ++++--- pkg/services/signingkeys/signingkeystore/store.go | 2 +- .../signingkeys/signingkeystore/store_test.go | 14 +++++++------- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/pkg/services/signingkeys/signingkeys.go b/pkg/services/signingkeys/signingkeys.go index f5678829886..1f5b370e0c7 100644 --- a/pkg/services/signingkeys/signingkeys.go +++ b/pkg/services/signingkeys/signingkeys.go @@ -30,7 +30,7 @@ type Service interface { type SigningKey struct { KeyID string `xorm:"key_id"` - PrivateKey []byte `xorm:"private_key"` + PrivateKey string `xorm:"private_key"` AddedAt time.Time `xorm:"added_at"` ExpiresAt *time.Time `xorm:"expires_at"` Alg jose.SignatureAlgorithm `xorm:"alg"` diff --git a/pkg/services/signingkeys/signingkeysimpl/service.go b/pkg/services/signingkeys/signingkeysimpl/service.go index edd4bd11a37..b907392cf80 100644 --- a/pkg/services/signingkeys/signingkeysimpl/service.go +++ b/pkg/services/signingkeys/signingkeysimpl/service.go @@ -1,7 +1,6 @@ package signingkeysimpl import ( - "bytes" "context" "crypto" "crypto/ecdsa" @@ -182,7 +181,7 @@ func (s *Service) addPrivateKey(ctx context.Context, keyID string, alg jose.Sign expiry := now.Add(30 * 24 * time.Hour) key, err := s.store.Add(ctx, &signingkeys.SigningKey{ KeyID: keyID, - PrivateKey: encoded, + PrivateKey: string(encoded), ExpiresAt: &expiry, Alg: alg, AddedAt: now, @@ -232,7 +231,7 @@ func (s *Service) encodePrivateKey(ctx context.Context, privateKey crypto.Signer return encoded, nil } -func (s *Service) decodePrivateKey(ctx context.Context, privateKey []byte) (crypto.Signer, error) { +func (s *Service) decodePrivateKey(ctx context.Context, privateKey string) (crypto.Signer, error) { // Bail out if empty string since it'll cause a segfault in Decrypt if len(privateKey) == 0 { return nil, errors.New("private key is empty") @@ -240,10 +239,9 @@ func (s *Service) decodePrivateKey(ctx context.Context, privateKey []byte) (cryp // Backwards compatibility with old base64 encoding // Can be removed in the future - privateKey = bytes.TrimRight(privateKey, "=") + privateKey = strings.TrimRight(privateKey, "=") - payload := make([]byte, base64.RawStdEncoding.DecodedLen(len(privateKey))) - _, err := base64.RawStdEncoding.Decode(payload, privateKey) + payload, err := base64.RawStdEncoding.DecodeString(privateKey) if err != nil { return nil, err } diff --git a/pkg/services/signingkeys/signingkeysimpl/service_test.go b/pkg/services/signingkeys/signingkeysimpl/service_test.go index 8bca186d0b9..6b36ae91da7 100644 --- a/pkg/services/signingkeys/signingkeysimpl/service_test.go +++ b/pkg/services/signingkeys/signingkeysimpl/service_test.go @@ -13,10 +13,11 @@ import ( "time" "github.com/go-jose/go-jose/v3" - "github.com/grafana/grafana/pkg/services/signingkeys" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/services/signingkeys" + "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" @@ -34,14 +35,14 @@ ielIkb6/Ys51o7KjHxtANhPesw== -----END PRIVATE KEY-----` ) -func getPrivateKey(t *testing.T, svc *Service) []byte { +func getPrivateKey(t *testing.T, svc *Service) string { pemBlock, _ := pem.Decode([]byte(privateKeyPem)) privateKey, err := x509.ParsePKCS8PrivateKey(pemBlock.Bytes) require.NoError(t, err) bytes, err := svc.encodePrivateKey(context.Background(), privateKey.(*ecdsa.PrivateKey)) require.NoError(t, err) - return bytes + return string(bytes) } func TestEmbeddedKeyService_GetJWKS_OnlyPublicKeyShared(t *testing.T) { diff --git a/pkg/services/signingkeys/signingkeystore/store.go b/pkg/services/signingkeys/signingkeystore/store.go index 0c01a2850a1..05aad18d3e5 100644 --- a/pkg/services/signingkeys/signingkeystore/store.go +++ b/pkg/services/signingkeys/signingkeystore/store.go @@ -39,7 +39,7 @@ type Store struct { type SigningKey struct { ID int64 `json:"-" xorm:"id" db:"id"` KeyID string `json:"key_id" xorm:"key_id" db:"key_id"` - PrivateKey []byte `json:"private_key" xorm:"private_key" db:"private_key"` + PrivateKey string `json:"private_key" xorm:"private_key" db:"private_key"` AddedAt time.Time `json:"added_at" xorm:"added_at" db:"added_at"` ExpiresAt *time.Time `json:"expires_at" xorm:"expires_at" db:"expires_at"` Alg jose.SignatureAlgorithm `json:"alg" xorm:"alg" db:"alg"` diff --git a/pkg/services/signingkeys/signingkeystore/store_test.go b/pkg/services/signingkeys/signingkeystore/store_test.go index 2cb8cadc60c..92ad386af7f 100644 --- a/pkg/services/signingkeys/signingkeystore/store_test.go +++ b/pkg/services/signingkeys/signingkeystore/store_test.go @@ -25,37 +25,37 @@ func TestIntegrationSigningKeyStore(t *testing.T) { ctx, store := context.Background(), NewSigningKeyStore(db.InitTestDB(t)) t.Run("Should successfully add new singing key", func(_ *testing.T) { - key, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "1", AddedAt: time.Now().UTC(), PrivateKey: []byte{}}, false) + key, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "1", AddedAt: time.Now().UTC(), PrivateKey: ""}, false) require.NoError(t, err) assert.Equal(t, "1", key.KeyID) }) t.Run("Should return old key if already exists", func(_ *testing.T) { // try to add the same key again with a different AddedAt - key2, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "1", PrivateKey: []byte{}, AddedAt: time.Now().Add(10 * time.Minute).UTC()}, false) + key2, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "1", PrivateKey: "", AddedAt: time.Now().Add(10 * time.Minute).UTC()}, false) require.ErrorIs(t, err, signingkeys.ErrSigningKeyAlreadyExists) assert.Equal(t, "1", key2.KeyID) }) t.Run("Should update old key when force is true", func(t *testing.T) { - key, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "2", PrivateKey: []byte{}, AddedAt: time.Now().UTC()}, false) + key, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "2", PrivateKey: "", AddedAt: time.Now().UTC()}, false) require.NoError(t, err) assert.Equal(t, "2", key.KeyID) // try to add the same key again with a different AddedAt and force is true - key2, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "2", PrivateKey: []byte{}, AddedAt: time.Now().Add(10 * time.Minute).UTC()}, true) + key2, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "2", PrivateKey: "", AddedAt: time.Now().Add(10 * time.Minute).UTC()}, true) require.NoError(t, err) assert.Equal(t, "2", key2.KeyID) assert.NotEqual(t, key.AddedAt, key2.AddedAt) }) t.Run("Should update old key when expired", func(t *testing.T) { - key, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "3", PrivateKey: []byte{}, AddedAt: time.Now().UTC(), ExpiresAt: &time.Time{}}, false) + key, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "3", PrivateKey: "", AddedAt: time.Now().UTC(), ExpiresAt: &time.Time{}}, false) require.NoError(t, err) assert.Equal(t, "3", key.KeyID) // try to add the same key again with a different AddedAt and force is false - key2, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "3", PrivateKey: []byte{}, AddedAt: time.Now().Add(10 * time.Minute).UTC()}, false) + key2, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "3", PrivateKey: "", AddedAt: time.Now().Add(10 * time.Minute).UTC()}, false) require.NoError(t, err) assert.Equal(t, "3", key2.KeyID) assert.NotEqual(t, key.AddedAt, key2.AddedAt) @@ -63,7 +63,7 @@ func TestIntegrationSigningKeyStore(t *testing.T) { t.Run("List should return all keys that are not expired", func(t *testing.T) { // expire key 3 - _, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "3", PrivateKey: []byte{}, AddedAt: time.Now().UTC(), ExpiresAt: &time.Time{}}, true) + _, err := store.Add(ctx, &signingkeys.SigningKey{KeyID: "3", PrivateKey: "", AddedAt: time.Now().UTC(), ExpiresAt: &time.Time{}}, true) require.NoError(t, err) keys, err := store.List(ctx)