diff --git a/docs/docs/CHANGELOG.md b/docs/docs/CHANGELOG.md index c719e18..6215575 100644 --- a/docs/docs/CHANGELOG.md +++ b/docs/docs/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `RuntimeDirectory` to systemd unit settings so native packages can listen over unix sockets - Added SearXNG instance tracker whitelist policy - Added Qualys SSL Labs whitelist policy +- Fixed cookie deletion logic ([#520](https://github.com/TecharoHQ/anubis/issues/520), [#522](https://github.com/TecharoHQ/anubis/pull/522)) ## v1.18.0: Varis zos Galvus diff --git a/lib/anubis.go b/lib/anubis.go index 60a007c..c4fa4c8 100644 --- a/lib/anubis.go +++ b/lib/anubis.go @@ -96,6 +96,12 @@ func (s *Server) maybeReverseProxyOrPage(w http.ResponseWriter, r *http.Request) func (s *Server) maybeReverseProxy(w http.ResponseWriter, r *http.Request, httpStatusOnly bool) { lg := internal.GetRequestLogger(r) + // Adjust cookie path if base prefix is not empty + cookiePath := "/" + if anubis.BasePrefix != "" { + cookiePath = strings.TrimSuffix(anubis.BasePrefix, "/") + "/" + } + cr, rule, err := s.check(r) if err != nil { lg.Error("check failed", "err", err) @@ -121,21 +127,21 @@ func (s *Server) maybeReverseProxy(w http.ResponseWriter, r *http.Request, httpS ckie, err := r.Cookie(s.cookieName) if err != nil { lg.Debug("cookie not found", "path", r.URL.Path) - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) s.RenderIndex(w, r, rule, httpStatusOnly) return } if err := ckie.Valid(); err != nil { lg.Debug("cookie is invalid", "err", err) - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) s.RenderIndex(w, r, rule, httpStatusOnly) return } if time.Now().After(ckie.Expires) && !ckie.Expires.IsZero() { lg.Debug("cookie expired", "path", r.URL.Path) - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) s.RenderIndex(w, r, rule, httpStatusOnly) return } @@ -146,7 +152,7 @@ func (s *Server) maybeReverseProxy(w http.ResponseWriter, r *http.Request, httpS if err != nil || !token.Valid { lg.Debug("invalid token", "path", r.URL.Path, "err", err) - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) s.RenderIndex(w, r, rule, httpStatusOnly) return } @@ -156,13 +162,19 @@ func (s *Server) maybeReverseProxy(w http.ResponseWriter, r *http.Request, httpS } func (s *Server) checkRules(w http.ResponseWriter, r *http.Request, cr policy.CheckResult, lg *slog.Logger, rule *policy.Bot) bool { + // Adjust cookie path if base prefix is not empty + cookiePath := "/" + if anubis.BasePrefix != "" { + cookiePath = strings.TrimSuffix(anubis.BasePrefix, "/") + "/" + } + switch cr.Rule { case config.RuleAllow: lg.Debug("allowing traffic to origin (explicit)") s.ServeHTTPNext(w, r) return true case config.RuleDeny: - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) lg.Info("explicit deny") if rule == nil { lg.Error("rule is nil, cannot calculate checksum") @@ -181,7 +193,7 @@ func (s *Server) checkRules(w http.ResponseWriter, r *http.Request, cr policy.Ch s.RenderBench(w, r) return true default: - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) slog.Error("CONFIG ERROR: unknown rule", "rule", cr.Rule) s.respondWithError(w, r, "Internal Server Error: administrator has misconfigured Anubis. Please contact the administrator and ask them to look for the logs around \"maybeReverseProxy.Rules\"") return true @@ -233,7 +245,7 @@ func (s *Server) MakeChallenge(w http.ResponseWriter, r *http.Request) { lg = lg.With("check_result", cr) challenge := s.challengeFor(r, rule.Challenge.Difficulty) - s.SetCookie(w, anubis.TestCookieName, challenge, "") + s.SetCookie(w, anubis.TestCookieName, challenge, "/") err = encoder.Encode(struct { Rules *config.ChallengeRules `json:"rules"` @@ -254,6 +266,14 @@ func (s *Server) MakeChallenge(w http.ResponseWriter, r *http.Request) { func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { lg := internal.GetRequestLogger(r) + // Adjust cookie path if base prefix is not empty + cookiePath := "/" + if anubis.BasePrefix != "" { + cookiePath = strings.TrimSuffix(anubis.BasePrefix, "/") + "/" + } + + s.ClearCookie(w, anubis.TestCookieName, "/") + redir := r.FormValue("redir") redirURL, err := url.ParseRequestURI(redir) if err != nil { @@ -274,7 +294,7 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { nonceStr := r.FormValue("nonce") if nonceStr == "" { - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) lg.Debug("no nonce") s.respondWithError(w, r, "missing nonce") return @@ -282,7 +302,7 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { elapsedTimeStr := r.FormValue("elapsedTime") if elapsedTimeStr == "" { - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) lg.Debug("no elapsedTime") s.respondWithError(w, r, "missing elapsedTime") return @@ -290,7 +310,7 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { elapsedTime, err := strconv.ParseFloat(elapsedTimeStr, 64) if err != nil { - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) lg.Debug("elapsedTime doesn't parse", "err", err) s.respondWithError(w, r, "invalid elapsedTime") return @@ -313,18 +333,16 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { challenge := s.challengeFor(r, rule.Challenge.Difficulty) if _, err := r.Cookie(anubis.TestCookieName); err == http.ErrNoCookie { - s.ClearCookie(w, s.cookieName) - s.ClearCookie(w, anubis.TestCookieName) + s.ClearCookie(w, s.cookieName, cookiePath) + s.ClearCookie(w, anubis.TestCookieName, cookiePath) lg.Warn("user has cookies disabled, this is not an anubis bug") s.respondWithError(w, r, "Your browser is configured to disable cookies. Anubis requires cookies for the legitimate interest of making sure you are a valid client. Please enable cookies for this domain") return } - s.ClearCookie(w, anubis.TestCookieName) - nonce, err := strconv.Atoi(nonceStr) if err != nil { - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) lg.Debug("nonce doesn't parse", "err", err) s.respondWithError(w, r, "invalid response") return @@ -334,7 +352,7 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { calculated := internal.SHA256sum(calcString) if subtle.ConstantTimeCompare([]byte(response), []byte(calculated)) != 1 { - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) lg.Debug("hash does not match", "got", response, "want", calculated) s.respondWithStatus(w, r, "invalid response", http.StatusForbidden) failedValidations.Inc() @@ -343,18 +361,13 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { // compare the leading zeroes if !strings.HasPrefix(response, strings.Repeat("0", rule.Challenge.Difficulty)) { - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) lg.Debug("difficulty check failed", "response", response, "difficulty", rule.Challenge.Difficulty) s.respondWithStatus(w, r, "invalid response", http.StatusForbidden) failedValidations.Inc() return } - // Adjust cookie path if base prefix is not empty - cookiePath := "/" - if anubis.BasePrefix != "" { - cookiePath = strings.TrimSuffix(anubis.BasePrefix, "/") + "/" - } // generate JWT cookie token := jwt.NewWithClaims(jwt.SigningMethodEdDSA, jwt.MapClaims{ "challenge": challenge, @@ -367,7 +380,7 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { tokenString, err := token.SignedString(s.priv) if err != nil { lg.Error("failed to sign JWT", "err", err) - s.ClearCookie(w, s.cookieName) + s.ClearCookie(w, s.cookieName, cookiePath) s.respondWithError(w, r, "failed to sign JWT") return } diff --git a/lib/http.go b/lib/http.go index 27f9fa0..70e083d 100644 --- a/lib/http.go +++ b/lib/http.go @@ -26,14 +26,16 @@ func (s *Server) SetCookie(w http.ResponseWriter, name, value, path string) { }) } -func (s *Server) ClearCookie(w http.ResponseWriter, name string) { +func (s *Server) ClearCookie(w http.ResponseWriter, name, path string) { http.SetCookie(w, &http.Cookie{ - Name: name, - Value: "", - Expires: time.Now().Add(-1 * time.Hour), - MaxAge: -1, - SameSite: http.SameSiteLaxMode, - Domain: s.opts.CookieDomain, + Name: name, + Value: "", + MaxAge: -1, + Expires: time.Now().Add(-1 * time.Minute), + SameSite: http.SameSiteLaxMode, + Partitioned: s.opts.CookiePartitioned, + Domain: s.opts.CookieDomain, + Path: path, }) } @@ -82,7 +84,12 @@ func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, rule *polic } } - s.SetCookie(w, anubis.TestCookieName, challenge, "") + http.SetCookie(w, &http.Cookie{ + Name: anubis.TestCookieName, + Value: challenge, + Expires: time.Now().Add(30 * time.Minute), + Path: "/", + }) component, err := web.BaseWithChallengeAndOGTags("Making sure you're not a bot!", web.Index(), challenge, rule.Challenge, ogTags) if err != nil { diff --git a/lib/http_test.go b/lib/http_test.go index 0a01799..add0706 100644 --- a/lib/http_test.go +++ b/lib/http_test.go @@ -11,7 +11,7 @@ func TestClearCookie(t *testing.T) { srv := spawnAnubis(t, Options{}) rw := httptest.NewRecorder() - srv.ClearCookie(rw, srv.cookieName) + srv.ClearCookie(rw, srv.cookieName, "/") resp := rw.Result() @@ -36,7 +36,7 @@ func TestClearCookieWithDomain(t *testing.T) { srv := spawnAnubis(t, Options{CookieDomain: "techaro.lol"}) rw := httptest.NewRecorder() - srv.ClearCookie(rw, srv.cookieName) + srv.ClearCookie(rw, srv.cookieName, "/") resp := rw.Result()