Skip to content

Commit

Permalink
characters in password can not be escaped.
Browse files Browse the repository at this point in the history
  • Loading branch information
aorcholski committed Apr 12, 2022
1 parent fd75261 commit 6361b3a
Show file tree
Hide file tree
Showing 5 changed files with 226 additions and 227 deletions.
2 changes: 1 addition & 1 deletion src/agproxysecret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (agProxySecretGenerator *ActiveGateProxySecretGenerator) proxyUrlFromUserSe
func parseProxyUrl(proxy string) (host string, port string, username string, password string, err error) {
proxyUrl, err := url.Parse(proxy)
if err != nil {
return "", "", "", "", fmt.Errorf("could not parse proxy URL")
return "", "", "", "", errors.New("could not parse proxy URL")
}

passwd, _ := proxyUrl.User.Password()
Expand Down
74 changes: 0 additions & 74 deletions src/webhook/validation/activegate.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
package validation

import (
"context"
"fmt"
"net/url"

"github.com/Dynatrace/dynatrace-operator/src/agproxysecret"
dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/src/api/v1beta1"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"sigs.k8s.io/controller-runtime/pkg/client"
)

const (
Expand All @@ -24,16 +19,6 @@ Make sure you correctly specify the ActiveGate capabilities in your custom resou
Make sure you don't duplicate an Activegate capability in your custom resource.
`
warningMissingActiveGateMemoryLimit = `ActiveGate specification missing memory limits. Can cause excess memory usage.`

errorInvalidActiveGateProxyUrl = `The DynaKube's specification has an invalid Proxy URL value set. Make sure you correctly specify the URL in your custom resource.`
errorInvalidEvalCharacter = `The DynaKube's specification has an invalid Proxy password value set. Make sure you correctly escape quotation mark, backtick and backslash characters using backslash.`

errorMissingActiveGateProxySecret = `The Proxy secret indicated by the DynaKube specification doesn't exist.`

errorInvalidProxySecretFormat = `The Proxy secret indicated by the DynaKube specification has an invalid format. Make sure you correctly creates the secret.`

errorInvalidProxySecretUrl = `The Proxy secret indicated by the DynaKube specification has an invalid URL value set. Make sure you correctly specify the URL in the secret.`
errorInvalidProxySecretEvalCharacter = `The Proxy secret indicated by the DynaKube specification has an invalid Proxy password value set. Make sure you correctly escape quotation mark, backtick and backslash characters using backslash.`
)

func conflictingActiveGateConfiguration(dv *dynakubeValidator, dynakube *dynatracev1beta1.DynaKube) string {
Expand Down Expand Up @@ -81,65 +66,6 @@ func missingActiveGateMemoryLimit(dv *dynakubeValidator, dynakube *dynatracev1be
return ""
}

func invalidActiveGateProxyUrl(dv *dynakubeValidator, dynakube *dynatracev1beta1.DynaKube) string {
if dynakube.Spec.Proxy != nil {
if len(dynakube.Spec.Proxy.ValueFrom) > 0 {
var proxySecret corev1.Secret
err := dv.clt.Get(context.TODO(), client.ObjectKey{Name: dynakube.Spec.Proxy.ValueFrom, Namespace: dynakube.Namespace}, &proxySecret)
if k8serrors.IsNotFound(err) {
return errorMissingActiveGateProxySecret
} else if err != nil {
return fmt.Sprintf("error occurred while reading PROXY secret indicated in the Dynakube specification (%s)", err.Error())
}
proxyUrl, ok := proxySecret.Data[agproxysecret.ProxySecretKey]
if !ok {
return errorInvalidProxySecretFormat
}
return validateProxyUrl(string(proxyUrl), errorInvalidProxySecretUrl, errorInvalidProxySecretEvalCharacter)
} else if len(dynakube.Spec.Proxy.Value) > 0 {
return validateProxyUrl(dynakube.Spec.Proxy.Value, errorInvalidActiveGateProxyUrl, errorInvalidEvalCharacter)
}
}
return ""
}

func memoryLimitSet(resources corev1.ResourceRequirements) bool {
return resources.Limits != nil && resources.Limits.Memory() != nil
}

// proxyUrl is valid if
// 1) encoded
// 2) "`\ are escaped using \
func validateProxyUrl(proxyUrl string, parseErrorMessage string, evalErrorMessage string) string {
if parsedUrl, err := url.Parse(proxyUrl); err != nil {
return parseErrorMessage
} else {
password, _ := parsedUrl.User.Password()
if isEvalEscapeNeeded(password) {
return evalErrorMessage
}
}
return ""
}

// 'eval' command is used by entrypoint.sh:readSecret function to return its result.
// For this reason quotation mark ", backtick ` and backslash \ characters have to be escaped using backslash.
func isEvalEscapeNeeded(str string) bool {
previousChar := '\000'
for _, char := range str {
if char == '"' || char == '`' {
if previousChar != '\\' {
return true
}
previousChar = char
} else if previousChar == '\\' {
if char != '\\' {
return true
}
previousChar = '\000'
} else {
previousChar = char
}
}
return false
}
152 changes: 0 additions & 152 deletions src/webhook/validation/activegate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,8 @@ import (
"testing"

dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/src/api/v1beta1"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
testProxySecret = "proxysecret"

plainTextProxyUrl = "http://test:test \"#%/<>?[\\]^`{|}pass@proxy-service.dynatrace:3128"
encodedProxyUrl = "http://test:test%20%5C%22%23%25%2F%3C%3E%3F%5B%5C%5C%5D%5E%5C%60%7B%7C%7Dpass@proxy-service.dynatrace:3128"
)

func TestConflictingActiveGateConfiguration(t *testing.T) {
Expand Down Expand Up @@ -158,146 +149,3 @@ func TestMissingActiveGateMemoryLimit(t *testing.T) {
})
})
}

func TestInvalidActiveGateProxy(t *testing.T) {
t.Run(`valid proxy url`, func(t *testing.T) {
assertAllowedResponseWithoutWarnings(t,
&dynatracev1beta1.DynaKube{
ObjectMeta: defaultDynakubeObjectMeta,
Spec: dynatracev1beta1.DynaKubeSpec{
APIURL: testApiUrl,
Proxy: &dynatracev1beta1.DynaKubeProxy{
Value: encodedProxyUrl,
ValueFrom: "",
},
},
})
})

t.Run(`invalid proxy url`, func(t *testing.T) {
assertDeniedResponse(t,
[]string{errorInvalidActiveGateProxyUrl},
&dynatracev1beta1.DynaKube{
ObjectMeta: defaultDynakubeObjectMeta,
Spec: dynatracev1beta1.DynaKubeSpec{
APIURL: testApiUrl,
Proxy: &dynatracev1beta1.DynaKubeProxy{
Value: plainTextProxyUrl,
ValueFrom: "",
},
},
})
})

t.Run(`valid proxy secret url`, func(t *testing.T) {
assertAllowedResponseWithoutWarnings(t,
&dynatracev1beta1.DynaKube{
ObjectMeta: defaultDynakubeObjectMeta,
Spec: dynatracev1beta1.DynaKubeSpec{
APIURL: testApiUrl,
Proxy: &dynatracev1beta1.DynaKubeProxy{
Value: "",
ValueFrom: testProxySecret,
},
},
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: testProxySecret,
Namespace: testNamespace,
},
Data: map[string][]byte{
"proxy": []byte(encodedProxyUrl),
},
})
})

t.Run(`missing proxy secret`, func(t *testing.T) {
assertDeniedResponse(t,
[]string{errorMissingActiveGateProxySecret},
&dynatracev1beta1.DynaKube{
ObjectMeta: defaultDynakubeObjectMeta,
Spec: dynatracev1beta1.DynaKubeSpec{
APIURL: testApiUrl,
Proxy: &dynatracev1beta1.DynaKubeProxy{
Value: "",
ValueFrom: testProxySecret,
},
},
})
})

t.Run(`invalid format of proxy secret`, func(t *testing.T) {
assertDeniedResponse(t,
[]string{errorInvalidProxySecretFormat},
&dynatracev1beta1.DynaKube{
ObjectMeta: defaultDynakubeObjectMeta,
Spec: dynatracev1beta1.DynaKubeSpec{
APIURL: testApiUrl,
Proxy: &dynatracev1beta1.DynaKubeProxy{
Value: "",
ValueFrom: testProxySecret,
},
},
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: testProxySecret,
Namespace: testNamespace,
},
Data: map[string][]byte{
"invalid-name": []byte(encodedProxyUrl),
},
})
})

t.Run(`invalid proxy secret url`, func(t *testing.T) {
assertDeniedResponse(t,
[]string{errorInvalidProxySecretUrl},
&dynatracev1beta1.DynaKube{
ObjectMeta: defaultDynakubeObjectMeta,
Spec: dynatracev1beta1.DynaKubeSpec{
APIURL: testApiUrl,
Proxy: &dynatracev1beta1.DynaKubeProxy{
Value: "",
ValueFrom: testProxySecret,
},
},
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: testProxySecret,
Namespace: testNamespace,
},
Data: map[string][]byte{
"proxy": []byte(plainTextProxyUrl),
},
})
})

t.Run(`invalid proxy secret url - eval`, func(t *testing.T) {
assert.Equal(t, false, isEvalEscapeNeeded("password"))

// single quotation mark
assert.Equal(t, true, isEvalEscapeNeeded("pass\"word"))
assert.Equal(t, false, isEvalEscapeNeeded("pass\\\"word"))

// backtick
assert.Equal(t, true, isEvalEscapeNeeded("pass`word"))
assert.Equal(t, false, isEvalEscapeNeeded("pass\\`word"))

// single backslash
assert.Equal(t, true, isEvalEscapeNeeded("pass\\word"))
assert.Equal(t, false, isEvalEscapeNeeded("pass\\\\word"))

// odd number of backslashes
assert.Equal(t, true, isEvalEscapeNeeded("pass\\\\\\word"))
assert.Equal(t, false, isEvalEscapeNeeded("pass\\\\\\\\word"))

// quotation mark, backtick, backslash
assert.Equal(t, false, isEvalEscapeNeeded("pass\\\"\\`\\\\word"))

// UTF-8 single character - U+1F600 grinning face
assert.Equal(t, false, isEvalEscapeNeeded("\xF0\x9F\x98\x80"))
})
}
75 changes: 75 additions & 0 deletions src/webhook/validation/proxy_url.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package validation

import (
"context"
"net/url"

"github.com/Dynatrace/dynatrace-operator/src/agproxysecret"
dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/src/api/v1beta1"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
"sigs.k8s.io/controller-runtime/pkg/client"
)

const (
errorInvalidActiveGateProxyUrl = `The DynaKube's specification has an invalid Proxy URL value set. Make sure you correctly specify the URL in your custom resource.`
errorInvalidEvalCharacter = `The DynaKube's specification has an invalid Proxy password value set. Make sure you don't use forbidden characters.`

errorMissingActiveGateProxySecret = `The Proxy secret indicated by the DynaKube specification doesn't exist.`

errorInvalidProxySecretFormat = `The Proxy secret indicated by the DynaKube specification has an invalid format. Make sure you correctly creates the secret.`

errorInvalidProxySecretUrl = `The Proxy secret indicated by the DynaKube specification has an invalid URL value set. Make sure you correctly specify the URL in the secret.`
errorInvalidProxySecretEvalCharacter = `The Proxy secret indicated by the DynaKube specification has an invalid Proxy password value set. Make sure you don't use forbidden characters.`
)

func invalidActiveGateProxyUrl(dv *dynakubeValidator, dynakube *dynatracev1beta1.DynaKube) string {
if dynakube.Spec.Proxy != nil {
if len(dynakube.Spec.Proxy.ValueFrom) > 0 {
var proxySecret corev1.Secret
err := dv.clt.Get(context.TODO(), client.ObjectKey{Name: dynakube.Spec.Proxy.ValueFrom, Namespace: dynakube.Namespace}, &proxySecret)
if k8serrors.IsNotFound(err) {
return errorMissingActiveGateProxySecret
} else if err != nil {
return errors.Wrap(err, "error occurred while reading PROXY secret indicated in the Dynakube specification").Error()
}
proxyUrl, ok := proxySecret.Data[agproxysecret.ProxySecretKey]
if !ok {
return errorInvalidProxySecretFormat
}
return validateProxyUrl(string(proxyUrl), errorInvalidProxySecretUrl, errorInvalidProxySecretEvalCharacter)
} else if len(dynakube.Spec.Proxy.Value) > 0 {
return validateProxyUrl(dynakube.Spec.Proxy.Value, errorInvalidActiveGateProxyUrl, errorInvalidEvalCharacter)
}
}
return ""
}

// proxyUrl is valid if
// 1) encoded
// 2) password does not contain '` characters
func validateProxyUrl(proxyUrl string, parseErrorMessage string, evalErrorMessage string) string {
if parsedUrl, err := url.Parse(proxyUrl); err != nil {
return parseErrorMessage
} else {
password, _ := parsedUrl.User.Password()
if evalIsStringInvalid(password) {
return evalErrorMessage
}
}
return ""
}

// 'eval' command is used by entrypoint.sh:readSecret function to return its result.
// For this reason apostrophe ' and backtick ` characters has to be escaped using backslash.
// On the other hand the operator needs a pure password at the same time.
// Finally, these 2 characters are forbidden.
func evalIsStringInvalid(str string) bool {
for _, char := range str {
if char == '\'' || char == '`' {
return true
}
}
return false
}
Loading

0 comments on commit 6361b3a

Please sign in to comment.