diff --git a/CHANGELOG.md b/CHANGELOG.md index cfbac834..0eed2ea2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ FIX: The migration `sqlite3/015_v0_4_19_share_unique_name_constraint.sql` has be FIX: Email addresses have been made case-insensitive. Please note that there is a migration included in this release (`016_v0_4_21_lowercase_email.sql`) which will attempt to ensure that all email addresses in your existing database are stored in lowercase; **if this migration fails you will need to manually remediate the duplicate account entries** (https://github.com/openziti/zrok/issues/517) +FIX: Stop sending authentication cookies to non-authenticated shares (https://github.com/openziti/zrok/issues/512) + ## v0.4.20 CHANGE: OpenZiti SDK updated to `v0.21.2`. All `ziti.ListenOptions` listener options configured to use `WaitForNEstablishedListeners: 1`. When a `zrok share` client or an `sdk.Share` client are connected to an OpenZiti router that supports "listener established" events, then listen calls will not return until the listener is fully established on the OpenZiti network. Previously a `zrok share` client could report that it is fully operational and listening before the listener is fully established on the OpenZiti network; in practice this produced a very small window of time when the share would not be ready to accept requests. This change eliminates this window of time (https://github.com/openziti/zrok/issues/490) diff --git a/endpoints/publicProxy/http.go b/endpoints/publicProxy/http.go index bcd69cc5..c2c2b841 100644 --- a/endpoints/publicProxy/http.go +++ b/endpoints/publicProxy/http.go @@ -19,6 +19,7 @@ import ( "net/http" "net/http/httputil" "net/url" + "slices" "strings" "time" ) @@ -157,7 +158,8 @@ func authHandler(handler http.Handler, pcfg *Config, key []byte, ctx ziti.Contex switch scheme { case string(sdk.None): logrus.Debugf("auth scheme none '%v'", shrToken) - deleteZrokCookie(w, r) + // ensure cookies from other shares are not sent to this share, in case it's malicious + deleteZrokCookies(w, r) handler.ServeHTTP(w, r) return @@ -203,7 +205,8 @@ func authHandler(handler http.Handler, pcfg *Config, key []byte, ctx ziti.Contex return } - deleteZrokCookie(w, r) + // ensure cookies from other shares are not sent to this share, in case it's malicious + deleteZrokCookies(w, r) handler.ServeHTTP(w, r) case string(sdk.Oauth): @@ -362,11 +365,26 @@ func SetZrokCookie(w http.ResponseWriter, cookieDomain, email, accessToken, prov }) } -func deleteZrokCookie(w http.ResponseWriter, r *http.Request) { - cookie, err := r.Cookie("zrok-access") - if err == nil { - cookie.MaxAge = -1 - http.SetCookie(w, cookie) +func deleteZrokCookies(w http.ResponseWriter, r *http.Request) { + // Get all cookies from the request + cookies := r.Cookies() + // List of cookies to delete, the pkce cookie might be okay to pass along to the HTTP backend, but zrok-access is + // not because it can contain the accessToken from any other OAuth enabled shares, so we delete it here when the + // current share is not OAuth-enabled. OAuth-enabled shares check the audience claim in the JWT to ensure it matches + // the requested share and will send the client back to the OAuth provider if it does not match. + deletedCookies := []string{"zrok-access", "pkce"} + // Filter the cookies to save + filteredCookies := make([]*http.Cookie, 0) + for _, cookie := range cookies { + if !slices.Contains(deletedCookies, cookie.Name) { + filteredCookies = append(filteredCookies, cookie) + } + } + + // Set the Cookie header to the filtered list of cookies + r.Header.Del("Cookie") + for _, cookie := range filteredCookies { + r.AddCookie(cookie) } }