Skip to content

Commit

Permalink
backport(3.1.x): fix(konnect) avoid collisions when redacting (#5964) (
Browse files Browse the repository at this point in the history
…#6001)

* fix(konnect) avoid collisions when redacting (#5964)

When generating sanitized configuration for Konnect, use random values
to redact unique fields.

Internally, go-database-reconciler builds an in-memory database with
certain fields as primary keys. Using static redacted values for these
results in collisions in that database.

(cherry picked from commit 4b60bf8)

* add CHANGELOG entry to backport

---------

Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com>
  • Loading branch information
randmonkey and rainest committed May 11, 2024
1 parent 949ab33 commit 61fb5ed
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ Adding a new version? You'll need three changes:

- Support to apply licenses to DB backed Kong gateway from `KongLicense`.
[#5648](https://github.com/Kong/kubernetes-ingress-controller/pull/5648)
- Redacted values no longer cause collisions in configuration reported to Konnect.
[5964](https://github.com/Kong/kubernetes-ingress-controller/pull/5964)

## [3.1.4]

Expand Down
4 changes: 2 additions & 2 deletions internal/dataplane/kong_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ func (c *KongClient) sendToClient(
// If the client is Konnect and the feature flag is turned on,
// we should sanitize the configuration before sending it out.
if client.IsKonnect() && config.SanitizeKonnectConfigDumps {
s = s.SanitizedCopy()
s = s.SanitizedCopy(util.DefaultUUIDGenerator{})
}
deckGenParams := deckgen.GenerateDeckContentParams{
SelectorTags: config.FilterTags,
Expand Down Expand Up @@ -632,7 +632,7 @@ func prepareSendDiagnosticFn(
if diagnosticConfig.DumpsIncludeSensitive {
redactedConfig := deckgen.ToDeckContent(ctx,
logger,
targetState.SanitizedCopy(),
targetState.SanitizedCopy(util.DefaultUUIDGenerator{}),
deckGenParams,
)
config = redactedConfig
Expand Down
5 changes: 3 additions & 2 deletions internal/dataplane/kongstate/consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/kong/go-kong/kong"

"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
)

Expand All @@ -27,13 +28,13 @@ type Consumer struct {
}

// SanitizedCopy returns a shallow copy with sensitive values redacted best-effort.
func (c *Consumer) SanitizedCopy() *Consumer {
func (c *Consumer) SanitizedCopy(uuidGenerator util.UUIDGenerator) *Consumer {
return &Consumer{
Consumer: c.Consumer,
Plugins: c.Plugins,
KeyAuths: func() (res []*KeyAuth) {
for _, v := range c.KeyAuths {
res = append(res, v.SanitizedCopy())
res = append(res, v.SanitizedCopy(uuidGenerator))
}
return
}(),
Expand Down
5 changes: 3 additions & 2 deletions internal/dataplane/kongstate/consumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/assert"

kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
"github.com/kong/kubernetes-ingress-controller/v3/test/mocks"
)

func int64Ptr(i int64) *int64 {
Expand Down Expand Up @@ -50,7 +51,7 @@ func TestConsumer_SanitizedCopy(t *testing.T) {
Tags: []*string{kong.String("5.1"), kong.String("5.2")},
},
Plugins: []kong.Plugin{{ID: kong.String("1")}},
KeyAuths: []*KeyAuth{{kong.KeyAuth{ID: kong.String("1"), Key: redactedString}}},
KeyAuths: []*KeyAuth{{kong.KeyAuth{ID: kong.String("1"), Key: kong.String("{vault://52fdfc07-2182-454f-963f-5f0f9a621d72}")}}},
HMACAuths: []*HMACAuth{{kong.HMACAuth{ID: kong.String("1"), Secret: redactedString}}},
JWTAuths: []*JWTAuth{{kong.JWTAuth{ID: kong.String("1"), Secret: redactedString}}},
BasicAuths: []*BasicAuth{{kong.BasicAuth{ID: kong.String("1"), Password: redactedString}}},
Expand All @@ -64,7 +65,7 @@ func TestConsumer_SanitizedCopy(t *testing.T) {
},
} {
t.Run(tt.name, func(t *testing.T) {
got := *tt.in.SanitizedCopy()
got := *tt.in.SanitizedCopy(mocks.StaticUUIDGenerator{UUID: "52fdfc07-2182-454f-963f-5f0f9a621d72"})
assert.Equal(t, tt.want, got)
})
}
Expand Down
13 changes: 11 additions & 2 deletions internal/dataplane/kongstate/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,22 @@ import (

"github.com/kong/go-kong/kong"
"github.com/mitchellh/mapstructure"

"github.com/kong/kubernetes-ingress-controller/v3/internal/util"
)

// redactedString is used to redact sensitive values in the KongState.
// It uses a vault URI to pass Konnect Admin API validations (e.g. when a TLS key is expected, it's only possible
// to pass a valid key or a vault URI).
var redactedString = kong.String("{vault://redacted-value}")

// randRedactedString is used to redact sensitive values in the KongState when the value must be random to avoid
// collisions.
func randRedactedString(uuidGenerator util.UUIDGenerator) *string {
s := fmt.Sprintf("{vault://%s}", uuidGenerator.NewString())
return &s
}

// KeyAuth represents a key-auth credential.
type KeyAuth struct {
kong.KeyAuth
Expand Down Expand Up @@ -152,13 +161,13 @@ func NewMTLSAuth(config interface{}) (*MTLSAuth, error) {
}

// SanitizedCopy returns a shallow copy with sensitive values redacted best-effort.
func (c *KeyAuth) SanitizedCopy() *KeyAuth {
func (c *KeyAuth) SanitizedCopy(uuidGenerator util.UUIDGenerator) *KeyAuth {
return &KeyAuth{
kong.KeyAuth{
// Consumer field omitted
CreatedAt: c.CreatedAt,
ID: c.ID,
Key: redactedString,
Key: randRedactedString(uuidGenerator),
Tags: c.Tags,
},
}
Expand Down
6 changes: 4 additions & 2 deletions internal/dataplane/kongstate/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (

"github.com/kong/go-kong/kong"
"github.com/stretchr/testify/assert"

"github.com/kong/kubernetes-ingress-controller/v3/test/mocks"
)

func TestKeyAuth_SanitizedCopy(t *testing.T) {
Expand All @@ -25,13 +27,13 @@ func TestKeyAuth_SanitizedCopy(t *testing.T) {
want: KeyAuth{kong.KeyAuth{
CreatedAt: kong.Int(1),
ID: kong.String("2"),
Key: redactedString,
Key: kong.String("{vault://52fdfc07-2182-454f-963f-5f0f9a621d72}"),
Tags: []*string{kong.String("4.1"), kong.String("4.2")},
}},
},
} {
t.Run(tt.name, func(t *testing.T) {
got := *tt.in.SanitizedCopy()
got := *tt.in.SanitizedCopy(mocks.StaticUUIDGenerator{UUID: "52fdfc07-2182-454f-963f-5f0f9a621d72"})
assert.Equal(t, tt.want, got)
})
}
Expand Down
4 changes: 2 additions & 2 deletions internal/dataplane/kongstate/kongstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type KongState struct {
}

// SanitizedCopy returns a shallow copy with sensitive values redacted best-effort.
func (ks *KongState) SanitizedCopy() *KongState {
func (ks *KongState) SanitizedCopy(uuidGenerator util.UUIDGenerator) *KongState {
return &KongState{
Services: ks.Services,
Upstreams: ks.Upstreams,
Expand All @@ -52,7 +52,7 @@ func (ks *KongState) SanitizedCopy() *KongState {
}),
Consumers: func() (res []Consumer) {
for _, v := range ks.Consumers {
res = append(res, *v.SanitizedCopy())
res = append(res, *v.SanitizedCopy(uuidGenerator))
}
return
}(),
Expand Down
5 changes: 3 additions & 2 deletions internal/dataplane/kongstate/kongstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
kongv1alpha1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1alpha1"
kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1"
"github.com/kong/kubernetes-ingress-controller/v3/test/mocks"
)

var kongConsumerTypeMeta = metav1.TypeMeta{
Expand Down Expand Up @@ -86,7 +87,7 @@ func TestKongState_SanitizedCopy(t *testing.T) {
Plugin: kong.Plugin{ID: kong.String("1"), Config: kong.Configuration{"secret": *redactedString}},
}},
Consumers: []Consumer{{
KeyAuths: []*KeyAuth{{kong.KeyAuth{ID: kong.String("1"), Key: redactedString}}},
KeyAuths: []*KeyAuth{{kong.KeyAuth{ID: kong.String("1"), Key: kong.String("{vault://52fdfc07-2182-454f-963f-5f0f9a621d72}")}}},
}},
Licenses: []License{{kong.License{ID: kong.String("1"), Payload: redactedString}}},
ConsumerGroups: []ConsumerGroup{{
Expand All @@ -104,7 +105,7 @@ func TestKongState_SanitizedCopy(t *testing.T) {
} {
t.Run(tt.name, func(t *testing.T) {
testedFields.Insert(extractNotEmptyFieldNames(tt.in)...)
got := *tt.in.SanitizedCopy()
got := *tt.in.SanitizedCopy(mocks.StaticUUIDGenerator{UUID: "52fdfc07-2182-454f-963f-5f0f9a621d72"})
assert.Equal(t, tt.want, got)
})
}
Expand Down
15 changes: 15 additions & 0 deletions internal/util/uuid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package util

import "github.com/google/uuid"

// UUIDGenerator is an interface to generate UUIDs.
type UUIDGenerator interface {
NewString() string
}

// DefaultUUIDGenerator is the default implementation of UUIDGenerator.
type DefaultUUIDGenerator struct{}

func (DefaultUUIDGenerator) NewString() string {
return uuid.NewString()
}
13 changes: 13 additions & 0 deletions test/mocks/uuid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package mocks

import "github.com/kong/kubernetes-ingress-controller/v3/internal/util"

var _ = util.UUIDGenerator(&StaticUUIDGenerator{})

type StaticUUIDGenerator struct {
UUID string
}

func (s StaticUUIDGenerator) NewString() string {
return s.UUID
}

0 comments on commit 61fb5ed

Please sign in to comment.