Skip to content

fix: harden the authorization core and the transport adapters - #115

Merged
euskadi31 merged 29 commits into
masterfrom
fix/audit-core-transport
Oct 13, 2026
Merged

euskadi31 merged 29 commits into
masterfrom
fix/audit-core-transport

Conversation

@euskadi31

@euskadi31 euskadi31 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes the following findings of the internal security audit of 2026-10-12 (commit c5c2ac3): CORE-01, CORE-02, CORE-05, CORE-07, CORE-10, CORE-13, CORE-15 (Engine, carriers, Authorities), CORE-I1, CORE-I2, CORE-I3, CORE-I4, CORE-I5, CORE-I6, CORE-I10, CORE-I11.

This PR hardens two layers:

  • the transport-agnostic core: Engine, Manager, the decision managers and the voters;
  • the three transport adapters: httpsec, grpcsec and connectrpcsec.

basic, bearer and session get only the edits these changes require; PR F builds its hardening on top of them. The PR also adds security.ErrUnavailable (503 / Unavailable). The JWT PR wraps it for key-set outages, so its name and contract are stable.

Findings

CORE-01 — the voters ignored the value of the attribute they checked (Medium)

Problem. The attributes a route listed only chose which voters ran. Each voter then checked the role it was built with, whatever role the route asked for. Reproduced with real voters:

  • a decision manager {HasRole("USER"), HasRole("ADMIN")} shared by two routes granted [Role("ADMIN")] to a USER;
  • {Authenticated(), HasRole("ADMIN")} granted any route to any logged-in principal, because Authenticated() voted on every attribute;
  • HasAnyRole("ADMIN", "MANAGER") granted a MANAGER on [Role("ADMIN")].

Fix. Voters now follow Spring Security's semantics.

  • Each voter supports an attribute only when both its family and its value match: HasRole("ADMIN") abstains on Role("USER").
  • HasAny* grants only for the names the route requests.
  • Authenticated() and Anonymous() vote only on the new security.IsAuthenticated / security.IsAnonymous attributes.
  • Decide hands each voter only the attributes it supports.
  • Decide denies when an attribute is supported by no voter.
  • Decide denies a decision without attributes or without voters, whatever the tie-break and WithAbstainFallback. Before, WithAbstainFallback(DecisionGrant) granted such a route to anyone, anonymous callers included.
  • voter.And is strict: an inner abstention denies when the composite votes.
  • An undefined Decision counts as a deny.

Breaking: yes. See MIGRATION "voter: voters compare attribute values; dedicated authentication-state attributes".

CORE-13 — Not() turned an anonymous refusal into a grant (Low)

Problem. Not(HasRole("BANNED")) admitted anonymous callers: the inner voter denies an unauthenticated principal, and Not inverted that deny into a grant.

Fix. Not grants only an authenticated principal. An undefined decision counts as a deny.

Breaking: yes. Use voter.Anonymous() with security.IsAnonymous to admit anonymous callers.

CORE-I2 — voter.FullyAuthenticated() behaved like Authenticated() (Info)

There is no remember-me or authentication-strength concept that could tell the two apart, so the stronger promise was false. FullyAuthenticated() is removed; MIGRATION says to use Authenticated().

CORE-I1 — roles and scopes shared one flat authority namespace (Info)

Problem. One bare admin authority satisfied HasRole("admin"), HasScope("admin") and HasAuthority("admin") at once. A scope a client obtained could therefore pass for a role. scope:a b was also split into two scopes.

Fix.

  • HasRole matches only ROLE_<name> (security.RolePrefix).
  • HasScope matches only one scope:<name> authority per scope (security.ScopePrefix). That is what jwtsec.DefaultAuthorityResolver emits.
  • HasAuthority matches verbatim.
  • The constructors panic on a name they could never match: none, an empty one, a role starting with ROLE_, or a scope that is not an RFC 6749 §3.3 scope-token or starts with scope:.

Breaking: yes. See MIGRATION "voter: roles and scopes live in their own authority namespaces".

CORE-02 — credentials and the session were read from the URL and from cookies (Medium)

Problem. When a header was absent, httpsec.Carrier.Get / Values fell back to a cookie or to the query parameter of the same name. As a result:

  • Basic and Bearer credentials were accepted from ?Authorization=… and from an Authorization cookie, so they leaked through logs, Referer and browser history;
  • a session was accepted from ?session=… or a Session header, and the header even took precedence over the victim's cookie. A link or a cross-site form could log a victim into the attacker's session.

Fix.

  • The carrier reads header fields only (RFC 6750 §2.1, RFC 7617 §2).
  • session.Manager reads its cookie through the explicit httpsec.Carrier.CookiesNamed accessor (the session.CookieReader interface), and finds no session on a carrier without it.

Breaking: yes. See MIGRATION "http, session: credentials from header fields only, the session from its cookie".

CORE-05 — the documented gRPC wiring served streaming RPCs unauthenticated (Medium)

Problem. The grpc-bearer example and the godoc example chained only unary interceptors. A test proved that Health.Watch was served without any token.

Fix. New helpers grpcsec.ServerOptions(engine, …) and grpcsec.AuthorizeServerOptions(adm, attrs, …) install the unary and the stream chains together, with the same options. Both examples use them, and tests check Watch on each path.

CORE-07 — personal data and error text in telemetry (Low)

Problem.

  • The failing core spans recorded the whole error chain as status description and exception event. That chain could include the username typed at a Basic prompt (often a password typed into the wrong field) and wrapped store errors.
  • The HTTP span recorded the raw path as http.route (reset tokens, e-mail addresses) and the raw method.

Fix.

  • Spans record only error.type: the class returned by the new security.ErrorClass(err) (OpenTelemetry semantic conventions), with no exception event and nothing on success.
  • basic no longer puts the username in its errors.
  • http.request.method falls back to _OTHER for unknown methods.
  • http.route is the ServeMux pattern's path template, or is absent.

CORE-10 — challenges and status codes (Low)

Problem.

  • A Basic-only server was answered with a bare Bearer challenge, and no challenge carried a realm (RFC 7617 §2 requires one, RFC 6750 §3 an auth-param).
  • The realm was rendered with Go escapes instead of an RFC 9110 §5.6.4 quoted-string.
  • invalid_token was sent only for expired or unknown tokens, and insufficient_scope never.
  • A wrong password joined with another authenticator's bail-out was answered 400.
  • Authorize answered an anonymous caller 403 with a hard-coded challenge.
  • An infrastructure failure was answered 401.
  • A Connect 401 carried no WWW-Authenticate at all.

Fix. One classification, security.ErrorClass, is shared by the core spans and the three default mappers. The default answers are:

Class HTTP gRPC / Connect
invalid_request 400 InvalidArgument
unavailable 503 Unavailable
token_expired, token_not_found, client_secret_mismatch, invalid_credentials 401 Unauthenticated
authentication_required, unsupported_credential 401 Unauthenticated
insufficient_scope, access_denied 403 PermissionDenied
no_extractor, no sentinel (_OTHER) 500 Internal

Challenge rules:

  • Every challenge carries a realm, api by default, rendered as a quoted-string; an invalid scheme or realm panics at construction.
  • A Basic challenge also carries charset="UTF-8" (RFC 7617 §2.1).
  • The RFC 6750 codes are sent only when the request carried a bearer token. invalid_token covers every failed check of the token (RFC 9068 §4); insufficient_scope a valid token refused by the access decision.
  • An unauthenticated principal refused by Authorize gets 401 with the middleware's challenge (RFC 9110 §15.5.2, §15.5.4).
  • The Connect mapper puts the same challenge in the error metadata.

Deviation from the code review (finding 5), deliberate. The review asked for a challenge on a Bearer resource's 400. This PR goes further: a Bearer resource challenges every refusal, 503 and 500 included.

  • RFC 6750 §3 says the resource server "MUST include the HTTP "WWW-Authenticate" response header field" when the request "does not include authentication credentials or does not contain an access token that enables access", and "MAY include it in response to other conditions as well".
  • A 5xx answered to a request without credentials falls under that MUST.
  • RFC 9110 §11.6.1 allows the field in any response ("A server MAY generate a WWW-Authenticate header field in other response messages").

So challenging every refusal is the only reading that never violates the letter. The challenge on a 5xx stays bare: §3.1 defines no code for it, and an empty error="" would break the grammar, which a test now pins. A Basic resource still challenges only its 401 and 403 (RFC 7617 has no such rule). The reviewer's re-review checked this reading and found it compliant.

Breaking: yes. See the MIGRATION sections for http, grpc and connectrpc. connectrpcsec.DefaultErrorMapper() becomes DefaultErrorMapper(scheme, realm string).

CORE-15 — credentials kept or printable (Low; the parts assigned to this PR)

Problem.

  • After a failed authentication, Engine.Process stored the extracted value (a cleartext password or a bearer token) in the request context and returned it.
  • grpcsec.Carrier and connectrpcsec.Carrier printed their incoming metadata, bearer token included, with %v.
  • Authorities() of the basic, bearer and session authentications returned their internal slice, so a caller could rewrite ROLE_USER into ROLE_ADMIN.

Fix.

  • The Engine and the Manager return and store security.Anonymous() on every failure.
  • The carriers print and log key counts only, whatever the fmt verb, and in slog.
  • Authorities() returns a copy, and the security.Authentication contract now requires it.

CORE-I3 — contract violations panicked on the request path (Info)

Problem.

  • An authenticator returning (nil, nil) panicked the request path, and so did a typed nil (var out *T; return out, nil).
  • So did a nil Attribute, a nil Authentication given to Decide, and a nil or broken custom Engine.
  • grpc-go recovers no panic, so on a gRPC server any of these crashed the process.

Fix.

  • A typed nil counts as nil wherever the core or an adapter checks a contract.
  • Success without an authenticated value is an internal error (500 / Internal).
  • A nil or typed-nil attribute or authentication given to Decide is denied, or evaluated as Anonymous().
  • An extractor's typed nil means "no credential".
  • FromContext reads a typed nil back as Anonymous().
  • At construction:
    • NewEngine, NewManager, the decision managers and the composites panic on a nil dependency;
    • the three authentication entry points panic on a nil engine;
    • the authorization constructors panic on a nil decision manager or a nil attribute, and copy their attributes.

CORE-I4 — grpcsec dropped the metadata staged on its carrier (Info)

The staged metadata is now sent as the response header, on success and on refusal. connectrpcsec merges it into the error metadata on a refusal or a handler error.

CORE-I5 — with several Authorization fields, the first one won (Info)

RFC 9110 §5.3 forbids repeating a singleton field. The new security.SingleValue(carrier, key) refuses a repeated field with ErrInvalidRequest, which is answered 400 / InvalidArgument. The Basic and Bearer extractors read through it.

CORE-I6 — status messages explained why authentication failed (Info)

The gRPC and Connect messages are now one fixed string per code (unauthenticated, permission denied, …).

  • Outages wrap ErrUnavailable and are answered Unavailable.
  • Errors wrapping no sentinel are answered Internal (user decision).
  • The bearer.TokenVerifier, basic.UserLoader and session.PrincipalLoader contracts state these rules.

CORE-I10 — the first extractor wins (Info, documented)

This is a deliberate design, as in Spring Security's filter chain. The NewEngine godoc and the "Extractor order" section of docs/security-considerations.md explain it: put header extractors before cookie sessions, protect cookie sessions against CSRF, and use one engine per audience.

CORE-I11 — documentation drift (Info)

  • AttrPrincipalSubject no longer references an option that does not exist.
  • The dead route override was removed.
  • LIMITATIONS lists ConnectRPC.
  • README, CLAUDE.md, architecture and security-considerations now match the code.

Tests

  • Red phase. Every finding starts from a failing test that reproduces it; the auditors' probes were rewritten to repository style. Examples:

    • an ADMIN route granted to a USER;
    • ?Authorization= accepted;
    • the stream served without a token;
    • the typed nil that crashed the gRPC test binary;
    • the 400 without a challenge.
  • End-to-end. The basic-http, bearer-jwt, session-web, grpc-bearer and connectrpc-bearer examples gained tests for:

    • where credentials are read from;
    • repeated fields;
    • the challenge per outcome;
    • the streaming RPCs.
  • Mutation testing.

    • Implementer: 231 mutants on every new guard (voters, mappers, carriers, nil checks, copies, example wiring); all are killed. The 3 initial survivors are each killed by an added test row.
    • Independent reviewer: 37 + 16 complementary mutants; every non-equivalent one is killed, the last (N6) by test(connectrpc).
  • Concurrency. Every module runs with -race.

  • Test doubles. The tests this PR adds build their interface doubles from the generated mockery mocks (make generate):

    • voters, extractors, authenticators and managers in the core;
    • ErrorMapper in http, grpc and connectrpc;
    • UserLoader and PasswordUser in basic, TokenVerifier in bearer, PrincipalLoader in session.

    Hand-written doubles remain only where no generated mock can stand in:

    • plain Authentication and Attribute values, which the tests compare by equality;
    • real carriers, such as the map-backed carrier and a carrier without cookie access;
    • doubles of core interfaces in the tests of other packages (the adapters, voter, the examples). The mocks are in-package _test.go files, so they are absent from every other package's test binary.

    After the migration, the mutants were re-run: all killed.

Checks

On Go 1.27.2 with golangci-lint v2.14.0:

  • make generate sync build test lint check-modules: green, 0 lint issues, no sync or tidy change.
  • govulncheck -test on every module: no reachable vulnerability. It ran against a local copy of the vuln.go.dev database, because the freshly built binary could not open outbound connections on this machine; CI's govulncheck workflow runs it as well.
Package master (before) this PR
security (core) 98.2 % 100.0 %
voter 97.2 % 100.0 %
http 92.0 % 100.0 %
grpc 100.0 % 100.0 %
connectrpc 100.0 % 100.0 %
basic 98.4 % 98.5 %
bearer 98.1 % 98.1 %
session 91.1 % 91.8 % (remaining gaps predate this PR; PR F)

Follow-ups (out of scope)

  • bearer-jwt end-to-end rows for an expired token and a wrong scope: PR F, which adds the token-minting hook they need. insufficient_scope is already proven end to end through connectrpc-bearer.
  • NFC normalization of Basic credentials under charset="UTF-8" (RFC 7617 §2.1): PR F. LIMITATIONS notes it meanwhile.
  • Several challenges per 401, for an engine accepting several schemes, is listed in LIMITATIONS.

Notes for the maintainer

  • User decisions applied:
    • realm api;
    • anonymous denial → 401 / Unauthenticated;
    • unsupported credential → 401;
    • repeated Authorization → 400 / InvalidArgument;
    • RFC 6750 codes only for bearer requests;
    • strict authority naming;
    • FullyAuthenticated() removed;
    • strict And();
    • Decide denies attributes no voter supports, and decisions without attributes or voters;
    • exported security.ErrorClass;
    • no sentinel → 500 / Internal;
    • security.ErrUnavailable → 503 / Unavailable.
  • http/go.mod: go.opentelemetry.io/otel/trace becomes a direct requirement (a test helper records spans).
  • No new module. Every behavior change has a MIGRATION section after "Toolchain" and a CHANGELOG entry.

@euskadi31
euskadi31 force-pushed the fix/audit-core-transport branch 2 times, most recently from 7a8762e to 3e87ff0 Compare October 13, 2026 01:02
After a failed authentication the Engine stored the extracted
Authentication (a cleartext password, a bearer token) in the request
context and returned it; the Manager returned it too. Engine.Process and
Manager.Authenticate now return the anonymous authentication on every
failure, and the Engine stores it in the context.

Contract violations no longer panic the request path: an authenticator
or a custom Manager reporting success without an authenticated value is
a refusal (an error wrapping no security sentinel), a nil Attribute
denies the decision, a nil Authentication is evaluated as anonymous and
a voter returning an undefined Decision votes Deny. Nil dependencies are
refused at construction (NewEngine, NewManager, the decision managers).

The Manager treats an authenticator error wrapping
ErrUnsupportedCredential, the documented runtime bail-out, as an
abstention instead of joining it with the real refusals: joined, it let
a wrong password be classified as an unsupported credential.

New sentinels for the transports' RFC 6750 and RFC 9110 mapping:
ErrAuthenticationRequired, ErrInvalidRequest and ErrUnavailable (a
dependency is temporarily down: not a verdict on the credentials).

Audit: CORE-I3, CORE-15, CORE-10.
The Engine and Manager spans carried the whole error chain of the
authenticators as status description and exception event: the username
typed at a Basic prompt (often a password typed in the wrong field) and
the wrapped errors of the application's stores reached the trace
backend, contrary to the documented secrets policy (OWASP Logging Cheat
Sheet, CWE-532).

security.ErrorClass(err) classifies a security error, joined errors
included, by the first sentinel it wraps in one precedence order shared
by every consumer: invalid_request, unavailable, the credential
verdicts, authentication_required / unsupported_credential, the access
decisions, no_extractor, else the OpenTelemetry fallback _OTHER. Failing
core spans now record only that class, as status description and as the
OpenTelemetry error.type attribute, and add no exception event. The
default transport mappers will answer from the same function.

basic.Authenticator no longer puts the typed username in its errors.
The AttrPrincipalSubject godoc no longer references a non-existent
SetSubjectAttributeMode.

Audit: CORE-07, CORE-I11.
Authentication values are documented as immutable snapshots, yet the
basic, bearer and session authentications returned their internal
authorities slice: any caller could rewrite the authorities of an
authenticated value (ROLE_USER into ROLE_ADMIN) for every later reader
of the same request context. They now return a copy, and the
security.Authentication contract requires implementations to do so
instead of recommending to share the backing slice.

Audit: CORE-15.
The attributes passed to an access decision only selected which voter
families ran: the role, scope and authority voters checked the names
they were built with whatever value the route asked for, and
Authenticated()/Anonymous() voted on every attribute. A decision manager
shared by several routes granted an ADMIN route to a USER, Authenticated()
next to a role voter granted any route to any logged-in principal,
HasAnyRole("ADMIN", "MANAGER") granted a MANAGER on Role("ADMIN"), and
Not() turned the refusal of an anonymous caller into a grant (CWE-863).

Voters now follow Spring Security's semantics. Each votes only on the
attributes whose family and value it supports and abstains otherwise;
HasAny* grants for the requested names only; Authenticated() and
Anonymous() vote only on the new security.IsAuthenticated and
security.IsAnonymous state attributes; Not() never grants an
unauthenticated principal. So that the new abstentions cannot silently
drop a requirement, And() is strict (a grant mixed with an abstention
denies) and a decision is denied when one of its attributes is supported
by no voter; the decision managers also pass each voter only the
attributes it supports. FullyAuthenticated(), identical to
Authenticated() for want of any remember-me concept, is removed.

Audit: CORE-01, CORE-13, CORE-I2.
Roles, scopes and plain authorities shared one flat namespace: HasRole
accepted a bare authority as well as its ROLE_ form, and HasScope a bare
authority, its scope: form and any scope: authority holding the name
among space-separated words. One bare "admin" authority (a scope a
client obtained, a fine-grained permission) therefore satisfied the
role, the scope and the authority checks at once, and HasRole("")
matched "ROLE_".

HasRole now matches only security.RolePrefix + name ("ROLE_ADMIN") and
HasScope only one security.ScopePrefix + name authority per scope
("scope:read:mail", what jwtsec.DefaultAuthorityResolver emits);
HasAuthority keeps its verbatim match. The constructors panic on a name
they could never match: none, an empty one, a role already starting
with ROLE_, a scope that is not an RFC 6749 section 3.3 scope-token or
starts with scope:. The basic-http example emits ROLE_ authorities and
its loader no longer echoes the typed username.

Audit: CORE-I1, CORE-07.
…cookie

httpsec.Carrier answered Get and Values from a cookie, then from the
query string, named like the key whenever the header was absent. Basic
and Bearer credentials were therefore accepted from ?Authorization=...
and from a cookie named Authorization, leaking through access logs,
Referer and history (RFC 6750 section 5.3, CWE-598). The session
manager read its cookie through the same Get, so a session was accepted
from ?session=... and from a Session request header that even took
precedence over the victim's cookie: a link or a cross-site form logged
the victim into the attacker's session (CWE-352, CWE-384).

Carrier.Get and Values now read the request's header fields only, and
never touch the URL (a request without URL no longer panics). The
session manager reads its cookie through a new explicit accessor,
httpsec.Carrier.CookiesNamed, declared on its side as
session.CookieReader; a carrier without it carries no session. The
session-web, basic-http and bearer-jwt examples pin the behaviour end
to end.

Audit: CORE-02, CORE-I3.
The default HTTP error mapping answered a Basic-only server with a bare
Bearer challenge, sent no realm (RFC 7617 section 2 requires one, RFC
6750 section 3 requires an auth-param after Bearer), rendered the realm
with Go escapes instead of an RFC 9110 quoted-string, sent invalid_token
only for expired or unknown tokens (RFC 9068 section 4: for every failed
check), never sent insufficient_scope, answered 400 when a wrong
password was joined with an unsupported credential, answered an
anonymous caller refused by Authorize 403 with a hard-coded bare Bearer
challenge, and an infrastructure failure 401.

The mapper now answers from security.ErrorClass: 400 for a malformed
request (invalid_request), 503 for ErrUnavailable, 401 with
invalid_token for a credential verdict, 401 without error code for no
credential or an unsupported one (RFC 6750 section 3.1), 403 with
insufficient_scope for an access decision, and 500 for an error that
wraps no sentinel. RFC 6750 codes are sent only when the request carried
a bearer token. Every challenge carries a realm (api by default), a
Basic one charset="UTF-8" (RFC 7617 section 2.1), and Middleware panics
at construction on a realm a quoted-string cannot carry. Authorize uses
the Middleware's error mapper and asks an unauthenticated principal to
authenticate (RFC 9110 sections 15.5.2 and 15.5.4).

A repeated Authorization field is refused (RFC 9110 section 5.3): the
Basic and Bearer extractors read it through the new
security.SingleValue. The middleware span records http.request.method
(_OTHER for unknown methods) and http.route as the ServeMux route
template only, never the raw path. A custom Engine returning nothing no
longer panics the middleware.

Audit: CORE-10, CORE-I5, CORE-I6, CORE-07, CORE-I3.
The default gRPC error mapper told the client why authentication failed
("token expired", "token not found"), contrary to the documented
policy, and answered an infrastructure failure Unauthenticated. It now
answers from security.ErrorClass with one fixed message per code:
InvalidArgument for a malformed request (a repeated authorization
value), Unavailable for ErrUnavailable, Unauthenticated for a credential
verdict, a missing or unsupported credential, PermissionDenied for an
access decision, Internal for an error that wraps no sentinel. An
anonymous principal refused by UnaryAuthorize or StreamAuthorize gets
Unauthenticated, and the authorization interceptors answer with the
authentication interceptor's mapper unless given their own.

The metadata an extractor staged on grpcsec.Carrier was never sent,
although the documentation said the interceptor flushed it: the
interceptors now send it as the response header, refused RPCs included.
Printing a Carrier printed its incoming metadata, the bearer token in
"authorization" included; it now prints and logs key counts only,
whatever the fmt verb. A custom Engine returning no authentication no
longer panics the server.

ServerOptions and AuthorizeServerOptions return the unary AND stream
interceptor chains as grpc.ServerOption values: the documented wiring
chained only unary interceptors, leaving streaming RPCs unauthenticated;
the godoc example now uses them.

Audit: CORE-I6, CORE-I4, CORE-10, CORE-15, CORE-05, CORE-I3.
A ConnectRPC unary call refused with Unauthenticated travels as an HTTP
401, which MUST carry a WWW-Authenticate challenge (RFC 9110 section
11.6.1); the Connect error mapper sent none. Its messages also told the
client why authentication failed, an infrastructure failure read as
Unauthenticated, and an unsupported credential as InvalidArgument.

connectrpcsec.DefaultErrorMapper(scheme, realm) answers from
security.ErrorClass with one fixed message per code, like the gRPC
mapper, and puts the RFC 6750 / RFC 7617 challenge of the HTTP adapter
in the error's metadata, sent as response headers for unary calls:
realm always present, charset="UTF-8" for Basic, invalid_token,
insufficient_scope and invalid_request only for requests that carried a
bearer token. WithChallengeScheme and WithRealm configure it; the
authorization interceptor uses the authentication interceptor's mapper
unless given its own, and asks an unauthenticated principal to
authenticate. The challenge helpers are a twin of httpsec's, the
dependency policy keeping the module off httpsec.

Printing a connectrpcsec.Carrier printed its incoming headers, the
bearer token included; it now prints and logs key counts only. Headers
staged on the carrier reach the client on a refusal and on a handler
error too, and a custom Engine returning no authentication no longer
panics the interceptors. The connectrpc-bearer example pins the HTTP
behaviour, the query string included (decision 6).

Audit: CORE-10, CORE-I6, CORE-15, CORE-I4, CORE-I3, CORE-02.
The grpc-bearer example chained only unary interceptors, and the health
service it registers also serves Watch, a server-streaming RPC: any
caller, without a token, received the health status stream. Copying the
documented wiring left every streaming RPC of an application
unauthenticated.

The example now builds its server from grpcsec.ServerOptions and
grpcsec.AuthorizeServerOptions, which install the unary and the stream
chains together, and a test proves Watch is refused without a token and
without the scope.

Audit: CORE-05.
The Engine tries its extractors in order and the first one finding a
credential wins: security-considerations now explains the CSRF exposure
of an engine that accepts a cookie session before header-based schemes,
and recommends one engine per audience. LIMITATIONS lists the ConnectRPC
adapter it omitted, and the operator checklist gains the voter,
challenge, gRPC stream and ErrUnavailable items of this change.

Audit: CORE-I10, CORE-I11.
- voter: a composite holds its combining function instead of a mode
  switch with an unreachable fallback.
- httpsec, connectrpcsec: isToken checks token = 1*tchar after the loop,
  pinned by an internal grammar test (the constructors map an empty
  scheme to "Bearer" first).
- grpcsec, connectrpcsec: drop the carriers' GoString; Format already
  serves %#v.
- Tests pin the default-deny baseline of every decision strategy (no
  attribute, all abstentions, no voter), a consensus majority of denies,
  the state attribute strings, grpcsec.Carrier.Get of an absent key, and
  httpsec.Middleware with an engine without extractors (500, or the
  anonymous fallback).

Audit: CORE-01, CORE-10, CORE-13, CORE-15.
- Decide denies a decision without attributes (or without voters)
  whatever the strategy, tie-break and abstain fallback: a unanimous
  manager with WithAbstainFallback(DecisionGrant) granted a route
  without attributes to anyone, anonymous callers included. The
  fallback only decides for custom voters that support an attribute
  and abstain.
- A typed nil (an interface holding a nil pointer) counts as nil in
  every contract check instead of panicking on its first method call:
  an authenticator or a Manager returning one breaks its contract, an
  extractor returning one found nothing, Decide treats one as
  Anonymous (or denies a typed-nil attribute), and FromContext reads
  one back as Anonymous.
- The Manager abstains on a bail-out only when the error wraps no
  higher-ranked sentinel: ErrUnsupportedCredential joined with
  ErrUnavailable is an outage, not an abstention.
- Tests pin the constructors' defensive copies, an extractor
  returning a value with its error, and identical repeated fields.
- Docs: a negated voter decides the attributes it supports, so a
  decision manager holding Not(HasRole("X")) must not serve routes
  that require X.

Audit: CORE-01, CORE-I3, CORE-15, CORE-I5.
HasAnyRole and the composites copy the slices they are given: a caller
reusing its slice for the next route cannot rewire a voter in use.

Audit: CORE-01.
- RFC 6750 §3: a Bearer resource answering a request without an access
  token that enables access MUST send WWW-Authenticate. The default
  mapper now challenges every refusal of a Bearer resource, 400 and
  5xx included, with the RFC 6750 error code only for a request that
  carried a bearer token; other schemes challenge the 401 and the 403.
- Middleware panics at construction on a nil engine, and answers a
  custom Engine's typed-nil Authentication 500 instead of panicking.
- Authorize and AuthorizeWith copy their attributes and panic at
  construction on a nil (or typed-nil) attribute: reusing the slice for
  the next route no longer rewrites the first route's policy.
- Tests pin the anonymous ErrInsufficientScope refusal, the nil-context
  guard, a bearer token in any Authorization field and the lowercase
  Basic charset.
- LIMITATIONS: one challenge scheme per middleware; Basic credentials
  are not NFC-normalized under charset="UTF-8".

Audit: CORE-10, CORE-I3.
- UnaryServerInterceptor, StreamServerInterceptor and ServerOptions
  panic at construction on a nil engine: grpc-go recovers no panic, so
  it crashed the server on the first RPC.
- A custom Engine returning a typed-nil Authentication is answered
  Internal instead of crashing the server.
- UnaryAuthorize, StreamAuthorize and AuthorizeServerOptions copy their
  attributes and panic at construction on a nil (or typed-nil) one.
- Tests pin the anonymous ErrInsufficientScope refusal, the nil-context
  guard and the options of the stream chains of ServerOptions and
  AuthorizeServerOptions.

Audit: CORE-I3, CORE-05, CORE-10.
- RFC 6750 §3: the default mapper puts the Bearer challenge in the
  metadata of every refusal of a Bearer resource — InvalidArgument,
  Unavailable and Internal included — with the RFC 6750 error code only
  for a request that carried a bearer token.
- NewAuthenticationInterceptor panics at construction on a nil engine,
  and a custom Engine's typed-nil Authentication is answered Internal
  instead of panicking.
- NewAuthorizationInterceptor copies its attributes and panics at
  construction on a nil (or typed-nil) one.
- Tests pin the anonymous ErrInsufficientScope refusal, the nil-context
  guard, the bearer fact of every Authorization field whatever the case
  of its scheme, the stream authorization's insufficient_scope and the
  lowercase Basic charset.

Audit: CORE-10, CORE-I3.
A verifier refusing a token wraps a credential sentinel (401
invalid_token), one that could not check it — key set or introspection
endpoint down — wraps security.ErrUnavailable (503 / Unavailable), and
an error wrapping no sentinel is an internal error. Authorities name
roles ROLE_<name> and scopes scope:<name>, what the voters match. A test
pins the classes the authenticator keeps.

Audit: CORE-I6, CORE-I1.
A loader reports an unknown user with security.ErrInvalidCredentials and
a user store that is down with security.ErrUnavailable: the outage is
answered 503 / Unavailable for every username, any other loader error is
a refusal. The AuthorityResolver names roles ROLE_<name> and scopes
scope:<name>. A test pins the classes.

Audit: CORE-I6, CORE-I1.
A loader refusing a session wraps security.ErrInvalidCredentials (401),
one whose store is down security.ErrUnavailable (503 / Unavailable); an
error wrapping no sentinel is an internal error (500). Authorities name
roles ROLE_<name> and scopes scope:<name>. A test pins the classes.

Audit: CORE-I6, CORE-I1.
A bearer request answered Unauthenticated without a token, Unavailable or
Internal carries the bare Bearer challenge: RFC 6750 §3.1 defines no code
for these answers, and an empty error="" would break the challenge
grammar.

Audit: CORE-10.
Before this change a nil attribute made Decide panic on every request —
crashing a gRPC server — rather than being answered 403 /
PermissionDenied.

Audit: CORE-I3.
The contract, decision and span tests added by this change use the
mockery mocks (make generate) instead of hand-written voters, managers,
extractors and authenticators: the typed nils are nil mock pointers, a
double that must never be consulted has no expectation, and the voters
expect exactly the attributes they support. Authentication and Attribute
values and the header-backed carrier stay plain values.

Audit: CORE-01, CORE-I3, CORE-07.
@euskadi31
euskadi31 force-pushed the fix/audit-core-transport branch from b99c99c to 290cbd1 Compare October 13, 2026 01:33
@euskadi31
euskadi31 merged commit cd82703 into master Oct 13, 2026
2 checks passed
@euskadi31
euskadi31 deleted the fix/audit-core-transport branch October 13, 2026 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant