Skip to content

Commit a5bdce7

Browse files
committed
fix(csrf): Replace gorilla/csrf with net/http.CrossOriginProtection
gorilla/csrf is pinned at v1.7.2 for CVE-2025-24358 and cannot move. Go 1.25's net/http.CrossOriginProtection covers the same ground, so drop the dependency and judge cross-origin writes with the standard library. No configured endpoint takes part: each request is judged on the origin the browser actually used, which is what lets one Harbor answer on several ingresses. The fallback that runs when a browser sends no Sec-Fetch-Site (plain HTTP) compares Origin against Host. A proxy that rewrites Host drops the port — the bundled nginx sets Host to $host — so a legitimate same-origin write looks cross-origin. Tolerate a port-only difference on an otherwise matching host; a differing host still fails, keeping the cross-site and CVE-2025-24358 rejections intact. Signed-off-by: Prasanth Baskar <prasanth@8gears.com>
1 parent 51dda34 commit a5bdce7

5 files changed

Lines changed: 289 additions & 140 deletions

File tree

.github/renovate.json

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,6 @@
134134
"docker"
135135
]
136136
},
137-
{
138-
"description": "gorilla/csrf is deliberately pinned to v1.7.2 (goharbor/harbor#22010) — block the v1.7.3+ updates Renovate keeps proposing, including via vulnerability alerts",
139-
"matchPackageNames": [
140-
"github.com/gorilla/csrf"
141-
],
142-
"allowedVersions": "<= 1.7.2"
143-
},
144137
{
145138
"description": "Bare deps: commit type everywhere — overrides :semanticPrefixFixDepsChoreOthers from config:recommended, which would otherwise emit fix(deps)/chore(deps)",
146139
"matchPackageNames": [

src/go.mod

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ require (
3636
github.com/gomodule/redigo v2.0.0+incompatible
3737
github.com/google/go-containerregistry v0.22.0
3838
github.com/google/uuid v1.6.0
39-
// pin the github.com/gorilla/csrf to v1.7.2 because of issue https://github.com/goharbor/harbor/issues/22010
40-
github.com/gorilla/csrf v1.7.2
4139
github.com/gorilla/handlers v1.5.2
4240
github.com/gorilla/mux v1.8.1
4341
github.com/graph-gophers/dataloader v5.0.0+incompatible
@@ -156,7 +154,6 @@ require (
156154
github.com/google/s2a-go v0.1.9 // indirect
157155
github.com/googleapis/enterprise-certificate-proxy v0.3.20 // indirect
158156
github.com/googleapis/gax-go/v2 v2.24.0 // indirect
159-
github.com/gorilla/securecookie v1.1.2 // indirect
160157
github.com/grpc-ecosystem/grpc-gateway/v2 v2.30.0 // indirect
161158
github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect
162159
github.com/hashicorp/golang-lru v1.0.2 // indirect

src/go.sum

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3860,18 +3860,13 @@ github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORR
38603860
github.com/gopherjs/gopherjs v1.17.2 h1:fQnZVsXk8uxXIStYb0N4bGk7jeyTalG/wsZjQ25dO0g=
38613861
github.com/gopherjs/gopherjs v1.17.2/go.mod h1:pRRIvn/QzFLrKfvEz3qUuEhtE/zLCWfreZ6J5gM2i+k=
38623862
github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg=
3863-
github.com/gorilla/csrf v1.7.2 h1:oTUjx0vyf2T+wkrx09Trsev1TE+/EbDAeHtSTbtC2eI=
3864-
github.com/gorilla/csrf v1.7.2/go.mod h1:F1Fj3KG23WYHE6gozCmBAezKookxbIvUJT+121wTuLk=
38653863
github.com/gorilla/handlers v1.5.2 h1:cLTUSsNkgcwhgRqvCNmdbRWG0A3N4F+M2nWKdScwyEE=
38663864
github.com/gorilla/handlers v1.5.2/go.mod h1:dX+xVpaxdSw+q0Qek8SSsl3dfMk3jNddUkMzo0GtH0w=
38673865
github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
38683866
github.com/gorilla/mux v1.7.3/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
38693867
github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
38703868
github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
38713869
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
3872-
github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4=
3873-
github.com/gorilla/securecookie v1.1.2 h1:YCIWL56dvtr73r6715mJs5ZvhtnY73hBvEF8kXD8ePA=
3874-
github.com/gorilla/securecookie v1.1.2/go.mod h1:NfCASbcHqRSY+3a8tlWJwsQap2VX5pwzwo4h3eOamfo=
38753870
github.com/gorilla/sessions v1.2.1/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM=
38763871
github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
38773872
github.com/graph-gophers/dataloader v5.0.0+incompatible h1:R+yjsbrNq1Mo3aPG+Z/EKYrXrXXUNJHOgbRt+U6jOug=

src/server/middleware/csrf/csrf.go

Lines changed: 77 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -15,78 +15,105 @@
1515
package csrf
1616

1717
import (
18+
"net"
1819
"net/http"
19-
"os"
20+
"net/url"
21+
"slices"
2022
"strings"
21-
"sync"
2223

23-
"github.com/gorilla/csrf"
24-
25-
"github.com/goharbor/harbor/src/common/utils"
2624
"github.com/goharbor/harbor/src/lib"
27-
"github.com/goharbor/harbor/src/lib/config"
2825
"github.com/goharbor/harbor/src/lib/errors"
2926
lib_http "github.com/goharbor/harbor/src/lib/http"
3027
"github.com/goharbor/harbor/src/lib/log"
3128
"github.com/goharbor/harbor/src/server/middleware"
3229
)
3330

34-
const (
35-
csrfKeyEnv = "CSRF_KEY"
36-
tokenHeader = "X-Harbor-CSRF-Token"
37-
)
31+
// protect judges each request on the origin that browser actually used, so it
32+
// needs no configuration and is safe for concurrent use.
33+
var protect = http.NewCrossOriginProtection()
3834

39-
var (
40-
once sync.Once
41-
secureFlag = true
42-
protect func(handler http.Handler) http.Handler
43-
)
35+
// safeMethods are the methods RFC 7231 defines as safe, which carry no state
36+
// change and so need no cross-origin check.
37+
var safeMethods = []string{http.MethodGet, http.MethodHead, http.MethodOptions}
4438

45-
// attachToken makes sure if csrf generate a new token it will be included in the response header
46-
func attachToken(w http.ResponseWriter, r *http.Request) {
47-
if t := csrf.Token(r); len(t) > 0 {
48-
w.Header().Set(tokenHeader, t)
49-
} else {
50-
log.Warningf("token not found in context, skip attaching")
39+
// check applies the cross-origin protection, then closes the one gap it leaves
40+
// open by design.
41+
//
42+
// CrossOriginProtection admits a request carrying neither Sec-Fetch-Site nor
43+
// Origin, reading it as non-browser traffic. That is the right default for a
44+
// library, but not here: csrfSkipper has already excused the registry, API and
45+
// service routes that non-browser clients use, so an unsafe request arriving
46+
// without either header is one we cannot vouch for. Refusing it matches what the
47+
// previous token scheme did, which rejected any write that failed to present a
48+
// token.
49+
func check(req *http.Request) error {
50+
if err := protect.Check(req); err != nil {
51+
// protect.Check compares Origin against Host including the port. Browsers
52+
// send no Sec-Fetch-Site over plain HTTP, so the check falls to that
53+
// comparison, and a proxy that rewrites Host drops the port from it — the
54+
// bundled nginx sets Host to $host — making a legitimate same-origin write
55+
// look cross-origin. Re-admit exactly that: no Sec-Fetch-Site, and an
56+
// Origin whose host matches the request Host once the port is set aside.
57+
if req.Header.Get("Sec-Fetch-Site") == "" && originHostMatchesIgnoringPort(req) {
58+
return nil
59+
}
60+
return err
5161
}
62+
63+
if slices.Contains(safeMethods, req.Method) {
64+
return nil
65+
}
66+
67+
if req.Header.Get("Sec-Fetch-Site") == "" && req.Header.Get("Origin") == "" {
68+
return errors.New("request carries neither Sec-Fetch-Site nor Origin, cannot confirm its origin")
69+
}
70+
71+
return nil
5272
}
5373

54-
func handleError(w http.ResponseWriter, r *http.Request) {
55-
attachToken(w, r)
56-
lib_http.SendError(w, errors.New(csrf.FailureReason(r)).WithCode(errors.ForbiddenCode))
74+
// originHostMatchesIgnoringPort reports whether the request's Origin names the
75+
// same host as the request targeted, disregarding the port. A differing host —
76+
// a foreign site or a sibling subdomain — never matches, so the cross-site and
77+
// CVE-2025-24358 rejections stand.
78+
func originHostMatchesIgnoringPort(req *http.Request) bool {
79+
origin := req.Header.Get("Origin")
80+
if origin == "" {
81+
return false
82+
}
83+
o, err := url.Parse(origin)
84+
if err != nil || o.Hostname() == "" {
85+
return false
86+
}
87+
return o.Hostname() == hostname(req.Host)
5788
}
5889

59-
func attach(handler http.Handler) http.Handler {
60-
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
61-
attachToken(rw, req)
62-
handler.ServeHTTP(rw, req)
63-
})
90+
// hostname returns the host of a Host header or authority without any port and
91+
// without IPv6 brackets, so it can be compared to url.URL.Hostname.
92+
func hostname(host string) string {
93+
if h, _, err := net.SplitHostPort(host); err == nil {
94+
return h
95+
}
96+
return strings.Trim(host, "[]")
6497
}
6598

66-
// Middleware initialize the middleware to apply csrf selectively
99+
// Middleware rejects cross-origin requests that carry a session. It reads the
100+
// Fetch metadata headers browsers have sent since 2023, falling back to
101+
// comparing Origin against the Host the client addressed.
102+
//
103+
// Deliberately no configured endpoint takes part in the decision. Harbor is
104+
// commonly reachable on several ingresses, over different schemes, behind
105+
// proxies that rewrite Host; a single configured origin describes none of that,
106+
// and every attempt to reconstruct "the" expected origin gets one of those
107+
// deployments wrong.
67108
func Middleware() func(handler http.Handler) http.Handler {
68-
once.Do(func() {
69-
key := os.Getenv(csrfKeyEnv)
70-
if len(key) == 0 {
71-
key = utils.GenerateRandomString()
72-
} else if len(key) != 32 {
73-
log.Errorf("Invalid CSRF key length from the environment: %s. Please ensure the key length is 32 characters.", key)
74-
protect = func(_ http.Handler) http.Handler {
75-
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
76-
lib_http.SendError(w, errors.New("invalid CSRF key length from the environment. Please ensure the key length is 32 characters"))
77-
})
78-
}
109+
return middleware.New(func(rw http.ResponseWriter, req *http.Request, next http.Handler) {
110+
if err := check(req); err != nil {
111+
log.Debugf("Rejected cross-origin request for %s: %v", lib.TrimLineBreaks(req.URL.Path), err)
112+
lib_http.SendError(rw, errors.New(err).WithCode(errors.ForbiddenCode))
79113
return
80114
}
81-
secureFlag = secureCookie()
82-
protect = csrf.Protect([]byte(key), csrf.RequestHeader(tokenHeader),
83-
csrf.Secure(secureFlag),
84-
csrf.ErrorHandler(http.HandlerFunc(handleError)),
85-
csrf.SameSite(csrf.SameSiteStrictMode),
86-
csrf.Path("/"))
87-
})
88-
return middleware.New(func(rw http.ResponseWriter, req *http.Request, next http.Handler) {
89-
protect(attach(next)).ServeHTTP(rw, req)
115+
116+
next.ServeHTTP(rw, req)
90117
}, csrfSkipper)
91118
}
92119

@@ -100,12 +127,3 @@ func csrfSkipper(req *http.Request) bool {
100127
}
101128
return false
102129
}
103-
104-
func secureCookie() bool {
105-
ep, err := config.ExtEndpoint()
106-
if err != nil {
107-
log.Warningf("Failed to get external endpoint: %v, set cookie secure flag to true", err)
108-
return true
109-
}
110-
return !strings.HasPrefix(strings.ToLower(ep), "http://")
111-
}

0 commit comments

Comments
 (0)