diff --git a/README.md b/README.md index ff057413..effa937b 100644 --- a/README.md +++ b/README.md @@ -45,16 +45,22 @@ Download the [latest release](https://github.com/BlindspotSoftware/dutctl/releas 1. **Start the DUT Agent** ```bash - dutagent -a localhost:1024 -c ./contrib/dutagent-cfg-example.yaml + dutagent -a localhost:1024 -c ./contrib/dutagent-cfg-example.yaml -insecure ``` Run the DUT Agent locally with an example configuration in a separate terminal session. This test configuration does not require a connected DUT. + `-insecure` keeps this walkthrough to plain HTTP. Without it the agent serves TLS and + writes a self-signed key pair to `/var/lib/dutagent/tls/`, which a normal user account + cannot do — pass `-tls-cert ./cert.pem -tls-key ./key.pem` to put it somewhere writable + instead. See [Transport Security](./docs/README.md#transport-security). + 2. **Play around with the DUT Client** ```bash - # dutctl connect to localhost:1024 by default. + # dutctl connects to localhost:1024 by default. + # Match the agent's transport: -insecure here, because the agent above uses it. # Use 'list' to see the available devices that are managed be the agent: - dutctl list + dutctl list -insecure # You can discover the available commands per device and run them. # Check out the usage information to learn how: diff --git a/cmds/dutagent/dutagent.go b/cmds/dutagent/dutagent.go index a8450c96..28d298d6 100644 --- a/cmds/dutagent/dutagent.go +++ b/cmds/dutagent/dutagent.go @@ -9,6 +9,7 @@ package main import ( "context" + "crypto/tls" "errors" "flag" "fmt" @@ -26,6 +27,7 @@ import ( "github.com/BlindspotSoftware/dutctl/internal/dutagent/locker" "github.com/BlindspotSoftware/dutctl/internal/log" "github.com/BlindspotSoftware/dutctl/internal/rpc" + "github.com/BlindspotSoftware/dutctl/internal/tlsutil" "github.com/BlindspotSoftware/dutctl/pkg/dut" "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1/dutctlv1connect" "gopkg.in/yaml.v3" @@ -42,6 +44,19 @@ const ( versionFlagInfo = `Print version information and exit` logLevelInfo = `Log level: debug, info, warn, or error` logJSONInfo = `Emit logs as JSON instead of human-readable text` + insecureInfo = `Disable TLS and serve plain HTTP/2 cleartext (h2c); clients must match` + tlsCertInfo = `Path to the TLS certificate file; a self-signed pair is generated if neither it nor the key exists` + tlsKeyInfo = `Path to the TLS key file; a self-signed pair is generated if neither it nor the certificate exists` +) + +// Default locations of the agent's TLS key pair. Both are generated on first +// start if neither exists — see internal/tlsutil. They live under /var/lib +// rather than /etc because the packaged unit runs unprivileged with +// ProtectSystem=strict, which makes /etc read-only; /var/lib/dutagent is the +// one path it may write (see packaging/dutagent.service). +const ( + defaultTLSCertPath = "/var/lib/dutagent/tls/cert.pem" + defaultTLSKeyPath = "/var/lib/dutagent/tls/key.pem" ) func newAgent(stdout io.Writer, exitFunc func(int), args []string) *agent { @@ -59,6 +74,9 @@ func newAgent(stdout io.Writer, exitFunc func(int), args []string) *agent { fs.BoolVar(&agt.versionFlag, "v", false, versionFlagInfo) fs.StringVar(&agt.logLevel, "log", "debug", logLevelInfo) fs.BoolVar(&agt.logJSON, "log-json", false, logJSONInfo) + fs.BoolVar(&agt.insecure, "insecure", false, insecureInfo) + fs.StringVar(&agt.tlsCertPath, "tls-cert", defaultTLSCertPath, tlsCertInfo) + fs.StringVar(&agt.tlsKeyPath, "tls-key", defaultTLSKeyPath, tlsKeyInfo) //nolint:errcheck // flag.Parse always returns no error because of flag.ExitOnError fs.Parse(args[1:]) @@ -79,10 +97,27 @@ type agent struct { server string logLevel string logJSON bool + insecure bool + tlsCertPath string + tlsKeyPath string // state config config modulesNeedDeinit bool + // cert is the key pair the RPC service serves with, resolved by loadTLSCert + // before module initialization. Unset under -insecure. + cert tls.Certificate +} + +// security maps the -insecure flag to the transport the agent dials the +// dutserver with. The serving side branches on agt.insecure directly, since it +// needs the certificate too and not just the transport. +func (agt *agent) security() rpc.Security { + if agt.insecure { + return rpc.Insecure + } + + return rpc.TLS } // config holds the dutagent configuration that is parsed from YAML data. @@ -167,10 +202,36 @@ func printInitErr(err error) { slog.Error("module error", "err", err) } +// loadTLSCert resolves the key pair the RPC service will serve with: it loads +// -tls-cert/-tls-key, generating a self-signed pair if neither file exists yet. +// It is a no-op under -insecure. See internal/tlsutil for what the generated +// pair does and does not protect. +func (agt *agent) loadTLSCert() error { + if agt.insecure { + return nil + } + + cert, generated, err := tlsutil.LoadOrGenerateCert(agt.tlsCertPath, agt.tlsKeyPath) + if err != nil { + return fmt.Errorf("loading TLS certificate: %w", err) + } + + if generated { + slog.Info("generated self-signed TLS certificate", "cert", agt.tlsCertPath, "key", agt.tlsKeyPath) + } + + agt.cert = cert + + return nil +} + // startRPCService starts the RPC service and serves until ctx is cancelled (a // signal), draining in-flight requests, or until the server stops on its own. It // returns the server error, if any; the caller classifies a graceful stop via // ctx.Err(). +// +// The service is served over TLS unless -insecure was given, using the key pair +// loadTLSCert already resolved. func (agt *agent) startRPCService(ctx context.Context) error { service := &rpcService{ devices: agt.config.Devices, @@ -187,15 +248,24 @@ func (agt *agent) startRPCService(ctx context.Context) error { ) mux.Handle(path, handler) - slog.Info("rpc service listening", "addr", agt.address) + if agt.insecure { + slog.Warn("rpc service listening WITHOUT TLS", "addr", agt.address) - return rpc.ListenAndServe(ctx, agt.address, mux) + return rpc.ListenAndServe(ctx, agt.address, mux) + } + + slog.Info("rpc service listening with TLS", "addr", agt.address, "cert", agt.tlsCertPath) + + return rpc.ListenAndServeTLS(ctx, agt.address, mux, agt.cert) } func (agt *agent) registerWithServer() error { slog.Info("registering with server", "server", agt.server) - client := rpc.NewRelayClient(agt.server) + // The registration RPC follows the agent's own transport setting. dutserver + // takes the same -insecure flag, so a deployment runs all three binaries on + // one setting; mixing them fails here at connect time. + client := rpc.NewRelayClient(agt.server, agt.security()) req := connect.NewRequest(&pb.RegisterRequest{ Devices: agt.config.Devices.Names(), Address: agt.address, @@ -261,6 +331,17 @@ func (agt *agent) start() { agt.cleanup(exit1) } + // Resolve the TLS key pair before touching any hardware. Doing it here rather + // than at listen time means an unwritable certificate directory fails startup + // cleanly, instead of after initModules has already driven the DUTs — and it + // puts the check inside the -dry-run path, which exists to catch exactly this + // kind of misconfiguration. + err = agt.loadTLSCert() + if err != nil { + slog.Error("loading TLS certificate failed", "err", err) + agt.cleanup(exit1) + } + // initCtx carries module initialization; it flows into every module's Init via // internal/log. It derives from the signal context so Ctrl-C interrupts a slow // startup, and is bounded by initTimeout so a wedged Init fails startup instead diff --git a/cmds/dutctl/dutctl.go b/cmds/dutctl/dutctl.go index 47b44c21..93d90720 100644 --- a/cmds/dutctl/dutctl.go +++ b/cmds/dutctl/dutctl.go @@ -15,6 +15,7 @@ import ( "log/slog" "os" "os/signal" + "strings" "syscall" "connectrpc.com/connect" @@ -57,6 +58,10 @@ releases it; add the force keyword to release a lock held by another user. Locks are advisory, so reserve a device only as long as you need it. When dutctl is run without any positional arguments, it defaults to the list command. + +dutctl talks to the agent over TLS by default. The agent's certificate is not +verified, so the connection is encrypted but not authenticated. Use -insecure to +talk to an agent that was started with -insecure; both ends must agree. ` // Usage strings for the command-line flags, shown in the OPTIONS section of dutctl -h. @@ -67,6 +72,7 @@ const ( noColorUsage = `Disable colored output` userUsage = `User Identity of the user of the device, defaults to @` logUsage = `Client-side diagnostic logging (on stderr), debug|warn|none, default is warn` + insecureUsage = `Disable TLS and talk plain HTTP/2 cleartext (h2c); for an agent started with -insecure` ) func newApp(stdin io.Reader, stdout, stderr io.Writer, exitFunc func(int), args []string) *application { @@ -94,6 +100,7 @@ func newApp(stdin io.Reader, stdout, stderr io.Writer, exitFunc func(int), args fs.BoolVar(&app.verbose, "v", false, verboseUsage) fs.BoolVar(&app.noColor, "no-color", false, noColorUsage) fs.StringVar(&app.user, "u", auth.Default().User(), userUsage) + fs.BoolVar(&app.insecure, "insecure", false, insecureUsage) mode := logModeWarn fs.Var(&mode, "log", logUsage) @@ -136,6 +143,7 @@ type application struct { verbose bool noColor bool user string + insecure bool args []string printFlagDefaults func() @@ -150,8 +158,14 @@ type application struct { } func (app *application) setupRPCClient() { + sec := rpc.TLS + if app.insecure { + sec = rpc.Insecure + } + app.rpcClient = rpc.NewDeviceClient( app.serverAddr, + sec, connect.WithInterceptors(rpc.NewVersionAdvisor(buildinfo.Version)), ) } @@ -316,7 +330,7 @@ func (app *application) exit(err error) { // Render the terminating error through the formatter (stderr, format-aware). app.formatter.WriteContent(output.Content{ Type: output.TypeGeneral, - Data: userFacingError(err, app.serverAddr), + Data: userFacingError(err, app.serverAddr, app.insecure), IsError: true, }) @@ -334,19 +348,48 @@ func (app *application) exit(err error) { // gives the common "agent unreachable" case a friendlier line; other errors — // including client-side ones like a bad command line or a missing local file — // render unchanged. -func userFacingError(err error, serverAddr string) string { +func userFacingError(err error, serverAddr string, insecure bool) string { var connErr *connect.Error if !errors.As(err, &connErr) { return err.Error() } if connErr.Code() == connect.CodeUnavailable { - return fmt.Sprintf("cannot reach dutagent at %s (%s)", serverAddr, connErr.Message()) + msg := fmt.Sprintf("cannot reach dutagent at %s (%s)", serverAddr, connErr.Message()) + + if hint := transportHint(connErr.Message(), insecure); hint != "" { + msg += "\n" + hint + } + + return msg } return connErr.Message() } +// transportHint names the fix for a client/agent transport mismatch, which is +// otherwise near-undiagnosable from the client side. The mismatch happens below +// the RPC layer, so the version interceptors never exchange headers and cannot +// report it; all the user gets is a dial failure whose message ("unexpected EOF") +// says nothing about TLS. It matches on the transport's message text because +// neither side surfaces a typed error for this. +func transportHint(msg string, insecure bool) string { + // A cleartext client reaching a TLS agent: the agent drops the connection on + // a request that is not a TLS handshake, which surfaces here as a truncated + // read. + if insecure && strings.Contains(msg, "unexpected EOF") { + return "hint: the agent may be serving TLS; retry without -insecure" + } + + // A TLS client reaching a cleartext agent: Go's transport recognises the + // plain HTTP response and says so. + if !insecure && strings.Contains(msg, "server gave HTTP response to HTTPS client") { + return "hint: the agent is serving cleartext; retry with -insecure, or upgrade the agent" + } + + return "" +} + func (app *application) printVersion() { app.formatter.WriteContent(output.Content{ Type: output.TypeVersion, diff --git a/cmds/dutctl/errmsg_test.go b/cmds/dutctl/errmsg_test.go index 047fd82f..d07b66c0 100644 --- a/cmds/dutctl/errmsg_test.go +++ b/cmds/dutctl/errmsg_test.go @@ -14,13 +14,16 @@ import ( // TestUserFacingError verifies error rendering: a non-connect error passes through // unchanged, a connect error drops its "code:" prefix (via Message), and // CodeUnavailable gets the friendly "cannot reach" line naming the server address. +// A transport mismatch additionally gets a hint naming the fix, since it fails +// below the RPC layer and the message alone does not mention TLS. func TestUserFacingError(t *testing.T) { const addr = "host:1234" tests := []struct { - name string - err error - want string + name string + err error + insecure bool + want string }{ { name: "non-connect error unchanged", @@ -37,11 +40,31 @@ func TestUserFacingError(t *testing.T) { err: connect.NewError(connect.CodeUnavailable, errors.New("connection refused")), want: "cannot reach dutagent at host:1234 (connection refused)", }, + { + name: "cleartext client against TLS agent gets a hint", + err: connect.NewError(connect.CodeUnavailable, errors.New("unexpected EOF")), + insecure: true, + want: "cannot reach dutagent at host:1234 (unexpected EOF)\n" + + "hint: the agent may be serving TLS; retry without -insecure", + }, + { + name: "TLS client against cleartext agent gets a hint", + err: connect.NewError(connect.CodeUnavailable, + errors.New("http: server gave HTTP response to HTTPS client")), + want: "cannot reach dutagent at host:1234 (http: server gave HTTP response to HTTPS client)\n" + + "hint: the agent is serving cleartext; retry with -insecure, or upgrade the agent", + }, + { + name: "no hint when the mismatch marker belongs to the other direction", + err: connect.NewError(connect.CodeUnavailable, errors.New("unexpected EOF")), + insecure: false, + want: "cannot reach dutagent at host:1234 (unexpected EOF)", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := userFacingError(tt.err, addr); got != tt.want { + if got := userFacingError(tt.err, addr, tt.insecure); got != tt.want { t.Errorf("userFacingError = %q, want %q", got, tt.want) } }) diff --git a/cmds/exp/dutserver/README.md b/cmds/exp/dutserver/README.md index ecd5f382..7a6b1fef 100644 --- a/cmds/exp/dutserver/README.md +++ b/cmds/exp/dutserver/README.md @@ -31,7 +31,7 @@ As a proof of concept, this implementation has several limitations that would ne - **No Agent Health Monitoring:** Lacks handshake or keep-alive mechanisms for registered agents. - **Limited Error Handling:** Error recovery in network failure scenarios is minimal. - **No Load Balancing:** Does not support multiple server instances or load balancing. -- **No TLS Configuration:** Communication is not secured by default. +- **No Certificate Verification:** TLS is on by default (see [Transport Security](../../../docs/README.md#transport-security)), but the self-signed certificate is not verified in either direction, and there is no client authentication. ## Demo @@ -43,12 +43,15 @@ go build ./cmds/exp/dutserver ``` Open up four terminal sessions referred to as `T1`, `T2`, `T3`, `T4`, +All three binaries below are started with `-insecure`, which keeps the demo on plain HTTP. The transport is a +deployment-wide setting: client, agents and server must all agree, so dropping the flag means dropping it everywhere. + ### Starting the Server In `T1` start the DUT Server with default settings: ```bash # Start the DUT Server locally on default port (1024) -./dutserver +./dutserver -insecure ``` ### Registering Devices @@ -57,7 +60,7 @@ Start a first agent in `T3` using a basic example configuration with one device ```bash # Configure the agent to connect to the server -./dutagent -a localhost:1025 -c ./cmds/exp/contrib/config-1.yaml -server localhost:1024 +./dutagent -a localhost:1025 -c ./cmds/exp/contrib/config-1.yaml -server localhost:1024 -insecure ``` @@ -65,7 +68,7 @@ In `T4` start a second agent with different port and another basic example confi ```bash # Configure the agent to connect to the server -./dutagent -a localhost:1026 -c ./cmds/exp/contrib/config-2.yaml -server localhost:1024 +./dutagent -a localhost:1026 -c ./cmds/exp/contrib/config-2.yaml -server localhost:1024 -insecure ``` @@ -75,13 +78,13 @@ Play around with the DUT Client in `T2` ```bash # List available devices using the default address localhost:1024 which is the DUT server -./dutctl list +./dutctl list -insecure # List the devices of a dedicated agent only -./dutctl -s localhost:1025 list +./dutctl -s localhost:1025 -insecure list # Run a interactive command on a remote device via the DUT server -./dutctl device2 repeat +./dutctl -insecure device2 repeat ``` The complete functionality of the client is available via the DUT Server, see `./dutctl -h` diff --git a/cmds/exp/dutserver/dutserver.go b/cmds/exp/dutserver/dutserver.go index 61bc171f..13c83e1b 100644 --- a/cmds/exp/dutserver/dutserver.go +++ b/cmds/exp/dutserver/dutserver.go @@ -9,6 +9,7 @@ package main import ( "context" "flag" + "fmt" "log/slog" "net/http" "os" @@ -17,6 +18,7 @@ import ( "github.com/BlindspotSoftware/dutctl/internal/log" "github.com/BlindspotSoftware/dutctl/internal/rpc" + "github.com/BlindspotSoftware/dutctl/internal/tlsutil" "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1/dutctlv1connect" ) @@ -24,6 +26,15 @@ const ( addressInfo = `Server address and port in the format: address:port` logLevelInfo = `Log level: debug, info, warn, or error` logJSONInfo = `Emit logs as JSON instead of human-readable text` + insecureInfo = `Disable TLS and serve plain HTTP/2 cleartext (h2c); also used to dial the agents` + tlsCertInfo = `Path to the TLS certificate file; a self-signed pair is generated if neither it nor the key exists` + tlsKeyInfo = `Path to the TLS key file; a self-signed pair is generated if neither it nor the certificate exists` +) + +// Default locations of the server's TLS key pair, mirroring dutagent's. +const ( + defaultTLSCertPath = "/var/lib/dutserver/tls/cert.pem" + defaultTLSKeyPath = "/var/lib/dutserver/tls/key.pem" ) func newServer(exitFunc func(int), args []string) *server { @@ -31,13 +42,16 @@ func newServer(exitFunc func(int), args []string) *server { svr.exit = exitFunc - f := flag.NewFlagSet(args[0], flag.ExitOnError) - f.StringVar(&svr.address, "s", "localhost:1024", addressInfo) - f.StringVar(&svr.logLevel, "log", "debug", logLevelInfo) - f.BoolVar(&svr.logJSON, "log-json", false, logJSONInfo) + fs := flag.NewFlagSet(args[0], flag.ExitOnError) + fs.StringVar(&svr.address, "s", "localhost:1024", addressInfo) + fs.StringVar(&svr.logLevel, "log", "debug", logLevelInfo) + fs.BoolVar(&svr.logJSON, "log-json", false, logJSONInfo) + fs.BoolVar(&svr.insecure, "insecure", false, insecureInfo) + fs.StringVar(&svr.tlsCertPath, "tls-cert", defaultTLSCertPath, tlsCertInfo) + fs.StringVar(&svr.tlsKeyPath, "tls-key", defaultTLSKeyPath, tlsKeyInfo) //nolint:errcheck // flag.Parse never returns an error because of flag.ExitOnError - f.Parse(args[1:]) + fs.Parse(args[1:]) return &svr } @@ -47,9 +61,23 @@ type server struct { exit func(int) // flags - address string - logLevel string - logJSON bool + address string + logLevel string + logJSON bool + insecure bool + tlsCertPath string + tlsKeyPath string +} + +// security is the transport the server serves on, and dials its agents with. +// One setting covers both directions: the relay sits between dutctl and the +// agents, and a deployment that encrypts one hop wants the other encrypted too. +func (svr *server) security() rpc.Security { + if svr.insecure { + return rpc.Insecure + } + + return rpc.TLS } type exitCode int @@ -71,10 +99,15 @@ func (svr *server) cleanup(code exitCode) { // signal), draining in-flight requests, or until the server stops on its own. It // returns the server error, if any; the caller classifies a graceful stop via // ctx.Err(). +// +// The service is served over TLS unless -insecure was given, on the same terms +// as dutagent: the certificate comes from -tls-cert/-tls-key and is generated +// self-signed if neither exists. See internal/tlsutil for what that protects. func (svr *server) startRPCService(ctx context.Context) error { // TODO: load registered DUTs from a file. service := &rpcService{ agents: make(map[string]*agent), + sec: svr.security(), } mux := http.NewServeMux() @@ -88,9 +121,24 @@ func (svr *server) startRPCService(ctx context.Context) error { path, handler = dutctlv1connect.NewRelayServiceHandler(service) mux.Handle(path, handler) - slog.Info("rpc service listening", "addr", svr.address) + if svr.insecure { + slog.Warn("rpc service listening WITHOUT TLS", "addr", svr.address) + + return rpc.ListenAndServe(ctx, svr.address, mux) + } + + cert, generated, err := tlsutil.LoadOrGenerateCert(svr.tlsCertPath, svr.tlsKeyPath) + if err != nil { + return fmt.Errorf("loading TLS certificate: %w", err) + } + + if generated { + slog.Info("generated self-signed TLS certificate", "cert", svr.tlsCertPath, "key", svr.tlsKeyPath) + } + + slog.Info("rpc service listening with TLS", "addr", svr.address, "cert", svr.tlsCertPath) - return rpc.ListenAndServe(ctx, svr.address, mux) + return rpc.ListenAndServeTLS(ctx, svr.address, mux, cert) } // start orchestrates the dutserver execution. diff --git a/cmds/exp/dutserver/rpc.go b/cmds/exp/dutserver/rpc.go index 4f1d73f7..59f63a50 100644 --- a/cmds/exp/dutserver/rpc.go +++ b/cmds/exp/dutserver/rpc.go @@ -29,6 +29,8 @@ type agent struct { // address is the address of the DUT agent. address string + // sec is the transport to dial the agent with, inherited from the server. + sec rpc.Security // client is the Connect RPC client for the DUT agent. // Do not use this client directly, but use agent's conn method. client dutctlv1connect.DeviceServiceClient @@ -45,7 +47,7 @@ func (a *agent) conn(ctx context.Context) dutctlv1connect.DeviceServiceClient { // tying it to a single request's context would be a bug. A per-RPC // deadline belongs on the call context passed to the forwarders. log.FromContext(ctx).Debug("spawning client for agent", "agent", a.address) - a.client = rpc.NewDeviceClient(a.address) + a.client = rpc.NewDeviceClient(a.address, a.sec) } return a.client @@ -65,6 +67,11 @@ type rpcService struct { // agents holds handles of the registered DUT agents. agents map[string]*agent + + // sec is the transport used to dial the agents. It follows the server's own + // -insecure flag: the relay does not know how each agent was started, so a + // deployment runs all three binaries on the same setting. + sec rpc.Security } // findAgent returns the handle for the DUT agent, that controls the device with the given name. @@ -96,7 +103,7 @@ func (s *rpcService) addAgent(ctx context.Context, address string, devices []str } for _, device := range devices { - s.agents[device] = &agent{address: address} + s.agents[device] = &agent{address: address, sec: s.sec} } l.Info("agent registered", "agent", address, "devices", devices) @@ -149,7 +156,7 @@ func (s *rpcService) Commands( return nil, connect.NewError(connect.CodeNotFound, err) } - res, err := forwardCommandsReq(log.WithScope(ctx, "relay"), agent.address, req) + res, err := forwardCommandsReq(log.WithScope(ctx, "relay"), agent.address, s.sec, req) if err != nil { l.Error("forwarding to agent failed", "agent", agent.address, "err", err) @@ -189,7 +196,7 @@ func (s *rpcService) Details( return nil, connect.NewError(connect.CodeNotFound, err) } - res, err := forwardDetailsReq(log.WithScope(ctx, "relay"), agent.address, req) + res, err := forwardDetailsReq(log.WithScope(ctx, "relay"), agent.address, s.sec, req) if err != nil { l.Error("forwarding to agent failed", "agent", agent.address, "err", err) @@ -481,12 +488,13 @@ func responseKind(res *pb.RunResponse) string { func forwardCommandsReq( ctx context.Context, url string, + sec rpc.Security, req *connect.Request[pb.CommandsRequest], ) (*connect.Response[pb.CommandsResponse], error) { log.FromContext(ctx).Debug("forwarding commands request to agent", "agent", url) // TODO: potential resource leak. Investigate how clients can be reused or closed. // For now, we spawn a new client for each request. - client := rpc.NewDeviceClient(url) + client := rpc.NewDeviceClient(url, sec) // ctx carries the caller's cancellation and, since the dutctl client now sets a // per-call deadline that connect propagates as a grpc-timeout header, an @@ -501,12 +509,13 @@ func forwardCommandsReq( func forwardDetailsReq( ctx context.Context, url string, + sec rpc.Security, req *connect.Request[pb.DetailsRequest], ) (*connect.Response[pb.DetailsResponse], error) { log.FromContext(ctx).Debug("forwarding details request to agent", "agent", url) // TODO: potential resource leak. Investigate how clients can be reused or closed. // For now, we spawn a new client for each request. - client := rpc.NewDeviceClient(url) + client := rpc.NewDeviceClient(url, sec) // ctx carries the caller's cancellation and, since the dutctl client now sets a // per-call deadline that connect propagates as a grpc-timeout header, an diff --git a/docs/README.md b/docs/README.md index 636df7d2..b273066d 100644 --- a/docs/README.md +++ b/docs/README.md @@ -31,6 +31,38 @@ relations. Its interface towards a DUT Client is the same as the one from a DUT from the client side to which instance to talk to. Additionally, the DUT Server could expose further interfaces like a REST API to observe the fleet of DUTs. +# Transport Security + +RPCs are carried over TLS by default, in every direction: DUT Client to DUT Agent, DUT Client to DUT Server, DUT Server +to DUT Agent, and the DUT Agent's registration call to the DUT Server. + +**What this protects, and what it does not.** The DUT Agent and DUT Server serve a self-signed certificate, which they +generate on first start if none is present. No peer verifies the certificate it is offered. The connection is therefore +**encrypted but not authenticated**: it stops passive eavesdropping on the wire, but not an active man-in-the-middle. +There is no client authentication either — any client that can reach an agent may talk to it, exactly as before. Supplying +your own CA-issued certificate via `-tls-cert`/`-tls-key` does not change this today, because no client in the project +verifies what it is offered. + +**Both ends must agree.** The transport is a deployment-wide setting, not something the peers negotiate: a mismatch fails +below the RPC layer, before any headers are exchanged, so it cannot be reported as a protocol error. `dutctl` recognises +the two mismatch cases and prints a hint naming the fix. + +| Flag | Applies to | Effect | +| --- | --- | --- | +| `-insecure` | `dutctl`, `dutagent`, `dutserver` | Use plain HTTP/2 cleartext (h2c). All participants must be started with it. | +| `-tls-cert` | `dutagent`, `dutserver` | Path to the certificate. A self-signed pair is generated if neither it nor the key exists. | +| `-tls-key` | `dutagent`, `dutserver` | Path to the private key, written mode `0600`. | + +**Generated key pairs.** `dutagent` defaults to `/var/lib/dutagent/tls/{cert,key}.pem` and `dutserver` to +`/var/lib/dutserver/tls/{cert,key}.pem` — under `/var/lib` rather than `/etc` because the packaged systemd unit runs +unprivileged with `ProtectSystem=strict`. Generation happens only when *neither* file exists; if one is present the pair +is loaded as-is and a load failure is reported rather than silently overwritten. Nothing rotates a certificate, and +expiry is never checked, since no peer verifies it. + +**Upgrading.** This is a breaking change. A `dutctl` from before this release cannot talk to an upgraded agent, and an +upgraded `dutctl` cannot talk to an older agent. Either upgrade both ends together, or run every participant with +`-insecure` to keep the previous cleartext behaviour until you can. + # Communication Design The distributed entities of the DUT Control system communicate via Remote Procedure Calls (RPCs), which are defined in diff --git a/internal/rpc/client.go b/internal/rpc/client.go index 99be8c17..6b4c854d 100644 --- a/internal/rpc/client.go +++ b/internal/rpc/client.go @@ -5,6 +5,7 @@ package rpc import ( + "crypto/tls" "fmt" "net" "net/http" @@ -14,23 +15,51 @@ import ( "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1/dutctlv1connect" ) +// Security selects the transport a client dials with. It is an explicit argument +// rather than a default because the choice is not observable at the call site +// otherwise, and getting it wrong fails at connect time with an opaque protocol +// error rather than a clear mismatch. +type Security int + +const ( + // TLS dials over HTTPS with HTTP/2 negotiated by ALPN. The server certificate + // is not verified — see newTLSClient. It is the zero value, and every value + // other than Insecure is treated as TLS, so a forgotten or malformed argument + // fails closed. + TLS Security = iota + // Insecure dials over HTTP/2 cleartext (h2c), with no encryption at all. + Insecure +) + // NewDeviceClient returns a DeviceService client for the agent (or server) at -// addr, speaking gRPC over HTTP/2 cleartext (h2c). Extra options — typically -// connect.WithInterceptors(NewVersionAdvisor(...)) — are appended after the -// mandatory WithGRPC. -func NewDeviceClient(addr string, opts ...connect.ClientOption) dutctlv1connect.DeviceServiceClient { - return dutctlv1connect.NewDeviceServiceClient(newH2CClient(), url(addr), clientOptions(opts)...) +// addr, speaking gRPC over HTTP/2 — TLS or cleartext (h2c) as sec selects. Extra +// options — typically connect.WithInterceptors(NewVersionAdvisor(...)) — are +// appended after the mandatory WithGRPC. +func NewDeviceClient(addr string, sec Security, opts ...connect.ClientOption) dutctlv1connect.DeviceServiceClient { + return dutctlv1connect.NewDeviceServiceClient(newClient(sec), url(addr, sec), clientOptions(opts)...) } // NewRelayClient returns a RelayService client for the server at addr, speaking -// gRPC over HTTP/2 cleartext (h2c). +// gRPC over HTTP/2 — TLS or cleartext (h2c) as sec selects. // //nolint:ireturn // returns the connect-generated RelayServiceClient interface by design -func NewRelayClient(addr string, opts ...connect.ClientOption) dutctlv1connect.RelayServiceClient { - return dutctlv1connect.NewRelayServiceClient(newH2CClient(), url(addr), clientOptions(opts)...) +func NewRelayClient(addr string, sec Security, opts ...connect.ClientOption) dutctlv1connect.RelayServiceClient { + return dutctlv1connect.NewRelayServiceClient(newClient(sec), url(addr, sec), clientOptions(opts)...) } -func url(addr string) string { return fmt.Sprintf("http://%s", addr) } +func url(addr string, sec Security) string { return fmt.Sprintf("%s://%s", sec.scheme(), addr) } + +// scheme is the URL scheme matching the transport s selects. +func (s Security) scheme() string { + switch s { + case Insecure: + return "http" + case TLS: + return "https" + default: + return "https" // Unknown value: fail closed, as documented on TLS. + } +} func clientOptions(opts []connect.ClientOption) []connect.ClientOption { return append([]connect.ClientOption{connect.WithGRPC()}, opts...) @@ -38,9 +67,10 @@ func clientOptions(opts []connect.ClientOption) []connect.ClientOption { // dialTimeout bounds establishing a new TCP connection. It is stream-safe — it // caps connection setup only, never the lifetime of a streaming RPC — and it is -// the one transport-level bound that matters to the one-shot dutctl CLI: a -// per-RPC context deadline (see cmds/dutctl) already bounds a unary dial, but the -// deadline-less streaming Run relies on this to fail fast on an unreachable agent. +// one of the two transport-level bounds that matter to the one-shot dutctl CLI +// (tlsHandshakeTimeout is the other): a per-RPC context deadline (see cmds/dutctl) +// already bounds a unary dial, but the deadline-less streaming Run relies on this +// to fail fast on an unreachable agent. const dialTimeout = 10 * time.Second // idleConnTimeout reaps a pooled connection after it has been idle this long. It @@ -50,9 +80,59 @@ const dialTimeout = 10 * time.Second // keep a dead keep-alive across an agent restart instead of re-dialing fresh. const idleConnTimeout = 90 * time.Second -// newH2CClient builds the shared HTTP/2-cleartext client used for every RPC -// connection. It is unexported: callers obtain a typed client via NewDeviceClient -// or NewRelayClient rather than the raw transport. +// tlsHandshakeTimeout bounds the TLS handshake, so a host that accepts the TCP +// connection but never completes the handshake fails fast instead of hanging the +// deadline-less streaming Run. It is the TLS counterpart to dialTimeout. +const tlsHandshakeTimeout = 10 * time.Second + +// newClient builds the shared HTTP client used for every RPC connection over the +// transport sec selects. It is unexported: callers obtain a typed client via +// NewDeviceClient or NewRelayClient rather than the raw transport. +func newClient(sec Security) *http.Client { + if sec == Insecure { + return newH2CClient() + } + + return newTLSClient() +} + +// newTLSClient builds the HTTP/2-over-TLS client. It does NOT verify the server +// certificate: the agent typically serves a self-signed certificate it generates +// itself (see internal/tlsutil), so there is usually no CA to chain to. +// Verification is skipped unconditionally, so an operator-supplied CA-issued +// certificate is not checked either. The connection is therefore encrypted but +// not authenticated — it stops passive eavesdropping on the wire, not an active +// man-in-the-middle. Client authentication is a separate concern and is not +// provided here either: any client may connect. +// +// TLS 1.3 is the floor in both directions: both peers are built from this repo, +// so there is no legacy implementation to accommodate, and it pairs with the +// Ed25519 key tlsutil generates. +func newTLSClient() *http.Client { + transport := &http.Transport{ + // Bound connection establishment only; safe for the streaming Run. + DialContext: (&net.Dialer{Timeout: dialTimeout}).DialContext, + // Reap idle pooled connections; never affects an active stream. + IdleConnTimeout: idleConnTimeout, + TLSHandshakeTimeout: tlsHandshakeTimeout, + TLSClientConfig: &tls.Config{ + //nolint:gosec // self-signed agent certificate; encryption only, see doc comment + InsecureSkipVerify: true, + MinVersion: tls.VersionTLS13, + }, + } + // A Transport with a custom TLSClientConfig does not negotiate HTTP/2 on its + // own; request it explicitly, since gRPC (connect.WithGRPC) needs HTTP/2. + transport.Protocols = new(http.Protocols) + transport.Protocols.SetHTTP2(true) + + return &http.Client{ + Transport: transport, + // No http.Client.Timeout — see newH2CClient. + } +} + +// newH2CClient builds the HTTP/2-cleartext client used when TLS is disabled. func newH2CClient() *http.Client { // Use the HTTP/2 protocol without TLS (h2c). transport := &http.Transport{ diff --git a/internal/rpc/client_test.go b/internal/rpc/client_test.go new file mode 100644 index 00000000..9cbb518c --- /dev/null +++ b/internal/rpc/client_test.go @@ -0,0 +1,114 @@ +// Copyright 2025 Blindspot Software +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package rpc + +import ( + "crypto/tls" + "net/http" + "testing" +) + +// TestNewClientTLS pins the transport properties the TLS client must have: it +// speaks HTTP/2 (gRPC does not work without it, and a custom TLSClientConfig +// disables the automatic upgrade), it accepts the agent's unverifiable +// self-signed certificate, and it floors the version at TLS 1.3. +func TestNewClientTLS(t *testing.T) { + if got := TLS.scheme(); got != "https" { + t.Errorf("scheme() = %q, want %q", got, "https") + } + + transport := transportOf(t, newClient(TLS)) + + if !transport.Protocols.HTTP2() { + t.Error("Protocols.HTTP2() = false, want true - gRPC requires HTTP/2") + } + + if transport.Protocols.UnencryptedHTTP2() { + t.Error("Protocols.UnencryptedHTTP2() = true, want false on the TLS transport") + } + + if transport.TLSClientConfig == nil { + t.Fatal("TLSClientConfig is nil") + } + + if got := transport.TLSClientConfig.MinVersion; got != tls.VersionTLS13 { + t.Errorf("MinVersion = %#x, want TLS 1.3 (%#x)", got, tls.VersionTLS13) + } + + // The agent serves a self-signed certificate, so verification is off by + // design - encryption without authentication. + if !transport.TLSClientConfig.InsecureSkipVerify { + t.Error("InsecureSkipVerify = false, want true for the self-signed agent certificate") + } + + if transport.TLSHandshakeTimeout == 0 { + t.Error("TLSHandshakeTimeout is unset, want a bound on the handshake") + } +} + +// TestNewClientInsecure pins the cleartext client: h2c, and no TLS settings at +// all. +func TestNewClientInsecure(t *testing.T) { + if got := Insecure.scheme(); got != "http" { + t.Errorf("scheme() = %q, want %q", got, "http") + } + + transport := transportOf(t, newClient(Insecure)) + + if !transport.Protocols.UnencryptedHTTP2() { + t.Error("Protocols.UnencryptedHTTP2() = false, want true - gRPC requires HTTP/2") + } + + if transport.Protocols.HTTP2() { + t.Error("Protocols.HTTP2() = true, want false on the cleartext transport") + } + + if transport.TLSClientConfig != nil { + t.Error("TLSClientConfig is set on the cleartext transport") + } +} + +// transportOf returns the client's *http.Transport, asserting the client carries +// no overall timeout: http.Client.Timeout bounds the whole exchange including the +// response body, which would abort the long-lived streaming Run. +func transportOf(t *testing.T, client *http.Client) *http.Transport { + t.Helper() + + if client == nil { + t.Fatal("newClient returned nil client") + } + + if client.Timeout != 0 { + t.Errorf("Client.Timeout = %v, want 0", client.Timeout) + } + + transport, ok := client.Transport.(*http.Transport) + if !ok { + t.Fatalf("transport is %T, want *http.Transport", client.Transport) + } + + return transport +} + +// TestURL pins the scheme each Security maps onto the dialled address. +func TestURL(t *testing.T) { + tests := []struct { + name string + addr string + sec Security + want string + }{ + {name: "TLS", addr: "agent:1024", sec: TLS, want: "https://agent:1024"}, + {name: "insecure", addr: "agent:1024", sec: Insecure, want: "http://agent:1024"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := url(tt.addr, tt.sec); got != tt.want { + t.Errorf("url(%q, %v) = %q, want %q", tt.addr, tt.sec, got, tt.want) + } + }) + } +} diff --git a/internal/rpc/server.go b/internal/rpc/server.go index 630205e9..40fd00a9 100644 --- a/internal/rpc/server.go +++ b/internal/rpc/server.go @@ -6,6 +6,7 @@ package rpc import ( "context" + "crypto/tls" "net" "net/http" "time" @@ -29,26 +30,66 @@ const shutdownGracePeriod = 15 * time.Second func ListenAndServe(ctx context.Context, addr string, handler http.Handler) error { var lc net.ListenConfig - ln, err := lc.Listen(ctx, "tcp", addr) + listener, err := lc.Listen(ctx, "tcp", addr) if err != nil { return err } - return serve(ctx, ln, handler) + return serve(ctx, listener, handler, nil) } -// serve runs handler on the listener ln. It blocks until ctx is cancelled — then it stops +// ListenAndServeTLS is ListenAndServe over TLS, serving handler with cert and +// HTTP/2 negotiated by ALPN. Everything else — binding, graceful shutdown, how +// the caller classifies the return — matches ListenAndServe. +// +// The certificate is typically the agent's own self-signed one (see +// internal/tlsutil), so the connection is encrypted but the client cannot +// authenticate the server, and the server does not authenticate the client: +// any client may connect. See newTLSClient for the peer's side of this. +func ListenAndServeTLS(ctx context.Context, addr string, handler http.Handler, cert tls.Certificate) error { + var lc net.ListenConfig + + listener, err := lc.Listen(ctx, "tcp", addr) + if err != nil { + return err + } + + return serve(ctx, listener, handler, serverTLSConfig(cert)) +} + +// serverTLSConfig is the TLS configuration the RPC service serves with. TLS 1.3 +// is the floor in both directions: both peers are built from this repo, so there +// is no legacy implementation to accommodate, and it pairs with the Ed25519 key +// tlsutil generates. See newTLSClient for the peer's side. +func serverTLSConfig(cert tls.Certificate) *tls.Config { + return &tls.Config{ + Certificates: []tls.Certificate{cert}, + MinVersion: tls.VersionTLS13, + } +} + +// serve runs handler on the given listener, over TLS if tlsConfig is non-nil and +// over h2c otherwise. It blocks until ctx is cancelled — then it stops // accepting and drains in-flight requests (bounded by shutdownGracePeriod) before // returning — or until the server stops on its own, in which case it returns that -// error. It is unexported: ListenAndServe is the entry point; serve is factored out -// so a test can drive a real in-flight request against a listener whose address it -// controls. -func serve(ctx context.Context, ln net.Listener, handler http.Handler) error { - srv := newH2CServer(ln.Addr().String(), handler) +// error. It is unexported: ListenAndServe and ListenAndServeTLS are the entry +// points; serve is factored out so a test can drive a real in-flight request +// against a listener whose address it controls. +func serve(ctx context.Context, listener net.Listener, handler http.Handler, tlsConfig *tls.Config) error { + srv := newServer(listener.Addr().String(), handler, tlsConfig) errCh := make(chan error, 1) - go func() { errCh <- srv.Serve(ln) }() + go func() { + if tlsConfig != nil { + // Empty paths: the certificate is already in srv.TLSConfig. + errCh <- srv.ServeTLS(listener, "", "") + + return + } + + errCh <- srv.Serve(listener) + }() select { case err := <-errCh: @@ -71,18 +112,29 @@ func serve(ctx context.Context, ln net.Listener, handler http.Handler) error { } } -// newH2CServer builds the h2c *http.Server. It is unexported: ListenAndServe is -// the only intended entry point and drives the server's graceful Shutdown itself. -func newH2CServer(addr string, handler http.Handler) *http.Server { +// newServer builds the *http.Server — over TLS if tlsConfig is non-nil, h2c +// otherwise. It is unexported: ListenAndServe and ListenAndServeTLS are the only +// intended entry points and drive the server's graceful Shutdown themselves. +func newServer(addr string, handler http.Handler, tlsConfig *tls.Config) *http.Server { srv := &http.Server{ Addr: addr, Handler: handler, + TLSConfig: tlsConfig, ReadHeaderTimeout: readHeaderTimeout, } - // Serve HTTP/2 without TLS (h2c), keeping HTTP/1 for upgrade. srv.Protocols = new(http.Protocols) srv.Protocols.SetHTTP1(true) + + if tlsConfig != nil { + // Over TLS, HTTP/2 is negotiated by ALPN; HTTP/1 stays for clients that + // do not offer h2. + srv.Protocols.SetHTTP2(true) + + return srv + } + + // Serve HTTP/2 without TLS (h2c), keeping HTTP/1 for upgrade. srv.Protocols.SetUnencryptedHTTP2(true) return srv diff --git a/internal/rpc/server_test.go b/internal/rpc/server_test.go index c684b6fa..872afafe 100644 --- a/internal/rpc/server_test.go +++ b/internal/rpc/server_test.go @@ -37,7 +37,7 @@ func TestServeDrainsInFlightRequest(t *testing.T) { served := make(chan error, 1) - go func() { served <- serve(ctx, ln, handler) }() + go func() { served <- serve(ctx, ln, handler, nil) }() reqDone := make(chan error, 1) @@ -94,7 +94,7 @@ func TestServeReturnsOnCancel(t *testing.T) { served := make(chan error, 1) - go func() { served <- serve(ctx, ln, http.NewServeMux()) }() + go func() { served <- serve(ctx, ln, http.NewServeMux(), nil) }() select { case err := <-served: diff --git a/internal/rpc/tls_test.go b/internal/rpc/tls_test.go new file mode 100644 index 00000000..c7beac76 --- /dev/null +++ b/internal/rpc/tls_test.go @@ -0,0 +1,128 @@ +// Copyright 2025 Blindspot Software +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package rpc + +import ( + "context" + "crypto/tls" + "errors" + "net" + "net/http" + "path/filepath" + "testing" + + "github.com/BlindspotSoftware/dutctl/internal/tlsutil" +) + +// testCert generates a throwaway self-signed key pair in a temp dir, the same +// way an agent does on first start. +func testCert(t *testing.T) tls.Certificate { + t.Helper() + + dir := t.TempDir() + + cert, _, err := tlsutil.LoadOrGenerateCert(filepath.Join(dir, "cert.pem"), filepath.Join(dir, "key.pem")) + if err != nil { + t.Fatalf("generate cert: %v", err) + } + + return cert +} + +// serveTLSOnListener runs serve over TLS on a listener the caller owns, and +// returns a channel carrying serve's eventual return. Handing serve an +// already-bound listener is what server_test.go does for the cleartext case: it +// removes any need to poll for readiness, because the socket is accepting before +// the first request is made. +func serveTLSOnListener(ctx context.Context, listener net.Listener, handler http.Handler, cert tls.Certificate) chan error { + served := make(chan error, 1) + + go func() { served <- serve(ctx, listener, handler, serverTLSConfig(cert)) }() + + return served +} + +// TestTLSRoundTrip drives a real request from the TLS client against a server +// started over TLS, both using a self-signed certificate. It pins the two +// properties the transports must hold together: the client accepts the +// unverifiable certificate, and the connection ends up on HTTP/2 — gRPC +// (connect.WithGRPC) does not work over HTTP/1.1, and a Transport with a custom +// TLSClientConfig does not negotiate h2 unless asked to. +func TestTLSRoundTrip(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + // Echo the negotiated protocol back so the client can assert on it. + w.Header().Set("X-Proto", r.Proto) + w.WriteHeader(http.StatusNoContent) + }) + + served := serveTLSOnListener(ctx, listener, mux, testCert(t)) + + client := newTLSClient() + + resp, err := client.Get("https://" + listener.Addr().String() + "/") //nolint:noctx // short-lived test request + if err != nil { + t.Fatalf("get: %v", err) + } + + if got := resp.Header.Get("X-Proto"); got != "HTTP/2.0" { + t.Errorf("server saw %q, want HTTP/2.0 - gRPC requires HTTP/2", got) + } + + if resp.TLS == nil { + t.Fatal("response carries no TLS state") + } + + if resp.TLS.Version != tls.VersionTLS13 { + t.Errorf("negotiated TLS version = %#x, want TLS 1.3 (%#x)", resp.TLS.Version, tls.VersionTLS13) + } + + // Release the connection before cancelling: Shutdown waits for pooled + // connections to go idle, so a live one would stall the drain. + resp.Body.Close() + client.CloseIdleConnections() + cancel() + + if err := <-served; err != nil && !errors.Is(err, http.ErrServerClosed) { + t.Errorf("graceful shutdown returned %v", err) + } +} + +// TestTransportMismatch pins the failure mode the Security argument exists to +// prevent: a cleartext client against a TLS server fails at connect time rather +// than hanging or silently downgrading. +func TestTransportMismatch(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + served := serveTLSOnListener(ctx, listener, http.NewServeMux(), testCert(t)) + + client := newH2CClient() + + resp, err := client.Get("http://" + listener.Addr().String() + "/") //nolint:noctx // short-lived test request + if err == nil { + resp.Body.Close() + t.Fatal("cleartext client reached the TLS server, want a connect failure") + } + + cancel() + + if err := <-served; err != nil && !errors.Is(err, http.ErrServerClosed) { + t.Errorf("graceful shutdown returned %v", err) + } +} diff --git a/internal/tlsutil/tlsutil.go b/internal/tlsutil/tlsutil.go new file mode 100644 index 00000000..85f71a96 --- /dev/null +++ b/internal/tlsutil/tlsutil.go @@ -0,0 +1,272 @@ +// Copyright 2025 Blindspot Software +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package tlsutil provides the TLS key pair that dutagent and dutserver serve +// with: it loads an existing certificate/key pair from disk, or generates a +// self-signed one on first start. +// +// A generated certificate chains to no CA, and the peers in this project skip +// verification anyway (see internal/rpc newTLSClient), so the pair buys +// encryption without authentication: it stops passive eavesdropping on the +// wire, not an active man-in-the-middle, and it does not authenticate clients +// either. Supplying a CA-issued pair via -tls-cert/-tls-key does not change +// that today, because no client in this project verifies what it is offered. +package tlsutil + +import ( + "crypto/ed25519" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "net" + "os" + "path/filepath" + "time" +) + +const ( + // File and directory permissions. + certFileMode = 0o644 // Owner read/write, world-readable. + keyFileMode = 0o600 // Owner read/write only. + dirMode = 0o750 // Owner access, group traverse; keeps the key out of world reach. + + // Certificate serial number bit size. + serialNumberBits = 128 +) + +// certValidity is how long a generated certificate stays valid. Nothing in this +// project enforces it — every peer sets InsecureSkipVerify, so NotAfter is never +// checked, and there is no rotation path. It is deliberately long so that the +// day verification is turned on, existing deployments do not all expire at once. +const certValidity = 10 * 365 * 24 * time.Hour + +// LoadOrGenerateCert returns the TLS key pair at certPath/keyPath, generating a +// self-signed one on first start. +// +// It generates only when neither file exists, creating the parent directories as +// needed. If either file is present it loads the pair and returns an error rather +// than overwriting, so a half-provisioned or unreadable pair fails startup instead +// of silently replacing an operator's certificate. An existing pair is loaded +// as-is: expiry is not checked and nothing is ever rotated. +// +// It reports whether it generated the pair, so the caller can log that once — +// this package does not log itself. +func LoadOrGenerateCert(certPath, keyPath string) (tls.Certificate, bool, error) { + certExists := fileExists(certPath) + keyExists := fileExists(keyPath) + + // If either file exists, load rather than generate: overwriting half a key + // pair is worse than failing loudly. + if certExists || keyExists { + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + if err != nil { + return tls.Certificate{}, false, fmt.Errorf( + "loading key pair (cert exists: %v, key exists: %v): %w", certExists, keyExists, err) + } + + return cert, false, nil + } + + certDir := filepath.Dir(certPath) + keyDir := filepath.Dir(keyPath) + + err := os.MkdirAll(certDir, dirMode) + if err != nil { + return tls.Certificate{}, false, fmt.Errorf("creating certificate directory: %w", err) + } + + if certDir != keyDir { + err := os.MkdirAll(keyDir, dirMode) + if err != nil { + return tls.Certificate{}, false, fmt.Errorf("creating key directory: %w", err) + } + } + + err = generateSelfSignedCert(certPath, keyPath) + if err != nil { + return tls.Certificate{}, false, err + } + + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + if err != nil { + return tls.Certificate{}, false, fmt.Errorf("loading generated key pair: %w", err) + } + + return cert, true, nil +} + +// generateSelfSignedCert writes a new self-signed certificate to certPath and its +// Ed25519 private key to keyPath. +// +// Both files are written to temporary paths alongside their targets and renamed +// into place only once both have been written. A half-written pair would be worse +// than none: LoadOrGenerateCert refuses to generate when either file exists, so a +// stray cert.pem with no key would make every subsequent start fail until an +// operator deleted it by hand. +// +// Ed25519 is used because it keeps key generation cheap on the small boards +// dutagent runs on. The certificate carries localhost, the system hostname, +// 127.0.0.1 and ::1 as SANs; those are advisory only, since no peer in this +// project verifies the certificate at all. +func generateSelfSignedCert(certPath, keyPath string) error { + publicKey, privateKey, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return fmt.Errorf("generating keys: %w", err) + } + + derBytes, err := createSelfSignedCertificate(publicKey, privateKey) + if err != nil { + return err + } + + certTmp := certPath + ".tmp" + keyTmp := keyPath + ".tmp" + + err = writeCertificate(certTmp, derBytes) + if err != nil { + os.Remove(certTmp) + + return err + } + + err = writePrivateKey(keyTmp, privateKey) + if err != nil { + os.Remove(certTmp) + os.Remove(keyTmp) + + return err + } + + // Rename the key first: if the second rename fails, the leftover is a key + // without a certificate, which LoadOrGenerateCert reports as a load failure + // naming both files — clearer than a certificate whose key vanished. + err = os.Rename(keyTmp, keyPath) + if err != nil { + os.Remove(certTmp) + os.Remove(keyTmp) + + return fmt.Errorf("installing private key: %w", err) + } + + err = os.Rename(certTmp, certPath) + if err != nil { + os.Remove(certTmp) + + return fmt.Errorf("installing certificate: %w", err) + } + + return nil +} + +func createSelfSignedCertificate(publicKey ed25519.PublicKey, privateKey ed25519.PrivateKey) ([]byte, error) { + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), serialNumberBits) + + serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) + if err != nil { + return nil, fmt.Errorf("generating serial number: %w", err) + } + + // A hostname SAN is a convenience for the day verification is turned on, not + // a requirement today, so a host that cannot name itself is not an error — + // localhost alone is a valid SAN list. + dnsNames := []string{"localhost"} + + hostname, err := os.Hostname() + if err == nil && hostname != "localhost" { + dnsNames = append(dnsNames, hostname) + } + + template := &x509.Certificate{ + SerialNumber: serialNumber, + Subject: pkix.Name{ + Organization: []string{"Blindspot Software"}, + CommonName: "dutagent", + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(certValidity), + // Ed25519 signs; it cannot do key encipherment, so KeyUsageDigitalSignature + // is the whole of what this leaf needs for a TLS 1.3 handshake. + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + // Marks the (absent) IsCA and path-length fields as meaningful, so the + // leaf explicitly says "not a CA" rather than leaving it unstated. + BasicConstraintsValid: true, + DNSNames: dnsNames, + IPAddresses: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("::1")}, + } + + derBytes, err := x509.CreateCertificate(rand.Reader, template, template, publicKey, privateKey) + if err != nil { + return nil, fmt.Errorf("creating certificate: %w", err) + } + + return derBytes, nil +} + +// writeCertificate writes the PEM-encoded certificate to path. Close is explicit +// rather than deferred: a failure to flush leaves an unusable file, and the +// caller must learn about it to clean up. +func writeCertificate(path string, derBytes []byte) error { + certOut, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, certFileMode) + if err != nil { + return fmt.Errorf("creating certificate file: %w", err) + } + + err = pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes}) + if err != nil { + certOut.Close() + + return fmt.Errorf("writing certificate: %w", err) + } + + err = certOut.Close() + if err != nil { + return fmt.Errorf("closing certificate file: %w", err) + } + + return nil +} + +// writePrivateKey writes the PKCS#8 PEM-encoded private key to path, with +// keyFileMode so it is never group- or world-readable. Close is explicit for the +// same reason as in writeCertificate. +func writePrivateKey(path string, privateKey ed25519.PrivateKey) error { + privBytes, err := x509.MarshalPKCS8PrivateKey(privateKey) + if err != nil { + return fmt.Errorf("marshaling private key: %w", err) + } + + keyOut, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, keyFileMode) + if err != nil { + return fmt.Errorf("creating key file: %w", err) + } + + err = pem.Encode(keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: privBytes}) + if err != nil { + keyOut.Close() + + return fmt.Errorf("writing private key: %w", err) + } + + err = keyOut.Close() + if err != nil { + return fmt.Errorf("closing key file: %w", err) + } + + return nil +} + +// fileExists checks if a file exists and is not a directory. +func fileExists(path string) bool { + info, err := os.Stat(path) + if err != nil { + return false + } + + return !info.IsDir() +} diff --git a/internal/tlsutil/tlsutil_test.go b/internal/tlsutil/tlsutil_test.go new file mode 100644 index 00000000..1ad62702 --- /dev/null +++ b/internal/tlsutil/tlsutil_test.go @@ -0,0 +1,203 @@ +// Copyright 2025 Blindspot Software +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package tlsutil + +import ( + "crypto/tls" + "crypto/x509" + "os" + "path/filepath" + "testing" +) + +// TestGenerateSelfSignedCert covers the generator itself: the pair it writes must +// load back as a usable key pair, the private key must not be readable by anyone +// but its owner, and a failed generation must leave nothing behind. +func TestGenerateSelfSignedCert(t *testing.T) { + tests := []struct { + name string + setupFunc func(t *testing.T) (certPath, keyPath string) + wantErr bool + }{ + { + name: "generates valid certificate", + setupFunc: func(t *testing.T) (string, string) { + t.Helper() + + tmpDir := t.TempDir() + + return filepath.Join(tmpDir, "cert.pem"), filepath.Join(tmpDir, "key.pem") + }, + wantErr: false, + }, + { + name: "fails with invalid path", + setupFunc: func(t *testing.T) (string, string) { + t.Helper() + + return "/nonexistent/directory/cert.pem", "/nonexistent/directory/key.pem" + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + certPath, keyPath := tt.setupFunc(t) + + err := generateSelfSignedCert(certPath, keyPath) + + if (err != nil) != tt.wantErr { + t.Errorf("generateSelfSignedCert() error = %v, wantErr %v", err, tt.wantErr) + + return + } + + if tt.wantErr { + // A failed generation must not leave either file behind: a stray + // cert.pem would make every later start refuse to regenerate. + if fileExists(certPath) || fileExists(keyPath) { + t.Error("failed generation left files behind") + } + + return + } + + keyInfo, err := os.Stat(keyPath) + if err != nil { + t.Fatalf("stat key: %v", err) + } + + if perm := keyInfo.Mode().Perm(); perm != keyFileMode { + t.Errorf("key mode = %#o, want %#o - the private key must stay owner-only", perm, keyFileMode) + } + + cert, err := tls.LoadX509KeyPair(certPath, keyPath) + if err != nil { + t.Fatalf("load key pair: %v", err) + } + + x509Cert, err := x509.ParseCertificate(cert.Certificate[0]) + if err != nil { + t.Fatalf("parse certificate: %v", err) + } + + if x509Cert.Subject.CommonName != "dutagent" { + t.Errorf("CommonName = %q, want %q", x509Cert.Subject.CommonName, "dutagent") + } + }) + } +} + +// TestLoadOrGenerateCert covers the load-or-generate decision, including the +// refusal to overwrite a half-provisioned pair in either direction. +func TestLoadOrGenerateCert(t *testing.T) { + tests := []struct { + name string + setupFunc func(t *testing.T) (certPath, keyPath string) + wantGenerated bool + wantErr bool + }{ + { + name: "generates when files don't exist", + setupFunc: func(t *testing.T) (string, string) { + t.Helper() + + tmpDir := t.TempDir() + + return filepath.Join(tmpDir, "cert.pem"), filepath.Join(tmpDir, "key.pem") + }, + wantGenerated: true, + }, + { + name: "creates the target directory", + setupFunc: func(t *testing.T) (string, string) { + t.Helper() + + tmpDir := filepath.Join(t.TempDir(), "tls") + + return filepath.Join(tmpDir, "cert.pem"), filepath.Join(tmpDir, "key.pem") + }, + wantGenerated: true, + }, + { + name: "loads existing certificate", + setupFunc: func(t *testing.T) (string, string) { + t.Helper() + + tmpDir := t.TempDir() + certPath := filepath.Join(tmpDir, "cert.pem") + keyPath := filepath.Join(tmpDir, "key.pem") + + if err := generateSelfSignedCert(certPath, keyPath); err != nil { + t.Fatalf("setup: %v", err) + } + + return certPath, keyPath + }, + wantGenerated: false, + }, + { + name: "fails when only cert exists", + setupFunc: func(t *testing.T) (string, string) { + t.Helper() + + tmpDir := t.TempDir() + certPath := filepath.Join(tmpDir, "cert.pem") + keyPath := filepath.Join(tmpDir, "key.pem") + + if err := os.WriteFile(certPath, []byte("invalid"), certFileMode); err != nil { + t.Fatalf("setup: %v", err) + } + + return certPath, keyPath + }, + wantErr: true, + }, + { + name: "fails when only key exists", + setupFunc: func(t *testing.T) (string, string) { + t.Helper() + + tmpDir := t.TempDir() + certPath := filepath.Join(tmpDir, "cert.pem") + keyPath := filepath.Join(tmpDir, "key.pem") + + if err := os.WriteFile(keyPath, []byte("invalid"), keyFileMode); err != nil { + t.Fatalf("setup: %v", err) + } + + return certPath, keyPath + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + certPath, keyPath := tt.setupFunc(t) + + cert, generated, err := LoadOrGenerateCert(certPath, keyPath) + + if (err != nil) != tt.wantErr { + t.Errorf("LoadOrGenerateCert() error = %v, wantErr %v", err, tt.wantErr) + + return + } + + if tt.wantErr { + return + } + + if generated != tt.wantGenerated { + t.Errorf("generated = %v, want %v", generated, tt.wantGenerated) + } + + if len(cert.Certificate) == 0 { + t.Error("certificate is empty") + } + }) + } +} diff --git a/packaging/README.md b/packaging/README.md index 0fa05718..0613417b 100644 --- a/packaging/README.md +++ b/packaging/README.md @@ -17,3 +17,5 @@ The systemd service needs a non-root user to run, with correct privileges (for e ## `packaging/dutagent.tmpfiles` Just like with `sysusers` case, we use `systemd-tmpfiles` tool to create directories needed by `dutagent`, with correct ownership and permissions. This is done automatically by `systemd` on installation of the distribution package. + +`/var/lib/dutagent/tls` holds the agent's TLS key pair, which `dutagent` generates on first start. It is mode `0750` because the private key must not be world-readable, and it lives under `/var/lib` rather than `/etc` because the service unit sets `ProtectSystem=strict` and only lists `/var/lib/dutagent` and `/var/log/dutagent` in `ReadWritePaths`. diff --git a/packaging/dutagent.tmpfiles b/packaging/dutagent.tmpfiles index 457f1358..7817b52a 100644 --- a/packaging/dutagent.tmpfiles +++ b/packaging/dutagent.tmpfiles @@ -1,2 +1,3 @@ d /var/lib/dutagent 0750 dutagent dutagent - +d /var/lib/dutagent/tls 0750 dutagent dutagent - d /var/log/dutagent 0755 dutagent dutagent -