fix(lib): properly clear out test cookie (#522)

Closes #520

For some reason, Chrome and Firefox are very picky over what they use to
match cookies that need to be deleted. Listen to me for my tale of woe:

The basic problem here is that cookies were an early hack added on the
side of the HTTP spec and they're basically impossible to upgrade or
change because who knows what relies on the exact behavior cookies use.
As a result, cookies don't just match by name, but by every setting that
exists on them. You can also have two cookies with the same name but
different values. This spec is a nightmare lol.

Even more fun: browsers will make up values for cookies if they aren't
set, meaning that getting a challenge token at `/docs` is semantically
different than a challenge token you got from `/`.

This PR fixes this issue by explicitly setting the "make sure cookie
support is working" cookie's path to `/`, meaning that it will always be
sent. Additionally, cookies are expired by setting the expiry time to
one minute in the past.

Hopefully this will fix it. I'm testing this locally and it seems to
work fine.

Signed-off-by: Xe Iaso <me@xeiaso.net>
This commit is contained in:
Xe Iaso 2025-05-18 18:41:26 -04:00 committed by GitHub
parent e31e1ca5e7
commit a6045d6698
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 54 additions and 33 deletions

View File

@ -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

View File

@ -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
}

View File

@ -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 {

View File

@ -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()