Skip to content

metalman: bind bootstrap credentials to the attested Machine - #946

Merged
Jordan Olshevski (jveski) merged 32 commits into
mainfrom
metalman-attest-hardening
Oct 20, 2026
Merged

Jordan Olshevski (jveski) merged 32 commits into
mainfrom
metalman-attest-hardening

Conversation

@jveski

@jveski Jordan Olshevski (jveski) commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Metalman bootstrap tokens are now bound to the specific Machine they were issued/encrypted for, and CSR signing is effectively guarded behind TPM auth. This is defense in depth to protect against replay attacks where a decrypted token is exfiltrated from the requesting node.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Admission installation can leave bootstrap permissions unrestricted, and CSR approval defects block the smoke provisioning flows.

3 open issues
1 file not reviewed
  • api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file
What changed in this PR

This PR binds metalman bootstrap credentials to an attested Machine, tightening node identity and certificate approval.

Changes:

  • Identify netboot clients by direct TCP peer and pin their node names.
  • Record bootstrap credential IDs and validate kubelet certificate requests against Machines.
  • Restrict bootstrap status writes, remove legacy auto-approval, and update tests and documentation.
File Description
pkg/​agent/​daemoncred/​reconciler.go Support multiple CSR evaluators.
pkg/​agent/​daemoncred/​kubelet_approver.go Validate kubelet certificate requests.
pkg/​agent/​daemoncred/​kubelet_approver_test.go Test approval and rejection cases.
internal/​operator/​components/​metalman/​vap_test.go Test admission restrictions.
internal/​operator/​components/​metalman/​metalman.go Install policies and remove stale RBAC.
internal/​operator/​components/​metalman/​metalman_test.go Test deployment plans and cleanup.
internal/​metalman/​peeraddr/​peeraddr.go Extract direct peer addresses.
internal/​metalman/​peeraddr/​peeraddr_test.go Test address extraction and header rejection.
internal/​metalman/​netboot/​peer_test.go Simulate peer addresses in tests.
internal/​metalman/​netboot/​netboot.go Pin agent node names.
internal/​metalman/​netboot/​netboot_test.go Update peer tests and verify node naming.
internal/​metalman/​netboot/​media_test.go Update forwarded-header expectations.
internal/​metalman/​netboot/​media_site_test.go Verify site fallback ignores forwarding.
internal/​metalman/​netboot/​media_legacy_test.go Require direct peers for legacy serving.
internal/​metalman/​netboot/​media_http.go Use peer addresses for attestation lookup.
internal/​metalman/​netboot/​media_attestation_test.go Reject forwarded attestation identities.
internal/​metalman/​netboot/​http.go Remove forwarding-header identity lookup.
internal/​metalman/​netboot/​forwarded_spoof_test.go Test header-spoofing rejection.
internal/​metalman/​machineops/​virtualmedia.go Use shared node-name resolution.
internal/​metalman/​machineops/​controller.go Remove duplicate node-name logic.
internal/​metalman/​attestation/​attestation.go Persist credential IDs with EK pins.
internal/​metalman/​attestation/​attestation_test.go Test credential binding and failure handling.
hack/​smoke-metalman.py Add explicit smoke CSR approval.
hack/​smoke-metalman-http.py Remove legacy automatic approval.
docs/​content/​reference/​machina-crd.md Document bootstrap status fields.
docs/​content/​reference/​architecture.md Describe bootstrap security changes.
docs/​content/​guides/​pxe.md Explain restrictions and requirements.
deploy/​machina/​crd/​unbounded-cloud.io_machines.yaml Add bootstrap status schema.
deploy/​machina/​06-metalman-rbac.yaml.tmpl Restrict bootstrap permissions and status writes.
deploy/​machina/​02-rbac.yaml.tmpl Grant machina kubelet approval permission.
cmd/​metalman/​README.md Document Machine-bound bootstrap credentials.
cmd/​machina/​machina/​controller/​metalman_csr_approver.go Authorize CSRs against Machine credentials.
cmd/​machina/​machina/​controller/​metalman_csr_approver_test.go Test credential matching and retries.
cmd/​machina/​machina/​controller/​manager.go Register approver and credential index.
api/​machina/​v1alpha3/​zz_generated.deepcopy.go Copy new bootstrap status fields.
api/​machina/​v1alpha3/​machine_types.go Define bootstrap status and node-name helper.
api/​machina/​v1alpha3/​machine_types_test.go Test node-name fallback behavior.

Review effort: Balanced

Comment thread hack/smoke-metalman.py Outdated
Comment thread hack/smoke-metalman.py Outdated
Comment thread internal/operator/components/metalman/metalman.go
Comment thread hack/smoke-metalman.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Security-sensitive credential authorization and admission rollout require human review, with unresolved authorization and upgrade-safety concerns.

4 open issues
1 resolved since last review
1 file not reviewed
  • api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file

Review effort: Balanced

Comment thread cmd/machina/machina/controller/metalman_csr_approver.go Outdated
Comment thread internal/operator/components/metalman/metalman.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Disabled-site upgrade protection, live-name CSR validation, and revocation-error reporting need correction.

5 open issues
2 resolved since last review
1 file not reviewed
  • api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file

Review effort: Balanced

Comment thread cmd/machina/machina/controller/metalman_csr_approver.go Outdated
Comment thread internal/operator/component/execute.go Outdated
Comment thread internal/operator/components/metalman/metalman.go Outdated
The cached Machine now only locates the candidate by credential ID. The node-name check runs against the live Machine so informer lag after a nodeRef change cannot deny a CSR for the authorized name.
A revocation that fails to delete the object is now recorded as OpFailed so the deletion error reaches ExecutionResult.Err() and CombineResult(). Successful or unnecessary revocations remain OpSkipped.
CleanupPlan now plans the shared metalman support set exactly as Plan does, so the bootstrap grant left by an earlier release stays behind the admission restriction and is revoked if the restriction fails, even when every Site has metalman disabled. Identical shared operations dedupe across enabled and disabled Sites.
@jveski
Jordan Olshevski (jveski) requested a balanced review from Copilot October 19, 2026 16:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-controller authorization and fail-closed upgrade behavior require human security review, alongside resolution of the retained correctness issue.

3 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity NodeRef changes during replacement cause stale-node completion timeout

internal/​metalman/​netboot/​netboot.go:236

If spec.kubernetes.nodeRef.name changes after a virtual-media HostReplace captures its boot-media state but before first-boot config is fetched, this config uses the new name and the CSR approver accepts it. Completion still queries the old target.BootMedia.NodeName (internal/metalman/machineops/virtualmedia.go:456,628-638). If that old Node is absent, the replacement times out even though the new Node joined successfully. Use one consistent node identity for config generation, CSR authorization, and replacement completion, and add a regression test for a nodeRef edit during replacement.

1 resolved since last review
1 file not reviewed
  • api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file

Review effort: Balanced

Record the node name a bootstrap credential was issued for in
status.bootstrap.nodeName and have the machina CSR approver match the
kubelet CSR against it instead of the live nodeRef. For virtual-media
first boot, the media owner lookup pins the Machine's node name to the
name captured in the operation's boot media status, so the agent config,
the recorded credential binding, and HostReplace completion use the same
name even if spec.kubernetes.nodeRef changes mid-replacement.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Coordinated credential issuance, certificate authorization, admission restrictions, and upgrade behavior require human review and end-to-end verification.

3 open issues
1 file not reviewed
  • api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file

Review effort: Balanced

Comment thread internal/metalman/peeraddr/peeraddr.go Outdated
Comment thread cmd/machina/machina/controller/metalman_csr_approver.go Outdated
@jveski

Copy link
Copy Markdown
Contributor Author

Copilot address my pr review comments

Copilot AI and others added 2 commits October 19, 2026 18:51
Co-authored-by: jveski <7576912+jveski@users.noreply.github.com>
Co-authored-by: jveski <7576912+jveski@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-cutting authentication, admission enforcement, and upgrade-time permission changes require final human security review.

1 open issue
3 resolved since last review
1 file not reviewed
  • api/machina/v1alpha3/zz_generated.deepcopy.go: Generated file

Review effort: Balanced

Comment thread hack/smoke-metalman.py Outdated
Ensure check=True for the delete command to enforce error checking.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@jveski
Jordan Olshevski (jveski) added this pull request to the merge queue Oct 20, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 20, 2026
@jveski
Jordan Olshevski (jveski) added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 4d9b369 Oct 20, 2026
56 of 57 checks passed
@jveski
Jordan Olshevski (jveski) deleted the metalman-attest-hardening branch October 20, 2026 19:47
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.

4 participants