From 5b003a5657c7fdc489d2a2b3e05e25424be22fc2 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 19 Nov 2020 19:58:50 +0000 Subject: [PATCH] SecretSource.Value should be plain text in memory --- oauthproxy_test.go | 4 ++-- pkg/apis/options/legacy_options.go | 3 +-- pkg/apis/options/legacy_options_test.go | 5 ++--- pkg/apis/options/util/util.go | 5 +---- pkg/apis/options/util/util_test.go | 15 +++------------ pkg/header/injector_test.go | 14 +++++++------- pkg/validation/common.go | 11 +---------- pkg/validation/common_test.go | 11 +---------- pkg/validation/header_test.go | 4 ++-- 9 files changed, 20 insertions(+), 52 deletions(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index fe68c90e..866109ca 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -515,7 +515,7 @@ func TestBasicAuthPassword(t *testing.T) { ClaimSource: &options.ClaimSource{ Claim: "email", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthPassword))), + Value: []byte(basicAuthPassword), }, }, }, @@ -1408,7 +1408,7 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("This is a secure password"))), + Value: []byte("This is a secure password"), }, }, }, diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index ae45bc7e..35edf680 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -1,7 +1,6 @@ package options import ( - "encoding/base64" "fmt" "net/url" "strconv" @@ -235,7 +234,7 @@ func getBasicAuthHeader(preferEmailToUser bool, basicAuthPassword string) Header Claim: claim, Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthPassword))), + Value: []byte(basicAuthPassword), }, }, }, diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index 684d7874..dbac5793 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -1,7 +1,6 @@ package options import ( - "encoding/base64" "time" . "github.com/onsi/ginkgo" @@ -332,7 +331,7 @@ var _ = Describe("Legacy Options", func() { Claim: "user", Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthSecret))), + Value: []byte(basicAuthSecret), }, }, }, @@ -372,7 +371,7 @@ var _ = Describe("Legacy Options", func() { Claim: "email", Prefix: "Basic ", BasicAuthPassword: &SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte(basicAuthSecret))), + Value: []byte(basicAuthSecret), }, }, }, diff --git a/pkg/apis/options/util/util.go b/pkg/apis/options/util/util.go index 918da13a..99988bdc 100644 --- a/pkg/apis/options/util/util.go +++ b/pkg/apis/options/util/util.go @@ -1,7 +1,6 @@ package util import ( - "encoding/base64" "errors" "io/ioutil" "os" @@ -13,9 +12,7 @@ import ( func GetSecretValue(source *options.SecretSource) ([]byte, error) { switch { case len(source.Value) > 0 && source.FromEnv == "" && source.FromFile == "": - value := make([]byte, base64.StdEncoding.DecodedLen(len(source.Value))) - decoded, err := base64.StdEncoding.Decode(value, source.Value) - return value[:decoded], err + return source.Value, nil case len(source.Value) == 0 && source.FromEnv != "" && source.FromFile == "": return []byte(os.Getenv(source.FromEnv)), nil case len(source.Value) == 0 && source.FromEnv == "" && source.FromFile != "": diff --git a/pkg/apis/options/util/util_test.go b/pkg/apis/options/util/util_test.go index 5ca76a04..8ee8cc30 100644 --- a/pkg/apis/options/util/util_test.go +++ b/pkg/apis/options/util/util_test.go @@ -1,7 +1,6 @@ package util import ( - "encoding/base64" "io/ioutil" "os" "path" @@ -31,20 +30,12 @@ var _ = Describe("GetSecretValue", func() { os.RemoveAll(fileDir) }) - It("returns the correct value from base64", func() { - originalValue := []byte("secret-value-1") - b64Value := base64.StdEncoding.EncodeToString((originalValue)) - - // Once encoded, the originalValue could have a decoded length longer than - // its actual length, ensure we trim this. - // This assertion ensures we are testing the triming - Expect(len(originalValue)).To(BeNumerically("<", base64.StdEncoding.DecodedLen(len(b64Value)))) - + It("returns the correct value from the string value", func() { value, err := GetSecretValue(&options.SecretSource{ - Value: []byte(b64Value), + Value: []byte("secret-value-1"), }) Expect(err).ToNot(HaveOccurred()) - Expect(value).To(Equal(originalValue)) + Expect(string(value)).To(Equal("secret-value-1")) }) It("returns the correct value from the environment", func() { diff --git a/pkg/header/injector_test.go b/pkg/header/injector_test.go index af034fd9..63f1e87a 100644 --- a/pkg/header/injector_test.go +++ b/pkg/header/injector_test.go @@ -49,14 +49,14 @@ var _ = Describe("Injector Suite", func() { }, expectedErr: nil, }), - Entry("with a static valued header from base64", newInjectorTableInput{ + Entry("with a static valued header from string", newInjectorTableInput{ headers: []options.Header{ { Name: "Secret", Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("super-secret"))), + Value: []byte("super-secret"), }, }, }, @@ -200,7 +200,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: []byte("basic-password"), }, }, }, @@ -349,7 +349,7 @@ var _ = Describe("Injector Suite", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("basic-password"))), + Value: []byte("basic-password"), }, }, }, @@ -380,17 +380,17 @@ var _ = Describe("Injector Suite", func() { Values: []options.HeaderValue{ { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("major=1"))), + Value: []byte("major=1"), }, }, { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("minor=2"))), + Value: []byte("minor=2"), }, }, { SecretSource: &options.SecretSource{ - Value: []byte(base64.StdEncoding.EncodeToString([]byte("patch=3"))), + Value: []byte("patch=3"), }, }, }, diff --git a/pkg/validation/common.go b/pkg/validation/common.go index bc9dba28..b9cb8e6f 100644 --- a/pkg/validation/common.go +++ b/pkg/validation/common.go @@ -1,7 +1,6 @@ package validation import ( - "encoding/base64" "fmt" "os" @@ -13,7 +12,7 @@ const multipleValuesForSecretSource = "multiple values specified for secret sour func validateSecretSource(source options.SecretSource) string { switch { case len(source.Value) > 0 && source.FromEnv == "" && source.FromFile == "": - return validateSecretSourceValue(source.Value) + return "" case len(source.Value) == 0 && source.FromEnv != "" && source.FromFile == "": return validateSecretSourceEnv(source.FromEnv) case len(source.Value) == 0 && source.FromEnv == "" && source.FromFile != "": @@ -23,14 +22,6 @@ func validateSecretSource(source options.SecretSource) string { } } -func validateSecretSourceValue(value []byte) string { - dst := make([]byte, len(value)) - if _, err := base64.StdEncoding.Decode(dst, value); err != nil { - return fmt.Sprintf("error decoding secret value: %v", err) - } - return "" -} - func validateSecretSourceEnv(key string) string { if value := os.Getenv(key); value == "" { return fmt.Sprintf("error loading secret from environent: no value for for key %q", key) diff --git a/pkg/validation/common_test.go b/pkg/validation/common_test.go index bdce5415..10c179b9 100644 --- a/pkg/validation/common_test.go +++ b/pkg/validation/common_test.go @@ -1,7 +1,6 @@ package validation import ( - "encoding/base64" "io/ioutil" "os" @@ -17,7 +16,7 @@ var _ = Describe("Common", func() { var validSecretSourceFile string BeforeEach(func() { - validSecretSourceValue = []byte(base64.StdEncoding.EncodeToString([]byte("This is a secret source value"))) + validSecretSourceValue = []byte("This is a secret source value") Expect(os.Setenv(validSecretSourceEnv, "This is a secret source env")).To(Succeed()) tmp, err := ioutil.TempFile("", "oauth2-proxy-secret-source-test") Expect(err).ToNot(HaveOccurred()) @@ -110,14 +109,6 @@ var _ = Describe("Common", func() { }, expectedMsg: "", }), - Entry("with an invalid Value", validateSecretSourceTableInput{ - source: func() options.SecretSource { - return options.SecretSource{ - Value: []byte("Invalid Base64 Value"), - } - }, - expectedMsg: "error decoding secret value: illegal base64 data at input byte 7", - }), Entry("with an invalid FromEnv", validateSecretSourceTableInput{ source: func() options.SecretSource { return options.SecretSource{ diff --git a/pkg/validation/header_test.go b/pkg/validation/header_test.go index fee4525d..ccc90857 100644 --- a/pkg/validation/header_test.go +++ b/pkg/validation/header_test.go @@ -148,7 +148,7 @@ var _ = Describe("Headers", func() { ClaimSource: &options.ClaimSource{ Claim: "user", BasicAuthPassword: &options.SecretSource{ - Value: []byte("secret"), + FromEnv: "UNKNOWN_ENV", }, }, }, @@ -157,7 +157,7 @@ var _ = Describe("Headers", func() { validHeader1, }, expectedMsgs: []string{ - "invalid header \"With-Invalid-Basic-Auth\": invalid values: invalid basicAuthPassword: error decoding secret value: illegal base64 data at input byte 4", + "invalid header \"With-Invalid-Basic-Auth\": invalid values: invalid basicAuthPassword: error loading secret from environent: no value for for key \"UNKNOWN_ENV\"", }, }), )