diff --git a/.gitleaksignore b/.gitleaksignore new file mode 100644 index 0000000..2ada374 --- /dev/null +++ b/.gitleaksignore @@ -0,0 +1 @@ +205dd6475254419977b831d4b9feb35bc2b37caa:internal/config/protocols_test.go:generic-api-key:30 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..83c0138 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: ACME off, EST on (not running, reboot pending) +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..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-20260929181434-442c13e13c15 + 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 a9fe1de..558e2f8 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-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/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..691826a --- /dev/null +++ b/internal/config/protocols_test.go @@ -0,0 +1,377 @@ +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" +) + +// 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: testPasswordDigest()}}, + } + 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/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) + } + }) + } +} 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) }