Repository navigation
feat(config): switch ACME and EST on and off through machine config - #286
Merged
Merged
Conversation
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>
This was referenced Sep 30, 2026
Bugs5382
marked this pull request as ready for review
September 30, 2026 06:28
This was referenced Sep 30, 2026
13 tasks
4 tasks done
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>
Merged
4 tasks done
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
force-pushed
the
feat/284-protocol-switches
branch
from
September 30, 2026 14:31
901552f to
ce0e20d
Compare
4 tasks
4 tasks done
Contributor
Author
|
Merging for the v0.1.0 test run: CI green and mergeable at merge time. |
This was referenced Sep 30, 2026
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.
What and why
ACME and EST could only be switched on by a hand-staged YAML file:
MachineConfighad no field for them, soconfig apply, the maintenance install and the Fleet Manager could not switch either protocol on, change it or switch it off, andGetStatuscould 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.FromProto/ToProtocarry both blocks field for field, andCarryForwardProtoGapsis gone.ToProtoalways sends an explicit block,enabled: falsefor an off protocol.FromPrototurnsenabled: false(or no block) into nil, the node's existing "off".config.FromProtoOver, used byApplyConfig, the maintenance install and re-provision): an absent block keeps the stored one;enabled: falseswitches off;enabled: trueswitches on and is validated.GetConfigreturnshmac_key_base64andpassword_sha256blank. An apply with a blank secret keeps the stored value for the samekey_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 applyaccepts the blanks client-side (config.ParseForApply), soconfig getoutput can be fed straight back.Validaterefusespki.acmeorpki.eston a Root. That one rule coversconfig apply, the maintenance install, re-provision andceremony 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.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 answersrequires_reboot: true.NodeStatus.protocolsreports each protocol's configured state (stored config), running state (this boot) andreboot_pending(stored block differs from the boot one);config_reboot_pendingisNeedsReboot(boot config, stored config).cryptosctl statusprintsProtocols: ACME on (not running, reboot pending), EST offand aReboot: pendingline.requires_reboot=true(same digest), because gRPC turns an empty list into nil andNeedsRebootusedreflect.DeepEqualagainst the stored YAML's empty slices.NeedsRebootnow compares the stored YAML form (config.Equivalent).Design calls:
enabled: falsestores 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.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/apiis pinned to apimainat 81494db, which carries CryptOS-PKI/api#115 (this contract), CryptOS-PKI/api#123 (the manager'sSetNodeProtocol) and CryptOS-PKI/api#104. Apimainalso has the SCEP challenge and approval RPCs (CryptOS-PKI/api#117) andListTsaCertificates(CryptOS-PKI/api#120), which this node does not serve yet, sointernal/grpc/unserved.goanswers themUnimplemented(tested inunserved_test.go). #288 replaces the SCEP stubs with its handlers when it rebases. Thewip/284-api-integrationapi branch is no longer used.Tests removed, because the function they covered is gone:
TestCarryForwardProtoGaps,TestCarryForwardProtoGaps_DoesNotOverwriteandTestCarryForwardProtoGaps_NilSafe(the absent-block keep they guarded is nowTestFromProtoOverAbsentBlockKeepsStoredandTestConfigStoreApply_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 ininternal/config,TestConfigStoreApply_PreservesProtocolsand theTestConfigStoreApply_RejectsInvalidConfigseed.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.AllowAnonymousAccountsargument is kept, with this PR's "ACME: off in the boot config" branch. Every commit is signed off. New files carry the headertask licensechecks on main today; #291 restamps them with the rest of the tree.Checked against the Fleet Manager on main:
SetNodeProtocolreadsGetConfig, flips one block'senabledflag, 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 inNodeStatus.protocols, and refuses a Root, which is what the manager's reboot tracking and error passthrough expect. Becauseenabled: falsestores 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.mdanddocs/est.mdgain 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. Thedocs/est.mdstatus example now lists ACME before EST, ascryptosctl statusprints it. Companion docs-site pages, all already merged and checked against this code after the rebase:docs/20-reference-pages, 3a1ae8d): machine-config-enrollment and machine-config: switching, reboot and maintenance-window callout, write-only secrets, Root refusal, the new error messages.docs/25-use-cases-pages, ec537ec): overview, internal-tls, devices-iot, kubernetes no longer say ACME and EST can't be switched on.docs/21-using-pages, 5de6b06): config-apply, status (Protocols and Reboot lines), ceremony-start (Root refusal).docs/27-concepts-pages, 7b714dc): declarative-config and ca-roles.Verification
golangci-lint run ./...v2.12.2, also withGOOS=linux;gofmt;go vet)go test ./...)task build)How this was verified
CurrentfailsTestConfigStoreApply_ACMEOnThenOff; disabling the Root rule failsTestStart_RootWithProtocolRefused.internal/config/protocols_test.go(round trip, enabled flag, redaction, absent/disabled/blank-secret/unknown-identifier/revoked-entry apply rules, Root refusal inValidateandParse,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 getthenconfig applyagainst the test server keeps the secret withrequires_reboot=false).task license(golic dry run): 0 files to change.testPasswordDigest()helper, and.gitleaksignorerecords that single finding (now commit 205dd64 after the rebase,internal/config/protocols_test.go:30) rather than rewriting the branch.gitleaks detect8.30.1 locally: 1 finding without the entry, none with it.GOTOOLCHAIN=go1.26.6with the 26.5 SDK, andinternal/grpc's Unix-socket test needs a shortTMPDIR; both are local-environment limits, not code.config applyACME on, confirmcryptosctl statusshows 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.goand 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).