From 922f99a61e4889c0c37e9262a03610faad67cc2c Mon Sep 17 00:00:00 2001 From: Xe Iaso Date: Sat, 26 Jul 2025 18:27:28 -0400 Subject: [PATCH] fix(lib): make challenge validation fully deterministic Signed-off-by: Xe Iaso --- lib/anubis.go | 65 +++++++++++++++++++++++++++----------- lib/challenge/challenge.go | 1 + lib/http.go | 11 ++++--- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/lib/anubis.go b/lib/anubis.go index ca4c14b..cd41d88 100644 --- a/lib/anubis.go +++ b/lib/anubis.go @@ -90,11 +90,12 @@ func (s *Server) getTokenKeyfunc() jwt.Keyfunc { } } -func (s *Server) challengeFor(r *http.Request) (*challenge.Challenge, error) { +func (s *Server) challengeFor(r *http.Request, lg *slog.Logger, cr policy.CheckResult, rule *policy.Bot) (*challenge.Challenge, error) { ckies := r.CookiesNamed(anubis.TestCookieName) if len(ckies) == 0 { - return s.issueChallenge(r.Context(), r) + lg.Info("no test cookie found, issuing new challenge") + return s.issueChallenge(r.Context(), r, lg, cr, rule) } j := store.JSON[challenge.Challenge]{Underlying: s.store} @@ -102,8 +103,10 @@ func (s *Server) challengeFor(r *http.Request) (*challenge.Challenge, error) { ckie := ckies[0] chall, err := j.Get(r.Context(), "challenge:"+ckie.Value) if err != nil { + lg.Error("can't find challenge", "id", ckie.Value, "err", err) if errors.Is(err, store.ErrNotFound) { - return s.issueChallenge(r.Context(), r) + lg.Info("issuing new challenge because error is storeErrNotFound") + return s.issueChallenge(r.Context(), r, lg, cr, rule) } return nil, err @@ -112,7 +115,26 @@ func (s *Server) challengeFor(r *http.Request) (*challenge.Challenge, error) { return &chall, nil } -func (s *Server) issueChallenge(ctx context.Context, r *http.Request) (*challenge.Challenge, error) { +func (s *Server) getChallenge(r *http.Request) (*challenge.Challenge, error) { + ckies := r.CookiesNamed(anubis.TestCookieName) + if len(ckies) == 0 { + return nil, store.ErrNotFound + } + + j := store.JSON[challenge.Challenge]{Underlying: s.store} + + ckie := ckies[0] + chall, err := j.Get(r.Context(), "challenge:"+ckie.Value) + + return &chall, err +} + +func (s *Server) issueChallenge(ctx context.Context, r *http.Request, lg *slog.Logger, cr policy.CheckResult, rule *policy.Bot) (*challenge.Challenge, error) { + if cr.Rule != config.RuleChallenge { + slog.Error("this should be impossible, asked to issue a challenge but the rule is not a challenge rule", "cr", cr, "rule", rule) + //return nil, errors.New("[unexpected] this codepath should be impossible, asked to issue a challenge for a non-challenge rule") + } + id, err := uuid.NewV7() if err != nil { return nil, err @@ -125,6 +147,7 @@ func (s *Server) issueChallenge(ctx context.Context, r *http.Request) (*challeng chall := challenge.Challenge{ ID: id.String(), + Method: rule.Challenge.Algorithm, RandomData: fmt.Sprintf("%x", randomData), IssuedAt: time.Now(), Metadata: map[string]string{ @@ -138,6 +161,8 @@ func (s *Server) issueChallenge(ctx context.Context, r *http.Request) (*challeng return nil, err } + lg.Info("new challenge issued", "challenge", id.String()) + return &chall, err } @@ -185,21 +210,21 @@ func (s *Server) maybeReverseProxy(w http.ResponseWriter, r *http.Request, httpS if err != nil { lg.Debug("cookie not found", "path", r.URL.Path) s.ClearCookie(w, CookieOpts{Path: cookiePath, Host: r.Host}) - s.RenderIndex(w, r, rule, httpStatusOnly) + s.RenderIndex(w, r, cr, rule, httpStatusOnly) return } if err := ckie.Valid(); err != nil { lg.Debug("cookie is invalid", "err", err) s.ClearCookie(w, CookieOpts{Path: cookiePath, Host: r.Host}) - s.RenderIndex(w, r, rule, httpStatusOnly) + s.RenderIndex(w, r, cr, rule, httpStatusOnly) return } if time.Now().After(ckie.Expires) && !ckie.Expires.IsZero() { lg.Debug("cookie expired", "path", r.URL.Path) s.ClearCookie(w, CookieOpts{Path: cookiePath, Host: r.Host}) - s.RenderIndex(w, r, rule, httpStatusOnly) + s.RenderIndex(w, r, cr, rule, httpStatusOnly) return } @@ -208,7 +233,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, CookieOpts{Path: cookiePath, Host: r.Host}) - s.RenderIndex(w, r, rule, httpStatusOnly) + s.RenderIndex(w, r, cr, rule, httpStatusOnly) return } @@ -216,7 +241,7 @@ func (s *Server) maybeReverseProxy(w http.ResponseWriter, r *http.Request, httpS if !ok { lg.Debug("invalid token claims type", "path", r.URL.Path) s.ClearCookie(w, CookieOpts{Path: cookiePath, Host: r.Host}) - s.RenderIndex(w, r, rule, httpStatusOnly) + s.RenderIndex(w, r, cr, rule, httpStatusOnly) return } @@ -224,14 +249,14 @@ func (s *Server) maybeReverseProxy(w http.ResponseWriter, r *http.Request, httpS if !ok { lg.Debug("policyRule claim is not a string") s.ClearCookie(w, CookieOpts{Path: cookiePath, Host: r.Host}) - s.RenderIndex(w, r, rule, httpStatusOnly) + s.RenderIndex(w, r, cr, rule, httpStatusOnly) return } if policyRule != rule.Hash() { lg.Debug("user originally passed with a different rule, issuing new challenge", "old", policyRule, "new", rule.Name) s.ClearCookie(w, CookieOpts{Path: cookiePath, Host: r.Host}) - s.RenderIndex(w, r, rule, httpStatusOnly) + s.RenderIndex(w, r, cr, rule, httpStatusOnly) return } @@ -346,7 +371,7 @@ func (s *Server) MakeChallenge(w http.ResponseWriter, r *http.Request) { } lg = lg.With("check_result", cr) - chall, err := s.challengeFor(r) + chall, err := s.challengeFor(r, lg, cr, rule) if err != nil { lg.Error("failed to fetch or issue challenge", "err", err) w.WriteHeader(http.StatusInternalServerError) @@ -436,19 +461,21 @@ func (s *Server) PassChallenge(w http.ResponseWriter, r *http.Request) { } lg = lg.With("check_result", cr) - impl, ok := challenge.Get(rule.Challenge.Algorithm) + chall, err := s.getChallenge(r) + if err != nil { + lg.Error("getChallenge failed", "err", err) + s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), rule.Challenge.Algorithm)) + return + } + + impl, ok := challenge.Get(chall.Method) if !ok { lg.Error("check failed", "err", err) s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), rule.Challenge.Algorithm)) return } - chall, err := s.challengeFor(r) - if err != nil { - lg.Error("check failed", "err", err) - s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), rule.Challenge.Algorithm)) - return - } + lg = lg.With("challenge", chall.ID) in := &challenge.ValidateInput{ Challenge: chall, diff --git a/lib/challenge/challenge.go b/lib/challenge/challenge.go index 4c975c8..1200e33 100644 --- a/lib/challenge/challenge.go +++ b/lib/challenge/challenge.go @@ -5,6 +5,7 @@ import "time" // Challenge is the metadata about a single challenge issuance. type Challenge struct { ID string `json:"id"` // UUID identifying the challenge + Method string `json:"method"` // Challenge method RandomData string `json:"randomData"` // The random data the client processes IssuedAt time.Time `json:"issuedAt"` // When the challenge was issued Metadata map[string]string `json:"metadata"` // Challenge metadata such as IP address and user agent diff --git a/lib/http.go b/lib/http.go index 9ee0847..ad1a244 100644 --- a/lib/http.go +++ b/lib/http.go @@ -111,7 +111,7 @@ func randomChance(n int) bool { return rand.Intn(n) == 0 } -func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, rule *policy.Bot, returnHTTPStatusOnly bool) { +func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, cr policy.CheckResult, rule *policy.Bot, returnHTTPStatusOnly bool) { localizer := localization.GetLocalizer(r) if returnHTTPStatusOnly { @@ -125,17 +125,20 @@ func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, rule *polic if !strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") && randomChance(64) { lg.Error("client was given a challenge but does not in fact support gzip compression") s.respondWithError(w, r, localizer.T("client_error_browser")) + return } challengesIssued.WithLabelValues("embedded").Add(1) - chall, err := s.challengeFor(r) + chall, err := s.issueChallenge(r.Context(), r, lg, cr, rule) if err != nil { - lg.Error("can't get challenge", "err", "err") + lg.Error("can't get challenge", "err", err) s.ClearCookie(w, CookieOpts{Name: anubis.TestCookieName, Host: r.Host}) s.respondWithError(w, r, fmt.Sprintf("%s: %s", localizer.T("internal_server_error"), rule.Challenge.Algorithm)) return } + lg = lg.With("challenge", chall.ID) + var ogTags map[string]string = nil if s.opts.OpenGraph.Enabled { var err error @@ -153,7 +156,7 @@ func (s *Server) RenderIndex(w http.ResponseWriter, r *http.Request, rule *polic Expiry: 30 * time.Minute, }) - impl, ok := challenge.Get(rule.Challenge.Algorithm) + impl, ok := challenge.Get(chall.Method) if !ok { lg.Error("check failed", "err", "can't get algorithm", "algorithm", rule.Challenge.Algorithm) s.ClearCookie(w, CookieOpts{Name: anubis.TestCookieName, Host: r.Host})