Skip to content

feat(attestation): make the build-attestation trust anchor configurable - #64

Merged
xzyaoi merged 2 commits into
eth-easl:mainfrom
swiss-ai:feat/configurable-build-pubkeys
Sep 6, 2026
Merged

xzyaoi merged 2 commits into
eth-easl:mainfrom
swiss-ai:feat/configurable-build-pubkeys

Conversation

@AryanAhadinia

Copy link
Copy Markdown
Contributor

Problem

The Ed25519 public key that build attestations are verified against is a compile-time constant (maintainerPubKeyHex), so anyone distributing their own builds must patch attestation.go for them to be accepted. That is a source divergence in a file the project keeps changing, re-applied on every rebase — for a value that is not secret and is pure configuration.

Concretely: with security.require_signed_binary enabled, a downstream deployment building from source has no way to have its own nodes accept each other without forking this file.

Change

Adds security.build_pubkeys (env: OF_SECURITY_BUILD_PUBKEYS) — a list of hex-encoded Ed25519 public keys whose signatures Verify() accepts. It defaults to the existing upstream release key, so behaviour is unchanged when unset.

A list rather than a single value: replacing the key outright would mean a downstream node rejects every upstream-signed peer and vice versa — an all-or-nothing trust decision. Listing both lets a node accept its own builds alongside upstream's, which is also what a migration needs.

Details worth reviewing

  • The sync.Once is replaced with a mutex-guarded cache keyed on the raw config value. A Once permanently freezes whatever the first caller observed, which is fragile for config-derived state — the same ordering hazard that requires common.InitLogger() to be re-run at the end of initConfig once viper is populated.
  • Env values arrive as one delimited string. cast.ToStringSlice splits on whitespace, not commas, so entries are split on comma/semicolon/whitespace to accept both the config-file list form and the env form.
  • A malformed entry is skipped rather than fatal. With several keys configured, one typo must not disable the others. If no usable key remains, Verify() errors naming the config key.
  • maintainerPubKeyHex becomes the exported DefaultPubKeyHex so the default can be referenced (by the viper default) rather than duplicated.
  • buildsign's keygen hint now points at the config key instead of telling the user to edit attestation.go.

Tests

Six added: a configured key verifies; an untrusted key does not; the second of several keys is tried; the comma-separated env form parses; a malformed entry alongside a valid one still works; no-usable-key returns an error.

Two things I noticed while working here — happy to file separately

  1. security.require_signed_binary defaults to true in root.go, while docs/content/docs/advanced/security.mdx (three places) and this package's own doc comment all state false is the default. The docs' rationale ("allows gradual rollout without breaking existing deployments") suggests false was intended. I have not changed it here — it seems like your call which is authoritative.
  2. The signed message is version + "|" + commit, so it does not cover the binary or bind to the node's peer ID. A peer can copy a valid (version, commit, signature) triple from another node's record and pass verification while running different code. Signing a hash of the binary, or binding the attestation to the peer ID, would make the check meaningful.

🤖 Generated with Claude Code

AryanAhadinia and others added 2 commits September 4, 2026 18:49
The Ed25519 public key that build attestations are verified against is a
compile-time constant, so anyone distributing their own builds must patch
attestation.go to have them accepted. That is a source divergence in a
file this project keeps changing, and it has to be re-applied on every
rebase — for a value that is not secret and is pure configuration.

Add security.build_pubkeys (env: OF_SECURITY_BUILD_PUBKEYS), a list of
hex-encoded Ed25519 public keys whose signatures Verify() accepts. It
defaults to the upstream release key, so behaviour is unchanged when the
key is unset.

A list rather than a single value: replacing the key outright would mean
a downstream node rejects every upstream-signed peer and vice versa, an
all-or-nothing trust decision. Listing both lets a node accept its own
builds alongside upstream's, which is also what a migration needs.

Details worth reviewing:

- The sync.Once is replaced with a mutex-guarded cache keyed on the raw
  config value. A Once permanently freezes whatever the first caller
  observed, which is fragile for config-derived state — the same
  ordering hazard that requires common.InitLogger() to be re-run at the
  end of initConfig once viper is populated.
- Values arrive as a single delimited string when set through the
  environment (cast.ToStringSlice splits on whitespace, not commas), so
  entries are split on comma/semicolon/whitespace to accept both the
  config-file list form and the env form.
- A malformed entry is skipped rather than fatal: with several keys
  configured, one typo must not disable the others. If no usable key
  remains, Verify() returns an error naming the config key.

maintainerPubKeyHex becomes the exported DefaultPubKeyHex so the default
can be referenced (e.g. by the viper default) instead of duplicated, and
buildsign's keygen hint now points at the config key rather than telling
the user to edit attestation.go.

Adds six tests: a configured key verifies, an untrusted key does not,
the second of several keys is tried, the comma-separated env form parses,
a malformed entry alongside a valid one still works, and no-usable-key
returns an error.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Make DefaultPubKeyHex a var rather than a const so a distributor signing
its own builds can bake in its own default:

	-ldflags "-X opentela/internal/attestation.DefaultPubKeyHex=<hex>"

This mirrors how the signature itself already reaches the binary
(-X main.buildSig) and keeps the trust anchor with the build it belongs
to, rather than requiring every node to be configured — which, for a
deployment whose nodes are started by separate systems, means the same
key must be set in several places and silently partitions the mesh when
one of them is missed.

security.build_pubkeys still overrides at runtime, and behaviour is
unchanged for anyone who sets neither.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AryanAhadinia added a commit to swiss-ai/OpenTela that referenced this pull request Sep 4, 2026
Our builds are signed with the swiss-ai key, but every node verifies
against attestation.DefaultPubKeyHex, which is upstream's key. Until now
that was papered over per node — OF_SECURITY_BUILD_PUBKEYS in the rob-poc
dispatcher manifest and in model-launch's generated job scripts — two
separate config surfaces in two other repos, where missing one silently
partitions the mesh (each side drops the other's peer record).

Inject the trust anchor at link time instead, the same way the signature
already reaches the binary:

    -X opentela/internal/attestation.DefaultPubKeyHex=${BUILD_PUBKEY}

so the binary we sign is the binary that trusts us, and no downstream
configuration is needed. The env overrides in rob-poc and model-launch
are removed separately.

Applied only when the BUILD_PUBKEY repo variable is set: an empty -X
would leave the binary with no usable trust anchor at all, which with
security.require_signed_binary enabled rejects every peer. Unset, the
build falls back to upstream's key exactly as before.

DefaultPubKeyHex becomes a var to make -X possible; that half is
upstreamed in eth-easl#64 and will come back via rebase.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@xzyaoi
xzyaoi merged commit 119401e into eth-easl:main Sep 6, 2026
2 checks passed
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.

2 participants