~bigbes/sr-ht-spec

b643b0be64833bb1c989b70b57a5a7ded7142904 — Eugene Blikh 9 days ago c87a11b
bearer: refuse through the shared table and challenge
M authn/bearer.go => authn/bearer.go +55 -22
@@ 152,37 152,70 @@ func instanceTokenLabel(tok *bearer.Token) string {
// the status the surface must answer with. It is one function so that the three
// surfaces cannot each invent their own table.
//
// The mapping, and the one line of it that has to be defended:
// Everything the credential itself can be wrong about is bearer.StatusFor's
// answer, not a second copy of it: ErrForbidden is 403, ErrUnavailable is 503
// and never 401, and ErrInvalid, ErrRevoked and ErrNotOurs are 401. That last
// arm is only reached because ErrNotOurs is decided before we ask — a meta.sr.ht
// PAT used to fall through to spec's own token store, and with that store gone
// it is a refusal at the door.
//
//   - bearer.ErrUnavailable is 503 and never 401. Reading "I could not reach
//     tokens.sr.ht" as "your token is revoked" would refuse every live instance
//     token on the instance for as long as a daemon that is deliberately off the
//     hot path is restarting, and would tell a thousand clients their
//     credentials are bad when the truth is that one service is down. 503 says
//     the true thing and keeps the operator's attention where the fault is.
//   - ErrMissingGrant and ErrNotInstanceOwner are 403: the credential verifies
// The ErrUnavailable line is the one worth restating even though it is no longer
// spelled here. Reading "I could not reach tokens.sr.ht" as "your token is
// revoked" would refuse every live instance token for as long as a daemon that
// is deliberately off the hot path takes to restart, and would tell a thousand
// clients their credentials are bad when the truth is that one service is down.
//
// What this function adds is what bearer cannot know:
//
//   - ErrMissingGrant and ErrNotInstanceOwner are 403. The credential verifies
//     and the holder is who they say they are, so retrying is pointless and what
//     they need is a wider grant, not another login.
//   - Everything permanent about the credential itself — malformed, foreign,
//     revoked — is 401.
//   - ErrNoAgentPlane is 503 and not 401. An instance with no [tokens.sr.ht]
//     origin cannot check any credential, and telling the holder of a good token
//     that it is bad would send them to re-provision it.
//   - Everything else is transient by definition and answers 503, which is the
//     fail-closed direction: a backend outage never reads as a valid credential.
//     they need is a wider grant, not another login. Both are asked before the
//     bearer table, because ErrMissingGrant is raised beside a token that
//     verified and must not be read as one that did not.
//   - ErrNoToken is 401: nothing was presented on a surface that requires a
//     credential.
//   - ErrNoAgentPlane, and anything else at all, is 503. An instance with no
//     [tokens.sr.ht] origin cannot check any credential, and telling the holder
//     of a good token that it is bad would send them to re-provision it; an
//     unclassified error is a backend that could not answer. This is where the
//     two tables' defaults deliberately differ — bearer's unrecognised failure
//     is the caller's credential, because everything reaching it is about a
//     credential, while an unrecognised failure here can be the database this
//     resolver had to consult, which must never read as a bad token.
func StatusFor(err error) int {
	switch {
	case err == nil:
		return http.StatusOK
	case errors.Is(err, bearer.ErrUnavailable):
		return http.StatusServiceUnavailable
	case errors.Is(err, bearer.ErrForbidden),
		errors.Is(err, ErrMissingGrant),
		errors.Is(err, ErrNotInstanceOwner):
	case errors.Is(err, ErrMissingGrant), errors.Is(err, ErrNotInstanceOwner):
		return http.StatusForbidden
	case IsAuthFailure(err):
	case isBearerRefusal(err):
		return bearer.StatusFor(err)
	case errors.Is(err, ErrNoToken):
		return http.StatusUnauthorized
	default:
		return http.StatusServiceUnavailable
	}
}

// isBearerRefusal reports whether err is one of the sentinels bearer.StatusFor
// has an answer for. The list is here rather than in a helper over there because
// it is the question "did the shared validator decide this?", and a wrong answer
// to it is what would let the 503 default below swallow a 401 — or, worse, let
// bearer's own 401 default swallow a database outage.
func isBearerRefusal(err error) bool {
	return errors.Is(err, bearer.ErrForbidden) ||
		errors.Is(err, bearer.ErrUnavailable) ||
		errors.Is(err, bearer.ErrInvalid) ||
		errors.Is(err, bearer.ErrRevoked) ||
		errors.Is(err, bearer.ErrNotOurs)
}

// Challenge is the WWW-Authenticate value every 401 this service answers must
// carry, per RFC 9110 §11.6.1 — the scheme, and this service's config section as
// the realm, which is what names it in the config, in the nav and in a grant
// everywhere else on the instance.
//
// It is bearer.Challenge with our section already in it, so that the four
// surfaces that refuse a credential (the resolver's middleware, MCP, /query and
// the read plane's machine formats) cannot name four realms.
func Challenge() string { return bearer.Challenge(ConfigSection) }

M authn/bearer_test.go => authn/bearer_test.go +8 -0
@@ 466,3 466,11 @@ func TestStatusFor(t *testing.T) {
		})
	}
}

// RFC 9110 asks a 401 to name the scheme it would accept, and the realm is this
// service's config section — the string that identifies it in the config, in the
// nav and in a grant. One spelling, so that the four surfaces which refuse a
// credential cannot name four realms.
func TestChallenge(t *testing.T) {
	assert.Equal(t, `Bearer realm="spec.sr.ht"`, Challenge())
}

M authn/resolver.go => authn/resolver.go +7 -0
@@ 207,6 207,13 @@ func (rs *Resolver) Middleware() func(http.Handler) http.Handler {
						"method", r.Method, "path", r.URL.Path, "status", status,
						scribe.Err(err))
				}
				if status == http.StatusUnauthorized {
					// RFC 9110 requires the challenge on a 401, and the caller
					// here is always a machine holding a bearer token: naming the
					// scheme and the realm is what tells it which credential this
					// service was refusing.
					w.Header().Set("WWW-Authenticate", Challenge())
				}
				http.Error(w, resolveFailureMessage(status), status)
				return
			}

M authn/resolver_test.go => authn/resolver_test.go +24 -0
@@ 263,6 263,30 @@ func TestMiddleware_RejectsBadToken(t *testing.T) {
	}
}

// A 401 out of this middleware names the scheme and the realm, which is what an
// agent library reads to know which credential was refused — and what a 503 must
// not carry, since nothing about the credential was decided.
func TestMiddleware_RefusalCarriesTheChallenge(t *testing.T) {
	rs := newTestResolver(t)

	h := rs.Middleware()(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
	rec := httptest.NewRecorder()
	h.ServeHTTP(rec, request("", map[string]string{"Authorization": "Bearer never-issued"}))
	require.Equal(t, http.StatusUnauthorized, rec.Code)
	assert.Equal(t, Challenge(), rec.Header().Get("WWW-Authenticate"))

	f := newPlaneFixture(t, http.StatusNoContent)
	f.users.err = errors.New("connection refused")
	h = f.rs.Middleware()(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
	rec = httptest.NewRecorder()
	h.ServeHTTP(rec, request("", map[string]string{
		"Authorization": "Bearer " + instanceToken("spec:read"),
	}))
	require.Equal(t, http.StatusServiceUnavailable, rec.Code)
	assert.Empty(t, rec.Header().Get("WWW-Authenticate"),
		"a backend outage says nothing about the credential")
}

// A user lookup that cannot answer is a backend outage, not a bad credential:
// fail closed with a 503 rather than telling a live agent its token is bad.
func TestMiddleware_BackendOutageIs503(t *testing.T) {

M graph/server.go => graph/server.go +1 -0
@@ 165,6 165,7 @@ func gate(next http.Handler) http.Handler {
		p := authn.PrincipalFromContext(r.Context())
		if !p.CanRead() {
			w.Header().Set("Content-Type", "text/plain; charset=utf-8")
			w.Header().Set("WWW-Authenticate", authn.Challenge())
			http.Error(w, "authentication required", http.StatusUnauthorized)
			return
		}

M mcpsrv/mcpsrv.go => mcpsrv/mcpsrv.go +1 -0
@@ 297,6 297,7 @@ func Gate(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if !authn.PrincipalFromContext(r.Context()).CanRead() {
			w.Header().Set("Content-Type", "text/plain; charset=utf-8")
			w.Header().Set("WWW-Authenticate", authn.Challenge())
			http.Error(w, "authentication required", http.StatusUnauthorized)
			return
		}

M web/handlers.go => web/handlers.go +6 -0
@@ 95,6 95,12 @@ func (s *Server) denyRead(w http.ResponseWriter, r *http.Request, f format) {
		return
	}
	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
	// The challenge belongs on the machine shape and not on the browser one:
	// this is the branch a client asking for .md or .json takes, and RFC 9110
	// asks a 401 to name the scheme it would accept. A browser never reaches
	// here — it gets the redirect above, because the session is a cookie
	// meta.sr.ht sets and there is no HTTP authentication scheme for it.
	w.Header().Set("WWW-Authenticate", authn.Challenge())
	http.Error(w, "authentication required", http.StatusUnauthorized)
}