Repository navigation
feat(edge): Add the MASQUE grant authority - #1176
Conversation
67d7f28 to
1ce50d7
Compare
The MASQUE adapter's Authorizer had no implementation outside the conformance fixture, which compares plaintext credentials from a file, issues a fixed one-hour deadline and points at EdgeServices someone created by hand. This adds the authority the MASQUE contract assigns to #977. internal/edge/masque/authority stores only a SHA-256 digest of each 32-byte bearer credential, in one grant record Secret per connector, with its revision, an expiry of at most 24 hours, an enabled flag and allowed destinations. One ledger ConfigMap per VPC and cell holds each connector's /96 and attachment /120. Every write is conditioned on UID and resourceVersion, and retired entries stay as tombstones, so no /96 or pool is reassigned. Authorize matches the credential against every record in constant time, rereads the record, VPC and ledger without a cache, and issues one finite EdgeService per authorization incarnation, bound to the calling pod. Its validUntil is the credential expiry or the read start plus 28 s, whichever is earlier. Refresh renews it only while the revision is current and enabled. Retire, rotation, revocation and the sweep set validUntil in the past. A retired incarnation is never renewed, and is deleted only after core releases its pool addresses and a 10 s retry window has passed. Each Authorize call carries the time it was first sent, and a call older than the window is refused, so a late retry can't create a second incarnation. Incarnations are named for the connector and issued only once no other exists, so pods racing to issue contend for one name and the API server accepts one. After each create or renewal the authority rereads the grant record, and retires the incarnation if a rotation, disable or deletion raced the write. The sweep retires only incarnations its own cell issued, and ledger init refuses while an issued service of the VPC in that cell still exists, or when its ranges overlap another cell's ledger for the same VPC. Server serves the calls over TLS 1.3 to pods presenting a pod-bound ServiceAccount token, checked with a TokenReview. Client is the provider's masque.Authorizer and rereads its token and CA bundle when they change. cmd/masque-authority runs the server and provides the operator commands: ledger init, and grant create, rotate, disable and retire. masque.Grant gains the authorization ID and credential revision that Refresh needs.
The grant authority renews a connector by moving its EdgeService's validUntil forward in place, and each renewal is a new generation. Core's attachment reconciler re-admits on a generation change, and nothing tested that a renewed attachment keeps forwarding. TestAgainstCore now renews a live connector's service past its first deadline, through several new generations. The request stays open, and the attachment, route and claim epoch stay unchanged.
docs/edge/masque-authority.md covers the authority's state, calls, retry and replacement rules, revocation, operator commands, RBAC and test coverage. The provider guide, the edge index, the acceptance matrix and the contract link to it. The contract notes the two places the build differs from its design: the retry window, and revocation without push notifications. The conformance fixture can authorize through the authority by setting grantAuthority in place of its static services and connectors.
59ebb45 to
0a4f48d
Compare
|
Review record for head 0a4f48d. First pass at 67d7f28: pr-adversary returned "VERDICT: merge" with one warning (cross-cell ledger initialization) and two decision hedges that were settled with evidence afterwards (envtest had not been run, and the base was not local); pr-conventions-reviewer returned "VERDICT: merge" with no findings. The branch was then rebased onto main 389a297 by pr-rebaser, which resolved conflicts in the logging package and the fixture main, renamed two reserved log keys, and ran envtest 1.37 with TestAuthority, TestOverHTTPS and TestAgainstCore passing. The cross-cell warning was fixed at 59ebb45. Second pass at 59ebb45: pr-rereviewer returned "VERDICT: merge" and left one nit about a missing same-cell refusal assertion; pr-conventions-reviewer returned "VERDICT: hold" with warnings that two fix commits patched the feat commit and that the feat commit message was stale, plus a CI hedge. Because the verdicts split, the decision went to the user, who chose to squash and then run the ready phase. The squash folded the fix commits into three commits, rewrote the feat message so it states both ledger initialization rules, and added the same-cell refusal assertion; a tamper test showed that assertion fails when the check is removed. The tree equals 59ebb45 plus that one assertion, and the force-push was leased against 59ebb45. What the reviewers ran: pr-rereviewer reran envtest 1.37, ran tamper tests that removed each guard and confirmed each guard's test fails, and checked the conflict resolutions and the log key rename. The first-pass reviewers read the diff and the conventions, and pr-rebaser ran envtest 1.37 after the rebase. After the squash, lint reported 0 issues and the authority envtest suite passed on 1.37. CI on 0a4f48d: every check passed (build, lint, manifests, documentation, service API bundle, unit tests, root unit tests, e2e, image publishes and the CLA check). Auto-merge is enabled with the merge method, the only method the main ruleset allows. The ruleset requires one approving review, code owner review and approval of the last push, so the PR waits for a human approval. |
ecv
left a comment
ecv
left a comment
There was a problem hiding this comment.
Approve.
No security exposure or data-loss path found: callers are authenticated by audience-bound, pod-bound TokenReview and pinned to one ServiceAccount, only SHA-256 digests are stored, rotation/disable/expiry fail closed, and ledger and record writes are conditioned on UID and resourceVersion. CI is green at 0a4f48d, and the internal/logging change only adds masque-authority to Binaries, which hack/check-log-settings reads, so no other binary is affected.
Follow-ups:
Grants.Createallocates the ledger entry before writing the grant Secret, so a failed Secret create (or a duplicate--connector) leaves an unused allocation that counts toward the 256-entry cap and is never reclaimable. Consider creating the record first or retiring the entry on failure.Ledgers.Initchecks for overlapping cell ranges with a list-then-create, so two concurrentledger initruns for different cells of one VPC could still be handed overlapping /80s. Operator-only, but not enforced by the API.Sweepbuilds its connector map from records that decode cleanly, so a single malformed or hand-edited grant Secret is treated as "grant record deleted" and retires that connector's live service. This fails closed but is an easy outage from a bad edit; skip retirement for connectors whose record exists but does not decode.Authorizelists and decodes every grant Secret per call to match the digest. Fine at the 256-per-ledger scale, but cost grows with total connectors per namespace and is paid on every pod retry; a digest-indexed label would avoid it.Server.calldecodes only the first JSON value and ignores trailing bytes; harmless today, worth rejecting if the body ever carries more.contract.mdis amended for the retry-window and issue-time differences, but the authority is cluster-scoped foredgeserviceslist (sweep) while the doc's RBAC list is left to the unwritten manifests (#980); make sure that role is reviewed there, since it spans every tenant namespace.
Summary
MASQUE connectors had no real authority behind them, only the conformance fixture's plaintext test credentials and hand-made EdgeServices.
This adds the grant authority from the MASQUE contract. It stores only credential digests, keeps a durable ledger of each connector's /96, and issues one finite EdgeService per authorization that it renews only while the credential stays current, so expiry, rotation and revocation stop one connector without touching its neighbours.
The HTTP/2 rows, refusing probes before admission, deployment manifests and the lab measurement of the bounds stay with #981, #980 and #985.
Test plan
Fixes #977