Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-Recipient encryption for client credentials #262

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Multi-Recipient encryption for client credentials #262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
650a501
Multi-Recipient encryption
mj-palanker Dec 10, 2024
6fab38d
make linter happy
mj-palanker Dec 11, 2024
8c86259
temp
mj-palanker Dec 19, 2024
d25ce74
Introduce key_ids as replacement for key_id in EncryptedData (#273)
mj-palanker Dec 19, 2024
36cab1d
update for new proto
mj-palanker Dec 19, 2024
8b569b9
Merge branch 'main' into mjp/age-multi-recipient
mj-palanker Dec 19, 2024
5a9e19d
need to take thumb after determining type
mj-palanker Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/connectorbuilder/connectorbuilder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ func (b *builderImpl) RotateCredential(ctx context.Context, request *v2.RotateCr
return nil, fmt.Errorf("error: rotate credentials on resource failed: %w", err)
}

pkem, err := crypto.NewEncryptionManager(request.GetCredentialOptions(), request.GetEncryptionConfigs())
pkem, err := crypto.NewEncryptionManager(ctx, request.GetCredentialOptions(), request.GetEncryptionConfigs())
if err != nil {
l.Error("error: creating encryption manager failed", zap.Error(err))
b.m.RecordTaskFailure(ctx, tt, b.nowFunc().Sub(start))
Expand Down Expand Up @@ -889,7 +889,7 @@ func (b *builderImpl) CreateAccount(ctx context.Context, request *v2.CreateAccou
return nil, fmt.Errorf("error: create account failed: %w", err)
}

pkem, err := crypto.NewEncryptionManager(request.GetCredentialOptions(), request.GetEncryptionConfigs())
pkem, err := crypto.NewEncryptionManager(ctx, request.GetCredentialOptions(), request.GetEncryptionConfigs())
if err != nil {
l.Error("error: creating encryption manager failed", zap.Error(err))
b.m.RecordTaskFailure(ctx, tt, b.nowFunc().Sub(start))
Expand Down
31 changes: 21 additions & 10 deletions pkg/crypto/crypto.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,36 +15,47 @@ type PlaintextCredential struct {
}

type EncryptionManager struct {
opts *v2.CredentialOptions
configs []*v2.EncryptionConfig
opts *v2.CredentialOptions
providerConfigsMap map[string]([]*v2.EncryptionConfig)
}

// FIXME(morgabra) Be tolerant of failures here and return the encryptions that succeeded. We've likely already
// done things to generate the credentials we want to encrypt, so we should still return the created objects
// even if your encryption provider is misconfigured.
func (pkem *EncryptionManager) Encrypt(ctx context.Context, cred *v2.PlaintextData) ([]*v2.EncryptedData, error) {
encryptedDatas := make([]*v2.EncryptedData, 0, len(pkem.configs))
encryptedDatas := make([]*v2.EncryptedData, 0)

for _, config := range pkem.configs {
provider, err := providers.GetEncryptionProviderForConfig(ctx, config)
for providerName, configs := range pkem.providerConfigsMap {
provider, err := providers.GetEncryptionProvider(providerName)
if err != nil {
return nil, err
}

encryptedData, err := provider.Encrypt(ctx, config, cred)
encryptedData, err := provider.Encrypt(ctx, configs, cred)
if err != nil {
return nil, err
}

encryptedDatas = append(encryptedDatas, encryptedData)
encryptedDatas = append(encryptedDatas, encryptedData...)
}
return encryptedDatas, nil
}

func NewEncryptionManager(co *v2.CredentialOptions, ec []*v2.EncryptionConfig) (*EncryptionManager, error) {
// MJP creating the providerMap means parsing the configs and failing early instead of in Encrypt.
func NewEncryptionManager(ctx context.Context, co *v2.CredentialOptions, ec []*v2.EncryptionConfig) (*EncryptionManager, error) {
// Group the encryption configs by provider
providerMap := make(map[string]([]*v2.EncryptionConfig))
for _, config := range ec {
providerName, err := providers.GetEncryptionProviderName(ctx, config)
if err != nil {
return nil, err
}
providerMap[providerName] = append(providerMap[providerName], config)
}

em := &EncryptionManager{
opts: co,
configs: ec,
opts: co,
providerConfigsMap: providerMap,
}
return em, nil
}
76 changes: 74 additions & 2 deletions pkg/crypto/crypto_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,76 @@ func marshalJWK(t *testing.T, privKey interface{}) (*v2.EncryptionConfig, []byte
return config, privJWKBytes
}

func TestMultiRecipientEncrypton(t *testing.T) {
ctx := context.Background()
provider, err := providers.GetEncryptionProvider(jwk.EncryptionProviderJwk)
require.NoError(t, err)

config, key1, err := provider.GenerateKey(ctx)
require.NoError(t, err)
config2, key2, err := provider.GenerateKey(ctx)
require.NoError(t, err)
config3, key3, err := provider.GenerateKey(ctx)
require.NoError(t, err)

// try with an RSA key as well
privKey, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(t, err)
config4, key4 := marshalJWK(t, privKey)

plainText := &v2.PlaintextData{
Name: "password",
Description: "this is the password",
Schema: "",
Bytes: []byte("hunter2"),
}

cipherTexts, err := provider.Encrypt(ctx, []*v2.EncryptionConfig{config, config2, config3, config4}, plainText)
require.NoError(t, err)
require.Len(t, cipherTexts, 1)

cipherText := cipherTexts[0]
require.Equal(t, plainText.Name, cipherText.Name)
require.Equal(t, plainText.Description, cipherText.Description)
require.Equal(t, plainText.Schema, cipherText.Schema)
require.NotEqual(t, plainText.Bytes, cipherText.EncryptedBytes)
require.Greater(t, len(cipherText.EncryptedBytes), len(plainText.Bytes))

decryptedText, err := provider.Decrypt(ctx, cipherText, key1)
require.NoError(t, err)
require.Equal(t, plainText.Name, decryptedText.Name)
require.Equal(t, plainText.Description, decryptedText.Description)
require.Equal(t, plainText.Schema, decryptedText.Schema)
require.Equal(t, plainText.Bytes, decryptedText.Bytes)

decryptedText, err = provider.Decrypt(ctx, cipherText, key2)
require.NoError(t, err)
require.Equal(t, plainText.Name, decryptedText.Name)
require.Equal(t, plainText.Description, decryptedText.Description)
require.Equal(t, plainText.Schema, decryptedText.Schema)
require.Equal(t, plainText.Bytes, decryptedText.Bytes)

decryptedText, err = provider.Decrypt(ctx, cipherText, key3)
require.NoError(t, err)
require.Equal(t, plainText.Name, decryptedText.Name)
require.Equal(t, plainText.Description, decryptedText.Description)
require.Equal(t, plainText.Schema, decryptedText.Schema)
require.Equal(t, plainText.Bytes, decryptedText.Bytes)

decryptedText, err = provider.Decrypt(ctx, cipherText, key4)
require.NoError(t, err)
require.Equal(t, plainText.Name, decryptedText.Name)
require.Equal(t, plainText.Description, decryptedText.Description)
require.Equal(t, plainText.Schema, decryptedText.Schema)
require.Equal(t, plainText.Bytes, decryptedText.Bytes)

// but some random new key shouldn't work
_, badKey, err := provider.GenerateKey(ctx)
require.NoError(t, err)
_, err = provider.Decrypt(ctx, cipherText, badKey)
require.Error(t, err)
}

func testEncryptionProvider(t *testing.T, ctx context.Context, config *v2.EncryptionConfig, privKey []byte) {
provider, err := providers.GetEncryptionProvider(jwk.EncryptionProviderJwk)
require.NoError(t, err)
Expand All @@ -47,8 +117,10 @@ func testEncryptionProvider(t *testing.T, ctx context.Context, config *v2.Encryp
Schema: "",
Bytes: []byte("hunter2"),
}
cipherText, err := provider.Encrypt(ctx, config, plainText)
cipherTexts, err := provider.Encrypt(ctx, []*v2.EncryptionConfig{config}, plainText)
require.NoError(t, err)
require.Len(t, cipherTexts, 1)
cipherText := cipherTexts[0]

require.Equal(t, plainText.Name, cipherText.Name)
require.Equal(t, plainText.Description, cipherText.Description)
Expand Down Expand Up @@ -138,7 +210,7 @@ func TestEncryptionProviderJWKSymmetric(t *testing.T) {
Schema: "",
Bytes: []byte("hunter2"),
}
cipherText, err := provider.Encrypt(ctx, config, plainText)
cipherText, err := provider.Encrypt(ctx, []*v2.EncryptionConfig{config}, plainText)
require.ErrorIs(t, err, jwk.JWKUnsupportedKeyTypeError)
require.Nil(t, cipherText)
}
13 changes: 11 additions & 2 deletions pkg/crypto/providers/jwk/ed25519.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import (
"crypto/ed25519"
"fmt"

"filippo.io/age"
"filippo.io/age/agessh"
"golang.org/x/crypto/ssh"
)

func EncryptED25519(pubKey ed25519.PublicKey, plaintext []byte) ([]byte, error) {
func CreateED25519Recipient(pubKey ed25519.PublicKey) (*agessh.Ed25519Recipient, error) {
sshPubKey, err := ssh.NewPublicKey(pubKey)
if err != nil {
return nil, fmt.Errorf("jwk-ed25519: failed to convert public key to ssh format: %w", err)
Expand All @@ -18,8 +19,16 @@ func EncryptED25519(pubKey ed25519.PublicKey, plaintext []byte) ([]byte, error)
if err != nil {
return nil, fmt.Errorf("jwk-ed25519: failed to create recipient: %w", err)
}
return recipient, nil
}

func EncryptED25519(pubKey ed25519.PublicKey, plaintext []byte) ([]byte, error) {
recipient, err := CreateED25519Recipient(pubKey)
if err != nil {
return nil, err
}

ciphertext, err := ageEncrypt(recipient, plaintext)
ciphertext, err := ageEncrypt([]age.Recipient{recipient}, plaintext)
if err != nil {
return nil, fmt.Errorf("jwk-ed25519: %w", err)
}
Expand Down
110 changes: 72 additions & 38 deletions pkg/crypto/providers/jwk/jwk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,49 +72,83 @@ func (j *JWKEncryptionProvider) GenerateKey(ctx context.Context) (*v2.Encryption
}, privKeyJWKBytes, nil
}

func (j *JWKEncryptionProvider) Encrypt(ctx context.Context, conf *v2.EncryptionConfig, plainText *v2.PlaintextData) (*v2.EncryptedData, error) {
jwk, err := unmarshalJWK(conf.GetJwkPublicKeyConfig().GetPubKey())
if err != nil {
return nil, err
}
func (j *JWKEncryptionProvider) Encrypt(ctx context.Context, configs []*v2.EncryptionConfig, plainText *v2.PlaintextData) ([]*v2.EncryptedData, error) {
recipients := make([]age.Recipient, 0, len(configs))
recipientThumbs := make([]string, 0, len(configs))
encrypted := make([]*v2.EncryptedData, 0, len(configs))

var ciphertext []byte
switch pubKey := jwk.Public().Key.(type) {
case ed25519.PublicKey:
ciphertext, err = EncryptED25519(pubKey, plainText.Bytes)
if err != nil {
return nil, err
}
case *ecdsa.PublicKey:
ciphertext, err = EncryptECDSA(pubKey, plainText.Bytes)
for _, config := range configs {
jwk, err := unmarshalJWK(config.GetJwkPublicKeyConfig().GetPubKey())
if err != nil {
return nil, err
}
case *rsa.PublicKey:
ciphertext, err = EncryptRSA(pubKey, plainText.Bytes)
if err != nil {
return nil, err
}
default:
return nil, JWKUnsupportedKeyTypeError
}

tp, err := thumbprint(jwk)
if err != nil {
return nil, err
switch pubKey := jwk.Public().Key.(type) {
case ed25519.PublicKey:
tp, err := thumbprint(jwk)
if err != nil {
return nil, err
}
recipientThumbs = append(recipientThumbs, tp)
recipient, err := CreateED25519Recipient(pubKey)
if err != nil {
return nil, err
}
recipients = append(recipients, recipient)
case *rsa.PublicKey:
tp, err := thumbprint(jwk)
if err != nil {
return nil, err
}
recipientThumbs = append(recipientThumbs, tp)
recipient, err := CreateRSARecipient(pubKey)
if err != nil {
return nil, err
}
recipients = append(recipients, recipient)
case *ecdsa.PublicKey:
tp, err := thumbprint(jwk)
if err != nil {
return nil, err
}
ciphertext, err := EncryptECDSA(pubKey, plainText.Bytes)
if err != nil {
return nil, err
}
encCipherText := base64.StdEncoding.EncodeToString(ciphertext)
encrypted = append(encrypted, &v2.EncryptedData{
Provider: EncryptionProviderJwk,
KeyId: tp, // MJP remove me once we've depricated fully
Name: plainText.Name,
Description: plainText.Description,
Schema: plainText.Schema,
EncryptedBytes: []byte(encCipherText),
KeyIds: []string{tp},
})

default:
return nil, JWKUnsupportedKeyTypeError
}
}

encCipherText := base64.StdEncoding.EncodeToString(ciphertext)

return &v2.EncryptedData{
Provider: EncryptionProviderJwk,
KeyId: tp,
Name: plainText.Name,
Description: plainText.Description,
Schema: plainText.Schema,
EncryptedBytes: []byte(encCipherText),
KeyIds: []string{tp},
}, nil
if len(recipients) > 0 {
ciphertext, err := ageEncrypt(recipients, plainText.Bytes)
if err != nil {
return nil, fmt.Errorf("jwk: %w", err)
}
encCipherText := base64.StdEncoding.EncodeToString(ciphertext)

encrypted = append(encrypted, &v2.EncryptedData{
Provider: EncryptionProviderJwk,
KeyId: recipientThumbs[0], // MJP remove me once we've depricated fully
Name: plainText.Name,
Description: plainText.Description,
Schema: plainText.Schema,
EncryptedBytes: []byte(encCipherText),
KeyIds: recipientThumbs,
})
}
Comment on lines +134 to +150
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Deprecation plan needed for KeyId field

The field is used in two main contexts:

  • In the protobuf definition (already marked as deprecated)
  • In pkg/crypto/providers/jwk/jwk.go where it's set alongside KeyIds for backward compatibility

Action items:

  • Add a deprecation notice in code documentation
  • Ensure all new code uses KeyIds array instead of KeyId
  • Plan to remove KeyId in next major version as it's only used for backward compatibility
🔗 Analysis chain

Plan KeyId deprecation

The code uses KeyId field with a comment indicating future deprecation. Consider:

  1. Adding a deprecation timeline
  2. Documenting the migration path to using KeyIds
  3. Adding a deprecation warning when KeyId is used

Let's check for other usages of KeyId in the codebase:

Let's search for actual usages of KeyId in our codebase, excluding vendor directories and focusing on the specific struct field:

Let's check for actual usage of the KeyId field in the code to understand the impact:

Let's check for any code that might be reading or processing this field to understand the full deprecation impact:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for KeyId usages to assess impact of deprecation
rg -l "KeyId" --type go

Length of output: 2101

Script:

#!/bin/bash
# Search for KeyId field usage in non-vendor code
rg "KeyId\s+" --type go -g '!vendor/**' -A 2 -B 2

Length of output: 1140

Script:

#!/bin/bash
# Search for actual usage of KeyId in non-vendor code
rg "\.KeyId" --type go -g '!vendor/**' -A 2 -B 2

# Also check for protobuf definition
cat pb/c1/connector/v2/resource.proto

Length of output: 950

Script:

#!/bin/bash
# Search for any code handling or processing KeyId
rg -g '!vendor/**' -g '!*.pb.go' "KeyId" --type go -A 3 -B 3

# Also check for any tests that might be using this field
rg -g '!vendor/**' "KeyId" --type go -g '*_test.go' -A 3 -B 3

Length of output: 3138

return encrypted, nil
}

func (j *JWKEncryptionProvider) Decrypt(ctx context.Context, cipherText *v2.EncryptedData, privateKey []byte) (*v2.PlaintextData, error) {
Expand Down Expand Up @@ -169,9 +203,9 @@ func thumbprint(jwk *jose.JSONWebKey) (string, error) {
return hex.EncodeToString(tp), nil
}

func ageEncrypt(r age.Recipient, plaintext []byte) ([]byte, error) {
func ageEncrypt(r []age.Recipient, plaintext []byte) ([]byte, error) {
ciphertext := &bytes.Buffer{}
w, err := age.Encrypt(ciphertext, r)
w, err := age.Encrypt(ciphertext, r...)
if err != nil {
return nil, fmt.Errorf("age: failed to encrypt: %w", err)
}
Expand Down
14 changes: 12 additions & 2 deletions pkg/crypto/providers/jwk/rsa.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import (
"crypto/rsa"
"fmt"

"filippo.io/age"
"filippo.io/age/agessh"
"golang.org/x/crypto/ssh"
)

func EncryptRSA(pubKey *rsa.PublicKey, plaintext []byte) ([]byte, error) {
func CreateRSARecipient(pubKey *rsa.PublicKey) (*agessh.RSARecipient, error) {
sshPubKey, err := ssh.NewPublicKey(pubKey)
if err != nil {
return nil, fmt.Errorf("jwk-rsa: failed to convert public key to ssh format: %w", err)
Expand All @@ -19,7 +20,16 @@ func EncryptRSA(pubKey *rsa.PublicKey, plaintext []byte) ([]byte, error) {
return nil, fmt.Errorf("jwk-rsa: failed to create recipient: %w", err)
}

ciphertext, err := ageEncrypt(recipient, plaintext)
return recipient, nil
}

func EncryptRSA(pubKey *rsa.PublicKey, plaintext []byte) ([]byte, error) {
recipient, err := CreateRSARecipient(pubKey)
if err != nil {
return nil, err
}

ciphertext, err := ageEncrypt([]age.Recipient{recipient}, plaintext)
if err != nil {
return nil, fmt.Errorf("jwk-rsa: %w", err)
}
Comment on lines +26 to 35
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Check for nil recipient before encryption.

Ensure that the recipient is not nil before proceeding with encryption to prevent potential runtime panics.

Apply this diff to add a nil check:

 func EncryptRSA(pubKey *rsa.PublicKey, plaintext []byte) ([]byte, error) {
     recipient, err := CreateRSARecipient(pubKey)
     if err != nil {
         return nil, err
     }

+    if recipient == nil {
+        return nil, fmt.Errorf("jwk-rsa: recipient is nil")
+    }

     ciphertext, err := ageEncrypt([]age.Recipient{recipient}, plaintext)
     if err != nil {
         return nil, fmt.Errorf("jwk-rsa: %w", err)
     }
     return ciphertext, nil
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func EncryptRSA(pubKey *rsa.PublicKey, plaintext []byte) ([]byte, error) {
recipient, err := CreateRSARecipient(pubKey)
if err != nil {
return nil, err
}
ciphertext, err := ageEncrypt([]age.Recipient{recipient}, plaintext)
if err != nil {
return nil, fmt.Errorf("jwk-rsa: %w", err)
}
func EncryptRSA(pubKey *rsa.PublicKey, plaintext []byte) ([]byte, error) {
recipient, err := CreateRSARecipient(pubKey)
if err != nil {
return nil, err
}
if recipient == nil {
return nil, fmt.Errorf("jwk-rsa: recipient is nil")
}
ciphertext, err := ageEncrypt([]age.Recipient{recipient}, plaintext)
if err != nil {
return nil, fmt.Errorf("jwk-rsa: %w", err)
}

Expand Down
Loading
Loading