From aafdc2eb2915a6e7e2cbf4acb1339e75b50c7677 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 9 Jul 2024 17:29:32 +1000 Subject: [PATCH] fix: asm follower obfuscation (#2017) - Secret providers take in obfuscated bytes to store and should return obfuscated bytes when loading secrets - ASM follower was not doing this because it stores and retrieves secrets through the admin API - When storing secrets it would send obfuscated values to the admin endpoint, which would fail (can not parse json string because its base64) - When retrieving secrets it woudl get unobfuscated values from the admin endpoint and return them. The manager would then fail when then unobfuscate this value (resulting in gibberish) which would then fail when trying to parse it as json Solution: ASM follower now directly accesses `Secrets{}.obfuscator()` to translate into the correct form. --- common/configuration/asm_follower.go | 25 ++++-- common/configuration/asm_test.go | 113 ++++++++++++++++++--------- common/configuration/obfuscator.go | 2 +- 3 files changed, 98 insertions(+), 42 deletions(-) diff --git a/common/configuration/asm_follower.go b/common/configuration/asm_follower.go index f077179947..b7155d6d87 100644 --- a/common/configuration/asm_follower.go +++ b/common/configuration/asm_follower.go @@ -18,8 +18,11 @@ const asmFollowerSyncInterval = time.Minute * 1 // asmFollower uses AdminService to get/set secrets from the leader type asmFollower struct { + // client requests/responses use unobfuscated values client ftlv1connect.AdminServiceClient - cache *secretsCache + + // cache stores obfuscated values + cache *secretsCache } var _ asmClient = &asmFollower{} @@ -36,6 +39,7 @@ func newASMFollower(ctx context.Context, rpcClient ftlv1connect.AdminServiceClie } func (f *asmFollower) sync(ctx context.Context, secrets *xsync.MapOf[Ref, cachedSecret]) error { + obfuscator := Secrets{}.obfuscator() module := "" includeValues := true resp, err := f.client.SecretsList(ctx, connect.NewRequest(&ftlv1.ListSecretsRequest{ @@ -51,9 +55,13 @@ func (f *asmFollower) sync(ctx context.Context, secrets *xsync.MapOf[Ref, cached if err != nil { return fmt.Errorf("invalid ref %q: %w", s.RefPath, err) } + obfuscatedValue, err := obfuscator.Obfuscate(s.Value) + if err != nil { + return fmt.Errorf("asm follower could not obfuscate value for ref %q: %w", s.RefPath, err) + } visited[ref] = true secrets.Store(ref, cachedSecret{ - value: s.Value, + value: obfuscatedValue, }) } // delete old values @@ -85,20 +93,25 @@ func (f *asmFollower) load(ctx context.Context, ref Ref, key *url.URL) ([]byte, return f.cache.getSecret(ref) } -func (f *asmFollower) store(ctx context.Context, ref Ref, value []byte) (*url.URL, error) { +func (f *asmFollower) store(ctx context.Context, ref Ref, obfuscatedValue []byte) (*url.URL, error) { + obfuscator := Secrets{}.obfuscator() + unobfuscatedValue, err := obfuscator.Reveal(obfuscatedValue) + if err != nil { + return nil, fmt.Errorf("asm follower could not unobfuscate: %w", err) + } provider := ftlv1.SecretProvider_SECRET_ASM - _, err := f.client.SecretSet(ctx, connect.NewRequest(&ftlv1.SetSecretRequest{ + _, err = f.client.SecretSet(ctx, connect.NewRequest(&ftlv1.SetSecretRequest{ Provider: &provider, Ref: &ftlv1.ConfigRef{ Module: ref.Module.Ptr(), Name: ref.Name, }, - Value: value, + Value: unobfuscatedValue, })) if err != nil { return nil, err } - f.cache.updatedSecret(ref, value) + f.cache.updatedSecret(ref, obfuscatedValue) return asmURLForRef(ref), nil } diff --git a/common/configuration/asm_test.go b/common/configuration/asm_test.go index 8480f144c1..ed56328473 100644 --- a/common/configuration/asm_test.go +++ b/common/configuration/asm_test.go @@ -186,7 +186,7 @@ func testClientSync(ctx context.Context, t *testing.T, client asmClient, cache *secretsCache, - sm *secretsmanager.Client, + externalClient *secretsmanager.Client, progressByIntervalPercentage func(percentage float64)) { t.Helper() @@ -199,55 +199,40 @@ func testClientSync(ctx context.Context, // write a secret via asmClient clientRef := Ref{Module: Some("sync"), Name: "set-by-client"} - _, err = client.store(ctx, clientRef, jsonBytes(t, "client-first")) + err = storeUnobfuscatedValue(ctx, client, clientRef, jsonBytes(t, "client-first")) assert.NoError(t, err) waitForUpdatesToProcess(cache) - value, err := client.load(ctx, clientRef, asmURLForRef(clientRef)) + value, err := getUnobfuscatedValue(ctx, client, clientRef) assert.NoError(t, err, "failed to load secret via asm") assert.Equal(t, value, jsonBytes(t, "client-first"), "unexpected secret value") // write another secret via sm directly smRef := Ref{Module: Some("sync"), Name: "set-by-sm"} - _, err = sm.CreateSecret(ctx, &secretsmanager.CreateSecretInput{ - Name: aws.String(smRef.String()), - SecretString: aws.String(jsonString(t, "sm-first")), - Tags: []types.Tag{ - {Key: aws.String(asmTagKey), Value: aws.String(smRef.Module.Default(""))}, - }, - }) + err = storeUnobfuscatedValueInASM(ctx, externalClient, smRef, []byte(jsonString(t, "sm-first")), true) assert.NoError(t, err, "failed to create secret via sm") waitForUpdatesToProcess(cache) - value, err = client.load(ctx, smRef, asmURLForRef(smRef)) + value, err = getUnobfuscatedValue(ctx, client, smRef) assert.Error(t, err, "expected to fail because asm client has not synced secret yet") // write a secret via client and then by sm directly clientSmRef := Ref{Module: Some("sync"), Name: "set-by-client-then-sm"} - _, err = client.store(ctx, clientSmRef, jsonBytes(t, "client-sm-first")) + err = storeUnobfuscatedValue(ctx, client, clientSmRef, jsonBytes(t, "client-sm-first")) assert.NoError(t, err) - _, err = sm.UpdateSecret(ctx, &secretsmanager.UpdateSecretInput{ - SecretId: aws.String(clientSmRef.String()), - SecretString: aws.String(jsonString(t, "client-sm-second")), - }) + err = storeUnobfuscatedValueInASM(ctx, externalClient, clientSmRef, []byte(jsonString(t, "client-sm-second")), false) assert.NoError(t, err) waitForUpdatesToProcess(cache) - value, err = client.load(ctx, clientSmRef, asmURLForRef(clientSmRef)) + value, err = getUnobfuscatedValue(ctx, client, clientSmRef) assert.NoError(t, err, "failed to load secret via asm") assert.Equal(t, value, jsonBytes(t, "client-sm-first"), "expected initial value before client has a chance to sync newest value") // write a secret via sm directly and then by client smClientRef := Ref{Module: Some("sync"), Name: "set-by-sm-then-client"} - _, err = sm.CreateSecret(ctx, &secretsmanager.CreateSecretInput{ - Name: aws.String(smClientRef.String()), - SecretString: aws.String(jsonString(t, "sm-client-first")), - Tags: []types.Tag{ - {Key: aws.String(asmTagKey), Value: aws.String(smClientRef.Module.Default(""))}, - }, - }) + err = storeUnobfuscatedValueInASM(ctx, externalClient, smClientRef, []byte(jsonString(t, "sm-client-first")), true) assert.NoError(t, err, "failed to create secret via sm") - _, err = client.store(ctx, smClientRef, jsonBytes(t, "sm-client-second")) + err = storeUnobfuscatedValue(ctx, client, smClientRef, jsonBytes(t, "sm-client-second")) assert.NoError(t, err) waitForUpdatesToProcess(cache) - value, err = client.load(ctx, smClientRef, asmURLForRef(smClientRef)) + value, err = getUnobfuscatedValue(ctx, client, smClientRef) assert.NoError(t, err, "failed to load secret via asm") assert.Equal(t, value, jsonBytes(t, "sm-client-second"), "unexpected secret value") @@ -260,7 +245,7 @@ func testClientSync(ctx context.Context, assert.NoError(t, err) assert.Equal(t, len(list), 4, "expected 4 secrets") for _, entry := range list { - value, err = client.load(ctx, entry.Ref, asmURLForRef(entry.Ref)) + value, err = getUnobfuscatedValue(ctx, client, entry.Ref) assert.NoError(t, err, "failed to load secret via asm") var expectedValue string switch entry.Ref { @@ -280,12 +265,12 @@ func testClientSync(ctx context.Context, // delete 2 secrets without client knowing tr := true - _, err = sm.DeleteSecret(ctx, &secretsmanager.DeleteSecretInput{ + _, err = externalClient.DeleteSecret(ctx, &secretsmanager.DeleteSecretInput{ SecretId: aws.String(smRef.String()), ForceDeleteWithoutRecovery: &tr, }) assert.NoError(t, err) - _, err = sm.DeleteSecret(ctx, &secretsmanager.DeleteSecretInput{ + _, err = externalClient.DeleteSecret(ctx, &secretsmanager.DeleteSecretInput{ SecretId: aws.String(smClientRef.String()), ForceDeleteWithoutRecovery: &tr, }) @@ -299,17 +284,63 @@ func testClientSync(ctx context.Context, list, err = client.list(ctx) assert.NoError(t, err) assert.Equal(t, len(list), 2, "expected 2 secrets") - _, err = client.load(ctx, smRef, asmURLForRef(smRef)) + _, err = getUnobfuscatedValue(ctx, client, smRef) assert.Error(t, err, "expected to fail because secret was deleted") - _, err = client.load(ctx, smClientRef, asmURLForRef(smClientRef)) + _, err = getUnobfuscatedValue(ctx, client, smClientRef) assert.Error(t, err, "expected to fail because secret was deleted") } +func storeUnobfuscatedValue(ctx context.Context, client asmClient, ref Ref, value []byte) error { + obfuscator := Secrets{}.obfuscator() + obfuscatedValue, err := obfuscator.Obfuscate(value) + if err != nil { + return err + } + _, err = client.store(ctx, ref, obfuscatedValue) + return err +} + +func getUnobfuscatedValue(ctx context.Context, client asmClient, ref Ref) ([]byte, error) { + obfuscator := Secrets{}.obfuscator() + obfuscatedValue, err := client.load(ctx, ref, asmURLForRef(ref)) + if err != nil { + return nil, err + } + unobfuscatedValue, err := obfuscator.Reveal(obfuscatedValue) + if err != nil { + return nil, err + } + return unobfuscatedValue, nil +} + +func storeUnobfuscatedValueInASM(ctx context.Context, externalClient *secretsmanager.Client, ref Ref, value []byte, isNew bool) error { + obfuscator := Secrets{}.obfuscator() + obfuscatedValue, err := obfuscator.Obfuscate(value) + if err != nil { + return err + } + if isNew { + _, err = externalClient.CreateSecret(ctx, &secretsmanager.CreateSecretInput{ + Name: aws.String(ref.String()), + SecretString: aws.String(string(obfuscatedValue)), + Tags: []types.Tag{ + {Key: aws.String(asmTagKey), Value: aws.String(ref.Module.Default(""))}, + }, + }) + } else { + _, err = externalClient.UpdateSecret(ctx, &secretsmanager.UpdateSecretInput{ + SecretId: aws.String(ref.String()), + SecretString: aws.String(string(obfuscatedValue)), + }) + } + return err +} + func jsonBytes(t *testing.T, value string) []byte { t.Helper() json, err := json.Marshal(value) assert.NoError(t, err, "failed to marshal value") - return []byte("c" + string(json)) + return []byte(string(json)) } func jsonString(t *testing.T, value string) string { @@ -343,6 +374,7 @@ func (c *fakeAdminClient) ConfigUnset(ctx context.Context, req *connect.Request[ } func (c *fakeAdminClient) SecretsList(ctx context.Context, req *connect.Request[ftlv1.ListSecretsRequest]) (*connect.Response[ftlv1.ListSecretsResponse], error) { + obfuscator := Secrets{}.obfuscator() client, err := c.asm.coordinator.Get() if err != nil { return nil, err @@ -363,7 +395,11 @@ func (c *fakeAdminClient) SecretsList(ctx context.Context, req *connect.Request[ } var sv []byte if *req.Msg.IncludeValues { - sv, err = c.asm.Load(ctx, secret.Ref, asmURLForRef(secret.Ref)) + obfuscatedValue, err := c.asm.Load(ctx, secret.Ref, asmURLForRef(secret.Ref)) + if err != nil { + return nil, err + } + sv, err = obfuscator.Reveal(obfuscatedValue) if err != nil { return nil, err } @@ -378,17 +414,24 @@ func (c *fakeAdminClient) SecretsList(ctx context.Context, req *connect.Request[ // SecretGet returns the secret value for a given ref string. func (c *fakeAdminClient) SecretGet(ctx context.Context, req *connect.Request[ftlv1.GetSecretRequest]) (*connect.Response[ftlv1.GetSecretResponse], error) { + obfuscator := Secrets{}.obfuscator() ref := NewRef(*req.Msg.Ref.Module, req.Msg.Ref.Name) - vb, err := c.asm.Load(ctx, ref, asmURLForRef(ref)) + obfuscatedValue, err := c.asm.Load(ctx, ref, asmURLForRef(ref)) if err != nil { return nil, err } + vb, err := obfuscator.Reveal(obfuscatedValue) return connect.NewResponse(&ftlv1.GetSecretResponse{Value: vb}), nil } // SecretSet sets the secret at the given ref to the provided value. func (c *fakeAdminClient) SecretSet(ctx context.Context, req *connect.Request[ftlv1.SetSecretRequest]) (*connect.Response[ftlv1.SetSecretResponse], error) { - _, err := c.asm.Store(ctx, NewRef(*req.Msg.Ref.Module, req.Msg.Ref.Name), req.Msg.Value) + obfuscator := Secrets{}.obfuscator() + obfuscatedValue, err := obfuscator.Obfuscate(req.Msg.Value) + if err != nil { + return nil, err + } + _, err = c.asm.Store(ctx, NewRef(*req.Msg.Ref.Module, req.Msg.Ref.Name), obfuscatedValue) if err != nil { return nil, err } diff --git a/common/configuration/obfuscator.go b/common/configuration/obfuscator.go index 8d940131f5..01f39236a9 100644 --- a/common/configuration/obfuscator.go +++ b/common/configuration/obfuscator.go @@ -49,7 +49,7 @@ func (o Obfuscator) Reveal(input []byte) ([]byte, error) { obfuscated, err := base64.StdEncoding.DecodeString(string(input)) if err != nil { - return nil, fmt.Errorf("expected hexadecimal string: %w", err) + return nil, fmt.Errorf("expected base64 string: %w", err) } block, err := aes.NewCipher(o.key) if err != nil {