Skip to content

feat(config): switch ACME and EST on and off through machine config - #286

Merged
Bugs5382 merged 5 commits into
mainfrom
feat/284-protocol-switches
Sep 30, 2026
Merged

Bugs5382 merged 5 commits into
mainfrom
feat/284-protocol-switches

Conversation

@Bugs5382

@Bugs5382 Bugs5382 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

What and why

ACME and EST could only be switched on by a hand-staged YAML file: MachineConfig had no field for them, so config apply, the maintenance install and the Fleet Manager could not switch either protocol on, change it or switch it off, and GetStatus could not say what a node serves. CryptOS-PKI/api#115 adds the contract (Pki.acme, Pki.est, NodeStatus.protocols, config_reboot_pending). This is the node side.

  • Mapping. FromProto/ToProto carry both blocks field for field, and CarryForwardProtoGaps is gone. ToProto always sends an explicit block, enabled: false for an off protocol. FromProto turns enabled: false (or no block) into nil, the node's existing "off".
  • Apply rules (config.FromProtoOver, used by ApplyConfig, the maintenance install and re-provision): an absent block keeps the stored one; enabled: false switches off; enabled: true switches on and is validated.
  • Write-only secrets. GetConfig returns hmac_key_base64 and password_sha256 blank. An apply with a blank secret keeps the stored value for the same key_id/username; a blank secret for an unknown identifier is refused (... empty, and the node has no stored key for key_id "x" to keep). Removing an entry revokes it. cryptosctl config apply accepts the blanks client-side (config.ParseForApply), so config get output can be fed straight back.
  • Root refusal. Validate refuses pki.acme or pki.est on a Root. That one rule covers config apply, the maintenance install, re-provision and ceremony start --config, which parses the raw YAML before storing it, so a Root can no longer pick up ACME or EST at its next boot.
  • Boot only. The listeners already started only at boot from the stored config (steps 12c/12d in internal/init/run.go); that stays, with a log line when a protocol is off. Every protocol change is outside the hot fields, so it answers requires_reboot: true.
  • Status. NodeStatus.protocols reports each protocol's configured state (stored config), running state (this boot) and reboot_pending (stored block differs from the boot one); config_reboot_pending is NeedsReboot(boot config, stored config). cryptosctl status prints Protocols: ACME on (not running, reboot pending), EST off and a Reboot: pending line.
  • Found on the way. An unchanged re-apply over the wire always answered requires_reboot=true (same digest), because gRPC turns an empty list into nil and NeedsReboot used reflect.DeepEqual against the stored YAML's empty slices. NeedsReboot now compares the stored YAML form (config.Equivalent).

Design calls:

  • enabled: false stores nothing, not a disabled block with its settings. The proto allows either, and nil is the node's single meaning of "off"; keeping settings for a later switch-on can be added without a contract change.
  • A Root refusal lives in Validate, so every path gets it. A Root whose stored config already has one of these blocks would fail to parse at its next boot and drop to maintenance; no lab config has one, and a Root serving ACME already broke Root Mode, so failing closed is right.

api pin. github.com/CryptOS-PKI/api is pinned to api main at 81494db, which carries CryptOS-PKI/api#115 (this contract), CryptOS-PKI/api#123 (the manager's SetNodeProtocol) and CryptOS-PKI/api#104. Api main also has the SCEP challenge and approval RPCs (CryptOS-PKI/api#117) and ListTsaCertificates (CryptOS-PKI/api#120), which this node does not serve yet, so internal/grpc/unserved.go answers them Unimplemented (tested in unserved_test.go). #288 replaces the SCEP stubs with its handlers when it rebases. The wip/284-api-integration api branch is no longer used.

Tests removed, because the function they covered is gone: TestCarryForwardProtoGaps, TestCarryForwardProtoGaps_DoesNotOverwrite and TestCarryForwardProtoGaps_NilSafe (the absent-block keep they guarded is now TestFromProtoOverAbsentBlockKeepsStored and TestConfigStoreApply_AbsentBlockKeepsProtocol). Fixtures moved from a Root to an issuing node, because a Root can no longer carry these blocks: the ACME and EST validation tests in internal/config, TestConfigStoreApply_PreservesProtocols and the TestConfigStoreApply_RejectsInvalidConfig seed.

Closes #284

Related: #205, CryptOS-PKI/api#114, CryptOS-PKI/cryptos-manager#84, CryptOS-PKI/cryptos-manager#61.

Rebased onto main and landed by the Linux box at the owner's request. Main in this repo is not relicensed yet (#291 is still open). The rebase conflicted only in internal/init/run.go (the ACME listener log line): main's !cfg.PKI.ACME.AllowAnonymousAccounts argument is kept, with this PR's "ACME: off in the boot config" branch. Every commit is signed off. New files carry the header task license checks on main today; #291 restamps them with the rest of the tree.

Checked against the Fleet Manager on main: SetNodeProtocol reads GetConfig, flips one block's enabled flag, sends secrets blank and leaves the other block out. The node keeps stored secrets for a blank value, keeps an absent block, reports both ACME and EST in NodeStatus.protocols, and refuses a Root, which is what the manager's reboot tracking and error passthrough expect. Because enabled: false stores nothing, the block the manager reads back for an off protocol is empty, so switching a protocol on from the manager is refused by validation until the operator applies a full block; the docs site already says the manager does not fill in settings.

Docs: docs/acme.md and docs/est.md gain a "Switching it on, changing it and switching it off" section (Root refusal, reboot and maintenance-window callout, write-only secrets) and lose the "not yet carried in the proto" limit. The docs/est.md status example now lists ACME before EST, as cryptosctl status prints it. Companion docs-site pages, all already merged and checked against this code after the rebase:

Verification

  • Lint clean (golangci-lint run ./... v2.12.2, also with GOOS=linux; gofmt; go vet)
  • Tests pass (go test ./...)
  • Build succeeds (task build)
  • Documentation updated

How this was verified

  • Tests first for each point; each new test failed before its implementation (stubs showed the round trip, enabled flag, Root rule and status failures; the CLI round trip failed on the client-side secret check, then on the reboot misreport). Mutation checks: returning unredacted config from Current fails TestConfigStoreApply_ACMEOnThenOff; disabling the Root rule fails TestStart_RootWithProtocolRefused.
  • New tests: internal/config/protocols_test.go (round trip, enabled flag, redaction, absent/disabled/blank-secret/unknown-identifier/revoked-entry apply rules, Root refusal in Validate and Parse, ParseForApply, reboot classification), TestNeedsRebootIgnoresNilVersusEmpty, internal/node/protocols_test.go (apply round trip ACME on, read back, unchanged re-apply, off; absent block kept; secretless new credential refused; Root refused; status configured/running/pending across an apply and a reboot), internal/init/install_protocols_test.go (maintenance install and re-provision carry the blocks, empty secret refused before the disk is touched, Root refused), TestStart_RootWithProtocolRefused (ceremony path, nothing stored), cmd/cryptosctl/protocols_test.go (status line, config get then config apply against the test server keeps the secret with requires_reboot=false).
  • task license (golic dry run): 0 files to change.
  • Gitleaks read the hard-coded SHA-256 test digest in the first commit as a generic API key. The fixture now uses the existing testPasswordDigest() helper, and .gitleaksignore records that single finding (now commit 205dd64 after the rebase, internal/config/protocols_test.go:30) rather than rewriting the branch. gitleaks detect 8.30.1 locally: 1 finding without the entry, none with it.
  • On macOS the TPM-simulator tests need GOTOOLCHAIN=go1.26.6 with the 26.5 SDK, and internal/grpc's Unix-socket test needs a short TMPDIR; both are local-environment limits, not code.
  • Real-host check still owed: boot an issuing node, config apply ACME on, confirm cryptosctl status shows it pending, reboot, confirm the listener is up and the status line clears.

Merge order: the api contracts are merged, so this PR merges next, then #271 and #288 rebase onto it (#271 overlaps in go.mod, internal/config/config.go, internal/init/run.go and the status output; #288 replaces the SCEP stubs), then #291 restamps the headers. #296 carries copies of this PR's commits and rebases after it merges.


Before merging: add a closing comment summarizing what was actually done in this PR
(not just the checked boxes).

@Bugs5382 Bugs5382 added this to the v1.0.0 milestone Sep 30, 2026
@Bugs5382 Bugs5382 added the enhancement New feature (feat). Minor version bump. label Sep 30, 2026
@Bugs5382 Bugs5382 self-assigned this Sep 30, 2026
Bugs5382 added a commit to CryptOS-PKI/docs that referenced this pull request Sep 30, 2026
The enrolment page and the machine-config apply section now say the
blocks travel in MachineConfig: switching a protocol is a
reboot-required change best done in a maintenance window, the secrets
are write-only, and a Root refuses both blocks. Adds the two new
refusal messages to each error table.

Depends on CryptOS-PKI/cryptos-node#286.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Bugs5382 added a commit to CryptOS-PKI/docs that referenced this pull request Sep 30, 2026
The overview, internal TLS, devices and Kubernetes pages said neither
protocol could be switched on. They now say both run on an intermediate
or issuing node once their block is in the machine config, that a switch
takes effect at the next reboot and belongs in a maintenance window, and
that a Root refuses both.

Depends on CryptOS-PKI/cryptos-node#286.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Bugs5382 added a commit to CryptOS-PKI/docs that referenced this pull request Sep 30, 2026
config apply now carries the acme and est sections with write-only
secrets, so the page says how to switch each protocol on and off, that
the switch waits for a reboot in a maintenance window, and that a Root
refuses both. status gains the Protocols and Reboot lines, and the
ceremony page names the refusal a Root config with either section gets.

Depends on CryptOS-PKI/cryptos-node#286.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Bugs5382 added a commit to CryptOS-PKI/docs that referenced this pull request Sep 30, 2026
…rets

declarative-config said the acme and est blocks never crossed the API.
They do now, with their secrets returned blank and kept on apply, and a
switch waits for a reboot in a maintenance window. ca-roles adds that a
Root refuses both blocks.

Depends on CryptOS-PKI/cryptos-node#286.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
@Bugs5382
Bugs5382 marked this pull request as ready for review September 30, 2026 06:28
@Bugs5382 Bugs5382 linked an issue Sep 30, 2026 that may be closed by this pull request
13 tasks
Bugs5382 added a commit to CryptOS-PKI/docs that referenced this pull request Sep 30, 2026
config apply now carries the acme and est sections with write-only
secrets, so the page says how to switch each protocol on and off, that
the switch waits for a reboot in a maintenance window, and that a Root
refuses both. status gains the Protocols and Reboot lines, and the
ceremony page names the refusal a Root config with either section gets.

Depends on CryptOS-PKI/cryptos-node#286.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Bugs5382 added a commit to CryptOS-PKI/docs that referenced this pull request Sep 30, 2026
The overview, internal TLS, devices and Kubernetes pages said neither
protocol could be switched on. They now say both run on an intermediate
or issuing node once their block is in the machine config, that a switch
takes effect at the next reboot and belongs in a maintenance window, and
that a Root refuses both.

Depends on CryptOS-PKI/cryptos-node#286.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Bugs5382 added a commit to CryptOS-PKI/docs that referenced this pull request Sep 30, 2026
…rets

declarative-config said the acme and est blocks never crossed the API.
They do now, with their secrets returned blank and kept on apply, and a
switch waits for a reboot in a maintenance window. ca-roles adds that a
Root refuses both blocks.

Depends on CryptOS-PKI/cryptos-node#286.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Bugs5382 added a commit to CryptOS-PKI/docs that referenced this pull request Sep 30, 2026
The enrolment page and the machine-config apply section now say the
blocks travel in MachineConfig: switching a protocol is a
reboot-required change best done in a maintenance window, the secrets
are write-only, and a Root refuses both blocks. Adds the two new
refusal messages to each error table.

Depends on CryptOS-PKI/cryptos-node#286.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Map pki.acme and pki.est in FromProto and ToProto, following the
protocol-block rules in the api contract, and drop CarryForwardProtoGaps.

- enabled=false, or a block that was never set, is off. An absent block
  in an apply keeps the stored one, so an older client cannot switch a
  protocol off by leaving it out.
- Secrets are write-only: GetConfig returns hmac_key_base64 and
  password_sha256 blank, an apply with a blank secret keeps the stored
  value for the same key_id or username, and a blank secret for an
  unknown identifier is refused. config apply accepts the blanks
  client-side so config get output can be fed back.
- A Root refuses an enabled protocol in config validation, which covers
  config apply, the maintenance install, re-provision and
  ceremony start --config.
- The maintenance install and re-provision carry the blocks.
- The listeners still start only at boot from the stored config, and
  every protocol change answers requires_reboot.
- GetStatus fills NodeStatus.protocols (configured, running,
  reboot_pending) and config_reboot_pending, and cryptosctl status
  prints a Protocols line and flags a pending reboot.
- NeedsReboot compares the stored YAML form, so an unchanged re-apply
  over the wire (empty lists arrive as nil) no longer asks for a reboot.

The api module is pinned to a development merge of the CryptOS-PKI/api
PR heads this needs; it re-pins to api main once they merge.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Gitleaks reads the hard-coded SHA-256 test digest as a generic API key.
Use the package's testPasswordDigest helper, and record the one finding
in the earlier commit of this branch in .gitleaksignore, since that
commit stays in the history the scan reads.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
The ACME and EST contract and GetIssuedCertificate are both merged on api
main, so the branch no longer needs the combined development pin. Api main
also carries the SCEP challenge and approval RPCs and ListTsaCertificates,
which this node does not serve yet; they answer Unimplemented until their
servers land.

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
…prints it

Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com>
@Bugs5382

Copy link
Copy Markdown
Contributor Author

Merging for the v0.1.0 test run: CI green and mergeable at merge time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature (feat). Minor version bump.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(config): switch ACME and EST on and off through machine config

1 participant