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.

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

Change BulkDecrypt to not rely on type tests #9373

Merged
merged 3 commits into from Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG_PENDING.md
Expand Up @@ -15,4 +15,7 @@
### Bug Fixes

- [codegen/node] - Fix an issue with escaping deprecation messages.
[#9371](https://github.com/pulumi/pulumi/pull/9371)
[#9371](https://github.com/pulumi/pulumi/pull/9371)

- [cli] - StackReferences will now correctly use the service bulk decryption end point.
[#9373](https://github.com/pulumi/pulumi/pull/9373)
4 changes: 4 additions & 0 deletions pkg/engine/lifeycletest/pulumi_test.go
Expand Up @@ -338,6 +338,10 @@ func (b brokenDecrypter) DecryptValue(_ string) (string, error) {
return "", fmt.Errorf(b.ErrorMessage)
}

func (b brokenDecrypter) BulkDecrypt(_ []string) (map[string]string, error) {
return nil, fmt.Errorf(b.ErrorMessage)
}

// Tests that the engine presents a reasonable error message when a decrypter fails to decrypt a config value.
func TestBrokenDecrypter(t *testing.T) {
t.Parallel()
Expand Down
2 changes: 1 addition & 1 deletion pkg/resource/stack/deployment.go
Expand Up @@ -244,7 +244,7 @@ func DeserializeDeploymentV3(deployment apitype.DeploymentV3, secretsProv Secret
}

// Decrypt the collected secrets and create a decrypter that will use the result as a cache.
cache, err := config.BulkDecrypt(d, ciphertexts)
cache, err := d.BulkDecrypt(ciphertexts)
if err != nil {
return nil, err
}
Expand Down
41 changes: 41 additions & 0 deletions pkg/resource/stack/secrets.go
Expand Up @@ -129,6 +129,10 @@ func (c *cachingCrypter) DecryptValue(ciphertext string) (string, error) {
return c.decrypter.DecryptValue(ciphertext)
}

func (c *cachingCrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return c.decrypter.BulkDecrypt(ciphertexts)
}

// encryptSecret encrypts the plaintext associated with the given secret value.
func (c *cachingCrypter) encryptSecret(secret *resource.Secret, plaintext string) (string, error) {
// If the cache has an entry for this secret and the plaintext has not changed, re-use the ciphertext.
Expand Down Expand Up @@ -188,3 +192,40 @@ func (c *mapDecrypter) DecryptValue(ciphertext string) (string, error) {

return plaintext, nil
}

func (c *mapDecrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
// Loop and find the entries that are already cached, then BulkDecrypt the rest
secretMap := map[string]string{}
var toDecrypt []string
if c.cache == nil {
// Don't bother searching for the cached subset if the cache is nil
toDecrypt = ciphertexts
} else {
toDecrypt = make([]string, 0)
for _, ct := range ciphertexts {
if plaintext, ok := c.cache[ct]; ok {
secretMap[ct] = plaintext
} else {
toDecrypt = append(toDecrypt, ct)
}
}
}

// try and bulk decrypt the rest
decrypted, err := c.decrypter.BulkDecrypt(toDecrypt)
if err != nil {
return nil, err
}

// And add them to the cache
if c.cache == nil {
c.cache = make(map[string]string)
}

for ct, pt := range decrypted {
secretMap[ct] = pt
c.cache[ct] = pt
}

return secretMap, nil
}
Comment on lines +196 to +231
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think current data flow is such that no this will never be hit, but it was pretty easy to write and means we won't trip up on a bad method if we refactor things slightly in the future that cause it to start being hit.

6 changes: 5 additions & 1 deletion pkg/resource/stack/secrets_test.go
Expand Up @@ -46,6 +46,10 @@ func (t *testSecretsManager) DecryptValue(ciphertext string) (string, error) {
return ciphertext[i+1:], nil
}

func (t *testSecretsManager) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return config.DefaultBulkDecrypt(t, ciphertexts)
}

func deserializeProperty(v interface{}, dec config.Decrypter) (resource.PropertyValue, error) {
b, err := json.Marshal(v)
if err != nil {
Expand Down Expand Up @@ -228,7 +232,7 @@ func (t *mapTestDecrypter) DecryptValue(ciphertext string) (string, error) {

func (t *mapTestDecrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
t.bulkDecryptCalls++
return config.BulkDecrypt(t.d, ciphertexts)
return config.DefaultBulkDecrypt(t.d, ciphertexts)
}

func TestMapCrypter(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions pkg/secrets/b64/manager.go
Expand Up @@ -48,3 +48,7 @@ func (c *base64Crypter) DecryptValue(s string) (string, error) {
}
return string(b), nil
}

func (c *base64Crypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return config.DefaultBulkDecrypt(c, ciphertexts)
}
5 changes: 5 additions & 0 deletions pkg/secrets/passphrase/manager.go
Expand Up @@ -323,3 +323,8 @@ func (ec *errorCrypter) DecryptValue(_ string) (string, error) {
return "", errors.New("failed to decrypt: incorrect passphrase, please set PULUMI_CONFIG_PASSPHRASE to the " +
"correct passphrase or set PULUMI_CONFIG_PASSPHRASE_FILE to a file containing the passphrase")
}

func (ec *errorCrypter) BulkDecrypt(_ []string) (map[string]string, error) {
return nil, errors.New("failed to decrypt: incorrect passphrase, please set PULUMI_CONFIG_PASSPHRASE to the " +
"correct passphrase or set PULUMI_CONFIG_PASSPHRASE_FILE to a file containing the passphrase")
}
2 changes: 0 additions & 2 deletions pkg/secrets/service/manager.go
Expand Up @@ -33,8 +33,6 @@ import (

const Type = "service"

var _ config.BulkDecrypter = (*serviceCrypter)(nil)

// serviceCrypter is an encrypter/decrypter that uses the Pulumi servce to encrypt/decrypt a stack's secrets.
type serviceCrypter struct {
client *client.Client
Expand Down
41 changes: 29 additions & 12 deletions sdk/go/common/resource/config/crypt.go
Expand Up @@ -36,12 +36,8 @@ type Encrypter interface {
// Decrypter decrypts encrypted ciphertext to its plaintext representation.
type Decrypter interface {
DecryptValue(ciphertext string) (string, error)
}

// BulkDecrypter is a Decrypter that also supports bulk decryption of secrets.
type BulkDecrypter interface {
Decrypter

// BulkDecrypt supports bulk decryption of secrets.
BulkDecrypt(ciphertexts []string) (map[string]string, error)
}

Expand All @@ -61,6 +57,10 @@ func (nopCrypter) DecryptValue(ciphertext string) (string, error) {
return ciphertext, nil
}

func (nopCrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return DefaultBulkDecrypt(NopDecrypter, ciphertexts)
}

func (nopCrypter) EncryptValue(plaintext string) (string, error) {
return plaintext, nil
}
Expand Down Expand Up @@ -91,6 +91,10 @@ func (t *trackingDecrypter) DecryptValue(ciphertext string) (string, error) {
return v, nil
}

func (t *trackingDecrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return DefaultBulkDecrypt(t, ciphertexts)
}

func (t *trackingDecrypter) SecureValues() []string {
return t.secureValues
}
Expand All @@ -115,6 +119,10 @@ func (b blindingCrypter) EncryptValue(plaintext string) (string, error) {
return "[secret]", nil
}

func (b blindingCrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return DefaultBulkDecrypt(b, ciphertexts)
}

// NewPanicCrypter returns a new config crypter that will panic if used.
func NewPanicCrypter() Crypter {
return &panicCrypter{}
Expand All @@ -130,6 +138,10 @@ func (p panicCrypter) DecryptValue(_ string) (string, error) {
panic("attempt to decrypt value")
}

func (p panicCrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
panic("attempt to bulk decrypt values")
}

// NewSymmetricCrypter creates a crypter that encrypts and decrypts values using AES-256-GCM. The nonce is stored with
// the value itself as a pair of base64 values separated by a colon and a version tag `v1` is prepended.
func NewSymmetricCrypter(key []byte) Crypter {
Expand Down Expand Up @@ -182,6 +194,10 @@ func (s symmetricCrypter) DecryptValue(value string) (string, error) {
return decryptAES256GCM(enc, s.key, nonce)
}

func (s symmetricCrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return DefaultBulkDecrypt(s, ciphertexts)
}

// encryptAES256GCGM returns the ciphertext and the generated nonce
func encryptAES256GCGM(plaintext string, key []byte) ([]byte, []byte) {
contract.Requiref(len(key) == SymmetricCrypterKeyBytes, "key", "AES-256-GCM needs a 32 byte key")
Expand Down Expand Up @@ -234,17 +250,18 @@ func (c prefixCrypter) EncryptValue(plaintext string) (string, error) {
return c.prefix + plaintext, nil
}

// BulkDecrypt decrypts a list of ciphertexts. If decrypter implements BulkDecrypter, then its BulkDecrypt method will
// be called. Otherwise, each ciphertext is decrypted individually. The returned map maps from ciphertext to plaintext.
func BulkDecrypt(decrypter Decrypter, ciphertexts []string) (map[string]string, error) {
func (c prefixCrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return DefaultBulkDecrypt(c, ciphertexts)
}

// DefaultBulkDecrypt decrypts a list of ciphertexts. Each ciphertext is decrypted individually. The returned
// map maps from ciphertext to plaintext. This should only be used by implementers of Decrypter to implement
// their BulkDecrypt method in cases where they can't do more efficient than just individual decryptions.
func DefaultBulkDecrypt(decrypter Decrypter, ciphertexts []string) (map[string]string, error) {
if len(ciphertexts) == 0 {
return nil, nil
}

if bulk, ok := decrypter.(BulkDecrypter); ok {
return bulk.BulkDecrypt(ciphertexts)
}

secretMap := map[string]string{}
for _, ct := range ciphertexts {
pt, err := decrypter.DecryptValue(ct)
Expand Down
4 changes: 4 additions & 0 deletions sdk/go/common/resource/config/value_test.go
Expand Up @@ -219,6 +219,10 @@ func (d passThroughDecrypter) DecryptValue(ciphertext string) (string, error) {
return ciphertext, nil
}

func (d passThroughDecrypter) BulkDecrypt(ciphertexts []string) (map[string]string, error) {
return DefaultBulkDecrypt(d, ciphertexts)
}

func TestSecureValues(t *testing.T) {
t.Parallel()

Expand Down