From 2db782e4a37b00d77bd06eaa86be610d34b35909 Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:37:18 -0400 Subject: [PATCH 1/4] feat(config): keep a switched-off ACME or EST block's settings An enabled=false block now stores its settings, and GetConfig returns them with enabled=false, so a reader can switch the protocol back on by flipping enabled alone. This is how the Fleet Manager switches a protocol, and switching on from its read-back used to be refused for a missing base_url and profile. - The settings sit apart from the on block, so nothing reads them at boot and changing only them is not a reboot. - Secrets stay write-only: GetConfig blanks them in an off block, and a blank secret keeps the stored one whether the stored block is on or off. - An empty enabled=false block still drops the settings, and an absent block keeps what the node has, on or off. - In YAML the block carries enabled: false; a block without the flag is on, as stored configs written before it are. - A Root still refuses a protocol switched on, and accepts an off block. Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> --- internal/config/config.go | 29 ++- internal/config/protocols.go | 210 ++++++++++++--- internal/config/protocols_disabled_test.go | 289 +++++++++++++++++++++ internal/node/protocols_disabled_test.go | 147 +++++++++++ 4 files changed, 638 insertions(+), 37 deletions(-) create mode 100644 internal/config/protocols_disabled_test.go create mode 100644 internal/node/protocols_disabled_test.go diff --git a/internal/config/config.go b/internal/config/config.go index b822ef2..4a7d84c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -304,6 +304,14 @@ type PKI struct { // SCEP configures the RFC 8894 enrolment endpoint (see SCEP). Nil is // off, the default. Unlike ACME and EST it is carried in the proto. SCEP *SCEP `yaml:"scep"` + // DisabledACME and DisabledEST keep the settings of a protocol block + // that is switched off (enabled: false), so the same block can be + // switched back on without re-entering them. Nothing reads them at boot: + // a protocol runs only from ACME or EST. At most one of ACME and + // DisabledACME is set, and likewise for EST. In YAML a disabled block is + // the acme or est block with enabled: false. + DisabledACME *ACME `yaml:"-" json:",omitempty"` + DisabledEST *EST `yaml:"-" json:",omitempty"` } // EST configures the node's RFC 7030 server. @@ -521,12 +529,17 @@ func decode(raw []byte) (*Config, error) { if len(raw) == 0 { return nil, errors.New("config: empty input") } + raw, flags, err := popProtocolFlags(raw) + if err != nil { + return nil, err + } var c Config dec := yaml.NewDecoder(strings.NewReader(string(raw))) dec.KnownFields(true) if err := dec.Decode(&c); err != nil { return nil, fmt.Errorf("config: parse YAML: %w", err) } + c.applyProtocolFlags(flags) return &c, nil } @@ -1088,9 +1101,13 @@ func marshalSorted(v interface{}) ([]byte, error) { } } -// Marshal renders c as the canonical machine.yaml document. +// Marshal renders c as the canonical machine.yaml document. A disabled +// protocol block is written as its block with enabled: false. func (c *Config) Marshal() ([]byte, error) { - return yaml.Marshal(c) + if c.PKI.DisabledACME == nil && c.PKI.DisabledEST == nil { + return yaml.Marshal(c) + } + return marshalWithDisabledBlocks(c) } // FromProto converts a proto MachineConfig back to a Config. It is the @@ -1146,8 +1163,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) + c.PKI.ACME, c.PKI.DisabledACME = acmeFromProto(pb.Pki.Acme) + c.PKI.EST, c.PKI.DisabledEST = estFromProto(pb.Pki.Est) c.PKI.SCEP = scepFromProto(pb.Pki.Scep) } if pb.Install != nil { @@ -1193,8 +1210,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), + Acme: acmeToProto(c.PKI.ACME, c.PKI.DisabledACME), + Est: estToProto(c.PKI.EST, c.PKI.DisabledEST), AllowUnsyncedClock: c.PKI.AllowUnsyncedClock, Scep: scepToProto(c.PKI.SCEP), } diff --git a/internal/config/protocols.go b/internal/config/protocols.go index 709a322..094f5db 100644 --- a/internal/config/protocols.go +++ b/internal/config/protocols.go @@ -19,6 +19,9 @@ limitations under the License. import ( "errors" "fmt" + "reflect" + + "gopkg.in/yaml.v3" cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" ) @@ -29,18 +32,27 @@ import ( // - 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". +// missing block, into a nil ACME or EST, which is how this package spells +// "off". +// - An off block keeps its settings. FromProto stores the fields of an +// enabled=false block in DisabledACME or DisabledEST, and ToProto sends +// them back with enabled=false, so a reader can switch the protocol back +// on by flipping enabled alone. An empty enabled=false block stores +// nothing. In YAML the same block carries enabled: false. // - 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. +// stored one, on or off. A client that predates a block therefore cannot +// switch a protocol off by leaving it out. +// - Secrets are write-only, in an off block too. ToProtoRedacted blanks +// them for GetConfig, and FromProtoOver fills an empty secret from the +// stored entry with the same identifier, on or off, 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. +// listeners), so Validate refuses a Root with either protocol on. An off +// block is inert and allowed. +// - Every field of an on block is read at boot, so NeedsReboot classifies +// any change to one, including the switch itself, as reboot-required. An +// off block's settings are read by nothing, so changing only them is live. // validateProtocolRole refuses an enrolment protocol on a Root. func validateProtocolRole(role RoleKind, p PKI) error { @@ -82,23 +94,22 @@ func FromProtoOver(pb *cryptosv1.MachineConfig, prev *Config) (*Config, error) { if err != nil { return nil, err } - var prevACME *ACME - var prevEST *EST + var prevPKI PKI if prev != nil { - prevACME, prevEST = prev.PKI.ACME, prev.PKI.EST + prevPKI = prev.PKI } if pb.GetPki().GetAcme() == nil { - c.PKI.ACME = prevACME - } else if c.PKI.ACME != nil { - if err := keepACMESecrets(c.PKI.ACME, prevACME); err != nil { + c.PKI.ACME, c.PKI.DisabledACME = prevPKI.ACME, prevPKI.DisabledACME + } else if next := firstNonNil(c.PKI.ACME, c.PKI.DisabledACME); next != nil { + if err := keepACMESecrets(next, firstNonNil(prevPKI.ACME, prevPKI.DisabledACME)); 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 { + c.PKI.EST, c.PKI.DisabledEST = prevPKI.EST, prevPKI.DisabledEST + } else if next := firstNonNil(c.PKI.EST, c.PKI.DisabledEST); next != nil { + if err := keepESTSecrets(next, firstNonNil(prevPKI.EST, prevPKI.DisabledEST)); err != nil { return nil, err } } @@ -106,6 +117,22 @@ func FromProtoOver(pb *cryptosv1.MachineConfig, prev *Config) (*Config, error) { return c, nil } +// firstNonNil returns on when the protocol is on, else its off block. +func firstNonNil[T any](on, off *T) *T { + if on != nil { + return on + } + return off +} + +// offBlock returns b as a stored off block, or nil when it holds no settings. +func offBlock[T any](b *T) *T { + if b == nil || reflect.ValueOf(*b).IsZero() { + return nil + } + return b +} + func keepACMESecrets(next, prev *ACME) error { stored := map[string]string{} if prev != nil { @@ -150,12 +177,15 @@ func keepESTSecrets(next, prev *EST) error { return nil } -func acmeToProto(a *ACME) *cryptosv1.Acme { +// acmeToProto renders the ACME block: on's settings with enabled=true, else +// off's with enabled=false. +func acmeToProto(on, off *ACME) *cryptosv1.Acme { + a := firstNonNil(on, off) if a == nil { return &cryptosv1.Acme{Enabled: false} } pb := &cryptosv1.Acme{ - Enabled: true, + Enabled: on != nil, BaseUrl: a.BaseURL, HttpPort: a.HTTPPort, Profile: a.Profile, @@ -174,9 +204,11 @@ func acmeToProto(a *ACME) *cryptosv1.Acme { return pb } -func acmeFromProto(pb *cryptosv1.Acme) *ACME { - if !pb.GetEnabled() { - return nil +// acmeFromProto returns the block as on (enabled=true) or off settings +// (enabled=false); a missing or empty off block is neither. +func acmeFromProto(pb *cryptosv1.Acme) (on, off *ACME) { + if pb == nil { + return nil, nil } a := &ACME{ BaseURL: pb.GetBaseUrl(), @@ -194,15 +226,21 @@ func acmeFromProto(pb *cryptosv1.Acme) *ACME { HMACKeyBase64: k.GetHmacKeyBase64(), }) } - return a + if pb.GetEnabled() { + return a, nil + } + return nil, offBlock(a) } -func estToProto(e *EST) *cryptosv1.Est { +// estToProto renders the EST block: on's settings with enabled=true, else +// off's with enabled=false. +func estToProto(on, off *EST) *cryptosv1.Est { + e := firstNonNil(on, off) if e == nil { return &cryptosv1.Est{Enabled: false} } pb := &cryptosv1.Est{ - Enabled: true, + Enabled: on != nil, Hostnames: e.Hostnames, HttpPort: e.HTTPPort, Profile: e.Profile, @@ -220,9 +258,11 @@ func estToProto(e *EST) *cryptosv1.Est { return pb } -func estFromProto(pb *cryptosv1.Est) *EST { - if !pb.GetEnabled() { - return nil +// estFromProto returns the block as on (enabled=true) or off settings +// (enabled=false); a missing or empty off block is neither. +func estFromProto(pb *cryptosv1.Est) (on, off *EST) { + if pb == nil { + return nil, nil } e := &EST{ Hostnames: pb.GetHostnames(), @@ -239,5 +279,113 @@ func estFromProto(pb *cryptosv1.Est) *EST { PasswordSHA256: cred.GetPasswordSha256(), }) } - return e + if pb.GetEnabled() { + return e, nil + } + return nil, offBlock(e) +} + +// protocolKeys are the pki keys whose blocks carry an enabled flag in YAML. +var protocolKeys = []string{"acme", "est"} + +// popProtocolFlags removes the enabled flag from the pki.acme and pki.est +// blocks of a YAML document, which the Config structs do not carry, and +// returns the document without them plus the flags it found. A document with +// no flag comes back unchanged. +func popProtocolFlags(raw []byte) ([]byte, map[string]bool, error) { + var doc yaml.Node + if err := yaml.Unmarshal(raw, &doc); err != nil { + return nil, nil, fmt.Errorf("config: parse YAML: %w", err) + } + pki := mappingValue(&doc, "pki") + flags := map[string]bool{} + for _, key := range protocolKeys { + block := mappingValue(pki, key) + if block == nil || block.Kind != yaml.MappingNode { + continue + } + for i := 0; i+1 < len(block.Content); i += 2 { + if block.Content[i].Value != "enabled" { + continue + } + var on bool + if err := block.Content[i+1].Decode(&on); err != nil { + return nil, nil, fmt.Errorf("config: pki.%s.enabled: must be true or false: %w", key, err) + } + flags[key] = on + block.Content = append(block.Content[:i], block.Content[i+2:]...) + break + } + } + if len(flags) == 0 { + return raw, flags, nil + } + out, err := yaml.Marshal(&doc) + if err != nil { + return nil, nil, fmt.Errorf("config: parse YAML: %w", err) + } + return out, flags, nil +} + +// applyProtocolFlags moves a block decoded with enabled: false to its off +// field. A block without the flag is on. +func (c *Config) applyProtocolFlags(flags map[string]bool) { + if on, ok := flags["acme"]; ok && !on { + c.PKI.ACME, c.PKI.DisabledACME = nil, offBlock(c.PKI.ACME) + } + if on, ok := flags["est"]; ok && !on { + c.PKI.EST, c.PKI.DisabledEST = nil, offBlock(c.PKI.EST) + } +} + +// marshalWithDisabledBlocks renders c with each off block written in its +// protocol's place under enabled: false. +func marshalWithDisabledBlocks(c *Config) ([]byte, error) { + out := *c + var off []string + if out.PKI.ACME == nil && out.PKI.DisabledACME != nil { + out.PKI.ACME = out.PKI.DisabledACME + off = append(off, "acme") + } + if out.PKI.EST == nil && out.PKI.DisabledEST != nil { + out.PKI.EST = out.PKI.DisabledEST + off = append(off, "est") + } + var doc yaml.Node + if err := doc.Encode(&out); err != nil { + return nil, fmt.Errorf("config: marshal: %w", err) + } + pki := mappingValue(&doc, "pki") + for _, key := range off { + block := mappingValue(pki, key) + if block == nil || block.Kind != yaml.MappingNode { + return nil, fmt.Errorf("config: marshal: pki.%s did not render as a block", key) + } + flag := []*yaml.Node{ + {Kind: yaml.ScalarNode, Tag: "!!str", Value: "enabled"}, + {Kind: yaml.ScalarNode, Tag: "!!bool", Value: "false"}, + } + block.Content = append(flag, block.Content...) + } + return yaml.Marshal(&doc) +} + +// mappingValue returns the value under key in the mapping n (or in the +// document n wraps), or nil. +func mappingValue(n *yaml.Node, key string) *yaml.Node { + if n == nil { + return nil + } + if n.Kind == yaml.DocumentNode && len(n.Content) == 1 { + n = n.Content[0] + } + if n.Kind != yaml.MappingNode { + return nil + } + for i := 0; i+1 < len(n.Content); i += 2 { + if n.Content[i].Value == key { + return n.Content[i+1] + } + } + return nil } diff --git a/internal/config/protocols_disabled_test.go b/internal/config/protocols_disabled_test.go new file mode 100644 index 0000000..7b923f5 --- /dev/null +++ b/internal/config/protocols_disabled_test.go @@ -0,0 +1,289 @@ +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 ( + "reflect" + "strings" + "testing" + + "google.golang.org/protobuf/proto" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" +) + +// readBackOff is what the Fleet Manager applies to switch a protocol off: the +// node's redacted read-back with only enabled flipped. +func readBackOff(c *Config) *cryptosv1.MachineConfig { + pb := c.ToProtoRedacted() + pb.Pki.Acme.Enabled = false + pb.Pki.Est.Enabled = false + return pb +} + +func TestDisabledBlockRoundTripsThroughProto(t *testing.T) { + prev := issuingWithProtocols(t) + got, err := FromProtoOver(readBackOff(prev), prev) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + if got.PKI.ACME != nil || got.PKI.EST != nil { + t.Fatalf("enabled=false left a protocol on: acme=%+v est=%+v", got.PKI.ACME, got.PKI.EST) + } + if err := got.Validate(); err != nil { + t.Fatalf("a config with disabled blocks does not validate: %v", err) + } + + pb := got.ToProtoRedacted() + a, e := pb.GetPki().GetAcme(), pb.GetPki().GetEst() + if a.GetEnabled() || e.GetEnabled() { + t.Fatalf("a disabled block read back as enabled: acme=%v est=%v", a, e) + } + if a.GetBaseUrl() != prev.PKI.ACME.BaseURL || a.GetProfile() != prev.PKI.ACME.Profile || + a.GetHttpPort() != prev.PKI.ACME.HTTPPort || a.GetOrderTtlHours() != prev.PKI.ACME.OrderTTLHours || + !reflect.DeepEqual(a.GetAllowedIdentifierSuffixes(), prev.PKI.ACME.AllowedIdentifierSuffixes) { + t.Errorf("the disabled ACME block lost its settings: %v", a) + } + if len(a.GetExternalAccountKeys()) != 1 || a.GetExternalAccountKeys()[0].GetKeyId() != "ops" { + t.Errorf("the disabled ACME block lost its key_id: %v", a.GetExternalAccountKeys()) + } + if !reflect.DeepEqual(e.GetHostnames(), prev.PKI.EST.Hostnames) || e.GetProfile() != prev.PKI.EST.Profile || + e.GetLabel() != prev.PKI.EST.Label || e.GetRealm() != prev.PKI.EST.Realm || e.GetHttpPort() != prev.PKI.EST.HTTPPort { + t.Errorf("the disabled EST block lost its settings: %v", e) + } + if len(e.GetEnrollCredentials()) != 1 || e.GetEnrollCredentials()[0].GetUsername() != "router" { + t.Errorf("the disabled EST block lost its username: %v", e.GetEnrollCredentials()) + } +} + +func TestDisabledBlockKeepsItsSecretsWriteOnly(t *testing.T) { + prev := issuingWithProtocols(t) + got, err := FromProtoOver(readBackOff(prev), prev) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + pb := got.ToProtoRedacted() + if s := pb.GetPki().GetAcme().GetExternalAccountKeys()[0].GetHmacKeyBase64(); s != "" { + t.Errorf("GetConfig returned a disabled block's EAB secret %q", s) + } + if s := pb.GetPki().GetEst().GetEnrollCredentials()[0].GetPasswordSha256(); s != "" { + t.Errorf("GetConfig returned a disabled block's password digest %q", s) + } + + // The node stores the kept secrets, so the block can be switched back on. + full := got.ToProto() + if full.GetPki().GetAcme().GetExternalAccountKeys()[0].GetHmacKeyBase64() != validEABKey() { + t.Error("the disabled ACME block did not keep the stored EAB secret") + } + if full.GetPki().GetEst().GetEnrollCredentials()[0].GetPasswordSha256() != testPasswordDigest() { + t.Error("the disabled EST block did not keep the stored password digest") + } + + // A blank secret for an identifier the node never stored is still refused. + bad := readBackOff(prev) + bad.Pki.Acme.ExternalAccountKeys = append(bad.Pki.Acme.ExternalAccountKeys, + &cryptosv1.AcmeExternalAccountKey{KeyId: "new-team"}) + if _, err := FromProtoOver(bad, prev); err == nil || !strings.Contains(err.Error(), `"new-team"`) { + t.Errorf("a blank secret for an unknown key_id in a disabled block = %v, want a refusal naming it", err) + } +} + +// The Fleet Manager switches a protocol on by reading the config back and +// flipping enabled, so a read-back of a disabled block has to be enough. +func TestSwitchOnFromReadBack(t *testing.T) { + on := issuingWithProtocols(t) + off, err := FromProtoOver(readBackOff(on), on) + if err != nil { + t.Fatalf("FromProtoOver (off): %v", err) + } + + pb := off.ToProtoRedacted() + pb.Pki.Acme.Enabled = true + pb.Pki.Est.Enabled = true + got, err := FromProtoOver(pb, off) + if err != nil { + t.Fatalf("FromProtoOver (on from the read-back): %v", err) + } + if err := got.Validate(); err != nil { + t.Fatalf("the switched-on config does not validate: %v", err) + } + if !reflect.DeepEqual(got.PKI.ACME, on.PKI.ACME) { + t.Errorf("ACME after switching back on = %+v, want %+v", got.PKI.ACME, on.PKI.ACME) + } + if !reflect.DeepEqual(got.PKI.EST, on.PKI.EST) { + t.Errorf("EST after switching back on = %+v, want %+v", got.PKI.EST, on.PKI.EST) + } + if !NeedsReboot(off, got) { + t.Error("switching a protocol back on was classified as live") + } +} + +// An empty enabled=false block is still how a client switches a protocol off +// and drops its settings. +func TestEmptyDisabledBlockDropsSettings(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 a := got.ToProto().GetPki().GetAcme(); a.GetEnabled() || a.GetBaseUrl() != "" || a.GetProfile() != "" { + t.Errorf("an empty enabled=false block kept settings: %v", a) + } +} + +// An absent block keeps what the node stores, disabled settings included. +func TestAbsentBlockKeepsDisabledSettings(t *testing.T) { + on := issuingWithProtocols(t) + off, err := FromProtoOver(readBackOff(on), on) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + pb := off.ToProto() + pb.Pki.Acme, pb.Pki.Est = nil, nil + got, err := FromProtoOver(pb, off) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + if got.ToProto().GetPki().GetAcme().GetBaseUrl() != on.PKI.ACME.BaseURL || + got.ToProto().GetPki().GetEst().GetProfile() != on.PKI.EST.Profile { + t.Error("an apply without the blocks dropped the disabled settings") + } +} + +func TestDisabledBlockRoundTripsThroughYAML(t *testing.T) { + on := issuingWithProtocols(t) + off, err := FromProtoOver(readBackOff(on), on) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + raw, err := off.Marshal() + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if !strings.Contains(string(raw), "enabled: false") { + t.Fatalf("the stored YAML does not mark the disabled blocks:\n%s", raw) + } + got, err := Parse(raw) + if err != nil { + t.Fatalf("Parse of the stored YAML: %v", err) + } + if got.PKI.ACME != nil || got.PKI.EST != nil { + t.Fatalf("a stored disabled block parsed as on: acme=%+v est=%+v", got.PKI.ACME, got.PKI.EST) + } + if !proto.Equal(got.ToProto(), off.ToProto()) { + t.Errorf("the YAML round trip changed the config:\n got %v\nwant %v", got.ToProto(), off.ToProto()) + } +} + +func TestYAMLEnabledFlag(t *testing.T) { + block := ` acme: + enabled: %s + base_url: https://ca.example.org/acme + profile: leaf-server + allow_anonymous_accounts: true +` + withProfile := string(issuingYAML(t)) + ` profiles: + - name: leaf-server + key_alg: ECDSA-P384 + validity_days: 90 + key_usage: [digital_signature] + ext_key_usage: [server_auth] +` + for _, tc := range []struct { + flag string + on bool + }{{"true", true}, {"false", false}} { + t.Run(tc.flag, func(t *testing.T) { + c, err := Parse([]byte(withProfile + strings.Replace(block, "%s", tc.flag, 1))) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if (c.PKI.ACME != nil) != tc.on { + t.Errorf("enabled: %s parsed as on=%t", tc.flag, c.PKI.ACME != nil) + } + }) + } + // A block without the flag is on, as stored configs written before the + // flag existed are. + noFlag := strings.Replace(block, " enabled: %s\n", "", 1) + c, err := Parse([]byte(withProfile + noFlag)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if c.PKI.ACME == nil { + t.Error("an acme block without enabled parsed as off") + } + if _, err := Parse([]byte(withProfile + strings.Replace(block, "%s", "maybe", 1))); err == nil { + t.Error("a non-boolean enabled was accepted") + } +} + +// Nothing reads a disabled block at boot, so changing only its settings is +// not a reboot. +func TestDisabledSettingsChangeIsLive(t *testing.T) { + on := issuingWithProtocols(t) + off, err := FromProtoOver(readBackOff(on), on) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + pb := off.ToProto() + pb.Pki.Acme.HttpPort = 9000 + next, err := FromProtoOver(pb, off) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + if NeedsReboot(off, next) { + t.Error("a settings change to a disabled block asked for a reboot") + } +} + +func TestRootStillRefusesAnEnabledProtocol(t *testing.T) { + on := issuingWithProtocols(t) + off, err := FromProtoOver(readBackOff(on), on) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + off.Role.Kind = RoleRoot + off.PKI.Parent = nil + off.PKI.RootValidityYears = 20 + if err := off.Validate(); err != nil { + t.Fatalf("a root holding only disabled blocks was refused: %v", err) + } + + for _, name := range []string{"acme", "est"} { + t.Run(name, func(t *testing.T) { + pb := off.ToProtoRedacted() + if name == "acme" { + pb.Pki.Acme.Enabled = true + } else { + pb.Pki.Est.Enabled = true + } + got, err := FromProtoOver(pb, off) + if err != nil { + t.Fatalf("FromProtoOver: %v", err) + } + err = got.Validate() + if err == nil || !strings.Contains(err.Error(), "pki."+name) || !strings.Contains(err.Error(), "root") { + t.Errorf("switching %s on at a root = %v, want a root-role refusal", name, err) + } + }) + } +} diff --git a/internal/node/protocols_disabled_test.go b/internal/node/protocols_disabled_test.go new file mode 100644 index 0000000..090a950 --- /dev/null +++ b/internal/node/protocols_disabled_test.go @@ -0,0 +1,147 @@ +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 ( + "context" + "testing" + + cryptosv1 "github.com/CryptOS-PKI/api/go/cryptos/v1" +) + +// The Fleet Manager switches a protocol by reading the config, flipping +// enabled and applying it back. Off keeps the block's settings on the node, so +// the same flip switches it back on, with the secret kept and never returned. +func TestConfigStoreApply_SwitchOffAndBackOnFromReadBack(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) + } + + // Off, from the read-back. + current, err := cs.Current(ctx) + if err != nil { + t.Fatalf("Current: %v", err) + } + current.Pki.Acme.Enabled = false + resp, err := cs.Apply(ctx, current) + if err != nil { + t.Fatalf("Apply (ACME off from the read-back): %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.Fatalf("ACME is still on after switching it off: %+v", stored.PKI.ACME) + } + + 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" || a.GetProfile() != "leaf-server" { + t.Fatalf("GetConfig of a switched-off ACME = %v, want enabled=false with its settings", a) + } + if keys := a.GetExternalAccountKeys(); len(keys) != 1 || keys[0].GetKeyId() != "ops" || keys[0].GetHmacKeyBase64() != "" { + t.Errorf("GetConfig of a switched-off ACME returned keys %v, want key_id ops with the secret blank", keys) + } + + // Back on, from the read-back, flipping enabled only. + current.Pki.Acme.Enabled = true + resp, err = cs.Apply(ctx, current) + if err != nil { + t.Fatalf("Apply (ACME on from the read-back): %v", err) + } + if !resp.GetRequiresReboot() { + t.Error("switching ACME back on did not answer requires_reboot") + } + stored := storedConfig(t, fs) + if stored.PKI.ACME == nil || stored.PKI.ACME.BaseURL != "https://ca.example.org/acme" || + stored.PKI.ACME.ExternalAccountKeys[0].HMACKeyBase64 != testEABKey { + t.Fatalf("stored ACME after switching back on = %+v, want the block with its secret", stored.PKI.ACME) + } +} + +func TestConfigStoreApply_ESTSwitchOnFromReadBack(t *testing.T) { + ctx := context.Background() + fs, cs := seededStore(t, protocolsOffSeed) + pb := storedConfig(t, fs).ToProto() + pb.Pki.Est = &cryptosv1.Est{ + Enabled: false, + Hostnames: []string{"est.example.org"}, + Profile: "leaf-server", + Label: "devices", + } + resp, err := cs.Apply(ctx, pb) + if err != nil { + t.Fatalf("Apply (EST settings, off): %v", err) + } + if resp.GetRequiresReboot() { + t.Error("storing the settings of an EST block that stays off asked for a reboot") + } + + current, err := cs.Current(ctx) + if err != nil { + t.Fatalf("Current: %v", err) + } + if e := current.GetPki().GetEst(); e.GetEnabled() || e.GetProfile() != "leaf-server" || e.GetLabel() != "devices" { + t.Fatalf("GetConfig of an off EST block = %v, want its settings with enabled=false", e) + } + current.Pki.Est.Enabled = true + if _, err := cs.Apply(ctx, current); err != nil { + t.Fatalf("Apply (EST on from the read-back): %v", err) + } + if stored := storedConfig(t, fs); stored.PKI.EST == nil || stored.PKI.EST.Label != "devices" { + t.Fatalf("stored EST after switching on = %+v, want the block", stored.PKI.EST) + } +} + +// An off block is not a configured protocol. +func TestStatusProviderOffBlockIsNotConfigured(t *testing.T) { + ctx := context.Background() + s, _ := newTestStore(t) + fs, cs := seededStore(t, protocolsOffSeed) + pb := storedConfig(t, fs).ToProto() + pb.Pki.Acme = acmeOn() + pb.Pki.Acme.Enabled = false + if _, err := cs.Apply(ctx, pb); err != nil { + t.Fatalf("Apply (ACME settings, off): %v", err) + } + sp, err := NewStatusProvider(StatusConfig{ + Store: s, + Role: cryptosv1.NodeRole_NODE_ROLE_ISSUING, + BootConfig: storedConfig(t, fs), + ConfigFile: fs, + ProtocolRunning: func(cryptosv1.ServiceProtocol) bool { return false }, + }) + if err != nil { + t.Fatalf("NewStatusProvider: %v", err) + } + st, err := sp.Status(ctx) + if err != nil { + t.Fatalf("Status: %v", err) + } + if acme := protocolState(t, st, cryptosv1.ServiceProtocol_SERVICE_PROTOCOL_ACME); acme.GetConfigured() || acme.GetRebootPending() { + t.Errorf("an off ACME block with settings reports %v, want not configured and nothing pending", acme) + } +} From 9839f60dc0d2c9ea0ab57d46aadb5265bc4cd15e Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:37:18 -0400 Subject: [PATCH 2/4] docs(config): describe switching ACME and EST off with their settings kept Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> --- docs/acme.md | 34 +++++++++++++++++++++++++++------- docs/est.md | 34 +++++++++++++++++++++++++++------- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/docs/acme.md b/docs/acme.md index c4d2139..07e80d4 100644 --- a/docs/acme.md +++ b/docs/acme.md @@ -140,17 +140,37 @@ 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 +protocol, so a config that switches `pki.acme` on at 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. +> Don't switch `pki.acme` on in a Root's config. It is refused, and nothing +> is stored. Serve ACME from an issuing node under that Root instead. A +> Root accepts a block with `enabled: false`, and never serves it. 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. +switch it off and keep its settings, set `enabled: false` in the block and +apply again; to switch it back on, set `enabled: true` (or remove the line). +A block without `enabled` is on. Removing the block switches ACME off and +drops its settings. + +```yaml +pki: + acme: + enabled: false + # the rest of the block stays as it was +``` + +The node stores a switched-off block's settings and `config get` prints the +block with `enabled: false`, so flipping `enabled` alone is enough to switch +it back on. The Fleet Manager switches a protocol the same way. Nothing reads +a switched-off block at boot, so changing only its settings does not need a +reboot. + +Over the API the block is `Pki.acme` with an explicit `enabled` flag. +`enabled: false` switches it off and keeps the settings sent with it, an +`enabled: false` block with no settings drops them, and leaving the block out +of an `ApplyConfig` keeps what the node has, on or off. > [!WARNING] > Every ACME change, switching it on or off included, takes effect at the @@ -167,7 +187,7 @@ Protocols: ACME on (not running, reboot pending), EST off, SCEP 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 +`hmac_key_base64` is write-only, in a switched-off block too. `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 diff --git a/docs/est.md b/docs/est.md index d33b384..b10c48f 100644 --- a/docs/est.md +++ b/docs/est.md @@ -202,17 +202,37 @@ 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 +protocol, so a config that switches `pki.est` on at 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. +> Don't switch `pki.est` on in a Root's config. It is refused, and nothing +> is stored. Serve EST from an issuing node under that Root instead. A +> Root accepts a block with `enabled: false`, and never serves it. 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. +switch it off and keep its settings, set `enabled: false` in the block and +apply again; to switch it back on, set `enabled: true` (or remove the line). +A block without `enabled` is on. Removing the block switches EST off and +drops its settings. + +```yaml +pki: + est: + enabled: false + # the rest of the block stays as it was +``` + +The node stores a switched-off block's settings and `config get` prints the +block with `enabled: false`, so flipping `enabled` alone is enough to switch +it back on. The Fleet Manager switches a protocol the same way. Nothing reads +a switched-off block at boot, so changing only its settings does not need a +reboot. + +Over the API the block is `Pki.est` with an explicit `enabled` flag. +`enabled: false` switches it off and keeps the settings sent with it, an +`enabled: false` block with no settings drops them, and leaving the block out +of an `ApplyConfig` keeps what the node has, on or off. > [!WARNING] > Every EST change, switching it on or off included, takes effect at the @@ -229,7 +249,7 @@ Protocols: ACME off, EST on (not running, reboot pending), SCEP 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 +`password_sha256` is write-only, in a switched-off block too. `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 From db3d32d47d4324319401aba4cd014e0bd7676884 Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:04:33 -0400 Subject: [PATCH 3/4] chore(license): stamp the CryptOS Authors header on the disabled-protocol tests Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> --- internal/config/protocols_disabled_test.go | 4 +--- internal/node/protocols_disabled_test.go | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/config/protocols_disabled_test.go b/internal/config/protocols_disabled_test.go index 7b923f5..d217132 100644 --- a/internal/config/protocols_disabled_test.go +++ b/internal/config/protocols_disabled_test.go @@ -1,9 +1,7 @@ package config /* -Apache License 2.0 - -Copyright 2026 Shane +Copyright The CryptOS Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/node/protocols_disabled_test.go b/internal/node/protocols_disabled_test.go index 090a950..ead80ff 100644 --- a/internal/node/protocols_disabled_test.go +++ b/internal/node/protocols_disabled_test.go @@ -1,9 +1,7 @@ package node /* -Apache License 2.0 - -Copyright 2026 Shane +Copyright The CryptOS Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 1cd45b626c5e0e9db9617581c52e2e14b1666fa4 Mon Sep 17 00:00:00 2001 From: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:07:01 -0400 Subject: [PATCH 4/4] docs(cryptosctl): say a switched-off block's settings change needs no reboot The config get help and the ACME and EST warnings said every protocol change waits for a reboot. Changing only the settings of a block with enabled: false is live, so say so. Signed-off-by: Bugs5382 <12115015+Bugs5382@users.noreply.github.com> --- cmd/cryptosctl/config.go | 5 +++-- docs/acme.md | 4 ++-- docs/est.md | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/cryptosctl/config.go b/cmd/cryptosctl/config.go index 6016ef9..907defb 100644 --- a/cmd/cryptosctl/config.go +++ b/cmd/cryptosctl/config.go @@ -60,8 +60,9 @@ func newConfigGetCmd(opts *globalOpts) *cobra.Command { "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.", + "secret. Switching a protocol on or off, or changing the settings of one " + + "that is on, takes effect at the next reboot. A block with enabled: false " + + "keeps its settings; changing only those needs no reboot.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { client, closeConn, err := dial(opts) diff --git a/docs/acme.md b/docs/acme.md index 07e80d4..29033eb 100644 --- a/docs/acme.md +++ b/docs/acme.md @@ -173,8 +173,8 @@ Over the API the block is `Pki.acme` with an explicit `enabled` flag. of an `ApplyConfig` keeps what the node has, on or off. > [!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 +> Switching ACME on or off, or changing its settings while it is on, 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 diff --git a/docs/est.md b/docs/est.md index b10c48f..092510b 100644 --- a/docs/est.md +++ b/docs/est.md @@ -235,8 +235,8 @@ Over the API the block is `Pki.est` with an explicit `enabled` flag. of an `ApplyConfig` keeps what the node has, on or off. > [!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 +> Switching EST on or off, or changing its settings while it is on, 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