Skip to content

Commit e1e9450

Browse files
fix: handler overflow panic (#1066)
* fix: hanlding panic Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * fix: hanlding panic Signed-off-by: Oliver Baehler <oliver@sudo-i.net> --------- Signed-off-by: Oliver Baehler <oliver@sudo-i.net>
1 parent 7e9c43f commit e1e9450

7 files changed

Lines changed: 107 additions & 42 deletions

File tree

charts/capsule-proxy/templates/flowschema.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,8 @@ spec:
4040
apiGroups: ["authorization.k8s.io"]
4141
resources: ["subjectaccessreviews"]
4242
clusterScope: true
43+
- verbs: ["create"]
44+
apiGroups: ["authentication.k8s.io"]
45+
resources: ["tokenreviews"]
46+
clusterScope: true
4347
{{- end }}

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ go 1.26.4
55
require (
66
github.com/go-logr/logr v1.4.3
77
github.com/golang-jwt/jwt/v5 v5.3.1
8-
github.com/gorilla/handlers v1.5.2
98
github.com/gorilla/mux v1.8.1
109
github.com/onsi/ginkgo/v2 v2.29.0
1110
github.com/onsi/gomega v1.41.0

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,6 @@ github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 h1:EwtI+Al+DeppwYX2oX
134134
github.com/google/pprof v0.0.0-20260402051712-545e8a4df936/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI=
135135
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
136136
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
137-
github.com/gorilla/handlers v1.5.2 h1:cLTUSsNkgcwhgRqvCNmdbRWG0A3N4F+M2nWKdScwyEE=
138-
github.com/gorilla/handlers v1.5.2/go.mod h1:dX+xVpaxdSw+q0Qek8SSsl3dfMk3jNddUkMzo0GtH0w=
139137
github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
140138
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
141139
github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=

internal/webserver/errors/panic.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,11 @@ func HandleUnauthorized(w http.ResponseWriter, err error, message string) {
2727
}
2828

2929
w.Header().Set("content-type", "application/json")
30+
w.WriteHeader(http.StatusForbidden)
3031

3132
//nolint:errchkjson
3233
b, _ := json.Marshal(status)
3334
_, _ = w.Write(b)
34-
35-
panic(message)
3635
}
3736

3837
func HandleError(w http.ResponseWriter, err error, message string) {
@@ -42,15 +41,16 @@ func HandleError(w http.ResponseWriter, err error, message string) {
4241
Kind: "Status",
4342
APIVersion: "v1",
4443
},
44+
Status: metav1.StatusFailure,
4545
Message: message,
4646
Reason: metav1.StatusReasonInternalError,
47+
Code: http.StatusInternalServerError,
4748
}
4849

4950
w.Header().Set("content-type", "application/json")
51+
w.WriteHeader(http.StatusInternalServerError)
5052

5153
//nolint:errchkjson
5254
b, _ := json.Marshal(status)
5355
_, _ = w.Write(b)
54-
55-
panic(message)
5656
}

internal/webserver/middleware/jwt.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,19 @@ func CheckJWTMiddleware(client client.Writer) mux.MiddlewareFunc {
3939
}
4040
if err = client.Create(request.Context(), &tr); err != nil {
4141
errors.HandleError(writer, err, "cannot create TokenReview")
42+
43+
return
4244
}
4345

4446
if statusErr := tr.Status.Error; len(statusErr) > 0 {
45-
invalidatedToken.Insert(token)
46-
4747
errors.HandleUnauthorized(writer, goerrors.New(statusErr), "cannot authenticate the token due to error")
48+
49+
return
4850
}
4951
case invalidatedToken.Has(token):
5052
errors.HandleUnauthorized(writer, goerrors.New("token is invalid"), "cannot authenticate the token due to error")
53+
54+
return
5155
}
5256

5357
next.ServeHTTP(writer, request)

internal/webserver/middleware/user_in_group.go

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package middleware
55

66
import (
7+
"errors"
78
"net/http"
89
"regexp"
910
"slices"
@@ -15,31 +16,33 @@ import (
1516

1617
"github.com/projectcapsule/capsule-proxy/internal/controllers"
1718
req "github.com/projectcapsule/capsule-proxy/internal/request"
19+
weberrors "github.com/projectcapsule/capsule-proxy/internal/webserver/errors"
1820
)
1921

2022
func CheckUserInIgnoredGroupMiddleware(client client.Writer, log logr.Logger, claim string, authTypes []req.AuthType, ignoredUserGroups sets.Set[string], ignoredImpersonationGroups []string, impersonationGroupsRegexp *regexp.Regexp, skipImpersonationReview bool, xfcc_header string, fn func(writer http.ResponseWriter, request *http.Request)) mux.MiddlewareFunc {
2123
return func(next http.Handler) http.Handler {
2224
return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
23-
if ignoredUserGroups.Len() > 0 {
24-
var (
25-
err error
26-
user string
27-
groups []string
28-
)
29-
30-
request, user, groups, err = req.ResolveUserAndGroups(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview, xfcc_header)
31-
if err != nil {
32-
log.Error(err, "Cannot retrieve username and group from request")
33-
}
34-
35-
if slices.ContainsFunc(groups, func(group string) bool {
36-
return ignoredUserGroups.Has(group)
37-
}) {
38-
log.V(5).Info("current user belongs to ignored groups", "user", user)
39-
fn(writer, request)
40-
41-
return
42-
}
25+
if ignoredUserGroups.Len() == 0 {
26+
next.ServeHTTP(writer, request)
27+
28+
return
29+
}
30+
31+
request, user, groups, err := req.ResolveUserAndGroups(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview, xfcc_header)
32+
if err != nil {
33+
log.Error(err, "Cannot retrieve username and group from request")
34+
handleResolveUserAndGroupsError(writer, err)
35+
36+
return
37+
}
38+
39+
if slices.ContainsFunc(groups, func(group string) bool {
40+
return ignoredUserGroups.Has(group)
41+
}) {
42+
log.V(5).Info("current user belongs to ignored groups", "user", user)
43+
fn(writer, request)
44+
45+
return
4346
}
4447

4548
next.ServeHTTP(writer, request)
@@ -53,6 +56,9 @@ func CheckUserInCapsuleGroupMiddleware(client client.Writer, log logr.Logger, cl
5356
request, user, groups, err := req.ResolveUserAndGroups(request, authTypes, claim, client, ignoredImpersonationGroups, impersonationGroupsRegexp, skipImpersonationReview, xfcc_header)
5457
if err != nil {
5558
log.Error(err, "Cannot retrieve username and group from request")
59+
handleResolveUserAndGroupsError(writer, err)
60+
61+
return
5662
}
5763

5864
log.V(10).Info("request groups", "groups", groups)
@@ -76,3 +82,14 @@ func CheckUserInCapsuleGroupMiddleware(client client.Writer, log logr.Logger, cl
7682
})
7783
}
7884
}
85+
86+
func handleResolveUserAndGroupsError(writer http.ResponseWriter, err error) {
87+
var unauthorizedErr *req.ErrUnauthorized
88+
if errors.As(err, &unauthorizedErr) {
89+
weberrors.HandleUnauthorized(writer, err, "cannot retrieve user and group from the request")
90+
91+
return
92+
}
93+
94+
weberrors.HandleError(writer, err, "cannot retrieve user and group from the request")
95+
}

internal/webserver/webserver.go

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"crypto/tls"
99
"encoding/json"
10+
"errors"
1011
"fmt"
1112
"io"
1213
"net"
@@ -22,9 +23,8 @@ import (
2223

2324
"github.com/go-logr/logr"
2425
"github.com/golang-jwt/jwt/v5"
25-
"github.com/gorilla/handlers"
2626
"github.com/gorilla/mux"
27-
"github.com/pkg/errors"
27+
pkgerrors "github.com/pkg/errors"
2828
capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
2929
capsulerbac "github.com/projectcapsule/capsule/pkg/api/rbac"
3030
"golang.org/x/net/http/httpguts"
@@ -82,7 +82,7 @@ func NewKubeFilter(
8282

8383
reverseProxyTransport, err := opts.ReverseProxyTransport()
8484
if err != nil {
85-
return nil, errors.Wrap(err, "cannot create transport for reverse proxy")
85+
return nil, pkgerrors.Wrap(err, "cannot create transport for reverse proxy")
8686
}
8787

8888
reverseProxy.Transport = reverseProxyTransport
@@ -92,12 +92,12 @@ func NewKubeFilter(
9292

9393
err = corev1.AddToScheme(scheme)
9494
if err != nil {
95-
return nil, errors.Wrap(err, "cannot add corev1 to scheme")
95+
return nil, pkgerrors.Wrap(err, "cannot add corev1 to scheme")
9696
}
9797

9898
err = authorizationv1.AddToScheme(scheme)
9999
if err != nil {
100-
return nil, errors.Wrap(err, "cannot add authorizationv1 to scheme")
100+
return nil, pkgerrors.Wrap(err, "cannot add authorizationv1 to scheme")
101101
}
102102

103103
codecFactory := serializer.NewCodecFactory(scheme)
@@ -167,7 +167,7 @@ func (n *kubeFilter) NeedLeaderElection() bool {
167167
//nolint:funlen
168168
func (n *kubeFilter) Start(ctx context.Context) error {
169169
r := mux.NewRouter()
170-
r.Use(handlers.RecoveryHandler())
170+
r.Use(n.recoveryMiddleware)
171171

172172
r.Path("/_healthz").Subrouter().HandleFunc("", func(writer http.ResponseWriter, _ *http.Request) {
173173
writer.WriteHeader(http.StatusOK)
@@ -283,13 +283,13 @@ func (n *kubeFilter) ReadinessProbe(req *http.Request) (err error) {
283283
var r *http.Request
284284

285285
if r, err = http.NewRequestWithContext(req.Context(), http.MethodGet, url, nil); err != nil {
286-
return errors.Wrap(err, "cannot create request")
286+
return pkgerrors.Wrap(err, "cannot create request")
287287
}
288288

289289
var resp *http.Response
290290

291291
if resp, err = clt.Do(r); err != nil {
292-
return errors.Wrap(err, "cannot make local _healthz request")
292+
return pkgerrors.Wrap(err, "cannot make local _healthz request")
293293
}
294294

295295
defer func() {
@@ -349,13 +349,17 @@ func (n *kubeFilter) authorizationMiddleware(next http.Handler) http.Handler {
349349

350350
request, username, groups, err := req.ResolveUserAndGroups(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview, n.xfcc_header)
351351
if err != nil {
352-
server.HandleError(writer, err, "cannot retrieve user and group from the request")
352+
n.handleResolveUserAndGroupsError(writer, err)
353+
354+
return
353355
}
354356

355357
//nolint:contextcheck
356358
proxyTenants, err := n.getTenantsForOwner(request.Context(), username, groups)
357359
if err != nil {
358360
server.HandleError(writer, err, "cannot list Tenant resources")
361+
362+
return
359363
}
360364

361365
obj, gvk, err := n.universalDecoder.Decode(body, nil, nil)
@@ -547,12 +551,16 @@ func (n *kubeFilter) registerModules(ctx context.Context, root *mux.Router) {
547551
sr.HandleFunc("", func(writer http.ResponseWriter, request *http.Request) {
548552
request, username, groups, err := req.ResolveUserAndGroups(request, n.authTypes, n.usernameClaimField, n.writer, n.ignoredImpersonationGroups, n.impersonationGroupsRegexp, n.skipImpersonationReview, n.xfcc_header)
549553
if err != nil {
550-
server.HandleError(writer, err, "cannot retrieve user and group from the request")
554+
n.handleResolveUserAndGroupsError(writer, err)
555+
556+
return
551557
}
552558

553559
proxyTenants, err := n.getTenantsForOwner(ctx, username, groups)
554560
if err != nil {
555561
server.HandleError(writer, err, "cannot list Tenant resources")
562+
563+
return
556564
}
557565

558566
var selector labels.Selector
@@ -574,19 +582,23 @@ func (n *kubeFilter) registerModules(ctx context.Context, root *mux.Router) {
574582
case err != nil:
575583
var t moderrors.Error
576584
if errors.As(err, &t) {
585+
writer.Header().Set("Content-Type", "application/json")
586+
577587
if t.Status().Code > 0 {
578588
writer.WriteHeader(int(t.Status().Code))
589+
} else {
590+
writer.WriteHeader(http.StatusInternalServerError)
579591
}
580592

581-
writer.Header().Set("Content-Type", "application/json")
582-
583593
b, _ := json.Marshal(t.Status())
584594
_, _ = writer.Write(b)
585595

586-
panic(err.Error())
596+
return
587597
}
588598

589599
server.HandleError(writer, err, err.Error())
600+
601+
return
590602
case selector == nil:
591603
// if there's no selector, let it pass to the
592604
n.impersonateHandler(writer, request)
@@ -597,6 +609,37 @@ func (n *kubeFilter) registerModules(ctx context.Context, root *mux.Router) {
597609
}
598610
}
599611

612+
func (n *kubeFilter) recoveryMiddleware(next http.Handler) http.Handler {
613+
return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
614+
defer func() {
615+
recovered := recover()
616+
if recovered == nil {
617+
return
618+
}
619+
620+
if err, ok := recovered.(error); ok && errors.Is(err, http.ErrAbortHandler) {
621+
panic(err)
622+
}
623+
624+
n.log.Error(fmt.Errorf("%v", recovered), "panic while handling request")
625+
server.HandleError(writer, fmt.Errorf("internal server error"), "panic while handling request")
626+
}()
627+
628+
next.ServeHTTP(writer, request)
629+
})
630+
}
631+
632+
func (n *kubeFilter) handleResolveUserAndGroupsError(writer http.ResponseWriter, err error) {
633+
var unauthorizedErr *req.ErrUnauthorized
634+
if errors.As(err, &unauthorizedErr) {
635+
server.HandleUnauthorized(writer, err, "cannot retrieve user and group from the request")
636+
637+
return
638+
}
639+
640+
server.HandleError(writer, err, "cannot retrieve user and group from the request")
641+
}
642+
600643
func (n *kubeFilter) getTenantsForOwner(ctx context.Context, username string, groups []string) (proxyTenants []*tenant.ProxyTenant, err error) {
601644
if strings.HasPrefix(username, serviceaccount.ServiceAccountUsernamePrefix) {
602645
proxyTenants, err = n.getProxyTenantsForOwnerKind(ctx, capsulerbac.ServiceAccountOwner, username)

0 commit comments

Comments
 (0)