From 205dd6475254419977b831d4b9feb35bc2b37caa Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 02:21:34 -0400 Subject: [PATCH 1/5] feat(config): switch ACME and EST on and off through machine config 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> --- cmd/cryptosctl/config.go | 14 +- cmd/cryptosctl/protocols_test.go | 144 +++++++++ cmd/cryptosctl/status.go | 35 +++ docs/acme.md | 38 ++- docs/est.md | 38 ++- go.mod | 2 +- go.sum | 4 +- internal/ceremony/ceremony.go | 5 +- internal/ceremony/ceremony_test.go | 59 +++- internal/config/acme_test.go | 10 +- internal/config/config.go | 105 ++++--- internal/config/config_test.go | 52 ---- internal/config/est_test.go | 12 +- internal/config/protocols.go | 244 +++++++++++++++ internal/config/protocols_test.go | 379 ++++++++++++++++++++++++ internal/config/reboot.go | 26 +- internal/config/reboot_test.go | 17 ++ internal/init/install_protocols_test.go | 145 +++++++++ internal/init/maintenance_installer.go | 2 +- internal/init/reprovision.go | 2 +- internal/init/run.go | 25 +- internal/node/apply_validate_test.go | 17 +- internal/node/node_test.go | 26 +- internal/node/protocols_test.go | 360 ++++++++++++++++++++++ internal/node/providers.go | 119 ++++++-- 25 files changed, 1710 insertions(+), 170 deletions(-) create mode 100644 cmd/cryptosctl/protocols_test.go create mode 100644 internal/config/protocols.go create mode 100644 internal/config/protocols_test.go create mode 100644 internal/init/install_protocols_test.go create mode 100644 internal/node/protocols_test.go diff --git a/cmd/cryptosctl/config.go b/cmd/cryptosctl/config.go index 9f8904d..21819e8 100644 --- a/cmd/cryptosctl/config.go +++ b/cmd/cryptosctl/config.go @@ -58,10 +58,12 @@ func newConfigGetCmd(opts *globalOpts) *cobra.Command { Short: "Print the node's current machine configuration", Long: "Print the node's current machine configuration as YAML, ready to edit and " + "feed back to `config apply -f`.\n\n" + - "The acme and est blocks are not returned: they carry secrets and are " + - "deliberately absent from the wire format. That does not break a " + - "read-edit-apply cycle, because the node carries those blocks forward from " + - "its existing config when a new one is applied.", + "The acme and est blocks are returned with their secrets blank: " + + "hmac_key_base64 and password_sha256 are write-only. Leave a blank secret " + + "as it is and `config apply` keeps the one the node stores for that key_id " + + "or username; set it to replace it. A new key_id or username needs its " + + "secret. Switching a protocol on or off, or changing its settings, takes " + + "effect at the next reboot.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { client, closeConn, err := dial(opts) @@ -117,7 +119,9 @@ func newConfigApplyCmd(opts *globalOpts) *cobra.Command { } // Validate client-side so the operator gets a clear error // before the round-trip; the node re-validates authoritatively. - cfg, err := config.Parse(raw) + // A blank protocol secret passes here because the node fills it + // from the one it stores. + cfg, err := config.ParseForApply(raw) if err != nil { return err } diff --git a/cmd/cryptosctl/protocols_test.go b/cmd/cryptosctl/protocols_test.go new file mode 100644 index 0000000..482400c --- /dev/null +++ b/cmd/cryptosctl/protocols_test.go @@ -0,0 +1,144 @@ +package main + +/* +Apache License 2.0 + +Copyright 2026 Shane + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "encoding/base64" + "os" + "path/filepath" + "strings" + "testing" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" +) + +func TestHumanStatusShowsProtocols(t *testing.T) { + s := &cryptosv1.NodeStatus{ + Role: cryptosv1.NodeRole_NODE_ROLE_ISSUING, + Protocols: []*cryptosv1.ProtocolStatus{ + {Protocol: cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME, Configured: true, Running: true}, + {Protocol: cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_EST, Configured: true, Running: false, RebootPending: true}, + }, + ConfigRebootPending: true, + } + out := humanStatus(s) + for _, want := range []string{ + "Protocols:", "ACME on", "EST on (not running, reboot pending)", + "Reboot:", "pending", + } { + if !strings.Contains(out, want) { + t.Errorf("humanStatus missing %q in:\n%s", want, out) + } + } + + off := humanStatus(&cryptosv1.NodeStatus{ + Protocols: []*cryptosv1.ProtocolStatus{ + {Protocol: cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME}, + {Protocol: cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_EST, Running: true, RebootPending: true}, + }, + }) + for _, want := range []string{"ACME off", "EST off (still running, reboot pending)"} { + if !strings.Contains(off, want) { + t.Errorf("humanStatus missing %q in:\n%s", want, off) + } + } + if strings.Contains(off, "Reboot:") { + t.Errorf("humanStatus flagged a reboot the node did not report:\n%s", off) + } + + // A node that does not report protocols (maintenance mode, an older node) + // prints no Protocols line rather than an empty one. + if bare := humanStatus(&cryptosv1.NodeStatus{}); strings.Contains(bare, "Protocols:") { + t.Errorf("humanStatus printed unreported protocols:\n%s", bare) + } +} + +const protocolsConfig = `apiVersion: cryptos.dev/v1alpha1 +kind: MachineConfig +metadata: + name: pki-issuing-test +role: + kind: issuing +network: + interface: eth0 + address: 192.0.2.30/24 + gateway: 192.0.2.1 +pki: + root_key_alg: ECDSA-P384 + root_subject: + common_name: Protocols Issuing CA + parent: + ca_cert_sha256: abababababababababababababababababababababababababababababababab + profiles: + - name: leaf-server + key_alg: ECDSA-P384 + validity_days: 90 + key_usage: [digital_signature] + ext_key_usage: [server_auth] + acme: + base_url: https://ca.example.org/acme + profile: leaf-server + external_account_keys: + - key_id: ops + hmac_key_base64: EAB_KEY +bootstrap: + admin_cert_sha256: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +` + +// config get prints the EAB secret blank, and feeding that output back to +// config apply keeps the secret the node holds. +func TestConfigGetApply_KeepsWriteOnlySecret(t *testing.T) { + ts := startTestServer(t) + dir := t.TempDir() + secret := base64.RawURLEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")) + path := filepath.Join(dir, "machine.yaml") + if err := os.WriteFile(path, []byte(strings.Replace(protocolsConfig, "EAB_KEY", secret, 1)), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + out, err := ts.run(t, "config", "apply", "-f", path) + if err != nil { + t.Fatalf("config apply: %v (out=%s)", err, out) + } + if !strings.Contains(out, "requires_reboot=true") { + t.Errorf("switching ACME on did not report requires_reboot:\n%s", out) + } + + got, err := ts.run(t, "config", "get") + if err != nil { + t.Fatalf("config get: %v (out=%s)", err, got) + } + if strings.Contains(got, secret) { + t.Fatalf("config get printed the EAB secret:\n%s", got) + } + if !strings.Contains(got, "key_id: ops") { + t.Errorf("config get dropped the key_id:\n%s", got) + } + + back := filepath.Join(dir, "back.yaml") + if err := os.WriteFile(back, []byte(got), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + out, err = ts.run(t, "config", "apply", "-f", back) + if err != nil { + t.Fatalf("config apply of the config get output: %v (out=%s)", err, out) + } + if !strings.Contains(out, "requires_reboot=false") { + t.Errorf("an unchanged re-apply reported a reboot:\n%s", out) + } +} diff --git a/cmd/cryptosctl/status.go b/cmd/cryptosctl/status.go index 033d773..c010eed 100644 --- a/cmd/cryptosctl/status.go +++ b/cmd/cryptosctl/status.go @@ -72,9 +72,44 @@ func humanStatus(s *cryptosv1.NodeStatus) string { if r := s.GetResolver(); r != nil { fmt.Fprintf(&b, "DNS: %s\n", humanResolver(r)) } + if ps := s.GetProtocols(); len(ps) > 0 { + fmt.Fprintf(&b, "Protocols: %s\n", humanProtocols(ps)) + } + if s.GetConfigRebootPending() { + fmt.Fprintf(&b, "Reboot: pending (the stored config changes take effect at the next boot)\n") + } return b.String() } +// humanProtocols renders the enrolment protocols as one line: each one on or +// off as stored, with a note when this boot does not match it yet. +func humanProtocols(ps []*cryptosv1.ProtocolStatus) string { + parts := make([]string, 0, len(ps)) + for _, p := range ps { + part := trimEnum(p.GetProtocol().String(), "SERVICE_PROTOCOL_") + if p.GetConfigured() { + part += " on" + } else { + part += " off" + } + var notes []string + switch { + case p.GetConfigured() && !p.GetRunning(): + notes = append(notes, "not running") + case !p.GetConfigured() && p.GetRunning(): + notes = append(notes, "still running") + } + if p.GetRebootPending() { + notes = append(notes, "reboot pending") + } + if len(notes) > 0 { + part += " (" + strings.Join(notes, ", ") + ")" + } + parts = append(parts, part) + } + return strings.Join(parts, ", ") +} + // humanPreflight renders the revocation preflight as one line: the state, the // base URL checked, when, and the last error while failing. func humanPreflight(p *cryptosv1.RevocationPreflight) string { diff --git a/docs/acme.md b/docs/acme.md index 33d30b5..e79a897 100644 --- a/docs/acme.md +++ b/docs/acme.md @@ -137,6 +137,42 @@ Accepted reason codes are `unspecified` (0), `keyCompromise` (1), `privilegeWithdrawn` (9). `certificateHold` is refused: this CA has no hold workflow, and accepting the request would imply one. +## Switching it on, changing it and switching it off + +ACME runs on an intermediate or issuing node only. A Root serves no enrolment +protocol, so a config that sets `pki.acme` on a Root is refused by +`config apply`, by the maintenance install and by `ceremony start --config`. + +> [!CAUTION] +> Don't add `pki.acme` to a Root's config. It is refused, and nothing is +> stored. Serve ACME from an issuing node under that Root instead. + +To switch ACME on, add the `pki.acme` block and run `config apply`. To +switch it off, remove the block and apply again. Over the API the block is +`Pki.acme` with an explicit `enabled` flag: `enabled: false` switches it +off, and leaving the block out of an `ApplyConfig` keeps what the node has. + +> [!WARNING] +> Every ACME change, switching it on or off included, takes effect at the +> next reboot, not when you apply it. `config apply` stores the change and +> prints `requires_reboot=true`; the listener starts, stops or picks up the new +> settings only when the node boots again. Plan the reboot for a maintenance +> window, because the node stops issuing while it restarts, then use +> `cryptosctl reboot`. + +`cryptosctl status` shows what is stored against what is running until then: + +```text +Protocols: ACME on (not running, reboot pending), EST off +Reboot: pending (the stored config changes take effect at the next boot) +``` + +`hmac_key_base64` is write-only. `cryptosctl config get` prints it blank, with its +`key_id`. Leave a blank value as it is and `config apply` keeps the one the +node stores for that `key_id`; set a value to replace it. A new `key_id` +needs its value, and an apply that leaves it blank is refused. Removing an +entry revokes it. + ## Limits worth knowing before you deploy - **Key change is not implemented.** RFC 8555 section 7.3.5 is absent from the @@ -148,5 +184,3 @@ workflow, and accepting the request would imply one. order that justified it. - **Validation runs inline on the challenge POST**, bounded by a ten-second timeout. The client's first poll already carries the answer. -- **The `pki.acme` block is not yet carried in the proto machine config**, so - it survives a staged YAML boot but not an `ApplyConfig` from a manager. diff --git a/docs/est.md b/docs/est.md index e9a88c0..ff9079b 100644 --- a/docs/est.md +++ b/docs/est.md @@ -199,6 +199,42 @@ operation. Set `label: issuing` and every path moves under `/.well-known/est/issuing/`. The unlabelled paths then return 404, so a label is routing rather than decoration. +## Switching it on, changing it and switching it off + +EST runs on an intermediate or issuing node only. A Root serves no enrolment +protocol, so a config that sets `pki.est` on a Root is refused by +`config apply`, by the maintenance install and by `ceremony start --config`. + +> [!CAUTION] +> Don't add `pki.est` to a Root's config. It is refused, and nothing is +> stored. Serve EST from an issuing node under that Root instead. + +To switch EST on, add the `pki.est` block and run `config apply`. To +switch it off, remove the block and apply again. Over the API the block is +`Pki.est` with an explicit `enabled` flag: `enabled: false` switches it +off, and leaving the block out of an `ApplyConfig` keeps what the node has. + +> [!WARNING] +> Every EST change, switching it on or off included, takes effect at the +> next reboot, not when you apply it. `config apply` stores the change and +> prints `requires_reboot=true`; the listener starts, stops or picks up the new +> settings only when the node boots again. Plan the reboot for a maintenance +> window, because the node stops issuing while it restarts, then use +> `cryptosctl reboot`. + +`cryptosctl status` shows what is stored against what is running until then: + +```text +Protocols: EST on (not running, reboot pending), ACME off +Reboot: pending (the stored config changes take effect at the next boot) +``` + +`password_sha256` is write-only. `cryptosctl config get` prints it blank, with its +`username`. Leave a blank value as it is and `config apply` keeps the one the +node stores for that `username`; set a value to replace it. A new `username` +needs its value, and an apply that leaves it blank is refused. Removing an +entry revokes it. + ## Limits worth knowing before you deploy - **Wildcards are refused.** Nothing in EST establishes control of a label @@ -211,5 +247,3 @@ is routing rather than decoration. say answers 204. - **Issuance is synchronous.** There is no 202 Retry-After manual-approval flow; a request either yields a certificate or fails. -- **The `pki.est` block is not yet carried in the proto machine config**, so - it survives a staged YAML boot but not an `ApplyConfig` from a manager. diff --git a/go.mod b/go.mod index 42806d6..71a6fff 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/CryptOS-PKI/cryptos go 1.25.0 require ( - github.com/CryptOS-PKI/api v0.0.0-20260929181434-442c13e13c15 + github.com/CryptOS-PKI/api v0.0.0-20260930060427-d48438336d53 github.com/google/go-tpm v0.9.8 github.com/google/go-tpm-tools v0.4.8 github.com/spf13/cobra v1.10.2 diff --git a/go.sum b/go.sum index a9fe1de..5e36c18 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/CryptOS-PKI/api v0.0.0-20260929181434-442c13e13c15 h1:ZSw1gdY7yy2872QDzCnNfchRPA3+d5Bgm4bYVvNZ55A= -github.com/CryptOS-PKI/api v0.0.0-20260929181434-442c13e13c15/go.mod h1:tmeaYhNoUblqPYYNtTbGBGIUsZ7xLATgLdKq4S33rmE= +github.com/CryptOS-PKI/api v0.0.0-20260930060427-d48438336d53 h1:VEdz0R94JOWFlJPKMEYnos5Wks2kY3+hmfJa24XYDPM= +github.com/CryptOS-PKI/api v0.0.0-20260930060427-d48438336d53/go.mod h1:tmeaYhNoUblqPYYNtTbGBGIUsZ7xLATgLdKq4S33rmE= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= diff --git a/internal/ceremony/ceremony.go b/internal/ceremony/ceremony.go index f068c0b..736d251 100644 --- a/internal/ceremony/ceremony.go +++ b/internal/ceremony/ceremony.go @@ -180,7 +180,10 @@ func (e *Engine) Start(ctx context.Context, req *cryptosv1.StartCeremonyRequest, return status.Error(codes.FailedPrecondition, "IDENTITY_EXISTS") } - // Parse + validate the operator's machine config from the request. + // Parse + validate the operator's machine config from the request. The + // raw YAML is stored below and the Root boots from it, so this is the + // check that keeps an enrolment protocol off a Root: config.Parse refuses + // a Root with pki.acme or pki.est set. if len(req.MachineConfigYaml) == 0 { return status.Error(codes.InvalidArgument, "ceremony: machine_config_yaml is required") } diff --git a/internal/ceremony/ceremony_test.go b/internal/ceremony/ceremony_test.go index 57ac322..a097967 100644 --- a/internal/ceremony/ceremony_test.go +++ b/internal/ceremony/ceremony_test.go @@ -32,6 +32,7 @@ import ( "encoding/pem" "fmt" "math/big" + "strings" "testing" "time" @@ -75,6 +76,7 @@ type harness struct { adminCert *x509.Certificate adminPEM string adminFP [32]byte + cfgStore *config.FileStore } func newHarness(t *testing.T) (*harness, context.Context) { @@ -111,7 +113,8 @@ func newHarness(t *testing.T) (*harness, context.Context) { if _, err := rand.Read(seed); err != nil { t.Fatalf("seed: %v", err) } - eng, err := New(Config{RootKey: tpmTestBackend{tp}, Store: store, ConfigStore: config.NewFileStore(t.TempDir()), Trust: trust, Seed: seed}) + cfgStore := config.NewFileStore(t.TempDir()) + eng, err := New(Config{RootKey: tpmTestBackend{tp}, Store: store, ConfigStore: cfgStore, Trust: trust, Seed: seed}) if err != nil { t.Fatalf("New: %v", err) } @@ -125,6 +128,7 @@ func newHarness(t *testing.T) (*harness, context.Context) { adminCert: adminCert, adminPEM: adminPEM, adminFP: sha256.Sum256(adminCert.Raw), + cfgStore: cfgStore, }, ctx } @@ -381,6 +385,59 @@ func TestStart_NonRootRole_Refused(t *testing.T) { } } +// `ceremony start --config` hands the node raw YAML, which it stores and +// boots from. A Root serves no enrolment protocol, so a config that switches +// one on must be refused before anything is stored: otherwise the Root would +// serve it from its next boot. +func TestStart_RootWithProtocolRefused(t *testing.T) { + for _, tc := range []struct { + name string + block string + }{ + {"acme", ` + acme: + base_url: https://ca.example.org/acme + profile: leaf-server + allow_anonymous_accounts: true`}, + {"est", ` + est: + hostnames: [est.example.org] + profile: leaf-server + allow_any_identifier: true`}, + } { + t.Run(tc.name, func(t *testing.T) { + h, ctx := newHarness(t) + yaml := string(machineYAML(h.adminFP)) + ` + profiles: + - name: leaf-server + key_alg: ECDSA-P384 + validity_days: 90 + key_usage: [digital_signature] + ext_key_usage: [server_auth]` + tc.block + "\n" + c := &collector{} + err := h.engine.Start(ctx, &cryptosv1.StartCeremonyRequest{ + Kind: cryptosv1.CeremonyKind_CEREMONY_KIND_FIRST_BOOT_ROOT, + MachineConfigYaml: []byte(yaml), + }, c.send) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("code = %v, want InvalidArgument (err=%v)", status.Code(err), err) + } + if !strings.Contains(err.Error(), "root") { + t.Errorf("error %q does not say the root role is the reason", err) + } + if len(c.kinds) != 0 { + t.Errorf("events emitted on a refused ceremony: %v", c.kinds) + } + if _, _, ok, _ := h.cfgStore.Read(); ok { + t.Error("the refused config was stored, so the Root would serve it at its next boot") + } + if ok, _ := h.store.HasIdentity(ctx); ok { + t.Error("identity established from a refused config") + } + }) + } +} + func TestVerifyManifest_Tamper(t *testing.T) { h, ctx := newHarness(t) if err := h.engine.Start(ctx, &cryptosv1.StartCeremonyRequest{ diff --git a/internal/config/acme_test.go b/internal/config/acme_test.go index 7ff7889..cf53b7c 100644 --- a/internal/config/acme_test.go +++ b/internal/config/acme_test.go @@ -48,7 +48,7 @@ func validACME() *ACME { } func TestValidateACMEAccepts(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -61,7 +61,7 @@ func TestValidateACMEAccepts(t *testing.T) { // A nil block is the default and must not require anything. func TestValidateACMEAbsentIsFine(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -119,7 +119,7 @@ func TestValidateACMERejections(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -141,7 +141,7 @@ func TestValidateACMERejections(t *testing.T) { // Setting the explicit opt-out is the only way to run without binding keys. func TestValidateACMEAnonymousOptOut(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -158,7 +158,7 @@ func TestValidateACMEAnonymousOptOut(t *testing.T) { // The block must survive a YAML round-trip, since that is how an operator // actually supplies it. func TestParseACMEFromYAML(t *testing.T) { - yaml := string(validYAML(t)) + ` profiles: + yaml := string(issuingYAML(t)) + ` profiles: - name: leaf-server key_alg: ECDSA-P384 validity_days: 90 diff --git a/internal/config/config.go b/internal/config/config.go index b7f0986..dd07e76 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -266,15 +266,16 @@ type PKI struct { // omitted) disables ACME entirely and is the default: an enrolment // protocol is opened deliberately, never by forgetting to close it. // - // Unlike the revocation fields, this is NOT yet carried in the proto - // MachineConfig, so it survives a staged YAML boot but not an ApplyConfig - // from a manager. Carrying it needs a CryptOS-PKI/api change. + // It is carried in the proto MachineConfig as Pki.acme, under the + // protocol-block rules in protocols.go: a Root refuses it, its secrets + // are write-only over the API, and any change takes effect at the next + // boot. ACME *ACME `yaml:"acme"` // EST configures the RFC 7030 enrolment endpoint. Nil (the field // omitted) disables EST entirely and is the default, for the same reason // ACME is off by default: an enrolment protocol is opened deliberately. // - // Like ACME, this is NOT yet carried in the proto MachineConfig. + // Like ACME, it is carried as Pki.est under the same rules. EST *EST `yaml:"est"` } @@ -463,6 +464,33 @@ type Subject struct { // Parse parses a machine-config YAML document and runs every Phase 1 // validation rule. The returned Config is safe to apply. func Parse(raw []byte) (*Config, error) { + c, err := decode(raw) + if err != nil { + return nil, err + } + if err := c.Validate(); err != nil { + return nil, err + } + return c, nil +} + +// ParseForApply parses a config a client is about to apply. It is Parse with +// one allowance: an empty protocol secret (an ACME hmac_key_base64 or an EST +// password_sha256) passes, because it asks the node to keep the secret it +// stores under the same identifier, which is what `config get` prints. The +// node resolves those and validates strictly before storing anything. +func ParseForApply(raw []byte) (*Config, error) { + c, err := decode(raw) + if err != nil { + return nil, err + } + if err := c.validate(true); err != nil { + return nil, err + } + return c, nil +} + +func decode(raw []byte) (*Config, error) { if len(raw) == 0 { return nil, errors.New("config: empty input") } @@ -472,9 +500,6 @@ func Parse(raw []byte) (*Config, error) { if err := dec.Decode(&c); err != nil { return nil, fmt.Errorf("config: parse YAML: %w", err) } - if err := c.Validate(); err != nil { - return nil, err - } return &c, nil } @@ -482,6 +507,12 @@ func Parse(raw []byte) (*Config, error) { // the field path so callers can surface it via INVALID_ARGUMENT details // on the gRPC layer. func (c *Config) Validate() error { + return c.validate(false) +} + +// validate is Validate, optionally letting protocol secrets be empty (see +// ParseForApply). +func (c *Config) validate(keptSecrets bool) error { if c == nil { return errors.New("config: nil config") } @@ -538,10 +569,13 @@ func (c *Config) Validate() error { if err := validateRevocationBaseURL(c.PKI.RevocationBaseURL); err != nil { return err } - if err := validateACME(c.PKI.ACME, c.PKI.Profiles); err != nil { + if err := validateProtocolRole(c.Role.Kind, c.PKI); err != nil { return err } - if err := validateEST(c.PKI.EST, c.PKI.Profiles); err != nil { + if err := validateACME(c.PKI.ACME, c.PKI.Profiles, keptSecrets); err != nil { + return err + } + if err := validateEST(c.PKI.EST, c.PKI.Profiles, keptSecrets); err != nil { return err } if err := validateParent(c.Role.Kind, c.PKI.Parent); err != nil { @@ -621,7 +655,9 @@ func validateRevocationBaseURL(raw string) error { // // As elsewhere in this file, no DNS resolution happens here: the box may // validate its config before the network is up. -func validateACME(a *ACME, profiles []CertificateProfile) error { +// +// keptSecrets lets an empty key through (see ParseForApply). +func validateACME(a *ACME, profiles []CertificateProfile, keptSecrets bool) error { if a == nil { return nil } @@ -659,6 +695,9 @@ func validateACME(a *ACME, profiles []CertificateProfile) error { return fmt.Errorf("config: pki.acme.external_account_keys[%d].key_id: %q is duplicated", i, k.KeyID) } seen[k.KeyID] = true + if keptSecrets && k.HMACKeyBase64 == "" { + continue + } raw, derr := base64.RawURLEncoding.DecodeString(k.HMACKeyBase64) if derr != nil { return fmt.Errorf("config: pki.acme.external_account_keys[%d].hmac_key_base64: "+ @@ -680,7 +719,9 @@ func validateACME(a *ACME, profiles []CertificateProfile) error { // required whenever credentials are configured. An operator who genuinely // wants an unrestricted endpoint has to say allow_any_identifier, which is a // line a reviewer can find. -func validateEST(e *EST, profiles []CertificateProfile) error { +// +// keptSecrets lets an empty password digest through (see ParseForApply). +func validateEST(e *EST, profiles []CertificateProfile, keptSecrets bool) error { if e == nil { return nil } @@ -722,6 +763,9 @@ func validateEST(e *EST, profiles []CertificateProfile) error { return fmt.Errorf("config: pki.est.enroll_credentials[%d].username: %q is duplicated", i, cred.Username) } seen[cred.Username] = true + if keptSecrets && cred.PasswordSHA256 == "" { + continue + } raw, err := hex.DecodeString(cred.PasswordSHA256) if err != nil || len(raw) != sha256.Size { return fmt.Errorf("config: pki.est.enroll_credentials[%d].password_sha256: "+ @@ -1067,6 +1111,8 @@ func FromProto(pb *cryptosv1.MachineConfig) (*Config, error) { CACertSHA256: pb.Pki.Parent.CaCertSha256, } } + c.PKI.ACME = acmeFromProto(pb.Pki.Acme) + c.PKI.EST = estFromProto(pb.Pki.Est) } if pb.Install != nil { c.Install.Disk = pb.Install.Disk @@ -1091,39 +1137,8 @@ func FromProto(pb *cryptosv1.MachineConfig) (*Config, error) { } // ToProto adapts the validated Config to the api/ proto MachineConfig -// for the gRPC layer. Only the Phase 1 subset is populated. -// CarryForwardProtoGaps copies into c the configuration that MachineConfig -// cannot express, taking it from prev -- the config currently on disk. -// -// FromProto can only populate what the proto carries, and the proto is a -// partial view: Pki has no acme or est field. Writing a config built solely -// from a proto therefore deleted both blocks, silently disabling the protocols -// the node was serving (#205). Any apply driven over the wire -- which is how -// the Fleet Manager applies a profile change -- did this. -// -// The blocks are not simply added to the proto because they carry secrets: -// ACME.ExternalAccountKeys are EAB HMAC keys and EST.EnrollCredentials are HTTP -// Basic credentials, and neither belongs in a response any admin caller can -// read. Preserving them here keeps them node-only. -// -// This list must grow whenever the config gains a field the proto does not -// carry. A field missing from both FromProto and here is a field an apply -// deletes. -func (c *Config) CarryForwardProtoGaps(prev *Config) { - if c == nil || prev == nil { - return - } - - // Only carry forward where the incoming config says nothing. A caller that - // did express one of these meant it. - if c.PKI.ACME == nil { - c.PKI.ACME = prev.PKI.ACME - } - if c.PKI.EST == nil { - c.PKI.EST = prev.PKI.EST - } -} - +// for the gRPC layer. Only the Phase 1 subset is populated. The protocol +// secrets are included; use ToProtoRedacted for a reader. func (c *Config) ToProto() *cryptosv1.MachineConfig { pki := &cryptosv1.Pki{ RootKeyAlg: string(c.PKI.RootKeyAlg), @@ -1142,6 +1157,8 @@ func (c *Config) ToProto() *cryptosv1.MachineConfig { CrlNextUpdateHours: c.PKI.CRLNextUpdateHours, RevocationHttpPort: c.PKI.RevocationHTTPPort, RootLeafIssuance: c.PKI.RootLeafIssuance, + Acme: acmeToProto(c.PKI.ACME), + Est: estToProto(c.PKI.EST), } if c.PKI.Parent != nil { pki.Parent = &cryptosv1.Parent{ diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9f3a289..9966baa 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -955,55 +955,3 @@ func TestRootKeyAlgKeyAlgorithm(t *testing.T) { }) } } - -// TestCarryForwardProtoGaps is the guard on #205: MachineConfig has no acme or -// est field, so a config built from a proto has neither, and writing that as -// the node's whole config disabled both protocols. -func TestCarryForwardProtoGaps(t *testing.T) { - prev := &Config{} - prev.PKI.ACME = &ACME{BaseURL: "https://ca.example/acme", Profile: "leaf-server"} - prev.PKI.EST = &EST{Hostnames: []string{"est.example"}, Profile: "leaf-server"} - - // What FromProto can produce: the protocol blocks are absent, because the - // proto cannot carry them. - fromProto := &Config{} - fromProto.CarryForwardProtoGaps(prev) - - if fromProto.PKI.ACME == nil { - t.Fatal("ACME was not carried forward, so an apply would disable it") - } - if fromProto.PKI.ACME.BaseURL != "https://ca.example/acme" { - t.Errorf("ACME.BaseURL = %q, want the previous value", fromProto.PKI.ACME.BaseURL) - } - if fromProto.PKI.EST == nil { - t.Fatal("EST was not carried forward, so an apply would disable it") - } - if fromProto.PKI.EST.Profile != "leaf-server" { - t.Errorf("EST.Profile = %q, want the previous value", fromProto.PKI.EST.Profile) - } -} - -// A caller that did express a block means it, so it is not overwritten. -func TestCarryForwardProtoGaps_DoesNotOverwrite(t *testing.T) { - prev := &Config{} - prev.PKI.EST = &EST{Profile: "old"} - - next := &Config{} - next.PKI.EST = &EST{Profile: "new"} - next.CarryForwardProtoGaps(prev) - - if next.PKI.EST.Profile != "new" { - t.Errorf("EST.Profile = %q, want the incoming value to win", next.PKI.EST.Profile) - } -} - -func TestCarryForwardProtoGaps_NilSafe(t *testing.T) { - var c *Config - c.CarryForwardProtoGaps(&Config{}) // must not panic - - next := &Config{} - next.CarryForwardProtoGaps(nil) - if next.PKI.ACME != nil { - t.Error("carrying forward from nil invented an ACME block") - } -} diff --git a/internal/config/est_test.go b/internal/config/est_test.go index efd3d63..8119235 100644 --- a/internal/config/est_test.go +++ b/internal/config/est_test.go @@ -44,7 +44,7 @@ func validEST() *EST { } func TestValidateESTAccepts(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -56,7 +56,7 @@ func TestValidateESTAccepts(t *testing.T) { } func TestValidateESTAbsentIsFine(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -114,7 +114,7 @@ func TestValidateESTRejections(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -136,7 +136,7 @@ func TestValidateESTRejections(t *testing.T) { // The explicit override is the only way to run simpleenroll unrestricted. func TestValidateESTAllowAnyIdentifier(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -153,7 +153,7 @@ func TestValidateESTAllowAnyIdentifier(t *testing.T) { // A reenroll-only deployment, with no simpleenroll credentials at all, is a // valid configuration and needs no allowlist. func TestValidateESTReenrollOnly(t *testing.T) { - cfg, err := Parse(validYAML(t)) + cfg, err := Parse(issuingYAML(t)) if err != nil { t.Fatalf("Parse: %v", err) } @@ -168,7 +168,7 @@ func TestValidateESTReenrollOnly(t *testing.T) { } func TestParseESTFromYAML(t *testing.T) { - yaml := string(validYAML(t)) + ` profiles: + yaml := string(issuingYAML(t)) + ` profiles: - name: leaf-server key_alg: ECDSA-P384 validity_days: 90 diff --git a/internal/config/protocols.go b/internal/config/protocols.go new file mode 100644 index 0000000..db9c8ea --- /dev/null +++ b/internal/config/protocols.go @@ -0,0 +1,244 @@ +package config + +/* +Apache License 2.0 + +Copyright 2026 Shane + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "errors" + "fmt" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" +) + +// The enrolment protocol blocks (pki.acme, pki.est) travel in MachineConfig +// under one set of rules, shared by every protocol that gains a block: +// +// - In the proto, a block carries an explicit enabled flag. ToProto always +// sends one, so a whole config is explicit about every protocol, and an off +// protocol goes out as enabled=false. FromProto turns enabled=false, or a +// missing block, into nil, which is how this package spells "off". +// - Applied over a stored config (FromProtoOver), a missing block keeps the +// stored one. A client that predates a block therefore cannot switch a +// protocol off by leaving it out. +// - Secrets are write-only. ToProtoRedacted blanks them for GetConfig, and +// FromProtoOver fills an empty secret from the stored entry with the same +// identifier, so a read, edit, apply cycle keeps them. An empty secret for +// an identifier the node does not know is refused. +// - A Root serves no enrolment protocol (Root Mode closes the service-plane +// listeners), so Validate refuses a Root with either block set. +// - Every protocol field is read at boot, so NeedsReboot classifies any +// change to a block, including the switch itself, as reboot-required. + +// validateProtocolRole refuses an enrolment protocol on a Root. +func validateProtocolRole(role RoleKind, p PKI) error { + if role != RoleRoot { + return nil + } + if p.ACME != nil { + return errors.New("config: pki.acme: must not be set on a root node; a root serves no " + + "enrolment protocol, so serve ACME from an intermediate or issuing node") + } + if p.EST != nil { + return errors.New("config: pki.est: must not be set on a root node; a root serves no " + + "enrolment protocol, so serve EST from an intermediate or issuing node") + } + return nil +} + +// ToProtoRedacted is ToProto with the protocol secrets blanked, for handing a +// config to a reader. The credential identifiers stay, so a caller can see +// which credentials exist and send the config back unchanged. +func (c *Config) ToProtoRedacted() *cryptosv1.MachineConfig { + pb := c.ToProto() + for _, k := range pb.GetPki().GetAcme().GetExternalAccountKeys() { + k.HmacKeyBase64 = "" + } + for _, cred := range pb.GetPki().GetEst().GetEnrollCredentials() { + cred.PasswordSha256 = "" + } + return pb +} + +// FromProtoOver converts an applied proto into the config to store over prev, +// the config the node holds now (nil when it holds none, as in a maintenance +// install). It is FromProto plus the protocol-block rules above: a missing +// block keeps prev's, and an empty secret is taken from prev's entry with the +// same identifier. The result still has to pass Validate. +func FromProtoOver(pb *cryptosv1.MachineConfig, prev *Config) (*Config, error) { + c, err := FromProto(pb) + if err != nil { + return nil, err + } + var prevACME *ACME + var prevEST *EST + if prev != nil { + prevACME, prevEST = prev.PKI.ACME, prev.PKI.EST + } + + if pb.GetPki().GetAcme() == nil { + c.PKI.ACME = prevACME + } else if c.PKI.ACME != nil { + if err := keepACMESecrets(c.PKI.ACME, prevACME); err != nil { + return nil, err + } + } + if pb.GetPki().GetEst() == nil { + c.PKI.EST = prevEST + } else if c.PKI.EST != nil { + if err := keepESTSecrets(c.PKI.EST, prevEST); err != nil { + return nil, err + } + } + return c, nil +} + +func keepACMESecrets(next, prev *ACME) error { + stored := map[string]string{} + if prev != nil { + for _, k := range prev.ExternalAccountKeys { + stored[k.KeyID] = k.HMACKeyBase64 + } + } + for i := range next.ExternalAccountKeys { + k := &next.ExternalAccountKeys[i] + if k.HMACKeyBase64 != "" { + continue + } + secret, ok := stored[k.KeyID] + if !ok { + return fmt.Errorf("config: pki.acme.external_account_keys[%d].hmac_key_base64: empty, and the node "+ + "has no stored key for key_id %q to keep; send the secret for a new key", i, k.KeyID) + } + k.HMACKeyBase64 = secret + } + return nil +} + +func keepESTSecrets(next, prev *EST) error { + stored := map[string]string{} + if prev != nil { + for _, cred := range prev.EnrollCredentials { + stored[cred.Username] = cred.PasswordSHA256 + } + } + for i := range next.EnrollCredentials { + cred := &next.EnrollCredentials[i] + if cred.PasswordSHA256 != "" { + continue + } + digest, ok := stored[cred.Username] + if !ok { + return fmt.Errorf("config: pki.est.enroll_credentials[%d].password_sha256: empty, and the node "+ + "has no stored credential for username %q to keep; send the digest for a new credential", i, cred.Username) + } + cred.PasswordSHA256 = digest + } + return nil +} + +func acmeToProto(a *ACME) *cryptosv1.Acme { + if a == nil { + return &cryptosv1.Acme{Enabled: false} + } + pb := &cryptosv1.Acme{ + Enabled: true, + BaseUrl: a.BaseURL, + HttpPort: a.HTTPPort, + Profile: a.Profile, + TermsOfService: a.TermsOfService, + Website: a.Website, + AllowAnonymousAccounts: a.AllowAnonymousAccounts, + AllowedIdentifierSuffixes: a.AllowedIdentifierSuffixes, + OrderTtlHours: a.OrderTTLHours, + } + for _, k := range a.ExternalAccountKeys { + pb.ExternalAccountKeys = append(pb.ExternalAccountKeys, &cryptosv1.AcmeExternalAccountKey{ + KeyId: k.KeyID, + HmacKeyBase64: k.HMACKeyBase64, + }) + } + return pb +} + +func acmeFromProto(pb *cryptosv1.Acme) *ACME { + if !pb.GetEnabled() { + return nil + } + a := &ACME{ + BaseURL: pb.GetBaseUrl(), + HTTPPort: pb.GetHttpPort(), + Profile: pb.GetProfile(), + TermsOfService: pb.GetTermsOfService(), + Website: pb.GetWebsite(), + AllowAnonymousAccounts: pb.GetAllowAnonymousAccounts(), + AllowedIdentifierSuffixes: pb.GetAllowedIdentifierSuffixes(), + OrderTTLHours: pb.GetOrderTtlHours(), + } + for _, k := range pb.GetExternalAccountKeys() { + a.ExternalAccountKeys = append(a.ExternalAccountKeys, ExternalAccountKey{ + KeyID: k.GetKeyId(), + HMACKeyBase64: k.GetHmacKeyBase64(), + }) + } + return a +} + +func estToProto(e *EST) *cryptosv1.Est { + if e == nil { + return &cryptosv1.Est{Enabled: false} + } + pb := &cryptosv1.Est{ + Enabled: true, + Hostnames: e.Hostnames, + HttpPort: e.HTTPPort, + Profile: e.Profile, + Label: e.Label, + Realm: e.Realm, + AllowedIdentifierSuffixes: e.AllowedIdentifierSuffixes, + AllowAnyIdentifier: e.AllowAnyIdentifier, + } + for _, cred := range e.EnrollCredentials { + pb.EnrollCredentials = append(pb.EnrollCredentials, &cryptosv1.EstEnrollCredential{ + Username: cred.Username, + PasswordSha256: cred.PasswordSHA256, + }) + } + return pb +} + +func estFromProto(pb *cryptosv1.Est) *EST { + if !pb.GetEnabled() { + return nil + } + e := &EST{ + Hostnames: pb.GetHostnames(), + HTTPPort: pb.GetHttpPort(), + Profile: pb.GetProfile(), + Label: pb.GetLabel(), + Realm: pb.GetRealm(), + AllowedIdentifierSuffixes: pb.GetAllowedIdentifierSuffixes(), + AllowAnyIdentifier: pb.GetAllowAnyIdentifier(), + } + for _, cred := range pb.GetEnrollCredentials() { + e.EnrollCredentials = append(e.EnrollCredentials, ESTEnrollCredential{ + Username: cred.GetUsername(), + PasswordSHA256: cred.GetPasswordSha256(), + }) + } + return e +} diff --git a/internal/config/protocols_test.go b/internal/config/protocols_test.go new file mode 100644 index 0000000..06c46af --- /dev/null +++ b/internal/config/protocols_test.go @@ -0,0 +1,379 @@ +package config + +/* +Apache License 2.0 + +Copyright 2026 Shane + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "encoding/base64" + "reflect" + "strings" + "testing" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" +) + +const testPasswordSHA256 = "5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8" + +// issuingWithProtocols is a valid issuing-node config serving both protocols, +// with one secret in each credential list. +func issuingWithProtocols(t *testing.T) *Config { + t.Helper() + c, err := Parse(validYAML(t)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + c.Role.Kind = RoleIssuing + c.PKI.Parent = &Parent{CACertSHA256: strings.Repeat("ab", 32)} + c.PKI.Profiles = acmeTestProfiles() + c.PKI.ACME = &ACME{ + BaseURL: "https://ca.example.org/acme", + HTTPPort: 8555, + Profile: "leaf-server", + TermsOfService: "https://ca.example.org/tos", + Website: "https://ca.example.org", + ExternalAccountKeys: []ExternalAccountKey{{KeyID: "ops", HMACKeyBase64: validEABKey()}}, + AllowedIdentifierSuffixes: []string{"example.org"}, + OrderTTLHours: 12, + } + c.PKI.EST = &EST{ + Hostnames: []string{"est.example.org"}, + HTTPPort: 8443, + Profile: "leaf-server", + Label: "devices", + Realm: "cryptos", + AllowedIdentifierSuffixes: []string{"devices.example.org"}, + EnrollCredentials: []ESTEnrollCredential{{Username: "router", PasswordSHA256: testPasswordSHA256}}, + } + if err := c.Validate(); err != nil { + t.Fatalf("fixture does not validate: %v", err) + } + return c +} + +// issuingYAML is validYAML for an issuing node, the role an enrolment protocol +// runs on. The pki block stays last, so a test can append to it. +func issuingYAML(t *testing.T) []byte { + t.Helper() + s := strings.Replace(string(validYAML(t)), " kind: root\n", " kind: issuing\n", 1) + return []byte(s + " parent:\n ca_cert_sha256: \"" + strings.Repeat("ab", 32) + "\"\n") +} + +func otherEABKey() string { + return base64.RawURLEncoding.EncodeToString([]byte("fedcba9876543210fedcba9876543210")) +} + +func TestValidateRejectsProtocolsOnRoot(t *testing.T) { + for _, tc := range []struct { + name string + mutate func(*Config) + expect string + }{ + {"acme", func(c *Config) { c.PKI.EST = nil }, "pki.acme"}, + {"est", func(c *Config) { c.PKI.ACME = nil }, "pki.est"}, + } { + t.Run(tc.name, func(t *testing.T) { + c := issuingWithProtocols(t) + tc.mutate(c) + c.Role.Kind = RoleRoot + c.PKI.Parent = nil + c.PKI.RootValidityYears = 20 + err := c.Validate() + if err == nil { + t.Fatal("a root node with an enrolment protocol switched on validated") + } + if !strings.Contains(err.Error(), tc.expect) || !strings.Contains(err.Error(), "root") { + t.Errorf("error %q does not name %s and the root role", err, tc.expect) + } + }) + } +} + +func TestParseRejectsProtocolsOnRoot(t *testing.T) { + raw := string(validYAML(t)) + ` profiles: + - name: leaf-server + key_alg: ECDSA-P384 + validity_days: 90 + key_usage: [digital_signature] + ext_key_usage: [server_auth] + acme: + base_url: https://ca.example.org/acme + profile: leaf-server + allow_anonymous_accounts: true +` + if _, err := Parse([]byte(raw)); err == nil || !strings.Contains(err.Error(), "root") { + t.Fatalf("Parse of a root config serving ACME = %v, want a root-role rejection", err) + } +} + +func TestProtoRoundTripCarriesProtocols(t *testing.T) { + c := issuingWithProtocols(t) + got, err := FromProto(c.ToProto()) + if err != nil { + t.Fatalf("FromProto: %v", err) + } + if !reflect.DeepEqual(got.PKI.ACME, c.PKI.ACME) { + t.Errorf("ACME after the round trip = %+v, want %+v", got.PKI.ACME, c.PKI.ACME) + } + if !reflect.DeepEqual(got.PKI.EST, c.PKI.EST) { + t.Errorf("EST after the round trip = %+v, want %+v", got.PKI.EST, c.PKI.EST) + } +} + +func TestToProtoMarksProtocolsEnabled(t *testing.T) { + c := issuingWithProtocols(t) + pb := c.ToProto() + if !pb.GetPki().GetAcme().GetEnabled() || !pb.GetPki().GetEst().GetEnabled() { + t.Fatalf("a configured protocol went out with enabled=false: acme=%v est=%v", + pb.GetPki().GetAcme(), pb.GetPki().GetEst()) + } + + // An off protocol is sent as an explicit enabled=false block, not left + // out: an absent block means "keep what the node has", so a config that + // leaves a protocol out could never switch it off. + c.PKI.ACME, c.PKI.EST = nil, nil + pb = c.ToProto() + if a := pb.GetPki().GetAcme(); a == nil || a.GetEnabled() { + t.Errorf("an off ACME went out as %v, want an explicit enabled=false block", a) + } + if e := pb.GetPki().GetEst(); e == nil || e.GetEnabled() { + t.Errorf("an off EST went out as %v, want an explicit enabled=false block", e) + } +} + +func TestFromProtoDisabledProtocolIsOff(t *testing.T) { + c := issuingWithProtocols(t) + pb := c.ToProto() + pb.Pki.Acme.Enabled = false + pb.Pki.Est.Enabled = false + got, err := FromProto(pb) + if err != nil { + t.Fatalf("FromProto: %v", err) + } + if got.PKI.ACME != nil || got.PKI.EST != nil { + t.Errorf("enabled=false left a protocol on: acme=%+v est=%+v", got.PKI.ACME, got.PKI.EST) + } +} + +func TestToProtoRedactedBlanksSecrets(t *testing.T) { + c := issuingWithProtocols(t) + pb := c.ToProtoRedacted() + keys := pb.GetPki().GetAcme().GetExternalAccountKeys() + if len(keys) != 1 || keys[0].GetKeyId() != "ops" || keys[0].GetHmacKeyBase64() != "" { + t.Errorf("redacted ACME keys = %v, want key_id ops with an empty secret", keys) + } + creds := pb.GetPki().GetEst().GetEnrollCredentials() + if len(creds) != 1 || creds[0].GetUsername() != "router" || creds[0].GetPasswordSha256() != "" { + t.Errorf("redacted EST credentials = %v, want username router with an empty digest", creds) + } + if pb.GetPki().GetAcme().GetBaseUrl() != c.PKI.ACME.BaseURL { + t.Error("redaction dropped a non-secret ACME field") + } + if c.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 == "" { + t.Error("redaction blanked the secret in the config itself") + } +} + +func TestFromProtoOverAbsentBlockKeepsStored(t *testing.T) { + prev := issuingWithProtocols(t) + pb := prev.ToProto() + pb.Pki.Acme = nil + pb.Pki.Est = nil + got, err := FromProtoOver(pb, prev) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + if !reflect.DeepEqual(got.PKI.ACME, prev.PKI.ACME) || !reflect.DeepEqual(got.PKI.EST, prev.PKI.EST) { + t.Errorf("an apply without the blocks changed them: acme=%+v est=%+v", got.PKI.ACME, got.PKI.EST) + } +} + +func TestFromProtoOverDisabledSwitchesOff(t *testing.T) { + prev := issuingWithProtocols(t) + pb := prev.ToProto() + pb.Pki.Acme = &cryptosv1.Acme{Enabled: false} + got, err := FromProtoOver(pb, prev) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + if got.PKI.ACME != nil { + t.Errorf("enabled=false left ACME on: %+v", got.PKI.ACME) + } + if got.PKI.EST == nil { + t.Error("switching ACME off switched EST off too") + } +} + +func TestFromProtoOverBlankSecretKeepsStored(t *testing.T) { + prev := issuingWithProtocols(t) + got, err := FromProtoOver(prev.ToProtoRedacted(), prev) + if err != nil { + t.Fatalf("FromProtoOver of the redacted config: %v", err) + } + if !reflect.DeepEqual(got.PKI.ACME, prev.PKI.ACME) { + t.Errorf("ACME after a redacted round trip = %+v, want the stored block", got.PKI.ACME) + } + if !reflect.DeepEqual(got.PKI.EST, prev.PKI.EST) { + t.Errorf("EST after a redacted round trip = %+v, want the stored block", got.PKI.EST) + } + if err := got.Validate(); err != nil { + t.Errorf("the resolved config does not validate: %v", err) + } +} + +func TestFromProtoOverSetSecretReplacesStored(t *testing.T) { + prev := issuingWithProtocols(t) + pb := prev.ToProto() + pb.Pki.Acme.ExternalAccountKeys[0].HmacKeyBase64 = otherEABKey() + got, err := FromProtoOver(pb, prev) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + if got.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 != otherEABKey() { + t.Error("a new secret for a known key_id did not replace the stored one") + } +} + +func TestFromProtoOverRemovedEntryIsRevoked(t *testing.T) { + prev := issuingWithProtocols(t) + pb := prev.ToProtoRedacted() + pb.Pki.Acme.ExternalAccountKeys = nil + pb.Pki.Acme.AllowAnonymousAccounts = true + got, err := FromProtoOver(pb, prev) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + if len(got.PKI.ACME.ExternalAccountKeys) != 0 { + t.Errorf("a key left out of the apply survived: %+v", got.PKI.ACME.ExternalAccountKeys) + } +} + +func TestFromProtoOverBlankSecretForUnknownIdentifierIsRejected(t *testing.T) { + for _, tc := range []struct { + name string + mutate func(*cryptosv1.MachineConfig) + expect string + }{ + { + name: "acme key_id", + mutate: func(pb *cryptosv1.MachineConfig) { + pb.Pki.Acme.ExternalAccountKeys = append(pb.Pki.Acme.ExternalAccountKeys, + &cryptosv1.AcmeExternalAccountKey{KeyId: "new-team"}) + }, + expect: `"new-team"`, + }, + { + name: "est username", + mutate: func(pb *cryptosv1.MachineConfig) { + pb.Pki.Est.EnrollCredentials = append(pb.Pki.Est.EnrollCredentials, + &cryptosv1.EstEnrollCredential{Username: "switch"}) + }, + expect: `"switch"`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + prev := issuingWithProtocols(t) + pb := prev.ToProtoRedacted() + tc.mutate(pb) + _, err := FromProtoOver(pb, prev) + if err == nil { + t.Fatal("an empty secret for an unknown identifier was accepted") + } + if !strings.Contains(err.Error(), tc.expect) { + t.Errorf("error %q does not name the identifier %s", err, tc.expect) + } + }) + } +} + +// With nothing stored (a maintenance install), every empty secret belongs to +// an identifier the node has never seen. +func TestFromProtoOverBlankSecretWithNothingStored(t *testing.T) { + c := issuingWithProtocols(t) + if _, err := FromProtoOver(c.ToProtoRedacted(), nil); err == nil { + t.Fatal("a redacted config was accepted with nothing stored to fill its secrets from") + } + got, err := FromProtoOver(c.ToProto(), nil) + if err != nil { + t.Fatalf("FromProtoOver with full secrets and nothing stored: %v", err) + } + if !reflect.DeepEqual(got.PKI.ACME, c.PKI.ACME) { + t.Errorf("ACME = %+v, want %+v", got.PKI.ACME, c.PKI.ACME) + } +} + +// config get prints secrets blank, and the operator feeds that YAML back to +// config apply. The client-side check has to accept the blanks the node will +// fill, while the node's own parse stays strict. +func TestParseForApplyAcceptsKeptSecrets(t *testing.T) { + c := issuingWithProtocols(t) + redacted, err := FromProto(c.ToProtoRedacted()) + if err != nil { + t.Fatalf("FromProto: %v", err) + } + raw, err := redacted.Marshal() + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if _, err := Parse(raw); err == nil { + t.Error("the strict Parse accepted a config with empty secrets") + } + got, err := ParseForApply(raw) + if err != nil { + t.Fatalf("ParseForApply of a config with empty secrets: %v", err) + } + if got.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 != "" { + t.Error("ParseForApply invented a secret") + } + + // A secret that is set is still checked. + redacted.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 = "short" + raw, err = redacted.Marshal() + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if _, err := ParseForApply(raw); err == nil { + t.Error("ParseForApply accepted a malformed secret") + } +} + +func TestNeedsRebootForEveryProtocolChange(t *testing.T) { + base := issuingWithProtocols(t) + for _, tc := range []struct { + name string + mutate func(*Config) + }{ + {"acme off", func(c *Config) { c.PKI.ACME = nil }}, + {"est off", func(c *Config) { c.PKI.EST = nil }}, + {"acme setting", func(c *Config) { c.PKI.ACME.HTTPPort = 9000 }}, + {"est setting", func(c *Config) { c.PKI.EST.Hostnames = []string{"other.example.org"} }}, + {"acme secret", func(c *Config) { c.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 = otherEABKey() }}, + } { + t.Run(tc.name, func(t *testing.T) { + next := issuingWithProtocols(t) + tc.mutate(next) + if !NeedsReboot(base, next) { + t.Error("a protocol change was classified as live") + } + }) + } + off := issuingWithProtocols(t) + off.PKI.ACME, off.PKI.EST = nil, nil + if !NeedsReboot(off, base) { + t.Error("switching the protocols on was classified as live") + } +} diff --git a/internal/config/reboot.go b/internal/config/reboot.go index 0f74af3..6feef98 100644 --- a/internal/config/reboot.go +++ b/internal/config/reboot.go @@ -18,7 +18,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -import "reflect" +import ( + "bytes" + + "gopkg.in/yaml.v3" +) // NeedsReboot reports whether applying newCfg over oldCfg on a running node // requires a reboot for the change to take effect. @@ -35,7 +39,25 @@ func NeedsReboot(oldCfg, newCfg *Config) bool { if oldCfg == nil || newCfg == nil { return true } - return !reflect.DeepEqual(hotNormalized(oldCfg), hotNormalized(newCfg)) + return !Equivalent(hotNormalized(oldCfg), hotNormalized(newCfg)) +} + +// Equivalent reports whether a and b store as the same YAML. It is the +// comparison for "would this change what the node boots from": a config that +// crossed the wire has nil where the stored YAML parsed an empty list, and +// reflect.DeepEqual calls those different, so an unchanged re-apply read as a +// reboot-required change. A value that fails to marshal compares unequal, +// which errs toward a reboot. +func Equivalent(a, b any) bool { + ra, err := yaml.Marshal(a) + if err != nil { + return false + } + rb, err := yaml.Marshal(b) + if err != nil { + return false + } + return bytes.Equal(ra, rb) } // hotNormalized returns a copy of c with the hot-reconfigurable fields blanked, diff --git a/internal/config/reboot_test.go b/internal/config/reboot_test.go index 5792796..74e1c75 100644 --- a/internal/config/reboot_test.go +++ b/internal/config/reboot_test.go @@ -95,3 +95,20 @@ func TestNeedsReboot(t *testing.T) { } }) } + +// A config that went over the wire comes back with nil where the stored YAML +// parsed an empty list. That is the same config, so it must not read as a +// reboot-required change. +func TestNeedsRebootIgnoresNilVersusEmpty(t *testing.T) { + stored := &Config{Network: Network{Nameservers: []string{}, Search: []string{}}} + stored.PKI.ACME = &ACME{BaseURL: "https://ca.example.org/acme", AllowedIdentifierSuffixes: []string{}} + wire := &Config{} + wire.PKI.ACME = &ACME{BaseURL: "https://ca.example.org/acme"} + if NeedsReboot(stored, wire) { + t.Error("nil and empty lists were classified as a reboot-required change") + } + wire.PKI.ACME.AllowedIdentifierSuffixes = []string{"example.org"} + if !NeedsReboot(stored, wire) { + t.Error("a real protocol change was classified as live") + } +} diff --git a/internal/init/install_protocols_test.go b/internal/init/install_protocols_test.go new file mode 100644 index 0000000..22916f3 --- /dev/null +++ b/internal/init/install_protocols_test.go @@ -0,0 +1,145 @@ +package init + +/* +Apache License 2.0 + +Copyright 2026 Shane + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "context" + "encoding/base64" + "strings" + "testing" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" + "github.com/CryptOS-PKI/cryptos/internal/config" + "github.com/CryptOS-PKI/cryptos/internal/install" +) + +var installEABKey = base64.RawURLEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")) + +// issuingMachineConfigWithProtocols is validMachineConfig as an issuing node +// with ACME and EST switched on. +func issuingMachineConfigWithProtocols() *cryptosv1.MachineConfig { + pb := validMachineConfig() + pb.Role.Kind = "issuing" + pb.Pki.RootValidityYears = 0 + pb.Pki.Parent = &cryptosv1.Parent{CaCertSha256: "abababababababababababababababababababababababababababababababab"} + pb.Pki.Profiles = []*cryptosv1.CertificateProfile{{ + Name: "leaf-server", KeyAlg: "ECDSA-P384", ValidityDays: 90, + KeyUsage: []string{"digital_signature"}, ExtKeyUsage: []string{"server_auth"}, + }} + pb.Pki.Acme = &cryptosv1.Acme{ + Enabled: true, + BaseUrl: "https://ca.example.org/acme", + Profile: "leaf-server", + ExternalAccountKeys: []*cryptosv1.AcmeExternalAccountKey{ + {KeyId: "ops", HmacKeyBase64: installEABKey}, + }, + } + pb.Pki.Est = &cryptosv1.Est{ + Enabled: true, + Hostnames: []string{"est.example.org"}, + Profile: "leaf-server", + AllowAnyIdentifier: true, + } + return pb +} + +// The maintenance install writes the staged YAML the installed node boots +// from, so the protocol blocks it was given must be in it. +func TestMaintenanceInstaller_CarriesProtocolBlocks(t *testing.T) { + var staged []byte + inst := &maintenanceInstaller{ + rebootCh: make(chan struct{}, 1), + doLocateUKI: func() (string, error) { return "/fake/uki.efi", nil }, + doInstall: func(_ context.Context, o install.Options, _ install.Runner, _ string, _ func(string, string) error, _ install.Deps) error { + staged = o.ConfigYAML + return nil + }, + } + if _, err := inst.Install(context.Background(), issuingMachineConfigWithProtocols()); err != nil { + t.Fatalf("Install: %v", err) + } + cfg, err := config.Parse(staged) + if err != nil { + t.Fatalf("parse the staged config: %v", err) + } + if cfg.PKI.ACME == nil || cfg.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 != installEABKey { + t.Errorf("staged ACME = %+v, want the applied block with its secret", cfg.PKI.ACME) + } + if cfg.PKI.EST == nil || cfg.PKI.EST.Hostnames[0] != "est.example.org" { + t.Errorf("staged EST = %+v, want the applied block", cfg.PKI.EST) + } +} + +// A fresh install has no stored secret to keep, so an empty one is refused +// with a message that says so, before anything touches the disk. +func TestMaintenanceInstaller_RejectsEmptySecret(t *testing.T) { + pb := issuingMachineConfigWithProtocols() + pb.Pki.Acme.ExternalAccountKeys[0].HmacKeyBase64 = "" + called := false + inst := &maintenanceInstaller{ + rebootCh: make(chan struct{}, 1), + doLocateUKI: func() (string, error) { return "/fake/uki.efi", nil }, + doInstall: func(context.Context, install.Options, install.Runner, string, func(string, string) error, install.Deps) error { + called = true + return nil + }, + } + _, err := inst.Install(context.Background(), pb) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("code = %v (err %v), want InvalidArgument", status.Code(err), err) + } + if !strings.Contains(err.Error(), `no stored key for key_id "ops"`) { + t.Errorf("error %q does not say the node has no key to keep", err) + } + if called { + t.Error("the disk was written for a config with an empty secret") + } +} + +func TestMaintenanceInstaller_RejectsProtocolOnRoot(t *testing.T) { + pb := validMachineConfig() + pb.Pki.Profiles = issuingMachineConfigWithProtocols().Pki.Profiles + pb.Pki.Est = &cryptosv1.Est{Enabled: true, Hostnames: []string{"est.example.org"}, Profile: "leaf-server", AllowAnyIdentifier: true} + inst := &maintenanceInstaller{rebootCh: make(chan struct{}, 1)} + if _, err := inst.Install(context.Background(), pb); status.Code(err) != codes.InvalidArgument { + t.Fatalf("install of a root serving EST: code = %v (err %v), want InvalidArgument", status.Code(err), err) + } +} + +func TestReprovisioner_CarriesProtocolBlocks(t *testing.T) { + store := config.NewFileStore(t.TempDir()) + rp := &reprovisioner{store: store, rebootCh: make(chan struct{}, 1)} + if _, err := rp.Install(context.Background(), issuingMachineConfigWithProtocols()); err != nil { + t.Fatalf("Install: %v", err) + } + raw, _, ok, err := store.Read() + if err != nil || !ok { + t.Fatalf("config not persisted: ok=%v err=%v", ok, err) + } + cfg, err := config.Parse(raw) + if err != nil { + t.Fatalf("parse the persisted config: %v", err) + } + if cfg.PKI.ACME == nil || cfg.PKI.EST == nil { + t.Errorf("persisted protocols: acme=%+v est=%+v, want both", cfg.PKI.ACME, cfg.PKI.EST) + } +} diff --git a/internal/init/maintenance_installer.go b/internal/init/maintenance_installer.go index 4bc5413..28353ee 100644 --- a/internal/init/maintenance_installer.go +++ b/internal/init/maintenance_installer.go @@ -64,7 +64,7 @@ type maintenanceInstaller struct { // Install validates cfg, locates the booted UKI, installs to disk, and triggers // a reboot by signalling runMaintenance to return. func (m *maintenanceInstaller) Install(ctx context.Context, cfg *cryptosv1.MachineConfig) (*cryptosv1.ApplyConfigResponse, error) { - parsed, err := config.FromProto(cfg) + parsed, err := config.FromProtoOver(cfg, nil) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "apply-config: parse: %v", err) } diff --git a/internal/init/reprovision.go b/internal/init/reprovision.go index 591eb14..46cbd68 100644 --- a/internal/init/reprovision.go +++ b/internal/init/reprovision.go @@ -56,7 +56,7 @@ type reprovisioner struct { // reboot, and returns RequiresReboot: true. It does not touch any disk: the // state partition is already present from the pre-reset install. func (r *reprovisioner) Install(_ context.Context, cfg *cryptosv1.MachineConfig) (*cryptosv1.ApplyConfigResponse, error) { - parsed, err := config.FromProto(cfg) + parsed, err := config.FromProtoOver(cfg, nil) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "reprovision: parse: %v", err) } diff --git a/internal/init/run.go b/internal/init/run.go index 72d0c31..dd0435d 100644 --- a/internal/init/run.go +++ b/internal/init/run.go @@ -29,6 +29,7 @@ import ( "net/netip" "os" "os/signal" + "sync/atomic" "syscall" "time" @@ -391,6 +392,10 @@ func boot(ctx context.Context, shutdown *shutdownRequests) (err error) { // driven after the revocation listener is up (below) and consumed by the // CA signer. GetStatus also reports the resolver written above. preflight := revocation.NewPreflight(cfg.PKI.RevocationBaseURL, revocation.DefaultResolver, revocation.DefaultProbe) + // The enrolment listeners start once, below, from this boot's config. A + // protocol switched by ApplyConfig waits for the next boot, and GetStatus + // shows it configured but not running until then. + var acmeRunning, estRunning atomic.Bool statusProv, err := node.NewStatusProvider(node.StatusConfig{ Store: store, Role: cfg.NodeRole(), @@ -399,7 +404,19 @@ func boot(ctx context.Context, shutdown *shutdownRequests) (err error) { RevocationPreflight: func() *cryptosv1.RevocationPreflight { return revocationPreflightStatus(cfg.PKI.RevocationBaseURL, preflight) }, - Resolver: func() *cryptosv1.ResolverStatus { return resolver }, + Resolver: func() *cryptosv1.ResolverStatus { return resolver }, + BootConfig: cfg, + ConfigFile: cfgStore, + ProtocolRunning: func(p cryptosv1.ServiceProtocol) bool { + switch p { + case cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME: + return acmeRunning.Load() + case cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_EST: + return estRunning.Load() + default: + return false + } + }, }) if err != nil { return err @@ -816,8 +833,11 @@ func boot(ctx context.Context, shutdown *shutdownRequests) (err error) { defer cancel() _ = stopACME(shutdownCtx) }() + acmeRunning.Store(true) log.Printf("ACME listener up: %s (base=%s profile=%s external_account_binding=%t)", acmeAddr, cfg.PKI.ACME.BaseURL, cfg.PKI.ACME.Profile, !cfg.PKI.ACME.AllowAnonymousAccounts) + } else { + log.Printf("ACME: off in the boot config; not listening") } // 12d. EST (RFC 7030) enrolment listener. Unlike ACME it terminates TLS @@ -850,8 +870,11 @@ func boot(ctx context.Context, shutdown *shutdownRequests) (err error) { defer cancel() _ = stopEST(shutdownCtx) }() + estRunning.Store(true) log.Printf("EST listener up: %s (hosts=%v profile=%s simpleenroll=%t)", estAddr, cfg.PKI.EST.Hostnames, cfg.PKI.EST.Profile, estOpts.EnrollAuth != nil) + } else { + log.Printf("EST: off in the boot config; not listening") } done() diff --git a/internal/node/apply_validate_test.go b/internal/node/apply_validate_test.go index e1171db..7d8dc5f 100644 --- a/internal/node/apply_validate_test.go +++ b/internal/node/apply_validate_test.go @@ -31,19 +31,18 @@ import ( ) // applyValidateSeed is a valid running-node config with a leaf profile that -// ACME references. The ACME block is not expressible in the proto, so Apply -// carries it forward from this seed. +// ACME references. const applyValidateSeed = `apiVersion: cryptos.dev/v1alpha1 kind: MachineConfig metadata: {name: apply-validate-test} -role: {kind: root} +role: {kind: issuing} network: {interface: eth0, address: 10.0.0.10/24, gateway: 10.0.0.1} bootstrap: {admin_cert_sha256: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"} pki: root_key_alg: ECDSA-P384 - root_subject: {common_name: "Apply Validate Root", organization: "Test", country: "US"} - root_validity_years: 10 - path_len_constraint: 1 + root_subject: {common_name: "Apply Validate Issuing CA", organization: "Test", country: "US"} + path_len_constraint: 0 + parent: {ca_cert_sha256: "abababababababababababababababababababababababababababababababab"} revocation_base_url: https://ca.example.org profiles: - name: leaf-server @@ -92,9 +91,9 @@ func TestConfigStoreApply_RejectsInvalidConfig(t *testing.T) { }, }, { - // Valid on its own, invalid once the carried-forward ACME block - // names a profile the incoming config dropped. Validation has to - // run on the config that would actually be written. + // Valid on its own, invalid once the ACME block names a profile + // the incoming config dropped. Validation has to run on the + // config that would actually be written. name: "carried-forward ACME references a removed profile", mutate: func(pb *cryptosv1.MachineConfig) { pb.Pki.Profiles[0].Name = "leaf-renamed" diff --git a/internal/node/node_test.go b/internal/node/node_test.go index 70a19a3..21ce2a1 100644 --- a/internal/node/node_test.go +++ b/internal/node/node_test.go @@ -362,23 +362,24 @@ func TestNewNilClient(t *testing.T) { // TestConfigStoreApply_PreservesProtocols is the regression guard for #205. // -// MachineConfig has no acme or est field, so a config built from a proto has -// neither. Apply used to write exactly that as the node's whole config, which -// silently disabled both protocols -- and the Fleet Manager applies config this -// way whenever it changes a profile, so a routine edit turned off enrollment. +// Apply used to write a config built solely from the proto, which then had no +// acme or est field, so it silently disabled both protocols -- and the Fleet +// Manager applies config this way whenever it changes a profile. The blocks +// now travel in the proto, with their secrets redacted on the way out, and a +// read-apply round trip must still leave both protocols as they were. func TestConfigStoreApply_PreservesProtocols(t *testing.T) { ctx := context.Background() withProtocols := []byte(`apiVersion: cryptos.dev/v1alpha1 kind: MachineConfig metadata: {name: protocols-test} -role: {kind: root} +role: {kind: issuing} network: {interface: eth0, address: 10.0.0.10/24, gateway: 10.0.0.1} bootstrap: {admin_cert_sha256: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"} pki: root_key_alg: ECDSA-P384 - root_subject: {common_name: "Protocols Test Root", organization: "Test", country: "US"} - root_validity_years: 10 - path_len_constraint: 1 + root_subject: {common_name: "Protocols Test Issuing CA", organization: "Test", country: "US"} + path_len_constraint: 0 + parent: {ca_cert_sha256: "abababababababababababababababababababababababababababababababab"} profiles: - name: leaf-server key_alg: ECDSA-P384 @@ -402,9 +403,7 @@ pki: t.Fatal("fixture does not configure both protocols") } - // Seed the file directly, the way a ceremony or an operator does. Seeding - // through Apply would go via the proto and drop the blocks before the round - // trip under test even began -- which is the bug, not the setup. + // Seed the file directly, the way a ceremony or an operator does. fs := config.NewFileStore(t.TempDir()) if _, err := fs.Write(withProtocols); err != nil { t.Fatalf("FileStore.Write (seed): %v", err) @@ -412,7 +411,7 @@ pki: cs := NewConfigStore(fs) // What the Fleet Manager does: read the config over the wire and apply it - // back. The proto it round-trips cannot carry the protocol blocks. + // back. current, err := cs.Current(ctx) if err != nil { t.Fatalf("Current: %v", err) @@ -424,8 +423,7 @@ pki: t.Fatalf("Apply (round trip): %v", err) } - // Current returns a proto, which by definition cannot express the - // protocols, so assert against what was actually persisted. + // Assert against what was actually persisted. raw, _, ok, err := fs.Read() if err != nil || !ok { t.Fatalf("FileStore.Read after apply: ok=%v err=%v", ok, err) diff --git a/internal/node/protocols_test.go b/internal/node/protocols_test.go new file mode 100644 index 0000000..bfc9a5e --- /dev/null +++ b/internal/node/protocols_test.go @@ -0,0 +1,360 @@ +package node + +/* +Apache License 2.0 + +Copyright 2026 Shane + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "bytes" + "context" + "encoding/base64" + "strings" + "testing" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" + "github.com/CryptOS-PKI/cryptos/internal/config" +) + +// protocolsOffSeed is an issuing node with a leaf profile and no enrolment +// protocol switched on. +const protocolsOffSeed = `apiVersion: cryptos.dev/v1alpha1 +kind: MachineConfig +metadata: {name: protocols-switch-test} +role: {kind: issuing} +network: {interface: eth0, address: 10.0.0.10/24, gateway: 10.0.0.1} +bootstrap: {admin_cert_sha256: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"} +pki: + root_key_alg: ECDSA-P384 + root_subject: {common_name: "Protocols Switch Issuing CA", organization: "Test", country: "US"} + path_len_constraint: 0 + parent: {ca_cert_sha256: "abababababababababababababababababababababababababababababababab"} + profiles: + - name: leaf-server + key_alg: ECDSA-P384 + validity_days: 90 + key_usage: [digital_signature] + ext_key_usage: [server_auth] +` + +var testEABKey = base64.RawURLEncoding.EncodeToString([]byte("0123456789abcdef0123456789abcdef")) + +func seededStore(t *testing.T, seed string) (*config.FileStore, *ConfigStore) { + t.Helper() + fs := config.NewFileStore(t.TempDir()) + if _, err := fs.Write([]byte(seed)); err != nil { + t.Fatalf("FileStore.Write (seed): %v", err) + } + return fs, NewConfigStore(fs) +} + +func storedConfig(t *testing.T, fs *config.FileStore) *config.Config { + t.Helper() + raw, _, ok, err := fs.Read() + if err != nil || !ok { + t.Fatalf("FileStore.Read: ok=%v err=%v", ok, err) + } + c, err := config.Parse(raw) + if err != nil { + t.Fatalf("parse the stored config: %v", err) + } + return c +} + +func acmeOn() *cryptosv1.Acme { + return &cryptosv1.Acme{ + Enabled: true, + BaseUrl: "https://ca.example.org/acme", + Profile: "leaf-server", + ExternalAccountKeys: []*cryptosv1.AcmeExternalAccountKey{ + {KeyId: "ops", HmacKeyBase64: testEABKey}, + }, + } +} + +// The round trip an operator or the Fleet Manager makes: switch ACME on, read +// the config back, apply it unchanged, then switch ACME off. Every switch is +// stored and answered requires_reboot, the secret never comes back over the +// wire, and an unchanged re-apply neither loses the secret nor asks for a +// reboot. +func TestConfigStoreApply_ACMEOnThenOff(t *testing.T) { + ctx := context.Background() + fs, cs := seededStore(t, protocolsOffSeed) + + current, err := cs.Current(ctx) + if err != nil { + t.Fatalf("Current: %v", err) + } + if a := current.GetPki().GetAcme(); a == nil || a.GetEnabled() { + t.Fatalf("GetConfig of a node without ACME = %v, want an explicit enabled=false block", a) + } + + // On. + current.Pki.Acme = acmeOn() + resp, err := cs.Apply(ctx, current) + if err != nil { + t.Fatalf("Apply (ACME on): %v", err) + } + if !resp.GetRequiresReboot() { + t.Error("switching ACME on did not answer requires_reboot") + } + stored := storedConfig(t, fs) + if stored.PKI.ACME == nil || stored.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 != testEABKey { + t.Fatalf("stored ACME after switching on = %+v, want the applied block with its secret", stored.PKI.ACME) + } + + // Read back: on, with the secret blank. + current, err = cs.Current(ctx) + if err != nil { + t.Fatalf("Current: %v", err) + } + a := current.GetPki().GetAcme() + if !a.GetEnabled() || a.GetBaseUrl() != "https://ca.example.org/acme" { + t.Fatalf("GetConfig after switching on = %v, want the ACME block", a) + } + if len(a.GetExternalAccountKeys()) != 1 || a.GetExternalAccountKeys()[0].GetHmacKeyBase64() != "" { + t.Errorf("GetConfig returned the EAB secret: %v", a.GetExternalAccountKeys()) + } + + // Apply what was read, unchanged. + resp, err = cs.Apply(ctx, current) + if err != nil { + t.Fatalf("Apply (unchanged read-back): %v", err) + } + if resp.GetRequiresReboot() { + t.Error("an unchanged re-apply asked for a reboot") + } + if got := storedConfig(t, fs).PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64; got != testEABKey { + t.Errorf("stored EAB secret after the read-back apply = %q, want it kept", got) + } + + // Off. + current.Pki.Acme = &cryptosv1.Acme{Enabled: false} + resp, err = cs.Apply(ctx, current) + if err != nil { + t.Fatalf("Apply (ACME off): %v", err) + } + if !resp.GetRequiresReboot() { + t.Error("switching ACME off did not answer requires_reboot") + } + if stored := storedConfig(t, fs); stored.PKI.ACME != nil { + t.Errorf("stored ACME after switching off = %+v, want none", stored.PKI.ACME) + } + current, err = cs.Current(ctx) + if err != nil { + t.Fatalf("Current: %v", err) + } + if current.GetPki().GetAcme().GetEnabled() { + t.Error("GetConfig still reports ACME on after switching it off") + } +} + +// A client that predates the protocol blocks sends none. That must leave the +// node's protocols as they are, not switch them off. +func TestConfigStoreApply_AbsentBlockKeepsProtocol(t *testing.T) { + ctx := context.Background() + fs, cs := seededStore(t, protocolsOffSeed) + pb := storedConfig(t, fs).ToProto() + pb.Pki.Acme = acmeOn() + if _, err := cs.Apply(ctx, pb); err != nil { + t.Fatalf("Apply (ACME on): %v", err) + } + + pb, err := cs.Current(ctx) + if err != nil { + t.Fatalf("Current: %v", err) + } + pb.Pki.Acme = nil + pb.Pki.Est = nil + pb.Pki.Profiles[0].ValidityDays = 30 + if _, err := cs.Apply(ctx, pb); err != nil { + t.Fatalf("Apply (no protocol blocks): %v", err) + } + stored := storedConfig(t, fs) + if stored.PKI.ACME == nil || stored.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 != testEABKey { + t.Errorf("an apply without the ACME block changed it: %+v", stored.PKI.ACME) + } + if stored.PKI.Profiles[0].ValidityDays != 30 { + t.Error("the rest of the apply was not stored") + } +} + +func TestConfigStoreApply_RejectsSecretlessNewCredential(t *testing.T) { + ctx := context.Background() + fs, cs := seededStore(t, protocolsOffSeed) + beforeRaw, beforeGen, _, _ := fs.Read() + + pb := storedConfig(t, fs).ToProto() + pb.Pki.Acme = acmeOn() + pb.Pki.Acme.ExternalAccountKeys[0].HmacKeyBase64 = "" + _, err := cs.Apply(ctx, pb) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("Apply with an empty secret for a new key_id: code = %v (err %v), want InvalidArgument", status.Code(err), err) + } + if !strings.Contains(err.Error(), `"ops"`) { + t.Errorf("error %q does not name the key_id", err) + } + afterRaw, afterGen, _, _ := fs.Read() + if afterGen != beforeGen || !bytes.Equal(afterRaw, beforeRaw) { + t.Error("a rejected apply changed the stored config") + } +} + +func TestConfigStoreApply_RejectsProtocolOnRoot(t *testing.T) { + ctx := context.Background() + fs, cs := seededStore(t, protocolsOffSeed) + beforeRaw, beforeGen, _, _ := fs.Read() + + for _, tc := range []struct { + name string + mutate func(*cryptosv1.MachineConfig) + }{ + {"acme", func(pb *cryptosv1.MachineConfig) { pb.Pki.Acme = acmeOn() }}, + {"est", func(pb *cryptosv1.MachineConfig) { + pb.Pki.Est = &cryptosv1.Est{Enabled: true, Hostnames: []string{"est.example.org"}, Profile: "leaf-server"} + }}, + } { + t.Run(tc.name, func(t *testing.T) { + pb := storedConfig(t, fs).ToProto() + pb.Role.Kind = string(config.RoleRoot) + pb.Pki.Parent = nil + pb.Pki.RootValidityYears = 10 + tc.mutate(pb) + _, err := cs.Apply(ctx, pb) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("Apply of a root serving %s: code = %v (err %v), want InvalidArgument", tc.name, status.Code(err), err) + } + if !strings.Contains(err.Error(), "root") { + t.Errorf("error %q does not say the root role is the reason", err) + } + afterRaw, afterGen, _, _ := fs.Read() + if afterGen != beforeGen || !bytes.Equal(afterRaw, beforeRaw) { + t.Error("a rejected apply changed the stored config") + } + }) + } +} + +func protocolState(t *testing.T, st *cryptosv1.NodeStatus, p cryptosv1.ServiceProtocol) *cryptosv1.ProtocolStatus { + t.Helper() + for _, ps := range st.GetProtocols() { + if ps.GetProtocol() == p { + return ps + } + } + t.Fatalf("NodeStatus.protocols has no entry for %v: %v", p, st.GetProtocols()) + return nil +} + +// Status compares the stored config with the one this boot started from, so a +// protocol switched on by ApplyConfig shows configured but not running, with a +// reboot pending, until the node reboots. +func TestStatusProviderReportsProtocols(t *testing.T) { + ctx := context.Background() + s, _ := newTestStore(t) + fs, cs := seededStore(t, protocolsOffSeed) + boot := storedConfig(t, fs) + + running := map[cryptosv1.ServiceProtocol]bool{} + sp, err := NewStatusProvider(StatusConfig{ + Store: s, + Role: cryptosv1.NodeRole_NODE_ROLE_ISSUING, + BootConfig: boot, + ConfigFile: fs, + ProtocolRunning: func(p cryptosv1.ServiceProtocol) bool { return running[p] }, + }) + if err != nil { + t.Fatalf("NewStatusProvider: %v", err) + } + + st, err := sp.Status(ctx) + if err != nil { + t.Fatalf("Status: %v", err) + } + for _, p := range []cryptosv1.ServiceProtocol{cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME, cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_EST} { + ps := protocolState(t, st, p) + if ps.GetConfigured() || ps.GetRunning() || ps.GetRebootPending() { + t.Errorf("%v on a node with nothing switched on = %v, want all false", p, ps) + } + } + if st.GetConfigRebootPending() { + t.Error("config_reboot_pending is set with the stored config unchanged since boot") + } + + pb := boot.ToProto() + pb.Pki.Acme = acmeOn() + if _, err := cs.Apply(ctx, pb); err != nil { + t.Fatalf("Apply (ACME on): %v", err) + } + st, err = sp.Status(ctx) + if err != nil { + t.Fatalf("Status: %v", err) + } + acme := protocolState(t, st, cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME) + if !acme.GetConfigured() || acme.GetRunning() || !acme.GetRebootPending() { + t.Errorf("ACME after an apply switched it on = %v, want configured, not running, reboot pending", acme) + } + if est := protocolState(t, st, cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_EST); est.GetRebootPending() { + t.Errorf("EST was untouched but reports a pending reboot: %v", est) + } + if !st.GetConfigRebootPending() { + t.Error("config_reboot_pending is not set after a reboot-required apply") + } + + // The next boot: it starts from the stored config and the listener runs. + booted := storedConfig(t, fs) + running[cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME] = true + sp, err = NewStatusProvider(StatusConfig{ + Store: s, + Role: cryptosv1.NodeRole_NODE_ROLE_ISSUING, + BootConfig: booted, + ConfigFile: fs, + ProtocolRunning: func(p cryptosv1.ServiceProtocol) bool { return running[p] }, + }) + if err != nil { + t.Fatalf("NewStatusProvider: %v", err) + } + st, err = sp.Status(ctx) + if err != nil { + t.Fatalf("Status: %v", err) + } + acme = protocolState(t, st, cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME) + if !acme.GetConfigured() || !acme.GetRunning() || acme.GetRebootPending() { + t.Errorf("ACME after the reboot = %v, want configured and running with nothing pending", acme) + } + if st.GetConfigRebootPending() { + t.Error("config_reboot_pending survived the reboot") + } +} + +// Without a boot config (maintenance mode) the protocol list stays unset. +func TestStatusProviderWithoutBootConfigLeavesProtocolsUnset(t *testing.T) { + s, ctx := newTestStore(t) + sp, err := NewStatusProvider(StatusConfig{Store: s, Role: cryptosv1.NodeRole_NODE_ROLE_ISSUING}) + if err != nil { + t.Fatalf("NewStatusProvider: %v", err) + } + st, err := sp.Status(ctx) + if err != nil { + t.Fatalf("Status: %v", err) + } + if len(st.GetProtocols()) != 0 || st.GetConfigRebootPending() { + t.Errorf("status without a boot config = protocols %v, pending %v; want unset", st.GetProtocols(), st.GetConfigRebootPending()) + } +} diff --git a/internal/node/providers.go b/internal/node/providers.go index 9fb4153..ef5b086 100644 --- a/internal/node/providers.go +++ b/internal/node/providers.go @@ -23,6 +23,7 @@ import ( "crypto/sha256" "errors" "fmt" + "log" "time" "google.golang.org/grpc/codes" @@ -67,6 +68,17 @@ type StatusConfig struct { // Resolver reports the DNS resolver written at boot; nil leaves // NodeStatus.resolver unset. Resolver func() *cryptosv1.ResolverStatus + // BootConfig is the machine config this boot started from. Nil (a + // maintenance boot, or a test) leaves NodeStatus.protocols unset and + // config_reboot_pending false. + BootConfig *config.Config + // ConfigFile is the stored machine config, read per GetStatus and + // compared with BootConfig: an ApplyConfig lands there and takes effect + // only at the next boot. + ConfigFile *config.FileStore + // ProtocolRunning reports whether a protocol's listener started this + // boot; nil reports every protocol as not running. + ProtocolRunning func(cryptosv1.ServiceProtocol) bool } // StatusProvider adapts a Store + live health probes to grpc.StatusProvider. @@ -109,6 +121,7 @@ func (p *StatusProvider) Status(ctx context.Context) (*cryptosv1.NodeStatus, err if p.cfg.Resolver != nil { resolver = p.cfg.Resolver() } + protocols, rebootPending := p.protocols() return &cryptosv1.NodeStatus{ Role: p.cfg.Role, IdentityState: phase.IdentityState(), @@ -122,9 +135,67 @@ func (p *StatusProvider) Status(ctx context.Context) (*cryptosv1.NodeStatus, err FleetManager: cryptosv1.FleetManagerState_FLEET_MANAGER_STATE_NOT_ENROLLED, RevocationPreflight: preflight, Resolver: resolver, + Protocols: protocols, + ConfigRebootPending: rebootPending, }, nil } +// protocols reports each enrolment protocol's configured and running state, +// and whether the stored config holds a reboot-required change this boot has +// not taken up. Configured comes from the stored config, because that is what +// the next boot starts; running comes from this boot. +// +// A stored config that cannot be read or parsed is reported against the boot +// config with the pending flag set: the next boot would not start from what +// is running now either way, since an unparseable config drops the node to +// maintenance. +func (p *StatusProvider) protocols() ([]*cryptosv1.ProtocolStatus, bool) { + boot := p.cfg.BootConfig + if boot == nil { + return nil, false + } + stored, err := p.storedConfig() + pending := false + if err != nil { + log.Printf("status: read the stored config: %v (reporting a pending reboot)", err) + stored, pending = boot, true + } else { + pending = config.NeedsReboot(boot, stored) + } + running := func(proto cryptosv1.ServiceProtocol) bool { + return p.cfg.ProtocolRunning != nil && p.cfg.ProtocolRunning(proto) + } + out := []*cryptosv1.ProtocolStatus{ + { + Protocol: cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME, + Configured: stored.PKI.ACME != nil, + Running: running(cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME), + RebootPending: !config.Equivalent(boot.PKI.ACME, stored.PKI.ACME), + }, + { + Protocol: cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_EST, + Configured: stored.PKI.EST != nil, + Running: running(cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_EST), + RebootPending: !config.Equivalent(boot.PKI.EST, stored.PKI.EST), + }, + } + return out, pending +} + +func (p *StatusProvider) storedConfig() (*config.Config, error) { + if p.cfg.ConfigFile == nil { + return nil, errors.New("no config store wired") + } + raw, _, ok, err := p.cfg.ConfigFile.Read() + if err != nil { + return nil, err + } + if !ok { + return nil, errors.New("no config stored") + } + return config.Parse(raw) +} + // ConfigStore adapts a config.FileStore to the grpc.ConfigStore interface. type ConfigStore struct { fs *config.FileStore @@ -155,9 +226,11 @@ func (c *ConfigStore) WithIssuer(issuer IssuerFunc) *ConfigStore { var ErrNoConfig = status.Error(codes.FailedPrecondition, "node: no config persisted yet") // Current returns the node's currently persisted machine config, parsed and -// converted to its proto representation. It returns ErrNoConfig if no config -// has been written yet: GetConfig and SetManagement have nothing to read or -// merge into before the first ApplyConfig/install has persisted one. +// converted to its proto representation with the protocol secrets blanked +// (they are write-only; Apply keeps them by identifier). It returns +// ErrNoConfig if no config has been written yet: GetConfig and SetManagement +// have nothing to read or merge into before the first ApplyConfig/install has +// persisted one. func (c *ConfigStore) Current(ctx context.Context) (*cryptosv1.MachineConfig, error) { raw, _, ok, err := c.fs.Read() if err != nil { @@ -170,12 +243,17 @@ func (c *ConfigStore) Current(ctx context.Context) (*cryptosv1.MachineConfig, er if err != nil { return nil, fmt.Errorf("node: Current: parse: %w", err) } - return parsed.ToProto(), nil + return parsed.ToProtoRedacted(), nil } // Apply converts cfg to YAML, validates it, persists it via the FileStore, // and returns the new generation, digest, and whether a reboot is required. // +// The enrolment protocol blocks are resolved against the stored config (see +// config.FromProtoOver): a block the caller left out is kept, and an empty +// secret keeps the stored one for the same identifier. Every protocol change +// is reboot-required, because the listeners start only at boot. +// // A config that fails the schema rules is rejected with codes.InvalidArgument // and nothing is written: the store's generation and contents are unchanged. // This is a live CA, so fail closed -- profiles are read live for signing, and @@ -184,35 +262,34 @@ func (c *ConfigStore) Apply(ctx context.Context, cfg *cryptosv1.MachineConfig) ( if cfg == nil { return nil, status.Error(codes.InvalidArgument, "node: Apply: nil config") } - parsed, err := config.FromProto(cfg) - if err != nil { - return nil, status.Errorf(codes.InvalidArgument, "node: Apply: %v", err) - } // Read the current config before anything is written. It serves two - // purposes: carrying forward what the proto cannot express, and - // classifying whether the change needs a reboot. + // purposes: resolving the protocol blocks and secrets the caller left to + // the node, and classifying whether the change needs a reboot. // // Classify BEFORE overwriting: a change limited to the hot-reconfigurable // fields (cert profiles, root-leaf-issuance acknowledgement) takes effect // live for signing, so the caller need not reboot. Any other change — or a // first apply with no prior config — requires a reboot. Fail safe to reboot // if the current config cannot be read or parsed. - requiresReboot := true + var oldCfg *config.Config if oldRaw, _, ok, rerr := c.fs.Read(); rerr == nil && ok { - if oldCfg, perr := config.Parse(oldRaw); perr == nil { - // MachineConfig has no acme or est field, so a config built from a - // proto has neither. Writing that as the whole config disabled the - // protocols the node was serving (#205), which any Fleet - // Manager-driven apply would do. - parsed.CarryForwardProtoGaps(oldCfg) - requiresReboot = config.NeedsReboot(oldCfg, parsed) + if prev, perr := config.Parse(oldRaw); perr == nil { + oldCfg = prev + } else { + log.Printf("node: Apply: stored config does not parse, nothing to keep from it: %v", perr) } } + parsed, err := config.FromProtoOver(cfg, oldCfg) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "node: Apply: %v", err) + } + requiresReboot := config.NeedsReboot(oldCfg, parsed) + log.Printf("node: Apply: acme=%t est=%t requires_reboot=%t", parsed.PKI.ACME != nil, parsed.PKI.EST != nil, requiresReboot) - // Validate exactly what will be written: after the carry-forward, so a - // carried ACME or EST block is checked against the incoming profiles, and - // before anything touches the store. + // Validate exactly what will be written: after the protocol blocks are + // resolved, so a kept ACME or EST block is checked against the incoming + // profiles, and before anything touches the store. if err := parsed.Validate(); err != nil { return nil, status.Errorf(codes.InvalidArgument, "node: Apply: validate: %v", err) } From 470249eb26b3b3ee52936589249bfef3114c1a95 Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 02:34:21 -0400 Subject: [PATCH 2/5] test(config): build the EST digest fixture instead of hard-coding it 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> --- .gitleaksignore | 1 + internal/config/protocols_test.go | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) create mode 100644 .gitleaksignore diff --git a/.gitleaksignore b/.gitleaksignore new file mode 100644 index 0000000..368ed74 --- /dev/null +++ b/.gitleaksignore @@ -0,0 +1 @@ +37897e3e7a87fbfbf38399b54637e2db1895b4f4:internal/config/protocols_test.go:generic-api-key:30 diff --git a/internal/config/protocols_test.go b/internal/config/protocols_test.go index 06c46af..691826a 100644 --- a/internal/config/protocols_test.go +++ b/internal/config/protocols_test.go @@ -27,8 +27,6 @@ import ( cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" ) -const testPasswordSHA256 = "5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8" - // issuingWithProtocols is a valid issuing-node config serving both protocols, // with one secret in each credential list. func issuingWithProtocols(t *testing.T) *Config { @@ -57,7 +55,7 @@ func issuingWithProtocols(t *testing.T) *Config { Label: "devices", Realm: "cryptos", AllowedIdentifierSuffixes: []string{"devices.example.org"}, - EnrollCredentials: []ESTEnrollCredential{{Username: "router", PasswordSHA256: testPasswordSHA256}}, + EnrollCredentials: []ESTEnrollCredential{{Username: "router", PasswordSHA256: testPasswordDigest()}}, } if err := c.Validate(); err != nil { t.Fatalf("fixture does not validate: %v", err) From 258b964749ac7143443b3e48b048bf4652df2023 Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:30:01 -0400 Subject: [PATCH 3/5] build(deps): pin the api module to main 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> --- go.mod | 2 +- go.sum | 4 +- internal/grpc/unserved.go | 62 ++++++++++++++++++++++++++ internal/grpc/unserved_test.go | 80 ++++++++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 internal/grpc/unserved.go create mode 100644 internal/grpc/unserved_test.go diff --git a/go.mod b/go.mod index 71a6fff..de7482d 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/CryptOS-PKI/cryptos go 1.25.0 require ( - github.com/CryptOS-PKI/api v0.0.0-20260930060427-d48438336d53 + github.com/CryptOS-PKI/api v0.0.0-20260930142054-81494db99991 github.com/google/go-tpm v0.9.8 github.com/google/go-tpm-tools v0.4.8 github.com/spf13/cobra v1.10.2 diff --git a/go.sum b/go.sum index 5e36c18..558e2f8 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/CryptOS-PKI/api v0.0.0-20260930060427-d48438336d53 h1:VEdz0R94JOWFlJPKMEYnos5Wks2kY3+hmfJa24XYDPM= -github.com/CryptOS-PKI/api v0.0.0-20260930060427-d48438336d53/go.mod h1:tmeaYhNoUblqPYYNtTbGBGIUsZ7xLATgLdKq4S33rmE= +github.com/CryptOS-PKI/api v0.0.0-20260930142054-81494db99991 h1:gRd3iY+smHPGl98k/T9Ezc9wG9/8hh+kJ8eZoGOOkW8= +github.com/CryptOS-PKI/api v0.0.0-20260930142054-81494db99991/go.mod h1:tmeaYhNoUblqPYYNtTbGBGIUsZ7xLATgLdKq4S33rmE= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= diff --git a/internal/grpc/unserved.go b/internal/grpc/unserved.go new file mode 100644 index 0000000..c6eb4f8 --- /dev/null +++ b/internal/grpc/unserved.go @@ -0,0 +1,62 @@ +package grpc + +/* +Apache License 2.0 + +Copyright 2026 Shane + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "context" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" +) + +// The SCEP and RFC 3161 TSA RPCs are in the api contract, but this build does +// not serve either protocol yet. Each answers Unimplemented until its server +// lands and replaces the stub here. + +func (s *Server) MintScepChallenge(context.Context, *cryptosv1.MintScepChallengeRequest) (*cryptosv1.MintScepChallengeResponse, error) { + return nil, errSCEPUnserved +} + +func (s *Server) ListScepChallenges(context.Context, *cryptosv1.ListScepChallengesRequest) (*cryptosv1.ListScepChallengesResponse, error) { + return nil, errSCEPUnserved +} + +func (s *Server) RevokeScepChallenge(context.Context, *cryptosv1.RevokeScepChallengeRequest) (*cryptosv1.RevokeScepChallengeResponse, error) { + return nil, errSCEPUnserved +} + +func (s *Server) ListScepEnrollments(context.Context, *cryptosv1.ListScepEnrollmentsRequest) (*cryptosv1.ListScepEnrollmentsResponse, error) { + return nil, errSCEPUnserved +} + +func (s *Server) ApproveScepEnrollment(context.Context, *cryptosv1.ApproveScepEnrollmentRequest) (*cryptosv1.ApproveScepEnrollmentResponse, error) { + return nil, errSCEPUnserved +} + +func (s *Server) RejectScepEnrollment(context.Context, *cryptosv1.RejectScepEnrollmentRequest) (*cryptosv1.RejectScepEnrollmentResponse, error) { + return nil, errSCEPUnserved +} + +func (s *Server) ListTsaCertificates(context.Context, *cryptosv1.ListTsaCertificatesRequest) (*cryptosv1.ListTsaCertificatesResponse, error) { + return nil, status.Error(codes.Unimplemented, "the RFC 3161 time-stamp authority is not served by this build") +} + +var errSCEPUnserved = status.Error(codes.Unimplemented, "SCEP is not served by this build") diff --git a/internal/grpc/unserved_test.go b/internal/grpc/unserved_test.go new file mode 100644 index 0000000..9f3c5f0 --- /dev/null +++ b/internal/grpc/unserved_test.go @@ -0,0 +1,80 @@ +package grpc + +/* +Apache License 2.0 + +Copyright 2026 Shane + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import ( + "context" + "testing" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" +) + +// TestUnservedRPCs_ReturnUnimplemented pins the answer for the NodeService +// RPCs in the api contract that this build does not serve yet: Unimplemented, +// never a panic or a silent empty success. +func TestUnservedRPCs_ReturnUnimplemented(t *testing.T) { + srv, err := New(ServerConfig{ + TLSConfig: newFixtures(t).serverConf, + Auditor: &mockAuditor{}, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + ctx := context.Background() + calls := map[string]func() error{ + "MintScepChallenge": func() error { + _, err := srv.MintScepChallenge(ctx, &cryptosv1.MintScepChallengeRequest{}) + return err + }, + "ListScepChallenges": func() error { + _, err := srv.ListScepChallenges(ctx, &cryptosv1.ListScepChallengesRequest{}) + return err + }, + "RevokeScepChallenge": func() error { + _, err := srv.RevokeScepChallenge(ctx, &cryptosv1.RevokeScepChallengeRequest{}) + return err + }, + "ListScepEnrollments": func() error { + _, err := srv.ListScepEnrollments(ctx, &cryptosv1.ListScepEnrollmentsRequest{}) + return err + }, + "ApproveScepEnrollment": func() error { + _, err := srv.ApproveScepEnrollment(ctx, &cryptosv1.ApproveScepEnrollmentRequest{}) + return err + }, + "RejectScepEnrollment": func() error { + _, err := srv.RejectScepEnrollment(ctx, &cryptosv1.RejectScepEnrollmentRequest{}) + return err + }, + "ListTsaCertificates": func() error { + _, err := srv.ListTsaCertificates(ctx, &cryptosv1.ListTsaCertificatesRequest{}) + return err + }, + } + for name, call := range calls { + t.Run(name, func(t *testing.T) { + if got := status.Code(call()); got != codes.Unimplemented { + t.Errorf("%s code = %v, want Unimplemented", name, got) + } + }) + } +} From 8d8ea36030ee2db4ba8935b42ca834d12b16a7a3 Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:30:01 -0400 Subject: [PATCH 4/5] chore(gitleaks): point the test digest entry at the rebased commit Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> --- .gitleaksignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitleaksignore b/.gitleaksignore index 368ed74..2ada374 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -1 +1 @@ -37897e3e7a87fbfbf38399b54637e2db1895b4f4:internal/config/protocols_test.go:generic-api-key:30 +205dd6475254419977b831d4b9feb35bc2b37caa:internal/config/protocols_test.go:generic-api-key:30 From ce0e20d5780a8e79510299ee8bf5a30b08589404 Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:31:14 -0400 Subject: [PATCH 5/5] docs(est): list ACME before EST in the status example, as cryptosctl prints it Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> --- docs/est.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/est.md b/docs/est.md index ff9079b..83c0138 100644 --- a/docs/est.md +++ b/docs/est.md @@ -225,7 +225,7 @@ off, and leaving the block out of an `ApplyConfig` keeps what the node has. `cryptosctl status` shows what is stored against what is running until then: ```text -Protocols: EST on (not running, reboot pending), ACME off +Protocols: ACME off, EST on (not running, reboot pending) Reboot: pending (the stored config changes take effect at the next boot) ```