Skip to content

Commit

Permalink
fix(konnect) avoid colliding redacted values
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rainest committed May 3, 2024
1 parent 755e2ce commit 36538e8
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ Adding a new version? You'll need three changes:
`parentRefs`'s group/kind is not `gateway.network.k8s.io/Gateway`, the item
is seen as a parent other than the controller and ignored in parentRef check.
[#5919](https://github.com/Kong/kubernetes-ingress-controller/pull/5919)
- Redacted values no longer cause collisions in configuration reported to Konnect.
[5964](https://github.com/Kong/kubernetes-ingress-controller/pull/5964)

### Changed

Expand Down
8 changes: 7 additions & 1 deletion internal/dataplane/kongstate/consumer_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package kongstate

import (
"math/rand"
"testing"

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

Expand All @@ -14,6 +16,8 @@ func int64Ptr(i int64) *int64 {
}

func TestConsumer_SanitizedCopy(t *testing.T) {
// this needs a static random seed because some auths generate random values
uuid.SetRand(rand.New(rand.NewSource(1)))

Check failure on line 20 in internal/dataplane/kongstate/consumer_test.go

View workflow job for this annotation

GitHub Actions / linters / lint

G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
for _, tt := range []struct {
name string
in Consumer
Expand Down Expand Up @@ -50,7 +54,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: randRedactedString()}}},
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 @@ -63,6 +67,8 @@ func TestConsumer_SanitizedCopy(t *testing.T) {
},
},
} {
// this needs a static random seed because some auths generate random values
uuid.SetRand(rand.New(rand.NewSource(1)))

Check failure on line 71 in internal/dataplane/kongstate/consumer_test.go

View workflow job for this annotation

GitHub Actions / linters / lint

G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
t.Run(tt.name, func(t *testing.T) {
got := *tt.in.SanitizedCopy()
assert.Equal(t, tt.want, got)
Expand Down
10 changes: 9 additions & 1 deletion internal/dataplane/kongstate/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package kongstate
import (
"fmt"

"github.com/google/uuid"
"github.com/kong/go-kong/kong"
"github.com/mitchellh/mapstructure"
)
Expand All @@ -12,6 +13,13 @@ import (
// 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() *string {
s := fmt.Sprintf("{vault://%s}", uuid.NewString())
return &s
}

// KeyAuth represents a key-auth credential.
type KeyAuth struct {
kong.KeyAuth
Expand Down Expand Up @@ -158,7 +166,7 @@ func (c *KeyAuth) SanitizedCopy() *KeyAuth {
// Consumer field omitted
CreatedAt: c.CreatedAt,
ID: c.ID,
Key: redactedString,
Key: randRedactedString(),
Tags: c.Tags,
},
}
Expand Down
8 changes: 7 additions & 1 deletion internal/dataplane/kongstate/credentials_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package kongstate

import (
"math/rand"
"testing"

"github.com/google/uuid"
"github.com/kong/go-kong/kong"
"github.com/stretchr/testify/assert"
)

func TestKeyAuth_SanitizedCopy(t *testing.T) {
// this needs a static random seed because some auths generate random values
uuid.SetRand(rand.New(rand.NewSource(1)))

Check failure on line 14 in internal/dataplane/kongstate/credentials_test.go

View workflow job for this annotation

GitHub Actions / linters / lint

G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
for _, tt := range []struct {
name string
in KeyAuth
Expand All @@ -25,11 +29,13 @@ func TestKeyAuth_SanitizedCopy(t *testing.T) {
want: KeyAuth{kong.KeyAuth{
CreatedAt: kong.Int(1),
ID: kong.String("2"),
Key: redactedString,
Key: randRedactedString(),
Tags: []*string{kong.String("4.1"), kong.String("4.2")},
}},
},
} {
// this needs a static random seed because some auths generate random values
uuid.SetRand(rand.New(rand.NewSource(1)))

Check failure on line 38 in internal/dataplane/kongstate/credentials_test.go

View workflow job for this annotation

GitHub Actions / linters / lint

G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
t.Run(tt.name, func(t *testing.T) {
got := *tt.in.SanitizedCopy()
assert.Equal(t, tt.want, got)
Expand Down
8 changes: 7 additions & 1 deletion internal/dataplane/kongstate/kongstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package kongstate

import (
"fmt"
"math/rand"
"reflect"
"strconv"
"strings"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/go-logr/logr"
"github.com/go-logr/logr/testr"
"github.com/go-logr/zapr"
"github.com/google/uuid"
"github.com/kong/go-kong/kong"
"github.com/samber/lo"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -45,6 +47,8 @@ var serviceTypeMeta = metav1.TypeMeta{

func TestKongState_SanitizedCopy(t *testing.T) {
testedFields := sets.New[string]()
// this needs a static random seed because some auths generate random values
uuid.SetRand(rand.New(rand.NewSource(1)))

Check failure on line 51 in internal/dataplane/kongstate/kongstate_test.go

View workflow job for this annotation

GitHub Actions / linters / lint

G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
for _, tt := range []struct {
name string
in KongState
Expand Down Expand Up @@ -86,7 +90,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: randRedactedString()}}},
}},
Licenses: []License{{kong.License{ID: kong.String("1"), Payload: redactedString}}},
ConsumerGroups: []ConsumerGroup{{
Expand All @@ -102,6 +106,8 @@ func TestKongState_SanitizedCopy(t *testing.T) {
},
},
} {
// this needs a static random seed because some auths generate random values
uuid.SetRand(rand.New(rand.NewSource(1)))

Check failure on line 110 in internal/dataplane/kongstate/kongstate_test.go

View workflow job for this annotation

GitHub Actions / linters / lint

G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
t.Run(tt.name, func(t *testing.T) {
testedFields.Insert(extractNotEmptyFieldNames(tt.in)...)
got := *tt.in.SanitizedCopy()
Expand Down
4 changes: 4 additions & 0 deletions internal/dataplane/kongstate/plugin_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package kongstate

import (
"math/rand"
"testing"

"github.com/google/uuid"
"github.com/kong/go-kong/kong"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -767,6 +769,8 @@ func TestKongPluginFromK8SPlugin(t *testing.T) {
}

func TestPlugin_SanitizedCopy(t *testing.T) {
// this needs a static random seed because some auths generate random values
uuid.SetRand(rand.New(rand.NewSource(1)))

Check failure on line 773 in internal/dataplane/kongstate/plugin_test.go

View workflow job for this annotation

GitHub Actions / linters / lint

G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
testCases := []struct {
name string
config kong.Configuration
Expand Down

0 comments on commit 36538e8

Please sign in to comment.