Skip to content

Commit 1383d01

Browse files
oliverbaehleroliverbaehler
authored
feat(controller): add SkipImpersonationReview featuregate (#422)
Signed-off-by: Oliver Bähler <[email protected]>
1 parent 7edba59 commit 1383d01

File tree

8 files changed

+77
-42
lines changed

8 files changed

+77
-42
lines changed

‎internal/features/features.go

+8
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,12 @@ const (
1111
// for resources in all tenant namespaces resulting in increased memory
1212
// usage and cluster-wide RBAC permissions (list and watch).
1313
ProxyAllNamespaced = "ProxyAllNamespaced"
14+
15+
// SkipImpersonationReview allows to skip the impersonation review
16+
// for all requests containing impersonation headers (user and groups)
17+
//
18+
// DANGER: Enabling this flag allows any user to impersonate as any user or group
19+
// essentially bypassing any authorization. Only use this option in trusted environments
20+
// where authorization/authentication is offloaded to external systems.
21+
SkipImpersonationReview = "SkipImpersonationReview"
1422
)

‎internal/options/kube.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,13 @@ type kubeOpts struct {
2424
url url.URL
2525
ignoredGroups []string
2626
ignoredImpersonationGroups []string
27+
skipImpersonationReview bool
2728
claimName string
2829
impersonationGroupsRegexp *regexp.Regexp
2930
config *rest.Config
3031
}
3132

32-
func NewKube(authTypes []request.AuthType, ignoredGroups []string, claimName string, config *rest.Config, ignoredImpersonationGroups []string, impersonationGroupsString string) (ListenerOpts, error) {
33+
func NewKube(authTypes []request.AuthType, ignoredGroups []string, claimName string, config *rest.Config, ignoredImpersonationGroups []string, impersonationGroupsString string, skipImpersonationReview bool) (ListenerOpts, error) {
3334
u, err := url.Parse(config.Host)
3435
if err != nil {
3536
return nil, fmt.Errorf("cannot create Kubernetes Options due to failed URL parsing: %w", err)
@@ -49,6 +50,7 @@ func NewKube(authTypes []request.AuthType, ignoredGroups []string, claimName str
4950
ignoredGroups: ignoredGroups,
5051
ignoredImpersonationGroups: ignoredImpersonationGroups,
5152
impersonationGroupsRegexp: impersonationGroupsRegexp,
53+
skipImpersonationReview: skipImpersonationReview,
5254
claimName: claimName,
5355
config: config,
5456
}, nil
@@ -82,6 +84,10 @@ func (k kubeOpts) PreferredUsernameClaim() string {
8284
return k.claimName
8385
}
8486

87+
func (k kubeOpts) SkipImpersonationReview() bool {
88+
return k.skipImpersonationReview
89+
}
90+
8591
func (k kubeOpts) ReverseProxyTransport() (*http.Transport, error) {
8692
transportConfig, err := k.config.TransportConfig()
8793
if err != nil {

‎internal/options/listener.go

+1
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ type ListenerOpts interface {
2020
PreferredUsernameClaim() string
2121
ReverseProxyTransport() (*http.Transport, error)
2222
BearerToken() string
23+
SkipImpersonationReview() bool
2324
}

‎internal/request/http.go

+38-31
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@ type http struct {
2222
usernameClaimField string
2323
ignoredImpersonationGroups []string
2424
impersonationGroupsRegexp *regexp.Regexp
25+
skipImpersonationReview bool
2526
client client.Writer
2627
}
2728

28-
func NewHTTP(request *h.Request, authTypes []AuthType, usernameClaimField string, client client.Writer, ignoredImpersonationGroups []string, impersonationGroupsRegexp *regexp.Regexp) Request {
29-
return &http{Request: request, authTypes: authTypes, usernameClaimField: usernameClaimField, client: client, ignoredImpersonationGroups: ignoredImpersonationGroups, impersonationGroupsRegexp: impersonationGroupsRegexp}
29+
func NewHTTP(request *h.Request, authTypes []AuthType, usernameClaimField string, client client.Writer, ignoredImpersonationGroups []string, impersonationGroupsRegexp *regexp.Regexp, skipImpersonationReview bool) Request {
30+
return &http{Request: request, authTypes: authTypes, usernameClaimField: usernameClaimField, client: client, ignoredImpersonationGroups: ignoredImpersonationGroups, impersonationGroupsRegexp: impersonationGroupsRegexp, skipImpersonationReview: skipImpersonationReview}
3031
}
3132

3233
func (h http) GetHTTPRequest() *h.Request {
@@ -49,14 +50,45 @@ func (h http) GetUserAndGroups() (username string, groups []string, err error) {
4950

5051
// In case the requester is asking for impersonation, we have to be sure that's allowed by creating a
5152
// SubjectAccessReview with the requested data, before proceeding.
53+
//nolint:nestif
5254
if impersonateGroups := GetImpersonatingGroups(h.Request, h.ignoredImpersonationGroups, h.impersonationGroupsRegexp); len(impersonateGroups) > 0 {
53-
for _, impersonateGroup := range impersonateGroups {
55+
if !h.skipImpersonationReview {
56+
for _, impersonateGroup := range impersonateGroups {
57+
ac := &authorizationv1.SubjectAccessReview{
58+
Spec: authorizationv1.SubjectAccessReviewSpec{
59+
ResourceAttributes: &authorizationv1.ResourceAttributes{
60+
Verb: "impersonate",
61+
Resource: "groups",
62+
Name: impersonateGroup,
63+
},
64+
User: username,
65+
Groups: groups,
66+
},
67+
}
68+
if err = h.client.Create(h.Request.Context(), ac); err != nil {
69+
return "", nil, err
70+
}
71+
72+
if !ac.Status.Allowed {
73+
return "", nil, NewErrUnauthorized(fmt.Sprintf("the current user %s cannot impersonate the group %s", username, impersonateGroup))
74+
}
75+
}
76+
}
77+
78+
defer func() {
79+
groups = impersonateGroups
80+
}()
81+
}
82+
83+
//nolint:nestif
84+
if impersonateUser := GetImpersonatingUser(h.Request); len(impersonateUser) > 0 {
85+
if !h.skipImpersonationReview {
5486
ac := &authorizationv1.SubjectAccessReview{
5587
Spec: authorizationv1.SubjectAccessReviewSpec{
5688
ResourceAttributes: &authorizationv1.ResourceAttributes{
5789
Verb: "impersonate",
58-
Resource: "groups",
59-
Name: impersonateGroup,
90+
Resource: "users",
91+
Name: impersonateUser,
6092
},
6193
User: username,
6294
Groups: groups,
@@ -67,35 +99,10 @@ func (h http) GetUserAndGroups() (username string, groups []string, err error) {
6799
}
68100

69101
if !ac.Status.Allowed {
70-
return "", nil, NewErrUnauthorized(fmt.Sprintf("the current user %s cannot impersonate the group %s", username, impersonateGroup))
102+
return "", nil, NewErrUnauthorized(fmt.Sprintf("the current user %s cannot impersonate the user %s", username, impersonateUser))
71103
}
72104
}
73105

74-
defer func() {
75-
groups = impersonateGroups
76-
}()
77-
}
78-
79-
if impersonateUser := GetImpersonatingUser(h.Request); len(impersonateUser) > 0 {
80-
ac := &authorizationv1.SubjectAccessReview{
81-
Spec: authorizationv1.SubjectAccessReviewSpec{
82-
ResourceAttributes: &authorizationv1.ResourceAttributes{
83-
Verb: "impersonate",
84-
Resource: "users",
85-
Name: impersonateUser,
86-
},
87-
User: username,
88-
Groups: groups,
89-
},
90-
}
91-
if err = h.client.Create(h.Request.Context(), ac); err != nil {
92-
return "", nil, err
93-
}
94-
95-
if !ac.Status.Allowed {
96-
return "", nil, NewErrUnauthorized(fmt.Sprintf("the current user %s cannot impersonate the user %s", username, impersonateUser))
97-
}
98-
99106
// Assign impersonate user after group impersonation with current user
100107
// As defer func works in LIFO, if user is also impersonating groups, they will be set to correct value in the previous defer func.
101108
// Otherwise, groups will be set to nil, meaning we are checking just user permissions.

‎internal/request/http_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func Test_http_GetUserAndGroups(t *testing.T) {
5252
authTypes []request.AuthType
5353
ignoreGroups []string
5454
ignoreImpersonationRegexp *regexp.Regexp
55+
skipImpersonationReview bool
5556
usernameClaimField string
5657
client client.Writer
5758
}
@@ -201,7 +202,7 @@ func Test_http_GetUserAndGroups(t *testing.T) {
201202
t.Run(tc.name, func(t *testing.T) {
202203
t.Parallel()
203204

204-
req := request.NewHTTP(tc.fields.Request, tc.fields.authTypes, tc.fields.usernameClaimField, tc.fields.client, tc.fields.ignoreGroups, tc.fields.ignoreImpersonationRegexp)
205+
req := request.NewHTTP(tc.fields.Request, tc.fields.authTypes, tc.fields.usernameClaimField, tc.fields.client, tc.fields.ignoreGroups, tc.fields.ignoreImpersonationRegexp, tc.fields.skipImpersonationReview)
205206
gotUsername, gotGroups, err := req.GetUserAndGroups()
206207
if (err != nil) != tc.wantErr {
207208
t.Errorf("GetUserAndGroups() error = %v, wantErr %v", err, tc.wantErr)

‎internal/webserver/middleware/user_in_group.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package middleware
55

66
import (
77
"net/http"
8+
"regexp"
89

910
"github.com/go-logr/logr"
1011
"github.com/gorilla/mux"
@@ -15,11 +16,11 @@ import (
1516
req "github.com/projectcapsule/capsule-proxy/internal/request"
1617
)
1718

18-
func CheckUserInIgnoredGroupMiddleware(client client.Writer, log logr.Logger, claim string, authTypes []req.AuthType, ignoredUserGroups sets.Set[string], fn func(writer http.ResponseWriter, request *http.Request)) mux.MiddlewareFunc {
19+
func CheckUserInIgnoredGroupMiddleware(client client.Writer, log logr.Logger, claim string, authTypes []req.AuthType, ignoredUserGroups sets.Set[string], ignoredImpersonationGroups []string, impersonationGroupsRegexp *regexp.Regexp, skipImpersonationReview bool, fn func(writer http.ResponseWriter, request *http.Request)) mux.MiddlewareFunc {
1920
return func(next http.Handler) http.Handler {
2021
return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
2122
if ignoredUserGroups.Len() > 0 {
22-
user, groups, err := req.NewHTTP(request, authTypes, claim, client, nil, nil).GetUserAndGroups()
23+
user, groups, err := req.NewHTTP(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview).GetUserAndGroups()
2324
if err != nil {
2425
log.Error(err, "Cannot retrieve username and group from request")
2526
}
@@ -39,10 +40,10 @@ func CheckUserInIgnoredGroupMiddleware(client client.Writer, log logr.Logger, cl
3940
}
4041
}
4142

42-
func CheckUserInCapsuleGroupMiddleware(client client.Writer, log logr.Logger, claim string, authTypes []req.AuthType, impersonate func(http.ResponseWriter, *http.Request)) mux.MiddlewareFunc {
43+
func CheckUserInCapsuleGroupMiddleware(client client.Writer, log logr.Logger, claim string, authTypes []req.AuthType, ignoredImpersonationGroups []string, impersonationGroupsRegexp *regexp.Regexp, skipImpersonationReview bool, impersonate func(http.ResponseWriter, *http.Request)) mux.MiddlewareFunc {
4344
return func(next http.Handler) http.Handler {
4445
return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
45-
_, groups, err := req.NewHTTP(request, authTypes, claim, client, nil, nil).GetUserAndGroups()
46+
_, groups, err := req.NewHTTP(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview).GetUserAndGroups()
4647
if err != nil {
4748
log.Error(err, "Cannot retrieve username and group from request")
4849
}

‎internal/webserver/webserver.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ func NewKubeFilter(opts options.ListenerOpts, srv options.ServerOptions, gates f
7575
ignoredUserGroups: sets.New(opts.IgnoredGroupNames()...),
7676
ignoredImpersonationGroups: opts.IgnoredImpersonationsGroups(),
7777
impersonationGroupsRegexp: opts.ImpersonationGroupsRegexp(),
78+
skipImpersonationReview: opts.SkipImpersonationReview(),
7879
reverseProxy: reverseProxy,
7980
bearerToken: opts.BearerToken(),
8081
usernameClaimField: opts.PreferredUsernameClaim(),
@@ -90,6 +91,7 @@ type kubeFilter struct {
9091
ignoredUserGroups sets.Set[string]
9192
ignoredImpersonationGroups []string
9293
impersonationGroupsRegexp *regexp.Regexp
94+
skipImpersonationReview bool
9395
reverseProxy *httputil.ReverseProxy
9496
bearerToken string
9597
usernameClaimField string
@@ -182,7 +184,7 @@ func (n *kubeFilter) handleRequest(request *http.Request, selector labels.Select
182184
}
183185

184186
func (n *kubeFilter) impersonateHandler(writer http.ResponseWriter, request *http.Request) {
185-
hr := req.NewHTTP(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp)
187+
hr := req.NewHTTP(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview)
186188

187189
username, groups, err := hr.GetUserAndGroups()
188190
if err != nil {
@@ -276,11 +278,11 @@ func (n *kubeFilter) registerModules(ctx context.Context, root *mux.Router) {
276278
sr.Use(
277279
middleware.CheckPaths(n.log, n.allowedPaths, n.impersonateHandler),
278280
middleware.CheckJWTMiddleware(n.writer),
279-
middleware.CheckUserInIgnoredGroupMiddleware(n.writer, n.log, n.usernameClaimField, n.authTypes, n.ignoredUserGroups, n.impersonateHandler),
280-
middleware.CheckUserInCapsuleGroupMiddleware(n.writer, n.log, n.usernameClaimField, n.authTypes, n.impersonateHandler),
281+
middleware.CheckUserInIgnoredGroupMiddleware(n.writer, n.log, n.usernameClaimField, n.authTypes, n.ignoredUserGroups, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview, n.impersonateHandler),
282+
middleware.CheckUserInCapsuleGroupMiddleware(n.writer, n.log, n.usernameClaimField, n.authTypes, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview, n.impersonateHandler),
281283
)
282284
sr.HandleFunc("", func(writer http.ResponseWriter, request *http.Request) {
283-
proxyRequest := req.NewHTTP(request, n.authTypes, n.usernameClaimField, n.writer, nil, nil)
285+
proxyRequest := req.NewHTTP(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview)
284286
username, groups, err := proxyRequest.GetUserAndGroups()
285287
if err != nil {
286288
server.HandleError(writer, err, "cannot retrieve user and group from the request")

‎main.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ func main() {
6666
LockToDefault: false,
6767
PreRelease: featuregate.Alpha,
6868
},
69+
features.SkipImpersonationReview: {
70+
Default: false,
71+
LockToDefault: false,
72+
PreRelease: featuregate.Alpha,
73+
},
6974
}))
7075

7176
authTypes := []request.AuthType{
@@ -150,6 +155,10 @@ First match is used and can be specified multiple times as comma separated value
150155
log.Info(fmt.Sprintf("The Groups dropped for impersonation %s", ignoreImpersonationGroups))
151156
}
152157

158+
if gates.Enabled(features.SkipImpersonationReview) {
159+
log.Info("SECURITY IMPLICATION: Skipping Impersonation reviews are enabled!")
160+
}
161+
153162
log.Info("---")
154163
log.Info("Creating the manager")
155164

@@ -209,7 +218,7 @@ First match is used and can be specified multiple times as comma separated value
209218

210219
var listenerOpts options.ListenerOpts
211220

212-
if listenerOpts, err = options.NewKube(authTypes, ignoredUserGroups, usernameClaimField, config, ignoreImpersonationGroups, impersonationGroupsRegexp); err != nil {
221+
if listenerOpts, err = options.NewKube(authTypes, ignoredUserGroups, usernameClaimField, config, ignoreImpersonationGroups, impersonationGroupsRegexp, gates.Enabled(features.SkipImpersonationReview)); err != nil {
213222
log.Error(err, "cannot create Kubernetes options")
214223
os.Exit(1)
215224
}

0 commit comments

Comments
 (0)