From 2b3f6bee40f03733741907f4807ed750515e1287 Mon Sep 17 00:00:00 2001 From: Yi-Min Lin Date: Wed, 7 Oct 2026 16:52:55 -0700 Subject: [PATCH] Bind deployed executors before requiring client certificates With tls.require_client_cert on, the dispatcher admits an executor only over the certificate bound to its ID in executor_enrollments. Executors deployed with generate-certs.sh and deploy-certs.yml never got a binding, so turning the requirement on disconnected the whole fleet. Add `debuglet-dispatcher -bind-executor ID -bind-certificate PATH`, which verifies an administrator-issued PEM certificate against tls.ca_file for client authentication, requires the ID as common name, refuses private key material and records the leaf fingerprint through the enrollment store. It is idempotent and replaces only that ID's binding. The deployment now binds every inventory executor from its deployed client.crt (public material only) as the service account: the dispatcher role and update-config.yml before the dispatcher runs with the requirement, and deploy-certs.yml on an enforcing dispatcher so a reissued certificate is re-bound. executor_enrollment_token renders [credentials] enrollment_token for executors bound by token instead, and the preflight refuses enforcement while an inventory executor has neither. Docs no longer imply a CA-issued certificate is sufficient. Fixes #415 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 26 +++ cmd/dispatcher/bind_executor_test.go | 196 ++++++++++++++++++ cmd/dispatcher/init_database_test.go | 2 +- cmd/dispatcher/main.go | 94 ++++++++- deploy/README.md | 90 +++++++- deploy/ansible/deploy-certs.yml | 54 +++++ deploy/ansible/group_vars/dispatcher.yml | 24 ++- deploy/ansible/group_vars/executors.yml | 9 + deploy/ansible/preflight-variables.yml | 65 ++++++ .../ansible/roles/dispatcher/tasks/main.yml | 10 + .../roles/executor/templates/executor.toml.j2 | 5 + deploy/ansible/tasks/bind-executors.yml | 81 ++++++++ deploy/ansible/update-config.yml | 9 + deploy/test/ansible-render.sh | 148 +++++++++++++ docs/operations/executor-onboarding.md | 16 +- docs/operations/remote-deployment.md | 34 ++- internal/dispatcher/enrollment/bind.go | 143 +++++++++++++ internal/dispatcher/enrollment/bind_test.go | 162 +++++++++++++++ 18 files changed, 1143 insertions(+), 25 deletions(-) create mode 100644 cmd/dispatcher/bind_executor_test.go create mode 100644 deploy/ansible/tasks/bind-executors.yml create mode 100644 internal/dispatcher/enrollment/bind.go create mode 100644 internal/dispatcher/enrollment/bind_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 405cbf96..61d72236 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,32 @@ changes; the linked API and deployment documentation contains operational detail listed as unconfirmed. Dispatcher databases require the explicit upgrade to schema 26. See `docs/operations/configuration.md#destination-limits-and-opt-outs`. +- `debuglet-dispatcher -bind-executor EXECUTOR_ID -bind-certificate PATH` + binds an executor ID to a client certificate the administrator issued, + without an enrollment token (#415). It verifies the PEM certificate against + the configured `tls.ca_file` for client authentication, requires the + executor ID as its common name and refuses a file holding a private key. It + records the leaf's SHA-256 fingerprint, the value an enforcing dispatcher + admits the executor by; binding the same certificate again changes nothing, + and a different one replaces that executor ID's binding only. +- Ansible: `executor_enrollment_token`, a per-host secret rendered as + `[credentials] enrollment_token` only when set, for an executor the + deployment does not bind from a certificate it holds (#415). + +### Fixed +- Turning on `dispatcher_require_client_cert` no longer disconnects every + executor deployed with `generate-certs.sh` and `deploy-certs.yml` (#415). + Enforcement admits an executor only over the certificate bound to its ID, + and nothing bound those certificates. With + `dispatcher_bind_inventory_executors` (default on), `site.yml`, + `update-config.yml` and `deploy-certs.yml` now copy each inventory + executor's public `client.crt` to the dispatcher and bind it as the service + account before the dispatcher runs with the requirement, and + `deploy-certs.yml` re-binds a certificate `generate-certs.sh` reissued. The + preflight refuses to enable enforcement while an inventory executor has + neither a certificate to bind nor an enrollment token. `deploy/README.md` + and the operations guides no longer suggest that a certificate from the + deployment CA is enough. ## [0.3.0-rc.1] - 2026-10-06 diff --git a/cmd/dispatcher/bind_executor_test.go b/cmd/dispatcher/bind_executor_test.go new file mode 100644 index 00000000..9bab9d31 --- /dev/null +++ b/cmd/dispatcher/bind_executor_test.go @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 ETH Zurich + +package main + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "flag" + "os" + "os/exec" + "path/filepath" + "slices" + "strings" + "testing" + "time" + + "github.com/netsec-ethz/debuglet/internal/dispatcher/enrollment" + "github.com/netsec-ethz/debuglet/internal/sqlitedb" + "github.com/netsec-ethz/debuglet/internal/storagecheck" + "github.com/netsec-ethz/debuglet/internal/testtls" +) + +// An executor deployed with a certificate the administrator issued has no +// token to enrol with, so the deployment binds it on the dispatcher host. The +// binding is what the enforcing dispatcher admits the executor by. +func TestBindExecutorAdmitsAnAdministratorIssuedCertificate(t *testing.T) { + const executorID = "5fe02882-0410-416c-9935-235090bcba0d" + dir := filepath.Join(t.TempDir(), "state") + if err := os.Mkdir(dir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.Chmod(dir, 0o700); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "dispatcher.sqlite") + if err := storagecheck.BootstrapFresh(context.Background(), storagecheck.Dispatcher, path); err != nil { + t.Fatalf("bootstrap database: %v", err) + } + certs := t.TempDir() + authority, err := testtls.NewAuthority(certs, "deployment-ca") + if err != nil { + t.Fatal(err) + } + client, err := authority.Issue(executorID, testtls.Options{Client: true}) + if err != nil { + t.Fatal(err) + } + // The deployment copies only the public certificate to the dispatcher. + certFile := filepath.Join(certs, "client.crt") + if err := os.WriteFile(certFile, client.CertPEM, 0o644); err != nil { + t.Fatal(err) + } + cfg := dispatcherConfig(path) + cfg.TLS.Disable = false + cfg.TLS.CAFile = authority.CertFile + cfg.TLS.RequireClientCert = true + sum := sha256.Sum256(client.Certificate.Certificate[0]) + fingerprint := hex.EncodeToString(sum[:]) + + bound := func(fingerprint string) error { + t.Helper() + db, err := sqlitedb.Open(path) + if err != nil { + t.Fatal(err) + } + defer db.Close() + return enrollment.NewStore(db).Bound(context.Background(), executorID, fingerprint) + } + if err := bound(fingerprint); !errors.Is(err, enrollment.ErrNotEnrolled) { + t.Fatalf("before binding: %v", err) + } + + var out bytes.Buffer + if err := administerBinding(context.Background(), cfg, executorID, certFile, &out); err != nil { + t.Fatalf("bind: %v", err) + } + if want := "executor " + executorID + " is now bound to certificate sha256:" + fingerprint; !strings.Contains(out.String(), want) { + t.Fatalf("output %q, want %q", out.String(), want) + } + if err := bound(fingerprint); err != nil { + t.Fatalf("the bound certificate is refused: %v", err) + } + + // Running the deployment again changes nothing, and says so. + out.Reset() + if err := administerBinding(context.Background(), cfg, executorID, certFile, &out); err != nil { + t.Fatalf("bind again: %v", err) + } + if !strings.Contains(out.String(), "is already bound") { + t.Fatalf("repeated binding printed %q", out.String()) + } + + // A reissued certificate replaces the binding, so the executor that + // installs it is admitted and the old certificate is not. + reissued, err := authority.Issue(executorID, testtls.Options{Client: true}) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(certFile, reissued.CertPEM, 0o644); err != nil { + t.Fatal(err) + } + out.Reset() + if err := administerBinding(context.Background(), cfg, executorID, certFile, &out); err != nil { + t.Fatalf("rebind: %v", err) + } + if !strings.Contains(out.String(), "replacing sha256:"+fingerprint) { + t.Fatalf("rebinding printed %q", out.String()) + } + if err := bound(fingerprint); !errors.Is(err, enrollment.ErrWrongNode) { + t.Fatalf("the replaced certificate: %v", err) + } + + // Refusals record nothing. + other, err := testtls.NewAuthority(t.TempDir(), "other-ca") + if err != nil { + t.Fatal(err) + } + foreign, err := other.Issue(executorID, testtls.Options{Client: true}) + if err != nil { + t.Fatal(err) + } + foreignFile := filepath.Join(certs, "foreign.crt") + if err := os.WriteFile(foreignFile, foreign.CertPEM, 0o644); err != nil { + t.Fatal(err) + } + for name, run := range map[string]func() error{ + "another authority": func() error { + return administerBinding(context.Background(), cfg, executorID, foreignFile, &out) + }, + "another executor ID": func() error { + return administerBinding(context.Background(), cfg, "1144ad6e-2c14-4e5c-ab72-c05a8e8770f2", certFile, &out) + }, + "a private key": func() error { + return administerBinding(context.Background(), cfg, executorID, reissued.KeyFile, &out) + }, + "a missing file": func() error { + return administerBinding(context.Background(), cfg, executorID, filepath.Join(certs, "absent.crt"), &out) + }, + "no authority configured": func() error { + unconfigured := *cfg + unconfigured.TLS.CAFile = "" + return administerBinding(context.Background(), &unconfigured, executorID, certFile, &out) + }, + "an absent database": func() error { + absent := *cfg + absent.Database.Path = filepath.Join(dir, "absent.sqlite") + return administerBinding(context.Background(), &absent, executorID, certFile, &out) + }, + } { + if err := run(); err == nil { + t.Errorf("%s was bound", name) + } + } + sum = sha256.Sum256(reissued.Certificate.Certificate[0]) + if err := bound(hex.EncodeToString(sum[:])); err != nil { + t.Fatalf("a refused binding changed the recorded one: %v", err) + } +} + +// The two flags name one binding, so either alone is refused before any +// configuration or database is read, and so is combining it with a check. +func TestBindExecutorFlagsGoTogether(t *testing.T) { + if os.Getenv("DEBUGLET_BIND_COMMAND_TEST") == "dispatcher" { + index := slices.Index(os.Args, "--") + os.Args = append([]string{os.Args[0]}, os.Args[index+1:]...) + flag.CommandLine = flag.NewFlagSet("dispatcher", flag.ExitOnError) + main() + return + } + executable, err := os.Executable() + if err != nil { + t.Fatal(err) + } + absent := filepath.Join(t.TempDir(), "absent.toml") + for _, tc := range []struct { + args []string + want string + }{ + {[]string{"-config", absent, "-bind-executor", "executor"}, "must be given together"}, + {[]string{"-config", absent, "-bind-certificate", "client.crt"}, "must be given together"}, + {[]string{"-config", absent, "-check-database", "-bind-executor", "executor", "-bind-certificate", "client.crt"}, "cannot be combined"}, + } { + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + command := exec.CommandContext(ctx, executable, append([]string{"-test.run=^TestBindExecutorFlagsGoTogether$", "--"}, tc.args...)...) + command.Env = append(os.Environ(), "DEBUGLET_BIND_COMMAND_TEST=dispatcher") + output, err := command.CombinedOutput() + cancel() + if err == nil || !strings.Contains(string(output), tc.want) { + t.Errorf("%v: %v, output %q, want a refusal mentioning %q", tc.args, err, output, tc.want) + } + } +} diff --git a/cmd/dispatcher/init_database_test.go b/cmd/dispatcher/init_database_test.go index c2bb85ce..f53d02e2 100644 --- a/cmd/dispatcher/init_database_test.go +++ b/cmd/dispatcher/init_database_test.go @@ -43,7 +43,7 @@ func TestInitializeDatabaseCommand(t *testing.T) { t.Fatal(err) } path := filepath.Join(directory, "dispatcher.sqlite") - for _, extra := range [][]string{{"-version"}, {"-upgrade-database"}, {"-check-database"}, {"-accept-data-loss"}, {"-config", "unused"}, {"-ready-file", "unused"}, {"-grant-operator", "unused"}, {"-revoke-operator", "unused"}, {"-enroll-executor", "unused"}, {"-revoke-executor", "unused"}, {"unexpected"}} { + for _, extra := range [][]string{{"-version"}, {"-upgrade-database"}, {"-check-database"}, {"-accept-data-loss"}, {"-config", "unused"}, {"-ready-file", "unused"}, {"-grant-operator", "unused"}, {"-revoke-operator", "unused"}, {"-enroll-executor", "unused"}, {"-revoke-executor", "unused"}, {"-bind-executor", "unused", "-bind-certificate", "unused"}, {"unexpected"}} { run(false, append([]string{"-init-database", path}, extra...)...) if _, err := os.Lstat(path); !os.IsNotExist(err) { t.Fatalf("refused arguments created database: %v", err) diff --git a/cmd/dispatcher/main.go b/cmd/dispatcher/main.go index 36f67339..4a716c16 100644 --- a/cmd/dispatcher/main.go +++ b/cmd/dispatcher/main.go @@ -10,6 +10,7 @@ import ( "errors" "flag" "fmt" + "io" "net" "net/http" "os" @@ -41,6 +42,7 @@ import ( "github.com/netsec-ethz/debuglet/internal/readiness" "github.com/netsec-ethz/debuglet/internal/sqlitedb" "github.com/netsec-ethz/debuglet/internal/storagecheck" + "github.com/netsec-ethz/debuglet/internal/tlsfiles" "github.com/google/uuid" @@ -55,6 +57,8 @@ func main() { revoke := flag.String("revoke-operator", "", "Return the account with this UUID to the ordinary role in the configured database, then exit") enroll := flag.String("enroll-executor", "", "Create a single-use enrollment token for this executor ID in the configured database, print it once, then exit") unenroll := flag.String("revoke-executor", "", "Delete the node credential enrolled for this executor ID in the configured database, then exit") + bind := flag.String("bind-executor", "", "Bind this executor ID to the client certificate named by -bind-certificate in the configured database, then exit") + bindCertificate := flag.String("bind-certificate", "", "With -bind-executor, the PEM client certificate issued for that executor by the authority in tls.ca_file") initDatabase := flag.String("init-database", "", "Create a new database at this path, then exit; its parent must be a private directory owned by this user") upgrade := flag.Bool("upgrade-database", false, "Apply the packaged migrations to the configured database, then exit. Stop the daemon and back the file up first") checkDatabase := flag.Bool("check-database", false, "Report whether the configured database is supported by this build, then exit; exit status 3 means it needs the upgrade, 4 that the upgrade drops recorded data") @@ -120,10 +124,14 @@ func main() { return } - if *checkDatabase && (*upgrade || *acceptDataLoss || *grant != "" || *revoke != "" || *enroll != "" || *unenroll != "") { + if *checkDatabase && (*upgrade || *acceptDataLoss || *grant != "" || *revoke != "" || *enroll != "" || *unenroll != "" || *bind != "") { fmt.Fprintln(os.Stderr, "dispatcher: -check-database cannot be combined with another administration flag") os.Exit(1) } + if (*bind == "") != (*bindCertificate == "") { + fmt.Fprintln(os.Stderr, "dispatcher: -bind-executor and -bind-certificate must be given together") + os.Exit(1) + } if *acceptDataLoss && !*upgrade { fmt.Fprintln(os.Stderr, "dispatcher: -accept-data-loss is only valid with -upgrade-database") os.Exit(1) @@ -164,7 +172,7 @@ func main() { // A database is upgraded only when its operator asks for it, never at // start: a normal start refuses an outdated schema instead. if *upgrade { - if *grant != "" || *revoke != "" || *enroll != "" || *unenroll != "" { + if *grant != "" || *revoke != "" || *enroll != "" || *unenroll != "" || *bind != "" { fmt.Fprintln(os.Stderr, "dispatcher: -upgrade-database cannot be combined with another administration flag") os.Exit(1) } @@ -195,6 +203,10 @@ func main() { // Role administration is deliberately not an HTTP operation: the operator // role is granted on the dispatcher host, by whoever already controls the // database, and never by anything reachable over the network. + if *bind != "" && (*grant != "" || *revoke != "" || *enroll != "" || *unenroll != "") { + fmt.Fprintln(os.Stderr, "dispatcher: -bind-executor cannot be combined with another administration flag") + os.Exit(1) + } if *grant != "" || *revoke != "" { if err := administerRole(context.Background(), cfg, *grant, *revoke); err != nil { fmt.Fprintf(os.Stderr, "dispatcher: %v\n", err) @@ -212,6 +224,15 @@ func main() { } return } + // A certificate the administrator issued is bound the same way: the + // deployment records the executors whose client certificates it issued. + if *bind != "" { + if err := administerBinding(context.Background(), cfg, *bind, *bindCertificate, os.Stdout); err != nil { + fmt.Fprintf(os.Stderr, "dispatcher: %v\n", err) + os.Exit(1) + } + return + } logger := daemonlog.New(cfg.Logging.LogLevel, cfg.Logging.JSONLogs) defer logger.Sync() @@ -688,6 +709,75 @@ func administerEnrollment(ctx context.Context, cfg *config.DispatcherConfig, enr return nil } +// administerBinding binds executorID to the client certificate in certPath, +// which the administrator issued from the authority in tls.ca_file, and +// returns. It is how executors deployed with administrator-issued +// certificates are admitted once tls.require_client_cert is on: no token is +// involved, because whoever controls this database already vouches for them. +// The certificate is verified as the transport would verify it, so a binding +// that could never admit its executor is refused rather than recorded. +// Binding the certificate already bound changes nothing; a different one +// replaces that executor ID's binding and no other. +func administerBinding(ctx context.Context, cfg *config.DispatcherConfig, executorID, certPath string, out io.Writer) error { + if cfg.TLS.CAFile == "" { + return errors.New("tls.ca_file is not set, so there is no authority to verify the certificate against; configure the authority executor certificates are checked with first") + } + now := time.Now() + roots, err := tlsfiles.TrustRoots("tls.ca_file", cfg.TLS.CAFile, now) + if err != nil { + return err + } + certPEM, err := readBounded(certPath, enrollment.MaxCertificateBytes) + if err != nil { + return fmt.Errorf("-bind-certificate: %w", err) + } + fingerprint, err := enrollment.CertificateFingerprint(executorID, certPEM, roots, now) + if err != nil { + return fmt.Errorf("-bind-certificate %s: %w", certPath, err) + } + if err := storagecheck.Check(ctx, storagecheck.Dispatcher, cfg.Database.Path); err != nil { + return err + } + db, err := sqlitedb.Open(cfg.Database.Path) + if err != nil { + return fmt.Errorf("open database: %w", err) + } + defer db.Close() + binding, err := enrollment.NewStore(db).Bind(ctx, executorID, fingerprint) + if err != nil { + return err + } + if !cfg.TLS.RequireClientCert { + fmt.Fprintln(os.Stderr, "dispatcher: tls.require_client_cert is not set, so this dispatcher does not check this binding until it is") + } + switch { + case !binding.Changed: + _, err = fmt.Fprintf(out, "executor %s is already bound to certificate sha256:%s\n", executorID, fingerprint) + case binding.Previous == "": + _, err = fmt.Fprintf(out, "executor %s is now bound to certificate sha256:%s\n", executorID, fingerprint) + default: + _, err = fmt.Fprintf(out, "executor %s is now bound to certificate sha256:%s, replacing sha256:%s\n", executorID, fingerprint, binding.Previous) + } + return err +} + +// readBounded reads a file of at most limit bytes. +func readBounded(path string, limit int64) ([]byte, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + data, err := io.ReadAll(io.LimitReader(f, limit+1)) + if err != nil { + return nil, err + } + if int64(len(data)) > limit { + return nil, fmt.Errorf("%s exceeds %d bytes", path, limit) + } + return data, nil +} + // localDevelopmentProfile reports whether the HTTP API serves its local // development profile. It takes both the operator's explicit opt-in and an // environment this daemon recognises as local: neither alone turns diff --git a/deploy/README.md b/deploy/README.md index bd96be51..b5b3cb8e 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -644,10 +644,10 @@ Two variables cover the cases where the default is not enough: address dialled, for instance `dispatcher_addr` holding an IP while the certificate names the host. Rendered only when set. - `dispatcher_require_client_cert` — makes the dispatcher require a client - certificate from the deployment CA on its own listeners. Off by default, - because turning it on refuses every executor that has not been given one - yet. Rendered only when on, together with the `ca_file` it is checked - against. + certificate on its own listeners and enforce executor enrollment. Off by + default. Rendered only when on, together with the `ca_file` it is checked + against. See [Requiring client certificates](#requiring-client-certificates): + a certificate from the deployment CA is necessary but not sufficient. The daemons accept both keys. The templates emit them only when they are set, so a deployment that needs neither renders neither. @@ -658,13 +658,83 @@ TEST rig does. The preflight refuses `executor_disable_tls` for any other `dispatcher_addr`, and refuses an executor that verifies TLS while the dispatcher serves none. +### Requiring client certificates + +With `dispatcher_require_client_cert: true` the dispatcher enforces executor +enrollment. It admits an executor ID only over the one client certificate +bound to that ID in its database: the lowercase hex SHA-256 fingerprint of the +leaf certificate's DER bytes. A certificate the deployment CA issued is +verified, but without a binding the executor is refused ("executor ID is not +enrolled") and its control session is rejected. Executors installed by +`generate-certs.sh` and `deploy-certs.yml` have no binding of their own, so the +deployment records one for them: + +- With `dispatcher_bind_inventory_executors: true`, the default, the + deployment binds every executor in the inventory to + `certs_dir/executors//client.crt`. It copies only that public + certificate to the dispatcher (`dispatcher_executor_certs_dir`) and runs the + installed dispatcher as the service account: + + ```sh + debuglet-dispatcher -config /etc/debuglet/dispatcher/dispatcher.toml \ + -bind-executor EXECUTOR_UUID -bind-certificate /path/to/client.crt + ``` + + The command verifies the certificate against `tls.ca_file` for client + authentication and requires the executor UUID as its common name. Binding + the certificate already bound changes nothing; a different certificate + replaces that executor's binding and no other. +- `site.yml` (the dispatcher role) binds after it renders the configuration + and before it starts or restarts the dispatcher; `update-config.yml` binds + before it restarts the dispatcher with a configuration that turns the + requirement on. Both run on every deployment, so a binding is never left + stale. +- `deploy-certs.yml` binds again on a dispatcher that already enforces the + requirement, before the executors install their certificates. A + certificate `generate-certs.sh` reissued (delete + `certs/executors//` and run it again) is therefore re-bound by + the same `make deploy-certs` that installs it. The old certificate is + refused from that moment, so the executor reconnects once its new + certificate is installed and it restarts, later in the same run. On a host + where the dispatcher is not installed yet, or does not require client + certificates yet, it binds nothing and says so: `site.yml` or + `update-config.yml` binds before enabling the requirement. +- An executor whose certificate the controller does not hold enrols with a + token instead: set its `executor_enrollment_token` (a secret; keep it in a + vault or a private `host_vars` file) to the output of + `debuglet-dispatcher -enroll-executor EXECUTOR_UUID`. It is rendered as + `[credentials] enrollment_token`, and the executor's first connection + spends it. The deployment does not bind such an executor itself. + +The preflight refuses to turn the requirement on while any inventory executor +has neither a certificate to bind nor a token, including on a run limited to +the dispatcher. Turn `dispatcher_bind_inventory_executors` off only when every +executor is enrolled some other way; the preflight then requires a token for +each one. Removing an executor from the inventory does not remove its +binding: `debuglet-dispatcher -revoke-executor EXECUTOR_UUID` does. + +To enable the requirement on a running fleet: + +1. Deploy a release that has `-bind-executor` with `site.yml`, still without + the requirement. +2. Make sure every executor has its certificate (`make deploy-certs`). +3. Set `dispatcher_require_client_cert: true` and run `update-config.yml` + (or `site.yml`). It binds every inventory executor and only then restarts + the dispatcher with the requirement. Check its "Report the executor + bindings" output and the dispatcher log for `Refused an executor identity`. + ### Self-service executor enrollment Enrollment through the console is disabled by default. It requires a release with API 1.10, an explicitly upgraded dispatcher database, native TLS on both -control endpoints, and certificates for every existing executor before enabling -`dispatcher_require_client_cert`. The browser API may have its own HTTPS proxy; -that proxy does not replace the native control listeners. +control endpoints, and `dispatcher_require_client_cert`. That requirement is +not satisfied by giving existing executors certificates alone: the dispatcher +then admits each executor only over the certificate bound to its ID, so every +existing executor needs a binding too. The deployment binds the inventory's +executors from their deployed certificates before it enables the requirement; +see [Requiring client certificates](#requiring-client-certificates). The +browser API may have its own HTTPS proxy; that proxy does not replace the +native control listeners. Preprovision a dedicated intermediate issuer certificate chain and matching private key on the deployment controller. Keep the key owner-only (`chmod 600`) @@ -725,7 +795,11 @@ both are leaves the deployment CA signed, and the generator refuses to issue anything without a name list. `ansible/deploy-certs.yml` then installs them into the fixture tree, and the rendered configurations are checked for the material on both sides — including that `tls.require_client_cert` and -`tls.server_name` appear only when their variables are set. No inventory, no +`tls.server_name` appear only when their variables are set. With client +certificates required, it deploys a dispatcher with a real database and checks +that the inventory executor is bound to its deployed certificate, that a +second run changes nothing, that a reissued certificate is re-bound and that a +certificate from another authority is refused. No inventory, no host key and no managed machine is involved, and nothing is deployed anywhere. ```sh diff --git a/deploy/ansible/deploy-certs.yml b/deploy/ansible/deploy-certs.yml index 23486990..96213e82 100644 --- a/deploy/ansible/deploy-certs.yml +++ b/deploy/ansible/deploy-certs.yml @@ -4,6 +4,11 @@ # dispatcher with plus that host's own client certificate. The dispatcher gets # the CA too, so requiring client certificates later needs no second run. # +# With dispatcher_require_client_cert on and a dispatcher already enforcing it, +# it also binds every inventory executor ID to its client certificate on the +# dispatcher before the executors install it, so a reissued certificate is +# admitted rather than refused (tasks/bind-executors.yml). +# # DISPATCHER_SANS="DNS:dispatcher.example.com" ./deploy/scripts/generate-certs.sh ... # cd deploy/ansible && ../scripts/provisioner.sh ansible-playbook -i hosts.yml -e @vars/prod.yml deploy-certs.yml # @@ -68,6 +73,55 @@ when: dispatcher_executor_onboarding_enabled | bool register: dispatcher_issuer_copy + # A dispatcher that requires client certificates admits an executor only + # over the certificate bound to its ID, so a certificate generate-certs.sh + # issued or reissued is bound here, before the executors below install + # it. On a host where the dispatcher is not installed yet, or whose + # installed configuration does not require client certificates yet, + # site.yml or update-config.yml binds them before enabling the requirement. + - name: Inspect the installed dispatcher for executor binding + ansible.builtin.stat: + path: "{{ item }}" + loop: + - "{{ payload_prefix }}/bin/debuglet-dispatcher" + - "{{ config_dir }}/dispatcher/dispatcher.toml" + - "{{ state_dir }}/dispatcher/dispatcher.db" + register: dispatcher_bind_installed + when: + - dispatcher_require_client_cert | bool + - dispatcher_bind_inventory_executors | bool + + - name: Read whether the installed configuration requires client certificates + ansible.builtin.command: + argv: [grep, -qxF, "require_client_cert = true", "{{ config_dir }}/dispatcher/dispatcher.toml"] + register: dispatcher_bind_enforced + changed_when: false + check_mode: false + failed_when: dispatcher_bind_enforced.rc not in [0, 1] + when: + - dispatcher_require_client_cert | bool + - dispatcher_bind_inventory_executors | bool + - dispatcher_bind_installed.results | map(attribute='stat.exists') | min + + - name: Bind the inventory executors to their client certificates + ansible.builtin.include_tasks: tasks/bind-executors.yml + when: + - dispatcher_require_client_cert | bool + - dispatcher_bind_inventory_executors | bool + - dispatcher_bind_enforced.rc | default(1) == 0 + + - name: Report executor binding left to the deployment + ansible.builtin.debug: + msg: >- + The dispatcher here is not installed or does not require client + certificates yet, so no executor was bound now. site.yml or + update-config.yml binds the inventory executors before it enables + the requirement. + when: + - dispatcher_require_client_cert | bool + - dispatcher_bind_inventory_executors | bool + - dispatcher_bind_enforced.rc | default(1) != 0 + - name: Inspect the dispatcher unit ansible.builtin.stat: path: "{{ systemd_unit_dir }}/debuglet-dispatcher.service" diff --git a/deploy/ansible/group_vars/dispatcher.yml b/deploy/ansible/group_vars/dispatcher.yml index 50be55ea..0ec53876 100644 --- a/deploy/ansible/group_vars/dispatcher.yml +++ b/deploy/ansible/group_vars/dispatcher.yml @@ -58,12 +58,28 @@ dispatcher_authentication_public_url: "" dispatcher_authentication_device_verification_url: "" # Whether the dispatcher requires a client certificate on its own listeners. -# Only meaningful when dispatcher_disable_tls is false, and off by default: -# turning it on refuses every executor that does not yet present a client -# certificate from the deployment CA, so run deploy-certs.yml on every -# executor first. It is rendered only when it is on. +# Only meaningful when dispatcher_disable_tls is false, and off by default. +# Turning it on also enforces enrollment: the dispatcher then admits an +# executor only over the one certificate bound to its ID in its database (the +# SHA-256 fingerprint of the leaf), and a certificate from the deployment CA +# alone is not enough. Run deploy-certs.yml on every executor first; the +# deployment then binds each inventory executor to the certificate it installed +# (dispatcher_bind_inventory_executors below) before it restarts the +# dispatcher with the requirement. It is rendered only when it is on. dispatcher_require_client_cert: false +# With dispatcher_require_client_cert on, bind every inventory executor ID to +# certs_dir/executors//client.crt on the dispatcher: site.yml, +# deploy-certs.yml and update-config.yml all do it, and deploy-certs.yml does it +# again whenever generate-certs.sh has reissued a certificate. Only public +# certificates are copied. An executor with executor_enrollment_token enrols +# with its token instead and is not bound here. Turn this off only when every +# executor is enrolled some other way: the preflight then requires a token for +# each inventory executor. +dispatcher_bind_inventory_executors: true +# Where the dispatcher keeps the public executor certificates it was bound to. +dispatcher_executor_certs_dir: "{{ config_dir }}/dispatcher/executors" + # The authority the dispatcher checks client certificates against. It is read # only when client certificates are required, and deploy-certs.yml installs it # next to the server keypair. diff --git a/deploy/ansible/group_vars/executors.yml b/deploy/ansible/group_vars/executors.yml index 8c2e64ca..b07a6171 100644 --- a/deploy/ansible/group_vars/executors.yml +++ b/deploy/ansible/group_vars/executors.yml @@ -193,6 +193,15 @@ executor_disable_tls: false # when set. executor_tls_server_name: "" +# A single-use enrollment token from `debuglet-dispatcher -enroll-executor +# `, for an executor the deployment does not bind from a +# certificate it holds (see dispatcher_bind_inventory_executors). Rendered as +# credentials.enrollment_token only when set; the first connection spends it +# and binds the certificate the executor presents. It is a secret: keep it in +# a vault or a private host_vars file, never in a committed inventory. A spent +# token buys nothing, so it may stay until the next configuration change. +executor_enrollment_token: "" + # Payments. The deployment runs without a wallet: the executor prices work in # TEST units and names no chain account. Fill in a currency and wallet only # together with the dispatcher's payment configuration. diff --git a/deploy/ansible/preflight-variables.yml b/deploy/ansible/preflight-variables.yml index 3f8df70d..aca53a40 100644 --- a/deploy/ansible/preflight-variables.yml +++ b/deploy/ansible/preflight-variables.yml @@ -276,6 +276,55 @@ quiet: true when: dispatcher_require_client_cert | bool + # With client certificates required, the dispatcher admits an executor + # only over the certificate bound to its ID in its database; a + # certificate the deployment CA issued is not enough on its own. Every + # inventory executor needs a way to be bound before enforcement is + # enabled: the certificate the deployment installs and binds + # (dispatcher_bind_inventory_executors), or its own enrollment token. + # Checked here for the whole inventory, so a run limited to the + # dispatcher cannot enable enforcement for an executor it would refuse. + - name: Inspect the client certificates the deployment binds + ansible.builtin.stat: + path: "{{ certs_dir }}/executors/{{ hostvars[item].executor_id | default('') }}/client.crt" + delegate_to: localhost + register: binding_certificates + loop: "{{ groups['executors'] | default([]) | sort }}" + when: + - dispatcher_require_client_cert | bool + - dispatcher_bind_inventory_executors | bool + - not (hostvars[item].executor_enrollment_token | default('')) + + - name: Require a binding for every inventory executor + ansible.builtin.assert: + that: + - unbound | length == 0 + fail_msg: >- + dispatcher_require_client_cert is on, and the dispatcher would then + refuse {{ unbound | join(', ') }}: an executor is admitted only over + the certificate bound to its executor_id. + {% if dispatcher_bind_inventory_executors | bool %}Issue its client + certificate with deploy/scripts/generate-certs.sh (`make + deploy-certs`) so that {{ certs_dir }}/executors//client.crt + exists and the deployment binds it, or give it an + executor_enrollment_token.{% else %}dispatcher_bind_inventory_executors + is off, so the deployment binds none of them: turn it back on, or + give each of them an executor_enrollment_token from + `debuglet-dispatcher -enroll-executor `.{% endif %} + quiet: true + vars: + unbound: >- + {%- set out = [] -%} + {%- for result in binding_certificates.results | default([]) -%} + {%- set h = hostvars[result.item] -%} + {%- if not (h.executor_enrollment_token | default('')) + and (not (dispatcher_bind_inventory_executors | bool) or not (result.stat.exists | default(false))) -%} + {%- set _ = out.append(result.item ~ ' (' ~ (h.executor_id | default('no executor_id')) ~ ')') -%} + {%- endif -%} + {%- endfor -%} + {{ out }} + when: dispatcher_require_client_cert | bool + - name: Require explicit public executor enrollment settings ansible.builtin.assert: that: @@ -585,6 +634,22 @@ deploy/ansible/group_vars/executors.yml. quiet: true + # The token is a secret, so neither it nor the expression testing it is + # shown. + - name: Require a well-formed enrollment token + ansible.builtin.assert: + that: + - executor_enrollment_token is string + - executor_enrollment_token is match('^dbx_[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$') + - executor_enrollment_token | length <= 128 + fail_msg: >- + executor_enrollment_token of {{ inventory_hostname }} must be the + token `debuglet-dispatcher -enroll-executor` printed, dbx_ followed + by two base64url parts joined by a dot. + quiet: true + no_log: true + when: executor_enrollment_token | default('') | length > 0 + - name: Require usable executor resources ansible.builtin.assert: that: diff --git a/deploy/ansible/roles/dispatcher/tasks/main.yml b/deploy/ansible/roles/dispatcher/tasks/main.yml index eed45357..78c9d3be 100644 --- a/deploy/ansible/roles/dispatcher/tasks/main.yml +++ b/deploy/ansible/roles/dispatcher/tasks/main.yml @@ -135,6 +135,16 @@ when: dispatcher_cilogon_oidc_enabled | bool notify: Restart dispatcher +# With client certificates required, the dispatcher admits only executors +# whose ID is bound to the certificate they present. Bind the inventory's +# executors with the configuration and database just installed, before the +# dispatcher is started or restarted with them. +- name: Bind the inventory executors to their client certificates + ansible.builtin.include_tasks: "{{ playbook_dir }}/tasks/bind-executors.yml" + when: + - dispatcher_require_client_cert | bool + - dispatcher_bind_inventory_executors | bool + - name: Install dispatcher systemd unit ansible.builtin.template: src: dispatcher.service.j2 diff --git a/deploy/ansible/roles/executor/templates/executor.toml.j2 b/deploy/ansible/roles/executor/templates/executor.toml.j2 index 96a0653b..8b31b500 100644 --- a/deploy/ansible/roles/executor/templates/executor.toml.j2 +++ b/deploy/ansible/roles/executor/templates/executor.toml.j2 @@ -30,6 +30,11 @@ server_name = "{{ executor_tls_server_name }}" ca_cert = "{{ executor_config_dir }}/ca.crt" client_cert = "{{ executor_config_dir }}/client.crt" client_key = "{{ executor_config_dir }}/client.key" +{% if executor_enrollment_token | default('') | length > 0 %} +# Spent by the first connection, which binds the certificate above to this +# executor ID (executor_enrollment_token). +enrollment_token = "{{ executor_enrollment_token }}" +{% endif %} [resources] capacity = {{ executor_capacity }} diff --git a/deploy/ansible/tasks/bind-executors.yml b/deploy/ansible/tasks/bind-executors.yml new file mode 100644 index 00000000..7ad5f7a3 --- /dev/null +++ b/deploy/ansible/tasks/bind-executors.yml @@ -0,0 +1,81 @@ +--- +# Bind every inventory executor ID to the client certificate the deployment +# issued it, so a dispatcher that requires client certificates admits it. +# +# With dispatcher_require_client_cert on, the dispatcher admits an executor +# only over the certificate recorded for its ID: the SHA-256 fingerprint of the +# leaf, kept in its database. A certificate from deploy/scripts/generate-certs.sh +# is trusted by the CA but has no such record until this binds it. An executor +# given executor_enrollment_token binds itself with that token instead and is +# skipped here. +# +# It copies each executor's public client.crt, never a key, to the dispatcher +# and runs the installed dispatcher's -bind-executor command as the service +# account against the configured database. The command verifies the +# certificate against the configured tls.ca_file, requires client +# authentication and the executor ID as common name, and changes nothing for a +# certificate already bound, so it is safe to run on every deployment. A +# reissued certificate replaces that executor's binding. +# +# deploy-certs.yml, the dispatcher role and update-config.yml include it when +# dispatcher_require_client_cert and dispatcher_bind_inventory_executors are on, +# after the configuration naming tls.ca_file is in place and before the +# dispatcher is restarted with it. + +- name: List the executors bound from their deployed certificates + ansible.builtin.set_fact: + dispatcher_bound_executors: >- + {%- set out = [] -%} + {%- for name in groups['executors'] | default([]) | sort -%} + {%- set h = hostvars[name] -%} + {%- if h.executor_id is defined and not (h.executor_enrollment_token | default('')) + and h.executor_id not in out -%} + {%- set _ = out.append(h.executor_id) -%} + {%- endif -%} + {%- endfor -%} + {{ out }} + +- name: Create the directory for executor client certificates + ansible.builtin.file: + path: "{{ dispatcher_executor_certs_dir }}" + state: directory + owner: "{{ debuglet_user }}" + group: "{{ debuglet_group }}" + mode: "0700" + +# Public material only: the certificate the executor presents, which the +# dispatcher sees in every handshake anyway. +- name: Install the executors' public client certificates + ansible.builtin.copy: + src: "{{ certs_dir }}/executors/{{ item }}/client.crt" + dest: "{{ dispatcher_executor_certs_dir }}/{{ item }}.crt" + owner: "{{ debuglet_user }}" + group: "{{ debuglet_group }}" + mode: "0644" + loop: "{{ dispatcher_bound_executors }}" + +# The command writes the database, which has to keep its owner. A play that +# already runs as the service account, as the offline fixture does, runs it +# directly; one running as root drops to that account through runuser. +- name: Read the account the deployment runs commands as + ansible.builtin.command: + argv: [id, -un] + register: dispatcher_bind_account + changed_when: false + check_mode: false + +- name: Bind each executor ID to its client certificate + ansible.builtin.command: + argv: >- + {{ ([] if dispatcher_bind_account.stdout == debuglet_user else ['runuser', '-u', debuglet_user, '--']) + + [payload_prefix ~ '/bin/debuglet-dispatcher', + '-config', config_dir ~ '/dispatcher/dispatcher.toml', + '-bind-executor', item, + '-bind-certificate', dispatcher_executor_certs_dir ~ '/' ~ item ~ '.crt'] }} + loop: "{{ dispatcher_bound_executors }}" + register: dispatcher_bind_result + changed_when: "'already bound' not in dispatcher_bind_result.stdout" + +- name: Report the executor bindings + ansible.builtin.debug: + msg: "{{ dispatcher_bind_result.results | default([]) | map(attribute='stdout', default='') | select | list }}" diff --git a/deploy/ansible/update-config.yml b/deploy/ansible/update-config.yml index cabfbf0f..449596c4 100644 --- a/deploy/ansible/update-config.yml +++ b/deploy/ansible/update-config.yml @@ -84,6 +84,15 @@ mode: "0644" register: dispatcher_unit + # The configuration may be the one that turns enforcement on, and a + # reissued certificate needs its binding replaced, so the inventory + # executors are bound before the dispatcher restarts with it. + - name: Bind the inventory executors to their client certificates + ansible.builtin.include_tasks: tasks/bind-executors.yml + when: + - dispatcher_require_client_cert | bool + - dispatcher_bind_inventory_executors | bool + - name: Restart dispatcher if config changed ansible.builtin.systemd: name: debuglet-dispatcher diff --git a/deploy/test/ansible-render.sh b/deploy/test/ansible-render.sh index fdcfeeb6..37fb5f53 100755 --- a/deploy/test/ansible-render.sh +++ b/deploy/test/ansible-render.sh @@ -33,6 +33,11 @@ # disabling it removes the units and the configuration entry. # 11. executor labels, the advertised address, the IPv4 reflector and the # executor's capability set render where the daemons read them. +# 12. with client certificates required, the deployment binds every inventory +# executor to the certificate it installs before the dispatcher runs with +# the requirement, re-binds a reissued certificate and refuses one from +# another authority; the preflight refuses enforcement for an executor +# with neither a certificate to bind nor an enrollment token. # # It needs a built release package in deploy/dist (./deploy/scripts/build-linux.sh) # and the pinned provisioner, so run it through deploy/test/provisioner-check.sh. @@ -1088,6 +1093,149 @@ fi expect 'legacy dispatcher database is preserved' \ "$host/etc/debuglet/dispatcher/dispatcher.db" 'legacy dispatcher accounts and runs' +# ------------------------------------------------------- executor binding --- +# With client certificates required, the dispatcher admits an executor only +# over the certificate bound to its ID in its database. The deployment binds +# every inventory executor from the certificate it installs, before the +# dispatcher runs with the requirement, and again when a certificate is +# reissued. This runs on a host tree of its own with a real dispatcher +# database, which the installed dispatcher creates. +mkdir -p "$work/no-client-certs" +cp -R "$certs/ca.crt" "$certs/dispatcher" "$work/no-client-certs/" +refuses 'enforcement is refused for an executor without a certificate to bind' \ + 'the dispatcher would then' -e dispatcher_require_client_cert=true \ + -e "certs_dir=$work/no-client-certs" --limit dispatcher +refuses 'enforcement without inventory binding is refused for an executor without a token' \ + 'dispatcher_bind_inventory_executors' -e dispatcher_require_client_cert=true \ + -e dispatcher_bind_inventory_executors=false +if run "$work/token-preflight.log" preflight-variables.yml -e dispatcher_require_client_cert=true \ + -e dispatcher_bind_inventory_executors=false -e executor_enrollment_token=dbx_fixtureselector.fixtureverifier; then + check 'enforcement without inventory binding is accepted for an executor with a token' pass +else + check 'enforcement without inventory binding is accepted for an executor with a token' fail + tail -20 "$work/token-preflight.log" >&2 +fi +refuses 'a malformed enrollment token is refused' 'Require a well-formed enrollment token' \ + -e executor_enrollment_token=not-a-token-secretvalue +refute 'a refused enrollment token is not shown' "$work/refuse.log" 'secretvalue' +mkdir -p "$work/token" +if run "$work/token-render.log" "$work/tls-render.yml" --limit executors \ + -e "tls_template_dir=$playbooks" -e "tls_render_dir=$work/token" \ + -e executor_enrollment_token=dbx_fixtureselector.fixtureverifier; then + expect 'an enrollment token renders into the executor credentials' "$work/token/executor.toml" \ + 'enrollment_token = "dbx_fixtureselector.fixtureverifier"' + output=$(timeout 10 "$host/opt/debuglet/prod/bin/debuglet-executor" \ + --config "$work/token/executor.toml" 2>&1 || true) + case $output in + *"$host/var/lib/debuglet/executor-prod/executor.db"*) + check 'the installed executor accepts an enrollment token' pass ;; + *) + check 'the installed executor accepts an enrollment token' fail + printf '%s\n' "$output" | head -5 >&2 ;; + esac +else + check 'an enrollment token renders into the executor credentials' fail + tail -20 "$work/token-render.log" >&2 +fi +refute 'no enrollment token is rendered by default' "$executor_toml" 'enrollment_token' + +bind_host=$work/bind-host +bind_certs=$work/bind-certs +bind_dist=$work/bind-dist +mkdir -p "$bind_host/etc/systemd/system" "$bind_dist" "$work/bind-seed" +chmod 0700 "$work/bind-seed" +cp -R "$certs" "$bind_certs" +cp "$dist"/* "$bind_dist/" +rm -f "$bind_dist/dispatcher-seed.db" +"$host/opt/debuglet/prod/bin/debuglet-dispatcher" -init-database "$work/bind-seed/dispatcher.db" \ + >"$work/bind-seed.log" 2>&1 && cp "$work/bind-seed/dispatcher.db" "$bind_dist/dispatcher-seed.db" +cat >"$work/bind.yml" <&2 +fi +if bind_run "$work/bind-apply.log" deploy-dispatcher.yml; then + check 'a deployment that requires client certificates applies' pass +else + check 'a deployment that requires client certificates applies' fail + tail -30 "$work/bind-apply.log" >&2 +fi +expect 'the deployment binds the inventory executor to its certificate' "$work/bind-apply.log" \ + "executor $fixture_executor is now bound to certificate sha256:$first_fingerprint" +if cmp -s "$bound_cert" "$bind_host/etc/debuglet/dispatcher/executors/$fixture_executor.crt"; then + check 'the dispatcher holds the public executor certificate it bound' pass +else + check 'the dispatcher holds the public executor certificate it bound' fail +fi +if find "$bind_host/etc/debuglet/dispatcher" -name 'client.key' | grep -q .; then + check 'no executor key reaches the dispatcher' fail +else + check 'no executor key reaches the dispatcher' pass +fi +if bind_run "$work/bind-update.log" update-config.yml -e "deploy_version=$release_version"; then + check 'a configuration update with client certificates required applies' pass + expect 'a repeated binding changes nothing' "$work/bind-update.log" \ + "executor $fixture_executor is already bound to certificate sha256:$first_fingerprint" +else + check 'a configuration update with client certificates required applies' fail + tail -30 "$work/bind-update.log" >&2 +fi +# Reissuing a certificate with the generator re-binds it on the next +# certificate installation, before the executor presents it. +rm -rf "${bind_certs:?}/executors/$fixture_executor" +CERTS_DIR=$bind_certs DISPATCHER_SANS=$fixture_sans \ + "$root/deploy/scripts/generate-certs.sh" "$fixture_executor" >"$work/bind-reissue.log" 2>&1 +second_fingerprint=$(fingerprint "$bound_cert") +if [ "$second_fingerprint" != "$first_fingerprint" ] && + bind_run "$work/bind-rebind.log" deploy-certs.yml; then + check 'a reissued certificate installs' pass + expect 'a reissued certificate replaces the binding' "$work/bind-rebind.log" \ + "executor $fixture_executor is now bound to certificate sha256:$second_fingerprint, replacing sha256:$first_fingerprint" +else + check 'a reissued certificate installs' fail + tail -30 "$work/bind-rebind.log" >&2 +fi +# A certificate the configured authority did not issue is refused rather than +# bound, and the deployment stops before the dispatcher restarts. +cp "$bound_cert" "$work/bind-good.crt" +CERTS_DIR=$work/foreign-ca DISPATCHER_SANS='DNS:other.fixture.invalid' \ + "$root/deploy/scripts/generate-certs.sh" "$fixture_executor" >/dev/null 2>&1 +cp "$work/foreign-ca/executors/$fixture_executor/client.crt" "$bound_cert" +if bind_run "$work/bind-foreign.log" update-config.yml -e "deploy_version=$release_version"; then + check 'a certificate from another authority is not bound' fail +elif grep -qF 'not a client certificate of the configured authority' "$work/bind-foreign.log"; then + check 'a certificate from another authority is not bound' pass +else + check 'a certificate from another authority is not bound' fail + tail -20 "$work/bind-foreign.log" >&2 +fi +cp "$work/bind-good.crt" "$bound_cert" + if [ "$failures" -ne 0 ]; then printf '%s check(s) failed\n' "$failures" >&2 exit 1 diff --git a/docs/operations/executor-onboarding.md b/docs/operations/executor-onboarding.md index 3c22decd..29a287ab 100644 --- a/docs/operations/executor-onboarding.md +++ b/docs/operations/executor-onboarding.md @@ -211,10 +211,18 @@ then start that same package and verify existing executors and measurements. Rolling back requires restoring the matching backup and package, not only the old binary. See the [deployment procedures](../../deploy/README.md). -Before enabling client-certificate enforcement, confirm that every existing -executor presents a certificate trusted by the retained client CA bundle. -Replacing that trust bundle or enabling enforcement for uncertified executors -will disconnect them. Keep enrollment disabled until this prerequisite and both +Client-certificate enforcement binds each executor ID to the SHA-256 +fingerprint of one certificate, and refuses an executor without a binding even +when its certificate is trusted. Before enabling it, confirm that every +existing executor presents a certificate trusted by the retained client CA +bundle and is bound to that certificate: an administrator-issued certificate +with `debuglet-dispatcher -bind-executor EXECUTOR_UUID -bind-certificate +client.crt`, or a token from `-enroll-executor`. The Ansible deployment binds +its inventory executors automatically before it enables enforcement, and +re-binds a certificate it reissues (see "Requiring client certificates" in +the [deployment procedures](../../deploy/README.md)). Replacing the trust +bundle, or enabling enforcement for uncertified or unbound executors, will +disconnect them. Keep enrollment disabled until this prerequisite and both advertised native control endpoints have been verified. Publish a matching full package and installation guide before enabling the console setup flow. diff --git a/docs/operations/remote-deployment.md b/docs/operations/remote-deployment.md index bc25d377..fc51f05e 100644 --- a/docs/operations/remote-deployment.md +++ b/docs/operations/remote-deployment.md @@ -77,16 +77,38 @@ The gRPC and reverse-control addresses must name their respective listeners. The first successful registration consumes the token and stores the certificate fingerprint bound to that UUID. Remove `credentials.enrollment_token` afterward. Restart with the same UUID, keypair and database; no new token is needed. A -replacement certificate needs a new token for the same UUID. The corresponding +replacement certificate needs a new token for the same UUID, or a binding as +described below. The corresponding `-revoke-executor EXECUTOR_UUID` command removes the binding and prevents new registration and lease renewal. An existing session retains its current lease until expiry or an earlier disconnect. -The current Ansible executor template renders the certificate paths but has no -enrollment-token input. It does not perform this initial binding automatically. -An on-host configuration edit is overwritten by the next Ansible deployment; -coordinate initial enrollment and configuration ownership with the operator. -Do not disable client-certificate verification to bypass missing enrollment. +When the administrator issued the executor's certificate, as +`deploy/scripts/generate-certs.sh` does, no token is needed: bind the UUID to +that certificate directly, as the database owner: + +```sh +debuglet-dispatcher -config /etc/debuglet/dispatcher/dispatcher.toml \ + -bind-executor EXECUTOR_UUID -bind-certificate /path/to/client.crt +``` + +Only the public certificate is needed. The command verifies it against +`tls.ca_file` for client authentication, requires the UUID as its common name +and records its SHA-256 fingerprint. Running it again for the same certificate +changes nothing; a reissued certificate replaces the binding, and the old one +is refused from then on. + +With `require_client_cert = true` the dispatcher enforces these bindings: a +certificate from the trusted CA without a binding for its executor's UUID is +refused. Enable the requirement only after every executor is bound or holds a +token. The Ansible deployment does this for its inventory: it binds every +executor from its deployed `client.crt` before it starts the dispatcher with +the requirement, re-binds a reissued certificate, renders +`executor_enrollment_token` into `[credentials]` for an executor it does not +bind, and its preflight refuses the requirement while an executor has neither; +see "Requiring client certificates" in `deploy/README.md`. An on-host +configuration edit is overwritten by the next Ansible deployment. Do not +disable client-certificate verification to bypass missing enrollment. ## Client trust and account access diff --git a/internal/dispatcher/enrollment/bind.go b/internal/dispatcher/enrollment/bind.go new file mode 100644 index 00000000..ebd52a4f --- /dev/null +++ b/internal/dispatcher/enrollment/bind.go @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 ETH Zurich + +package enrollment + +import ( + "bytes" + "context" + "crypto/sha256" + "crypto/x509" + "database/sql" + "encoding/hex" + "encoding/pem" + "errors" + "fmt" + "strings" + "time" + + "github.com/netsec-ethz/debuglet/internal/dispatcher/database" + "github.com/netsec-ethz/debuglet/internal/dispatcher/models" +) + +// MaxCertificateBytes bounds a certificate file read for binding: a leaf and +// a few intermediates fit many times over. +const MaxCertificateBytes = 64 << 10 + +// Binding reports what one administrator binding did. +type Binding struct { + // Fingerprint is the lowercase hex SHA-256 of the leaf's DER bytes, the + // value the transport reports for a verified client certificate. + Fingerprint string + // Previous is the fingerprint the executor ID was bound to before, empty + // when it had none. + Previous string + // Changed reports that the recorded binding was created or replaced. A + // binding to the same certificate is left as it is. + Changed bool +} + +// CertificateFingerprint verifies a PEM certificate file an administrator +// issued for executorID and returns the fingerprint the transport would +// report for it. The first certificate is the leaf; any further ones are +// intermediates. The leaf has to chain to roots for client authentication at +// now, must not be an authority, and must carry executorID as its common +// name, which is how deploy/scripts/generate-certs.sh and the enrollment +// signer both issue executor certificates. A file holding anything but +// certificates, a private key in particular, is refused: binding needs public +// material only. +func CertificateFingerprint(executorID string, certPEM []byte, roots *x509.CertPool, now time.Time) (string, error) { + if strings.TrimSpace(executorID) == "" || len(executorID) > 128 { + return "", errors.New("binding needs an executor ID") + } + if roots == nil { + return "", errors.New("binding needs the authority client certificates are verified against") + } + if len(certPEM) > MaxCertificateBytes { + return "", fmt.Errorf("certificate file exceeds %d bytes", MaxCertificateBytes) + } + var certs []*x509.Certificate + rest := certPEM + for { + var block *pem.Block + block, rest = pem.Decode(rest) + if block == nil { + break + } + if block.Type != "CERTIFICATE" { + return "", fmt.Errorf("certificate file holds a %q block; give the public certificate only", block.Type) + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return "", fmt.Errorf("parse certificate: %w", err) + } + certs = append(certs, cert) + } + if len(bytes.TrimSpace(rest)) != 0 { + return "", errors.New("certificate file holds data that is not PEM") + } + if len(certs) == 0 { + return "", errors.New("certificate file holds no PEM certificate") + } + leaf := certs[0] + if leaf.IsCA { + return "", errors.New("the certificate is an authority, not an executor's client certificate") + } + if leaf.Subject.CommonName != executorID { + return "", fmt.Errorf("the certificate was issued for %q, not for executor %q", leaf.Subject.CommonName, executorID) + } + intermediates := x509.NewCertPool() + for _, cert := range certs[1:] { + intermediates.AddCert(cert) + } + if _, err := leaf.Verify(x509.VerifyOptions{ + Roots: roots, Intermediates: intermediates, CurrentTime: now, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + }); err != nil { + return "", fmt.Errorf("the certificate is not a client certificate of the configured authority: %w", err) + } + sum := sha256.Sum256(leaf.Raw) + return hex.EncodeToString(sum[:]), nil +} + +// Bind records fingerprint as the node credential of executorID without a +// token: the administrator who controls the database vouches for a +// certificate they issued. It replaces only executorID's own binding, leaves +// any unused token alone, and changes nothing when the same certificate is +// already bound, so running it again is harmless. +func (s *Store) Bind(ctx context.Context, executorID, fingerprint string) (Binding, error) { + if executorID == "" { + return Binding{}, errors.New("binding needs an executor ID") + } + if len(fingerprint) != 2*sha256.Size || strings.ToLower(fingerprint) != fingerprint { + return Binding{}, errors.New("binding needs a lowercase hex SHA-256 certificate fingerprint") + } + if _, err := hex.DecodeString(fingerprint); err != nil { + return Binding{}, errors.New("binding needs a lowercase hex SHA-256 certificate fingerprint") + } + result := Binding{Fingerprint: fingerprint} + err := s.write(ctx, func(q *database.Queries) error { + previous, err := q.GetExecutorEnrollment(ctx, executorID) + switch { + case errors.Is(err, sql.ErrNoRows): + case err != nil: + return fmt.Errorf("read executor enrollment: %w", err) + default: + result.Previous = previous + } + if result.Previous == fingerprint { + return nil + } + if err := q.SetExecutorEnrollment(ctx, database.SetExecutorEnrollmentParams{ + ExecutorID: executorID, Fingerprint: fingerprint, EnrolledAt: models.NewUTCTime(s.now()), + }); err != nil { + return fmt.Errorf("record executor enrollment: %w", err) + } + result.Changed = true + return nil + }) + if err != nil { + return Binding{}, err + } + return result, nil +} diff --git a/internal/dispatcher/enrollment/bind_test.go b/internal/dispatcher/enrollment/bind_test.go new file mode 100644 index 00000000..0876fef6 --- /dev/null +++ b/internal/dispatcher/enrollment/bind_test.go @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 ETH Zurich + +package enrollment_test + +import ( + "crypto/sha256" + "encoding/hex" + "errors" + "strings" + "testing" + "time" + + "github.com/netsec-ethz/debuglet/internal/dispatcher/enrollment" + "github.com/netsec-ethz/debuglet/internal/testtls" +) + +const boundExecutor = "5fe02882-0410-416c-9935-235090bcba0d" + +// TestBindRecordsAnAdministratorIssuedCertificate covers the deployment path: +// an executor whose certificate the administrator issued is bound without a +// token, binding the same certificate again changes nothing, and a reissued +// certificate replaces the binding of that executor ID only. +func TestBindRecordsAnAdministratorIssuedCertificate(t *testing.T) { + ctx, store, _ := openStore(t) + first, err := store.Bind(ctx, boundExecutor, nodeA) + if err != nil { + t.Fatalf("bind: %v", err) + } + if !first.Changed || first.Previous != "" || first.Fingerprint != nodeA { + t.Fatalf("first binding = %+v", first) + } + if err := store.Bound(ctx, boundExecutor, nodeA); err != nil { + t.Fatalf("bound certificate refused: %v", err) + } + again, err := store.Bind(ctx, boundExecutor, nodeA) + if err != nil { + t.Fatalf("bind again: %v", err) + } + if again.Changed || again.Previous != nodeA { + t.Fatalf("repeated binding = %+v, want unchanged", again) + } + if _, err := store.Bind(ctx, "other-executor", nodeA); err != nil { + t.Fatalf("bind another executor: %v", err) + } + rotated, err := store.Bind(ctx, boundExecutor, nodeB) + if err != nil { + t.Fatalf("rebind: %v", err) + } + if !rotated.Changed || rotated.Previous != nodeA { + t.Fatalf("rebinding = %+v", rotated) + } + if err := store.Bound(ctx, boundExecutor, nodeA); !errors.Is(err, enrollment.ErrWrongNode) { + t.Fatalf("the replaced certificate: %v", err) + } + if err := store.Bound(ctx, boundExecutor, nodeB); err != nil { + t.Fatalf("the new certificate: %v", err) + } + if err := store.Bound(ctx, "other-executor", nodeA); err != nil { + t.Fatalf("another executor's binding changed: %v", err) + } +} + +// TestBindLeavesOutstandingTokensAlone keeps a token an operator issued +// usable: binding is not a revocation. +func TestBindLeavesOutstandingTokensAlone(t *testing.T) { + ctx, store, _ := openStore(t) + token := mint(ctx, t, store, boundExecutor) + if _, err := store.Bind(ctx, boundExecutor, nodeA); err != nil { + t.Fatalf("bind: %v", err) + } + if err := store.Admit(ctx, boundExecutor, nodeB, token); err != nil { + t.Fatalf("token after binding: %v", err) + } +} + +func TestBindRefusesMalformedInput(t *testing.T) { + ctx, store, _ := openStore(t) + for name, fingerprint := range map[string]string{ + "empty": "", + "short": nodeA[:10], + "uppercase": strings.ToUpper(nodeA), + "not hex": strings.Repeat("z", 64), + } { + if _, err := store.Bind(ctx, boundExecutor, fingerprint); err == nil { + t.Errorf("%s fingerprint was bound", name) + } + } + if _, err := store.Bind(ctx, "", nodeA); err == nil { + t.Error("an empty executor ID was bound") + } + if err := store.Bound(ctx, boundExecutor, nodeA); !errors.Is(err, enrollment.ErrNotEnrolled) { + t.Fatalf("a refused binding was recorded: %v", err) + } +} + +func TestCertificateFingerprint(t *testing.T) { + dir := t.TempDir() + authority, err := testtls.NewAuthority(dir, "deployment-ca") + if err != nil { + t.Fatal(err) + } + client, err := authority.Issue(boundExecutor, testtls.Options{Client: true}) + if err != nil { + t.Fatal(err) + } + now := time.Now() + fingerprint, err := enrollment.CertificateFingerprint(boundExecutor, client.CertPEM, authority.Pool(), now) + if err != nil { + t.Fatalf("a client certificate of the authority was refused: %v", err) + } + // The value the transport reports for this certificate once verified. + sum := sha256.Sum256(client.Certificate.Certificate[0]) + if fingerprint != hex.EncodeToString(sum[:]) { + t.Fatalf("fingerprint = %s, want the SHA-256 of the leaf DER", fingerprint) + } + // A chain file with the authority appended names the same leaf. + chain := append(append([]byte{}, client.CertPEM...), authority.CertPEM...) + if got, err := enrollment.CertificateFingerprint(boundExecutor, chain, authority.Pool(), now); err != nil || got != fingerprint { + t.Fatalf("chain file: %s, %v", got, err) + } + + other, err := testtls.NewAuthority(t.TempDir(), "other-ca") + if err != nil { + t.Fatal(err) + } + foreign, err := other.Issue(boundExecutor, testtls.Options{Client: true}) + if err != nil { + t.Fatal(err) + } + server, err := authority.Issue(boundExecutor+"-server", testtls.Options{Server: true}) + if err != nil { + t.Fatal(err) + } + serverForID, err := authority.Issue(boundExecutor, testtls.Options{Server: true}) + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name, id, want string + pem []byte + at time.Time + }{ + {"another authority", boundExecutor, "not a client certificate of the configured authority", foreign.CertPEM, now}, + {"server-only usage", boundExecutor, "not a client certificate", serverForID.CertPEM, now}, + {"another executor's name", "1144ad6e-2c14-4e5c-ab72-c05a8e8770f2", "was issued for", client.CertPEM, now}, + {"a different common name", boundExecutor, "was issued for", server.CertPEM, now}, + {"expired", boundExecutor, "not a client certificate", client.CertPEM, now.Add(48 * time.Hour)}, + {"the authority itself", "deployment-ca", "is an authority", authority.CertPEM, now}, + {"a private key", boundExecutor, "public certificate only", append(append([]byte{}, client.CertPEM...), client.KeyPEM...), now}, + {"no certificate", boundExecutor, "no PEM certificate", []byte("\n"), now}, + {"trailing garbage", boundExecutor, "not PEM", append(append([]byte{}, client.CertPEM...), "garbage"...), now}, + {"empty ID", "", "executor ID", client.CertPEM, now}, + } { + if _, err := enrollment.CertificateFingerprint(tc.id, tc.pem, authority.Pool(), tc.at); err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: err = %v, want it to mention %q", tc.name, err, tc.want) + } + } + if _, err := enrollment.CertificateFingerprint(boundExecutor, client.CertPEM, nil, now); err == nil { + t.Error("a certificate was accepted with no authority") + } +}