From 41bff9cd5e36f8f53dc47fdd63ea13bc19204a6f Mon Sep 17 00:00:00 2001 From: Jake Sanders Date: Tue, 3 Jul 2018 21:19:15 -0700 Subject: [PATCH] Escape illegal characters in remote extra keys Signed-off-by: Jake Sanders Kubernetes-commit: f35e3d07c9898f8ec156209a868fa4451eb9afe2 --- .../request/headerrequest/requestheader.go | 11 +++++- .../headerrequest/requestheader_test.go | 31 +++++++++++++++++ pkg/endpoints/filters/impersonation.go | 11 +++++- pkg/endpoints/filters/impersonation_test.go | 34 +++++++++++++++++++ 4 files changed, 85 insertions(+), 2 deletions(-) diff --git a/pkg/authentication/request/headerrequest/requestheader.go b/pkg/authentication/request/headerrequest/requestheader.go index 38f132b58..948478b80 100644 --- a/pkg/authentication/request/headerrequest/requestheader.go +++ b/pkg/authentication/request/headerrequest/requestheader.go @@ -21,6 +21,7 @@ import ( "fmt" "io/ioutil" "net/http" + "net/url" "strings" "k8s.io/apimachinery/pkg/util/sets" @@ -160,6 +161,14 @@ func allHeaderValues(h http.Header, headerNames []string) []string { return ret } +func unescapeExtraKey(encodedKey string) string { + key, err := url.PathUnescape(encodedKey) // Decode %-encoded bytes. + if err != nil { + return encodedKey // Always record extra strings, even if malformed/unencoded. + } + return key +} + func newExtra(h http.Header, headerPrefixes []string) map[string][]string { ret := map[string][]string{} @@ -170,7 +179,7 @@ func newExtra(h http.Header, headerPrefixes []string) map[string][]string { continue } - extraKey := strings.ToLower(headerName[len(prefix):]) + extraKey := unescapeExtraKey(strings.ToLower(headerName[len(prefix):])) ret[extraKey] = append(ret[extraKey], vv...) } } diff --git a/pkg/authentication/request/headerrequest/requestheader_test.go b/pkg/authentication/request/headerrequest/requestheader_test.go index d81d4ee64..28876dbc4 100644 --- a/pkg/authentication/request/headerrequest/requestheader_test.go +++ b/pkg/authentication/request/headerrequest/requestheader_test.go @@ -152,6 +152,37 @@ func TestRequestHeader(t *testing.T) { }, expectedOk: true, }, + + "escaped extra keys": { + nameHeaders: []string{"X-Remote-User"}, + groupHeaders: []string{"X-Remote-Group"}, + extraPrefixHeaders: []string{"X-Remote-Extra-"}, + requestHeaders: http.Header{ + "X-Remote-User": {"Bob"}, + "X-Remote-Group": {"one-a", "one-b"}, + "X-Remote-Extra-Alpha": {"alphabetical"}, + "X-Remote-Extra-Alph4num3r1c": {"alphanumeric"}, + "X-Remote-Extra-Percent%20encoded": {"percent encoded"}, + "X-Remote-Extra-Almost%zzpercent%xxencoded": {"not quite percent encoded"}, + "X-Remote-Extra-Example.com%2fpercent%2520encoded": {"url with double percent encoding"}, + "X-Remote-Extra-Example.com%2F%E4%BB%8A%E6%97%A5%E3%81%AF": {"url with unicode"}, + "X-Remote-Extra-Abc123!#$+.-_*\\^`~|'": {"header key legal characters"}, + }, + expectedUser: &user.DefaultInfo{ + Name: "Bob", + Groups: []string{"one-a", "one-b"}, + Extra: map[string][]string{ + "alpha": {"alphabetical"}, + "alph4num3r1c": {"alphanumeric"}, + "percent encoded": {"percent encoded"}, + "almost%zzpercent%xxencoded": {"not quite percent encoded"}, + "example.com/percent%20encoded": {"url with double percent encoding"}, + "example.com/今日は": {"url with unicode"}, + "abc123!#$+.-_*\\^`~|'": {"header key legal characters"}, + }, + }, + expectedOk: true, + }, } for k, testcase := range testcases { diff --git a/pkg/endpoints/filters/impersonation.go b/pkg/endpoints/filters/impersonation.go index 8e7e9eaee..726cbe4d5 100644 --- a/pkg/endpoints/filters/impersonation.go +++ b/pkg/endpoints/filters/impersonation.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "strings" "github.com/golang/glog" @@ -146,6 +147,14 @@ func WithImpersonation(handler http.Handler, a authorizer.Authorizer, s runtime. }) } +func unescapeExtraKey(encodedKey string) string { + key, err := url.PathUnescape(encodedKey) // Decode %-encoded bytes. + if err != nil { + return encodedKey // Always record extra strings, even if malformed/unencoded. + } + return key +} + // buildImpersonationRequests returns a list of objectreferences that represent the different things we're requesting to impersonate. // Also includes a map[string][]string representing user.Info.Extra // Each request must be authorized against the current user before switching contexts. @@ -175,7 +184,7 @@ func buildImpersonationRequests(headers http.Header) ([]v1.ObjectReference, erro } hasUserExtra = true - extraKey := strings.ToLower(headerName[len(authenticationv1.ImpersonateUserExtraHeaderPrefix):]) + extraKey := unescapeExtraKey(strings.ToLower(headerName[len(authenticationv1.ImpersonateUserExtraHeaderPrefix):])) // make a separate request for each extra value they're trying to set for _, value := range values { diff --git a/pkg/endpoints/filters/impersonation_test.go b/pkg/endpoints/filters/impersonation_test.go index 3357e038d..d309a2109 100644 --- a/pkg/endpoints/filters/impersonation_test.go +++ b/pkg/endpoints/filters/impersonation_test.go @@ -70,6 +70,10 @@ func (impersonateAuthorizer) Authorize(a authorizer.Attributes) (authorized auth return authorizer.DecisionAllow, "", nil } + if len(user.GetGroups()) > 1 && (user.GetGroups()[1] == "escaped-scopes" || user.GetGroups()[1] == "almost-escaped-scopes") { + return authorizer.DecisionAllow, "", nil + } + if len(user.GetGroups()) > 1 && user.GetGroups()[1] == "extra-setter-particular-scopes" && a.GetVerb() == "impersonate" && a.GetResource() == "userextras" && a.GetSubresource() == "scopes" && a.GetName() == "scope-a" { return authorizer.DecisionAllow, "", nil @@ -224,6 +228,36 @@ func TestImpersonationFilter(t *testing.T) { }, expectedCode: http.StatusOK, }, + { + name: "percent-escaped-userextras", + user: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel", "escaped-scopes"}, + }, + impersonationUser: "system:admin", + impersonationUserExtras: map[string][]string{"example.com%2fescaped%e1%9b%84scopes": {"scope-a", "scope-b"}}, + expectedUser: &user.DefaultInfo{ + Name: "system:admin", + Groups: []string{"system:authenticated"}, + Extra: map[string][]string{"example.com/escapedᛄscopes": {"scope-a", "scope-b"}}, + }, + expectedCode: http.StatusOK, + }, + { + name: "almost-percent-escaped-userextras", + user: &user.DefaultInfo{ + Name: "dev", + Groups: []string{"wheel", "almost-escaped-scopes"}, + }, + impersonationUser: "system:admin", + impersonationUserExtras: map[string][]string{"almost%zzpercent%xxencoded": {"scope-a", "scope-b"}}, + expectedUser: &user.DefaultInfo{ + Name: "system:admin", + Groups: []string{"system:authenticated"}, + Extra: map[string][]string{"almost%zzpercent%xxencoded": {"scope-a", "scope-b"}}, + }, + expectedCode: http.StatusOK, + }, { name: "allowed-users-impersonation", user: &user.DefaultInfo{