Add signed capabilities and enforce local authentication - #39
Merged
Conversation
Security review #3 records that a capability is self-asserted: the caller supplies plan and capabilities in one request, and the only subject binding (`issued_to == plan.task_id`) is satisfied by a task_id the same caller chose. Add the missing primitive -- a detached ed25519 signature by an issuer that is not the caller -- so "capabilities are unforgeable" becomes checkable rather than merely documented. `Capability` gains an optional `signature`. Optional, deliberately: nothing in this repository issues capabilities yet, so a mandatory field would reject every request a current client can make and orphan every persisted record. The field is skipped when absent, so an unsigned capability's JSON is byte-identical to what earlier builds wrote, and a record written without the key still parses as unsigned. A regression test pins both directions. Canonical bytes are domain-separated and cover the typed model with the signature stripped, mirroring the manifest scheme in andromeda-hardware. That scheme's canonical-JSON writer and hex codec move to `andromeda_core::encoding` and both schemes now call them, because two copies of a signature encoder are two things that can silently drift. The bytes are unchanged -- every existing hardware signing test still passes -- and a golden signature vector now locks them so a future edit to the shared encoder cannot quietly invalidate manifest signatures already issued. What this does NOT achieve: whoever can run the signing helper is the issuer, so on its own this does not close review #3. It closes when a trusted host component owns the key and the requesting process cannot reach it. No new packages enter Cargo.lock. ed25519-dalek 2.2.0 was already declared by andromeda-hardware and pinned in the lock; andromeda-core declares the same version and feature set so the two resolve to one entry, and serde_json is promoted from dev-dependency to dependency. Signatures are hex, like every other opaque byte string here -- no base64 crate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add `CapabilityAdmission`, the policy the control plane applies to incoming capabilities, and make it a required argument to `TaskService::new`. The type has no `Default` and the permissive variant is spelled `unsigned_for_development()`, so the weak posture cannot be reached by leaving an argument off -- reading a call site tells you what a deployment enforces. With `require_signed(keyring)`, both paths that attach capabilities to a task -- creation and `grant_capabilities` -- reject anything unsigned, signed by an unknown key, malformed, or altered after issuance. An empty keyring is refused at construction rather than accepted: it would reject every request while presenting as a hardened configuration, so a typo in a trusted-keys file becomes a startup failure with a reason. Verification runs only after the length bound. Forcing ed25519 over an unbounded caller-supplied vector is a local denial of service (the flaw fixed for the general case in #24). `create` calls `validate_plan` first, which enforces MAX_TASK_CAPABILITIES; `grant_capabilities` checks the post-grant total, which necessarily covers the request's own length. `admit` re-states the bound as `AdmissionError::Unbounded` instead of trusting its callers, so a future call site that forgets gets an error rather than an unbounded verification loop. A test pins the ordering by handing in a list that is both over the limit and full of inadmissible grants, and asserting which check reports. `ServiceError::Admission` maps to 422 `capability_not_admitted` rather than `invalid_task`: obtaining a grant the issuer signed is a different operator action from repairing a malformed plan. The developer CLI names the permissive posture explicitly. It drives a local store with no daemon and no caller to authenticate, so requiring signatures there would protect nothing the filesystem does not already decide. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`taskd` had no authentication: any local process reaching loopback drove the full API (security review #2). Add a local bearer token, and put the guarantee in the serve wiring rather than in a config check. `app` now takes an `Authenticator` by value. The type has no `Default`, no public fields, and no constructor yielding "accept everything" -- every constructor is fallible and rejects an empty or trivially short secret. There is therefore no way to spell an anonymous router: not a flag, not an environment variable, not an alternative constructor. The previous design was rejected for validating an `AuthConfig` while leaving anonymous listening representable; this leaves nothing to validate. Authentication is the outermost layer, so an unauthenticated request is rejected before the Host check, before body parsing, and before any store lock is taken -- including on `/healthz`. Identity and permissions are decided in exactly one place. The previous attempt created a key directory as `0700 root:root` from two separate locations while the unit ran under `DynamicUser=`, so taskd failed at startup. Here, `auth.rs` owns the constants (`TOKEN_DIR_MODE`, `TOKEN_FILE_MODE`, `PRIVATE_MODE_MASK`, the token path), `ensure_private` asserts them at startup with an actionable error, and a test asserts the shipped unit agrees with them -- including that no static `User=`/`Group=` is declared and that `UMask=` is not looser than the mode the code creates. No file in the tree names a uid or gid, so there is nothing to keep in sync. The unit declares `RuntimeDirectory=andromeda-taskd` (0700) and points `ANDROMEDA_AUTH_TOKEN_FILE` at the token inside it. systemd is the only creator of that directory; taskd never mkdir's it. `UMask=0077` is now coherent with a 0600 token rather than locking out a group that was meant to read it, and `StateDirectoryMode=0700` matches the same constant. The API is reachable by the service account and by root, and by nobody else. Token generation uses `Uuid::new_v4` -- the CSPRNG already behind every CapabilityId and event id. A crate depending on getrandom 0.3.x would put a third major version of it in the lockfile (0.2.17 and 0.4.3 are both present); this adds no package at all. Comparison is constant-time, the secret is redacted from Debug output, and the file is installed atomically at 0600 rather than chmod'ed after the fact. `--capability-keyring` selects the signature-requiring admission mode, and `/healthz` reports which mode is in force so a running system's posture is observable rather than inferred from documentation. The shipped unit deliberately does not set it: nothing issues signed capabilities yet, so requiring them would reject every request any current client can make. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The threat model lists local authentication and issuer-signed capabilities as blocking prerequisites for an executor (4.2), so it has to say which of them now hold and which do not. Update it, the control-plane contract, and every user-facing statement that claimed taskd has no authentication. Threat model 4.2: authentication is recorded as landed, with the guarantee located at the serve wiring and the length bound named as the precondition for mandatory verification. Capability signing is recorded as a landed *mechanism* whose enforcement is off in the shipped image, and the prerequisite is rewritten rather than struck out: whoever holds the private key is the issuer, so until a trusted host component owns it and the caller cannot reach it, this does not close review #3. Prerequisite 3 (bind the authenticated subject into policy evaluation) is explicitly still open, with the reason -- the token is a single shared secret, so feeding it in as `subject` would yield a constant, not a constraint. 6.2 becomes "capability self-issuance (taskd auth fixed; no issuer)" instead of being marked resolved; 5.3's attack table gains rows for the attacks that are now blocked and, deliberately, a row for the one that is not: an attacker who already holds root or the service account. 8 gains the honest half-step on audit subject, and 9 records this review as triggered and completed. task-control-plane.md gains the authentication and capability-admission contracts, the 401 and 422 `capability_not_admitted` codes, and the `/healthz` posture fields; the "`Ready` does not guarantee" list now says the capability-issuance guarantee depends on deployment mode instead of being absent outright. README, README.zh-CN, SECURITY.md, and getting-started.md drop "no authentication of any kind" and state the narrower truth: the token separates this host's service account and root from other local users, and is neither user identity nor remote authentication. docs/reviews/ is left untouched on purpose. Those are point-in-time records -- security-review.md says so in its own footer -- and rewriting a finding to match today would destroy the evidence trail. The living status lives in the threat model, which they link to. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`require_runtime_health` curled `/healthz` with no credential. Now that every taskd route requires a bearer token, that probe would have returned 401 and failed the OS end-to-end run -- the image contradicting the design, which is precisely the failure mode the rework constraints exist to prevent. Read the token systemd's RuntimeDirectory holds and present it. The unit sets no `User=`, so this runs as root, which is exactly the access a 0600 token in a 0700 directory is meant to require. `Type=simple` reports the service active as soon as the process is exec'd, marginally before the token file can exist, so the read retries on the same budget as the HTTP retry below it and emits `ANDROMEDA_TASKD_TOKEN_MISSING` if it never appears. The token is returned through a global rather than stdout: the failure path calls `emit`, and a command substitution would have swallowed the marker the serial harness reads instead of letting it reach the console. `--retry` covers connection failures only, so a wrong token surfaces immediately as curl's exit 22 rather than a minute of pointless retries. shellcheck clean (same file set and invocation as ci.yml). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`project-map.md` told contributors to keep taskd loopback-only "until authentication, identity, brokered execution, and multi-tenant boundaries exist". Authentication now exists, so the sentence read as partly satisfied while the rule it states is unchanged. Drop authentication from the list of missing prerequisites and say what the token actually is -- a single shared secret separating the service account and root from other local users, which is neither identity nor remote authentication -- so the guidance is not read as weaker than it is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Running the daemon against a group-accessible token directory printed:
Error: TooPermissive { path: "/run/andromeda-taskd", mode: 488 }
fn main() returning a Result prints an error's Debug, not its Display, so the
mode came out in decimal and none of the guidance the error type spells out
reached the operator. The startup assertions exist precisely to tell someone
what to change, so this made the assertion technically present and practically
useless.
Move the body into run() and have main return an ExitCode, logging the error's
Display and walking the source() chain. The same failure now reads:
/run/andromeda-taskd is mode 0750, which grants access beyond its owner;
the API token must be readable only by the service account
(directory 0700, file 0600)
This was not only the new code: NonLoopbackBind's long, carefully worded
refusal had the same problem and had never been visible either. It, the empty
keyring refusal, and the token errors all render properly now, and all still
exit non-zero.
Verified against the built binary, not just in tests: group-accessible
directory, non-loopback bind, and an empty keyring each abort with their
intended message and exit 1; a private directory yields a 0600 64-character
token, unauthenticated and wrong-token requests get 401, the correct token gets
200 with capability_admission unsigned_allowed, and a correct token with a
foreign Host still gets 403.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rework of the
capability-authdesign thatdocs/reviews/remediation-design-review.md§1rejected. The direction was judged correct; this is the implementation redone against the five
mandatory constraints, walked through one by one below.
Underlying issues:
docs/reviews/security-review.mdfindings #2 (taskdhas no authentication)and #3 (capabilities are self-issued).
The five rework constraints
1. Key directory ownership/permissions and the service's runtime identity defined in ONE place, with an assertion
The previous attempt created
/var/lib/andromeda/keysas0700 root:rootfrom two separatelocations while the unit ran under
DynamicUser=, so taskd failed at startup;UMask=0077additionally made signing-path files unreadable to the group meant to read them.
The single place is
crates/andromeda-taskd/src/auth.rs. It ownsTOKEN_DIR_MODE(0700),TOKEN_FILE_MODE(0600),PRIVATE_MODE_MASK(0o077),RUNTIME_DIRECTORY_NAME, andSYSTEM_TOKEN_PATH. Three things follow:service identity is whatever systemd allocates (
DynamicUser=yesis kept), systemd createsboth
StateDirectory=andRuntimeDirectory=owned by that identity, and taskd nevermkdirs the directory — one creator, not two.ensure_private()refuses to serve if the token directory or filegrants anything to group or other, with an error naming the path, the mode found, and the
modes required. A missing directory gets its own error pointing at
RuntimeDirectory=.the_shipped_unit_matches_the_token_permission_constantsparses
os/files/usr/lib/systemd/system/andromeda-taskd.serviceand asserts thatRuntimeDirectoryMode=/StateDirectoryMode=equalTOKEN_DIR_MODE, thatANDROMEDA_AUTH_TOKEN_FILEmatchesSYSTEM_TOKEN_PATH, that no staticUser=/Group=isdeclared, and that
UMask=is not looser than the mode the code creates. The two cannot drift.I mutation-tested this: flipping
RuntimeDirectoryModeto0750fails with"RuntimeDirectoryMode must equal auth::TOKEN_DIR_MODE".
UMask=0077is now coherent with the design rather than in conflict with it: everything thisservice creates is owner-only, and there is no group expected to read anything.
2. The authentication guarantee is unbypassable at the SERVE wiring
The previous attempt validated an
AuthConfigwhile leaving anonymous listening representable,and shipped a unit that only logged the uid without making an authorization decision.
pub fn app(service: TaskService, authenticator: Authenticator) -> Router.Authenticatorhas no
Default, no public fields, and no variant meaning "no authentication". Everyconstructor is fallible and rejects an empty or under-32-character secret. The route handlers and
AppStateare private, so no other crate can assemble an equivalent router. An unauthenticatedlistener is not representable — there is nothing to validate because there is no invalid state
to reach.
Authentication is the outermost layer, so an unauthenticated request is rejected before the
Host check, before body parsing, and before any store lock is taken. Every route,
/healthzincluded, returns 401
unauthorizedwithout distinguishing missing / malformed / wrong.Comparison is constant-time; the secret is redacted from
Debug.Image behaviour matches the documentation. No unit directive can disable authentication
because no such switch exists, and the test above also asserts the unit does not set
ANDROMEDA_ALLOW_NON_LOOPBACK./healthzreportsauthenticationandcapability_admission,so a running system's posture is observable rather than inferred from docs.
One consequence I had to fix rather than let CI discover:
os/files/usr/libexec/andromeda-ci-verifycurled
/healthzwith no credential and would have failed the OS end-to-end run — the imagecontradicting the design, which is exactly the failure mode this constraint targets. It now reads
the token from
RuntimeDirectory(it runs as root, precisely the access a 0600 token in a 0700directory requires), retries on the same budget as the HTTP retry because
Type=simplereportsactive marginally before the token file exists, and emits
ANDROMEDA_TASKD_TOKEN_MISSINGotherwise.
3. Dependency reality checked against
Cargo.toml/Cargo.lockbefore arguing anythingVerified in this tree, not assumed:
ed25519-dalek 2.2.0already presentcrates/andromeda-hardware/Cargo.toml, pinned atCargo.lock:416.andromeda-coredeclares the identical version and feature set (default-features = false,std,zeroize), so both resolve to one entry.getrandommajors already in the lockUuid::new_v4, which reachesgetrandom 0.4.3— already the source of everyCapabilityIdand audit event id.base64ArtifactPin.sha256andManifestSignature.sig.Cargo.lockdiff: zero new packages. 157[[package]]entries before, 157 after. The onlychanges are dependency edges between crates already in the graph:
serde_jsonwas promoted from dev-dependency to dependency inandromeda-core(already in thelock either way) because canonical JSON is now a shipped code path rather than test-only.
4. A length bound precedes every mandatory cryptographic verification
MAX_TASK_CAPABILITIES(10 000, added by #24) does bound every path verification was added to.Confirmed per path:
create—validate_plan()enforcescapabilities.len() > MAX_TASK_CAPABILITIESand runsbefore
admission.admit().grant_capabilities— the existing post-grant total check runs first, and sincetotal = existing + request.len(), it necessarily bounds the request's own length too.CapabilityAdmission::admitre-states the bound itself and returnsAdmissionError::Unboundedrather than trusting its callers, so a future call site that forgetsgets an error instead of an unbounded verification loop. axum's default 2 MB body limit bounds
it earlier still.
Tested at both layers by handing in a list that is simultaneously over the limit and full of
individually inadmissible grants, then asserting which check reports first — plus asserting that
at exactly the limit, verification is what reports, so the test cannot pass vacuously. The bound
applies in permissive mode too, so "signatures are off today" cannot be used to smuggle in an
unbounded vector.
5. Threat model §4.2 and the control-plane contract updated
docs/andromeda-threat-model.md§4.2 (the section listing these as executor-blockingprerequisites), plus §5.3, §6.2, §8, and §9; and
docs/development/task-control-plane.mdfor thecontract changes (401, 422
capability_not_admitted,/healthzposture fields, and the"
Readydoes not guarantee" list). AlsoREADME.md,README.zh-CN.md,SECURITY.md, andgetting-started.md, all of which asserted "no authentication of any kind".Prerequisite 1 is rewritten rather than struck out, prerequisite 2 is marked landed, and
prerequisite 3 is documented as still open, with the reason — see below.
docs/reviews/is deliberately untouched: those are point-in-time records (security-review.mdsays so in its own footer), and rewriting a finding to match today would destroy the evidence
trail. Current status lives in the threat model, which they link to.
What this does NOT achieve
Stated plainly, because the mechanism looks like more than it is:
private key is the issuer, and no component in this repository issues capabilities. That is
why the shipped image deliberately configures no keyring and runs
unsigned_allowed: a keyringthere would reject every request any current client can make. An authenticated caller still
mints its own grants today. The gap closes when a trusted host component owns the key and the
requesting process cannot reach it.
It narrows the caller set from "any local process or user" to "the service account and root" —
a real improvement over an open loopback API — but it does not defend against an attacker who
already holds either, and it is not remote authentication.
into
EvaluationContext::subjectwould feed a constant into the policy engine, not aconstraint, because one shared secret cannot distinguish callers. That needs
SO_PEERCRED/AF_UNIXor real user identity. I did not fake it;Authenticatoris an enum-ready shape thata peer-credential variant slots into without weakening this one.
Signatures are optional on the wire — and why
Making
Capability.signaturemandatory would reject every capability current clients can produceand orphan every persisted task record, with no issuer able to produce signed ones. It is
therefore
Option, skipped when absent, so unsigned capabilities serialize byte-identically towhat earlier builds wrote, and records written before the field existed still parse — as
unsigned, not as anything else. A regression test pins both directions. Enforcement is the
deployment decision (
--capability-keyring), and that is what actually breaks when turned on:every unsigned grant is refused with 422
capability_not_admitted.Incidental: one canonical encoder, not two
The manifest and capability signature schemes must agree on how a typed value becomes a message.
andromeda-hardware's canonical-JSON writer and hex codec moved toandromeda_core::encodingand both schemes now call them — two copies of a signature encoder are two things that can
silently drift. The bytes are unchanged (every existing hardware signing test passes untouched)
and a golden signature vector now locks them, so a future edit to the shared encoder cannot
quietly invalidate manifest signatures already issued.
Validation
cargo fmt --all— cleancargo clippy --workspace --all-targets --locked -- -D warnings— cleancargo test --workspace --locked— 242 passed, 0 failedshellcheckon ci.yml's exact file set and invocation — clean.andromeda-ci-verifyis theonly shell file this branch touches.
StateDirectoryMode/RuntimeDirectoryModecontradicting the runtime user,
UMaskconsistent with the file mode the code creates, and thewhole thing asserted by test.
Required test coverage:
create_accepts_a_capability_the_trusted_issuer_signed,a_valid_signed_capability_is_admitted,signed_capability_verifies_against_its_keycreate_refuses_a_capability_tampered_with_after_signing,widening_a_grant_after_signing_is_detected,removing_the_expiry_after_signing_is_detected,changing_the_subject_after_signing_is_detectedan_unknown_key_is_rejected,unknown_key_id_is_reported,a_different_key_under_a_trusted_id_fails_verification,empty_keyring_trusts_nothingthe_capability_bound_is_enforced_before_signature_verification,the_length_bound_is_enforced_before_any_verification,the_length_bound_applies_in_unsigned_mode_tooan_unauthenticated_listener_cannot_be_constructed,every_route_rejects_a_caller_without_the_token,only_the_exact_token_is_acceptedAll test keys derive from fixed seeds, mirroring
andromeda-hardware— no runtime RNG in anytest, and
signing_is_reproducible_from_the_seedasserts that this is meaningful rather thanmerely non-random.
🤖 Generated with Claude Code