diff --git a/CLAUDE.md b/CLAUDE.md index 58e0af7..c4ff00a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,11 +32,13 @@ cmd/network-agent/ # CLI entry point (main.go) internal/ protocol/ # Binary wire format (encode/decode) — no external deps acl/ # IP/CIDR access list parser - config/ # File paths, TLS config, env vars - caroot/ # Embedded CA certificate (ca.crt) + config/ # File paths, TLS config, CA bundle, env vars + caroot/ # Embedded CA bundle (ca.crt) + bundle parsing + certrenew/ # Validate + atomically install a renewed client cert tunnel/ # mTLS connection + goroutine lifecycle setup/ # Interactive setup wizard daemon/ # PID file, background process management + testca/ # Test-only: DeployHQ-shaped certificate fixtures ``` ## Protocol (Wire Format) @@ -50,7 +52,23 @@ Frame layout — must not change without coordinating with the server side: `total_frame_size` includes the 2-byte length field itself. Commands: `CREATE_REQUEST(1)`, `CREATE_RESPONSE(2)`, `DESTROY(3)`, `DATA(4)`, -`REJECT(5)`, `RECONNECT(6)`, `KEEPALIVE(7)`. +`REJECT(5)`, `RECONNECT(6)`, `KEEPALIVE(7)`, `RENEW_REQUEST(8)`, +`RENEW_RESPONSE(9)`. + +`RENEW_REQUEST` / `RENEW_RESPONSE` carry in-band certificate renewal and are +implemented identically in the DeployHQ backend and the Ruby `deploy-agent` +gem — the three must not drift: + +- `RENEW_REQUEST` (agent → server), sent once per connection immediately after + the handshake. Payload: UTF-8 `"go/"` (the gem sends `"ruby/"`). + Nothing else on the wire identifies the implementation or version. +- `RENEW_RESPONSE` (server → agent, never unsolicited). Payload + `[status:1][body]`: `0` RENEWED (body = new client certificate PEM), `1` + CURRENT (no body), `2` ERROR (body = UTF-8 message, logged and ignored). + +Unknown command bytes must stay tolerated — the `default:` branch in +`server_conn.go` only logs. Deployed agents rely on that to survive new +commands. Reference implementations: - Agent side: `../deploy-agent/lib/deploy_agent/server_connection.rb` @@ -73,6 +91,11 @@ Reference implementations: | `DEPLOY_AGENT_PROXY_IP` | `agent.deployhq.com` | | `DEPLOY_AGENT_CERTIFICATE_URL` | `https://api.deployhq.com/api/v1/agents/create` | | `DEPLOY_AGENT_NOVERIFY` | unset (set to skip TLS verification in dev) | +| `DEPLOY_AGENT_CA_FILE` | unset (uses `internal/caroot`'s embedded bundle) | + +`DEPLOY_AGENT_CA_FILE` replaces the embedded CA bundle with a PEM file on disk. +Read through `config.CACert(caroot.CACert)`, which both `run` and `check` go +through. An unreadable or empty file is an error, never a silent fall back. ## Testing @@ -100,5 +123,11 @@ GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go build ./cmd/network-agent - Keep the protocol package dependency-free — it is the ground truth for wire format - The access list is reloaded on each reconnect (see `tunnel/agent.go`) — no restart needed for ACL changes +- The client certificate is reloaded from disk on **every handshake** + (`config.NewTLSConfig`'s `GetClientCertificate`), not cached in + `tls.Config.Certificates`. One `*tls.Config` is built at start-up and reused + for every reconnect, so caching it there would mean a renewed `agent.crt` + never took effect without a process restart — the thing certificate renewal + exists to avoid. Do not "optimise" this back into `Certificates`. - Daemon backgrounding uses re-exec (not fork) — see `internal/daemon/` - Build tags `!windows` / `windows` separate Unix/Windows daemon code diff --git a/README.md b/README.md index b7e4ef4..db52f01 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,33 @@ Add `-v` / `--verbose` before the command for debug logging. 10.0.0.0/8 ``` +## Upgrading + +```bash +network-agent update && network-agent restart +``` + +`update` replaces the binary in place; `restart` is what makes the running +agent pick it up. Both steps are needed — there is no background update check. + +## Certificate renewal + +DeployHQ is rotating the certificate authority behind the agent connection. The +agent handles its side automatically: on each connection it tells DeployHQ which +version it is running, and if DeployHQ has re-issued its client certificate the +agent validates the new one, replaces `~/.deploy/agent.crt`, and reconnects to +start using it. Nothing is written unless the new certificate pairs with the +existing private key, keeps the same identity, and is signed by a trusted CA — +and the replacement is atomic, so a failure never leaves a half-written +certificate behind. + +Nothing is required of you beyond running a recent version. There is no +interruption to deployments, no new claim code, and no change to +`~/.deploy/agent.key`, which never leaves the machine. + +`network-agent check` prints the certificate authorities the binary trusts and +when they expire. + ## Migration from Ruby gem Users already running the Ruby gem can migrate in place — the Go binary uses the @@ -88,6 +115,7 @@ To roll back: `network-agent stop`, then `gem install deploy-agent` and `deploy- | `DEPLOY_AGENT_PROXY_IP` | `agent.deployhq.com` | Agent server hostname/IP | | `DEPLOY_AGENT_CERTIFICATE_URL` | `https://api.deployhq.com/api/v1/agents/create` | Certificate provisioning endpoint | | `DEPLOY_AGENT_NOVERIFY` | unset | Set to skip TLS server verification | +| `DEPLOY_AGENT_CA_FILE` | unset (uses the bundled CAs) | Path to a PEM bundle of certificate authorities to trust instead of the ones built into the binary. Unreadable or empty is an error, not a fall back. | ## Building from source diff --git a/ca.crt b/ca.crt deleted file mode 100644 index 9233736..0000000 --- a/ca.crt +++ /dev/null @@ -1,33 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIFszCCA5ugAwIBAgIBATANBgkqhkiG9w0BAQsFADB7MQswCQYDVQQGEwJHQjEP -MA0GA1UECAwGRG9yc2V0MSAwHgYDVQQDDBdEZXBsb3kgRGV2IENBIChjaGFybGll -KTEYMBYGA1UECgwPYVRlY2ggTWVkaWEgTHRkMQ4wDAYDVQQHDAVQb29sZTEPMA0G -A1UECwwGRGVwbG95MB4XDTE3MDMxNzE0MzE0NFoXDTI3MDMxNzE0MzE0NFowezEL -MAkGA1UEBhMCR0IxDzANBgNVBAgMBkRvcnNldDEgMB4GA1UEAwwXRGVwbG95IERl -diBDQSAoY2hhcmxpZSkxGDAWBgNVBAoMD2FUZWNoIE1lZGlhIEx0ZDEOMAwGA1UE -BwwFUG9vbGUxDzANBgNVBAsMBkRlcGxveTCCAiIwDQYJKoZIhvcNAQEBBQADggIP -ADCCAgoCggIBAM9uxfQPy7KGYOAgagcy1V+YqGsA2kXxtnmyg/tu+/JTJc+cfp6+ -UVh8T1uGf1gOVjTas4Zfz0iRvMxMNDz8AUBMv18Bxjrtw5J9vpSFvy5FKlxYBmbr -5CZf7OVtdpiCtK0dBxHnuasPbaBRLFfZuGEu8cA7Y2WgN1Sagj0254C0qGpVDgoF -ViB1cavGb9XZW4SDo4rTK/56YvxFCgy7E0Au/OSevvrfZloF6htUqkeq8RDl0Og6 -F0zFHUSK4w4tZhtpeov27cyacMoTP2ioXcWdUujuEz2yhgzVw/8h3obl84hqaXar -hWr+DUHbPw8EC0PI8qLNtmJHJ91zpEXgw0m2pyopyaOw60kmYo+P4HBEqYViWXMQ -2cYfi+5vWiVRQtaQKgd3+Ii8IPcU0GlQF1heMuX6Qnzf/3wIWKPTDKvly/seTC8A -8BYHABGFLWtir+rLaTD25fYySGzRlShWoT0otLZ5aOwA+W8CwJ+KLHvSNWaeRlqd -xz0NtJvsNnkY4mkIyWyDxCjiR14bOaC2TkCZgXh5teX9BuSKu6FX/ciwkZk4OpSW -VnDTQWERI1II/WjQlb6jPfkrcarld2PBaQidBdkZ72gCNp9lkvVeLLedkVhFPyIy -VXy6/tNOgPjlarjgDCrasWVKVFTGhbIobqgLPiKrJN6KWTjBIn4+o1TNAgMBAAGj -QjBAMB0GA1UdDgQWBBQqww4dAhh6km8+h6h+TQNOrbmPrTAPBgNVHRMBAf8EBTAD -AQH/MA4GA1UdDwEB/wQEAwIBBjANBgkqhkiG9w0BAQsFAAOCAgEAk3f/d6emYq5z -gTKa+mwvGZJEr4BvGUK47N+S9Se9zxXVL56gkxcPsfLqKkCijpx+0M1TR12+vY3u -niNj2FuxSdx9LOG5X+zXsNn0M3FkovMwIEulv5vQocHgONUvoQO8gh4QBhAADYhv -BFucBeda2zUtG/Tt1qREmpkOsBLFIl/9totULmXj47zpM57TqE8IVhfDD/hRB0Oy -sxRgtz5Ug4YYV0tf2rtO9pPbuKdZflAPS8S5x3bLHCMx6SFGpcP9jDxqCY4pA6RL -8HrT8kNjmgElKH8W/j6LAOK98znfKzWdXEuX/Wi2eb+wKSICvhY6qpuAmEAV0M95 -v799CtoDbiLdeSFR2i0KOoIcEkkyl66Icfbum8QMvyKqAP0+MFJw1f4u2ABvWrH2 -0IgC2ndJp5wopmYn2A0/cGHtlZs4COAK7whGQ3Y8cla+pat+/3YlG/tYnJm2yauv -pRlFQUHG1dQVTivWDCJReVbzFsxULxsLwRz+WteJVp3SL4J6A9cxZupNrr0EfHue -l4CPcGbB0L8yyIhGwiEfrZpjx6hOelX1daG8QPTvSSYpB6ODtQeb3zpDf8vU8M7T -oAwG8/0g1Owh/a970vIKu4TBa4D2IiCfA3KPWlsIUSoeu9uBTKmUQ0Raa0AhZWPv -JI4XgcL63KznYzLm0BOxvTYMxDfn7fs= ------END CERTIFICATE----- diff --git a/cmd/network-agent/main.go b/cmd/network-agent/main.go index 2ebc3be..b3816ea 100644 --- a/cmd/network-agent/main.go +++ b/cmd/network-agent/main.go @@ -153,12 +153,27 @@ func cmdRun(paths config.Paths, verbose bool) { } log := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: level})) - tlsCfg, err := config.NewTLSConfig(paths, caroot.CACert, config.VerifyTLS()) + caCert, err := config.CACert(caroot.CACert) + if err != nil { + log.Error("CA certificate error", "err", err) + os.Exit(1) + } + + tlsCfg, err := config.NewTLSConfig(paths, caCert, config.VerifyTLS()) if err != nil { log.Error("TLS configuration error", "err", err) os.Exit(1) } + // The same trust anchors gate in-band certificate renewal: a certificate + // the agent would not accept from the server is not one it will install + // for itself. + caRoots, err := config.NewCertPool(caCert) + if err != nil { + log.Error("CA certificate error", "err", err) + os.Exit(1) + } + // Handle SIGTERM / SIGINT for clean shutdown sigc := make(chan os.Signal, 1) signal.Notify(sigc, syscall.SIGTERM, syscall.SIGINT) @@ -173,7 +188,13 @@ func cmdRun(paths config.Paths, verbose bool) { // If PID already written by the parent (start), this is a no-op for our PID. _ = daemon.WritePID(paths.PID) - if err := tunnel.RunAgent(tlsCfg, paths, log); err != nil { + opts := tunnel.Options{ + Version: Version, + Paths: paths, + CARoots: caRoots, + } + + if err := tunnel.RunAgent(tlsCfg, opts, log); err != nil { log.Error("agent stopped with error", "err", err) daemon.RemovePID(paths.PID) os.Exit(1) @@ -260,11 +281,33 @@ func cmdCheck(paths config.Paths) { fmt.Println(")") } + // --- CA bundle --------------------------------------------------------- + // Which CAs this binary trusts is the thing a CA rotation turns on, so + // surface it rather than leaving operators to inspect the binary. + caCert, caErr := config.CACert(caroot.CACert) + if caErr != nil { + fmt.Printf(" CA bundle FAIL %v\n", caErr) + ok = false + } else if cas, err := caroot.ParseBundle(caCert); err != nil { + fmt.Printf(" CA bundle FAIL %v\n", err) + ok = false + } else { + source := "embedded" + if override := os.Getenv(config.CAFileEnv); override != "" { + source = override + } + fmt.Printf(" CA bundle OK %d CA(s) from %s\n", len(cas), source) + for _, c := range cas { + fmt.Printf(" %s (expires %s)\n", + c.Subject.CommonName, c.NotAfter.UTC().Format("2006-01-02")) + } + } + // --- Connectivity ------------------------------------------------------ serverAddr := config.ServerHost() + ":" + config.ServerPort fmt.Printf(" Connectivity ") - tlsCfg, err := config.NewTLSConfig(paths, caroot.CACert, config.VerifyTLS()) + tlsCfg, err := config.NewTLSConfig(paths, caCert, config.VerifyTLS()) if err != nil { fmt.Printf("FAIL TLS config error: %v\n", err) ok = false diff --git a/internal/caroot/ca.crt b/internal/caroot/ca.crt index 9233736..17c3696 100644 --- a/internal/caroot/ca.crt +++ b/internal/caroot/ca.crt @@ -31,3 +31,33 @@ l4CPcGbB0L8yyIhGwiEfrZpjx6hOelX1daG8QPTvSSYpB6ODtQeb3zpDf8vU8M7T oAwG8/0g1Owh/a970vIKu4TBa4D2IiCfA3KPWlsIUSoeu9uBTKmUQ0Raa0AhZWPv JI4XgcL63KznYzLm0BOxvTYMxDfn7fs= -----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIFGzCCAwOgAwIBAgIBATANBgkqhkiG9w0BAQsFADAvMRowGAYDVQQDDBFEZXBs +b3lIUSBBZ2VudCBDQTERMA8GA1UECgwIRGVwbG95SFEwHhcNMjYwOTE1MDczODI4 +WhcNMzYwOTE1MDczODI4WjAvMRowGAYDVQQDDBFEZXBsb3lIUSBBZ2VudCBDQTER +MA8GA1UECgwIRGVwbG95SFEwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoIC +AQC4bMAQgGZC7ZUO9Wq2+rWodOXL4IZ6Ae+Dfce+SlTsQUx5HrYrmMFTE9NJTxcE +dGYjAVVwpdvgT8flFKRiQa7j1xVOLr73ra7Uro0ZGHw1TOBFFAhXLZ25oXT8SsB7 +1aaRZksZAPGR1ZkWRvKqUkRge9LERtcmZDOSeKyf3i3PQp3AaODA4tRwsQnyKWoA +6h9Ruk9jnHqNRapiEjjdkubKKRjgU28IO0Abom5dRsDvvFYVquaQmud745kdaxDr +fQA9Kyz8lzNSglmTHACjzxtX1AGRpn5yLO7PqaV0M/QqnKZhaKW+tzP3pWdsDZ8v +RBGDhyVYeyHYSPAEOgsRdTqQYpnXkR1FLLS/FCFQ/UgyWFStFVv3l9mzq/fQLr/O +sdPzG9KNNTz4OqMrGOeRMcBXsKQJjIRS6TSzggPagcxT2iPRHCGQknko8/O3IyM1 +9YIsLm7kxw2oEIzhHx8Wv3s+SafJr4cuTu1n9RfyP3LmU/C6GbfrH/jDvVuLvFdH +9AY55RNCfCgCKWjqDeYrzGxIcuoZ57H6DJlGouw0MiSg4hUUqNnM1EQG410bJouA +itprB7FIGcZ9uHQvpEgs8NP/DTRsmUP+51Vdp6SLbFSNrt5v/APDrd3ya8R3f7g0 +XVLFl/jxNM4Ki+L6ZGwRz4SYThew0OZXsezbnYiGEro1JwIDAQABo0IwQDAdBgNV +HQ4EFgQUgnCdcTQCEbmADry95bq7UAoX71AwDwYDVR0TAQH/BAUwAwEB/zAOBgNV +HQ8BAf8EBAMCAQYwDQYJKoZIhvcNAQELBQADggIBAHaN63Zqs4W5FTTqQPYnkMH0 +/hMzRVVcQ8tZng6rGy4HdONixQ2XOwFOCMbl8uqRB/ihyLbwiLIVgsqG3wMEIiqV +lxNDYerUtdBolam/lqA371d18lEqKFiFfncCRrd+AzTMcYnYZZK026smx1JCxDCP +t1rB/T9Lv0g2aRyWTKsKBeI5GFOIEIHScP1k/R7549kNEwerFKX5USWszDfnouiV +hSz9UjlY1vQQ//qYgimevJxlGudt2b+73+BkT5THdNKBxeyn02/DbhaH/1zafE/7 +lxAKmN94HkAOM2TD27ZPRDIK4jmKK/Hx03X0kYD+KL/9Mr/4PIjlObRFF8BUawp1 +Kpn/Caw4gNz76IujGXGvJp9FxsZUscRRRdikIZ3AtR0/6Wyf8fFaAd89DoBGSzLM +mRynJbnK5ROlVzvxX43ow7sBLtoh1+/RCsWcCVgOC2EMCW2n4b5M0USv7FBrhPie +F50NkMqsTtEoxw8kcKvLd4FbPvWuIxPsXzxlnFaHTbVgTevgL6O8IVeWLqdqwFGK +owB7z1tEQTN6sV4+fDDtaVQFCalEV9QPAolVLYdBUApc866heDHmzW85tkq2zoDG +2cj80hCcI42DPvrb9nEcIcZiFZDCcIlDjRf6kDmqBW4oXr5V1yAxrC+i0UA2+LWj +NVgD1tDYCLJlNY3GA6j8 +-----END CERTIFICATE----- diff --git a/internal/caroot/caroot.go b/internal/caroot/caroot.go index 5e9bdc5..5507e08 100644 --- a/internal/caroot/caroot.go +++ b/internal/caroot/caroot.go @@ -1,7 +1,53 @@ // Package caroot embeds the DeployHQ CA certificate used to verify the agent server. +// +// ca.crt is a PEM bundle, not necessarily a single certificate: during a CA +// rotation it holds both the outgoing and the incoming CA so that one binary +// verifies an agent server presenting a leaf from either. package caroot -import _ "embed" +import ( + "crypto/x509" + "encoding/pem" + "fmt" + + _ "embed" +) //go:embed ca.crt var CACert []byte + +// Certificates parses every certificate in the embedded bundle, in file order. +// +// It exists so the number of trusted CAs is observable — by `network-agent +// check`, by tests, and by anyone verifying that a release actually shipped the +// bundle it was supposed to. +func Certificates() ([]*x509.Certificate, error) { + return ParseBundle(CACert) +} + +// ParseBundle parses every CERTIFICATE block in a PEM bundle. Non-certificate +// blocks are skipped; any unparsable certificate block, or a bundle holding no +// certificates at all, is an error. +func ParseBundle(pemData []byte) ([]*x509.Certificate, error) { + var certs []*x509.Certificate + rest := pemData + for { + var block *pem.Block + block, rest = pem.Decode(rest) + if block == nil { + break + } + if block.Type != "CERTIFICATE" { + continue + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("parsing certificate %d in bundle: %w", len(certs)+1, err) + } + certs = append(certs, cert) + } + if len(certs) == 0 { + return nil, fmt.Errorf("no certificates found in bundle") + } + return certs, nil +} diff --git a/internal/caroot/caroot_test.go b/internal/caroot/caroot_test.go new file mode 100644 index 0000000..6f0e155 --- /dev/null +++ b/internal/caroot/caroot_test.go @@ -0,0 +1,56 @@ +package caroot_test + +import ( + "testing" + "time" + + "github.com/deployhq/network-agent/internal/caroot" +) + +// TestEmbeddedBundleParses is the release-time guard for the CA bundle: it +// proves the embedded PEM is well formed and reports how many CAs it holds, so +// appending the new DeployHQ CA before a release is a visible, verifiable +// change rather than an unchecked assumption. +func TestEmbeddedBundleParses(t *testing.T) { + certs, err := caroot.Certificates() + if err != nil { + t.Fatalf("parsing embedded CA bundle: %v", err) + } + if len(certs) < 1 { + t.Fatal("embedded CA bundle holds no certificates") + } + + t.Logf("embedded CA bundle holds %d certificate(s)", len(certs)) + for i, c := range certs { + t.Logf(" [%d] subject=%q issuer=%q serial=%s notAfter=%s isCA=%v", + i, c.Subject.String(), c.Issuer.String(), c.SerialNumber, + c.NotAfter.UTC().Format(time.RFC3339), c.IsCA) + + if !c.IsCA { + t.Errorf("[%d] %q is not a CA certificate", i, c.Subject.CommonName) + } + } +} + +// TestParseBundleRejectsGarbage keeps ParseBundle honest: a bundle that does +// not parse must be an error, never an empty-but-successful result that would +// ship a binary trusting nothing. +func TestParseBundleRejectsGarbage(t *testing.T) { + tests := []struct { + name string + data []byte + }{ + {"empty", nil}, + {"not pem", []byte("hello, world")}, + {"pem but not a certificate", []byte("-----BEGIN RSA PRIVATE KEY-----\nZm9v\n-----END RSA PRIVATE KEY-----\n")}, + {"corrupt certificate block", []byte("-----BEGIN CERTIFICATE-----\nZm9v\n-----END CERTIFICATE-----\n")}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := caroot.ParseBundle(tt.data); err == nil { + t.Error("expected an error, got none") + } + }) + } +} diff --git a/internal/certrenew/certrenew.go b/internal/certrenew/certrenew.go new file mode 100644 index 0000000..326c457 --- /dev/null +++ b/internal/certrenew/certrenew.go @@ -0,0 +1,221 @@ +// Package certrenew validates a certificate the DeployHQ backend has re-issued +// for this agent and installs it over ~/.deploy/agent.crt. +// +// Renewal happens during a CA rotation: the backend re-signs the public key it +// already holds for the agent under the new CA, preserving the subject and the +// serial number that identify the agent, so the agent keeps its existing +// private key and only the certificate file changes. +// +// Two properties matter more than anything else here: +// +// - Nothing is written unless every check passes. A bad certificate that +// replaced a good one would take the agent offline behind a customer +// firewall, where nobody can fix it. +// - The replacement is atomic. A reader must see either the whole old +// certificate or the whole new one, never a truncated file. +package certrenew + +import ( + "bytes" + "crypto/tls" + "crypto/x509" + "encoding/pem" + "errors" + "fmt" + "os" + "path/filepath" + "time" +) + +// ErrUnchanged reports that the offered certificate is byte for byte the one +// already installed, so there is nothing to write. +// +// It is a normal outcome, not a failure: a server that keeps answering RENEWED +// with the certificate the agent already has must not push it through a +// connect-renew-reconnect loop forever. +var ErrUnchanged = errors.New("certificate is already installed") + +// renameAttempts / renameDelay bound the retry loop around the final rename. +// +// On Unix os.Rename is a single atomic syscall and never needs a retry. On +// Windows it is MoveFileEx(MOVEFILE_REPLACE_EXISTING), which fails with a +// sharing violation while another process (antivirus, a backup agent, an +// installer) holds the destination open without FILE_SHARE_DELETE. Those holds +// are short, so retrying a few times turns a transient collision into a +// successful renewal instead of a skipped one. +const ( + renameAttempts = 5 + renameDelay = 200 * time.Millisecond +) + +// Request describes a single renewal attempt. +type Request struct { + // CertPath is the certificate file to replace (~/.deploy/agent.crt). + CertPath string + + // KeyPath is the agent's private key. It is never written; the candidate + // certificate must pair with the key already on disk. + KeyPath string + + // Roots are the trust anchors the candidate certificate must chain to — + // the same CA bundle the agent uses to verify the server. + Roots *x509.CertPool + + // Current is the certificate the agent is using now. The candidate must + // carry the same subject and serial number, because those are how the + // backend identifies this agent. + Current *x509.Certificate + + // CandidatePEM is the PEM-encoded certificate offered by the server. + CandidatePEM []byte +} + +// Apply validates req.CandidatePEM and, only when every check passes and the +// certificate actually differs from the one installed, atomically replaces the +// file at req.CertPath with it. +// +// On any error — ErrUnchanged included — nothing has been written and the +// existing certificate file is untouched. The returned certificate is the newly +// installed one. +func Apply(req Request) (*x509.Certificate, error) { + candidate, err := Validate(req) + if err != nil { + return nil, err + } + if err := writeAtomic(req.CertPath, req.CandidatePEM); err != nil { + return nil, err + } + return candidate, nil +} + +// Validate runs every check on the candidate certificate without touching the +// filesystem except to read the private key. It is separated from Apply so the +// checks can be exercised — and reasoned about — on their own. +func Validate(req Request) (*x509.Certificate, error) { + if req.Current == nil { + return nil, fmt.Errorf("no current certificate to compare against") + } + if req.Roots == nil { + return nil, fmt.Errorf("no CA roots to verify against") + } + + block, _ := pem.Decode(req.CandidatePEM) + if block == nil || block.Type != "CERTIFICATE" { + return nil, fmt.Errorf("renewed certificate is not a PEM certificate") + } + candidate, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("parsing renewed certificate: %w", err) + } + + // (a) The certificate must pair with the private key already on disk. + // tls.X509KeyPair does exactly this comparison, for every key type Go + // supports, and fails with "private key does not match public key". + keyPEM, err := os.ReadFile(req.KeyPath) + if err != nil { + return nil, fmt.Errorf("reading private key %s: %w", req.KeyPath, err) + } + if _, err := tls.X509KeyPair(req.CandidatePEM, keyPEM); err != nil { + return nil, fmt.Errorf("renewed certificate does not pair with %s: %w", req.KeyPath, err) + } + + // (b) Subject and serial identify the agent to the backend. A renewal + // changes the issuer, never the identity — anything else is not a renewal + // of *this* agent's certificate and must not overwrite it. + if !bytes.Equal(candidate.RawSubject, req.Current.RawSubject) { + return nil, fmt.Errorf("renewed certificate subject %q does not match current %q", + candidate.Subject.String(), req.Current.Subject.String()) + } + if candidate.SerialNumber.Cmp(req.Current.SerialNumber) != 0 { + return nil, fmt.Errorf("renewed certificate serial %s does not match current %s", + candidate.SerialNumber, req.Current.SerialNumber) + } + + // (c) The certificate must chain to a CA this agent trusts, AND be usable + // for the one thing the agent does with it: client authentication. Roots + // only and no DNSName, because DeployHQ's certificates are CN-only with no + // SANs. ClientAuth rather than Any: a certificate carrying no extended key + // usage -- which is what the backend actually mints -- is valid for every + // usage and still passes, while one whose EKU rules client authentication + // out is refused here rather than on every reconnect after it has already + // replaced the working certificate. + if _, err := candidate.Verify(x509.VerifyOptions{ + Roots: req.Roots, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + }); err != nil { + return nil, fmt.Errorf("renewed certificate does not verify against the trusted CAs: %w", err) + } + + // Last, once the candidate is known good: is it the one already installed? + // Compare the DER, so PEM line endings or trailing whitespace cannot make + // an identical certificate look like a new one. + if bytes.Equal(candidate.Raw, req.Current.Raw) { + return candidate, ErrUnchanged + } + + return candidate, nil +} + +// LoadCurrent reads and parses the certificate currently installed at path. +func LoadCurrent(path string) (*x509.Certificate, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading %s: %w", path, err) + } + block, _ := pem.Decode(data) + if block == nil || block.Type != "CERTIFICATE" { + return nil, fmt.Errorf("%s is not a PEM certificate", path) + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("parsing %s: %w", path, err) + } + return cert, nil +} + +// writeAtomic writes data to ".tmp" in dst's own directory, flushes it to +// stable storage, and renames it over dst. Same directory so the rename stays +// within one filesystem, which is what makes it atomic. +func writeAtomic(dst string, data []byte) error { + tmp := dst + ".tmp" + + f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + if err != nil { + return fmt.Errorf("creating %s: %w", tmp, err) + } + if _, err := f.Write(data); err != nil { + f.Close() + _ = os.Remove(tmp) + return fmt.Errorf("writing %s: %w", tmp, err) + } + if err := f.Sync(); err != nil { + f.Close() + _ = os.Remove(tmp) + return fmt.Errorf("syncing %s: %w", tmp, err) + } + if err := f.Close(); err != nil { + _ = os.Remove(tmp) + return fmt.Errorf("closing %s: %w", tmp, err) + } + + if err := renameWithRetry(tmp, dst); err != nil { + _ = os.Remove(tmp) + return err + } + return nil +} + +// renameWithRetry renames src over dst, retrying a few times so a transient +// Windows sharing violation does not abandon an otherwise valid renewal. +func renameWithRetry(src, dst string) error { + var err error + for attempt := 1; attempt <= renameAttempts; attempt++ { + if err = os.Rename(src, dst); err == nil { + return nil + } + if attempt < renameAttempts { + time.Sleep(renameDelay) + } + } + return fmt.Errorf("replacing %s after %d attempts: %w", filepath.Base(dst), renameAttempts, err) +} diff --git a/internal/certrenew/certrenew_test.go b/internal/certrenew/certrenew_test.go new file mode 100644 index 0000000..0d2bdf4 --- /dev/null +++ b/internal/certrenew/certrenew_test.go @@ -0,0 +1,462 @@ +package certrenew_test + +import ( + "bytes" + "crypto/x509" + "encoding/pem" + "errors" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/deployhq/network-agent/internal/certrenew" + "github.com/deployhq/network-agent/internal/testca" +) + +// agentCN and agentSerial mirror how DeployHQ identifies an agent: the subject +// common name and the serial number are the identity, and a renewal must +// preserve both while changing only the issuer. +const ( + agentCN = "Deploy Agent #42" + agentSerial = 42 +) + +type fixture struct { + dir string + certPath string + keyPath string + current *x509.Certificate + originalPEM []byte + + // renewed is the certificate the backend would legitimately hand back: + // the same key, subject and serial, re-signed by the new CA. + renewed []byte + + // The refusable variants. + wrongSerial []byte + wrongSubject []byte + wrongKey []byte + unknownCA []byte + + // Kept so a test can mint a variant the fixture does not carry. + newCA *testca.CA + agent *testca.Identity + + roots *x509.CertPool +} + +func newFixture(t *testing.T) *fixture { + t.Helper() + + oldCA := testca.New(t, "old") + newCA := testca.New(t, "new") + otherCA := testca.New(t, "someone else") + + agent := oldCA.Issue(t, agentCN, agentSerial) + impostor := newCA.Issue(t, agentCN, agentSerial) // same identity, different key + + dir := t.TempDir() + certPath := filepath.Join(dir, "agent.crt") + keyPath := filepath.Join(dir, "agent.key") + if err := os.WriteFile(certPath, agent.CertPEM, 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyPath, agent.KeyPEM, 0600); err != nil { + t.Fatal(err) + } + + return &fixture{ + dir: dir, + certPath: certPath, + keyPath: keyPath, + current: agent.Cert, + originalPEM: agent.CertPEM, + renewed: newCA.Reissue(t, agent, agentCN, agentSerial), + wrongSerial: newCA.Reissue(t, agent, agentCN, agentSerial+1), + wrongSubject: newCA.Reissue(t, agent, "Deploy Agent #99", agentSerial), + wrongKey: impostor.CertPEM, + unknownCA: otherCA.Reissue(t, agent, agentCN, agentSerial), + newCA: newCA, + agent: agent, + roots: testca.Pool(oldCA, newCA), + } +} + +func (f *fixture) request(candidate []byte) certrenew.Request { + return certrenew.Request{ + CertPath: f.certPath, + KeyPath: f.keyPath, + Roots: f.roots, + Current: f.current, + CandidatePEM: candidate, + } +} + +// onDisk reads the certificate file, so a test can assert it was left alone. +func (f *fixture) onDisk(t *testing.T) []byte { + t.Helper() + data, err := os.ReadFile(f.certPath) + if err != nil { + t.Fatalf("reading %s: %v", f.certPath, err) + } + return data +} + +// TestApplyInstallsRenewedCertificate is the happy path: a certificate +// re-signed under the new CA, keeping key, subject and serial, replaces the +// file on disk. +func TestApplyInstallsRenewedCertificate(t *testing.T) { + f := newFixture(t) + + installed, err := certrenew.Apply(f.request(f.renewed)) + if err != nil { + t.Fatalf("Apply: %v", err) + } + + if got := f.onDisk(t); !bytes.Equal(got, f.renewed) { + t.Error("certificate file does not hold the renewed certificate") + } + if installed.Issuer.CommonName != "Deploy Dev CA (new)" { + t.Errorf("issuer = %q, want %q", installed.Issuer.CommonName, "Deploy Dev CA (new)") + } + if installed.Subject.CommonName != agentCN { + t.Errorf("subject CN = %q, want %q", installed.Subject.CommonName, agentCN) + } + if installed.SerialNumber.Int64() != agentSerial { + t.Errorf("serial = %s, want %d", installed.SerialNumber, agentSerial) + } + if installed.Issuer.CommonName == f.current.Issuer.CommonName { + t.Error("issuer did not change — the fixture is not exercising a renewal") + } + + // The renewed certificate must still be usable: it has to load as a key + // pair with the untouched private key, or the agent is offline. + reloaded, err := certrenew.LoadCurrent(f.certPath) + if err != nil { + t.Fatalf("LoadCurrent after Apply: %v", err) + } + if !reloaded.Equal(installed) { + t.Error("certificate read back from disk differs from the one Apply reported") + } +} + +// TestApplyRejectsACertificateThatCannotDoClientAuth: the agent presents its +// certificate for client authentication and nothing else. A replacement that +// pairs with the key, keeps subject and serial and chains to a trusted CA, but +// whose extended key usage rules client authentication out, would be written to +// disk and then refused by the server on every reconnect -- the agent is behind +// a customer's firewall with its working certificate already gone. A real agent +// certificate carries no extended key usage at all, which remains valid for any +// usage, so requiring ClientAuth costs nothing and closes that door. +func TestApplyRejectsACertificateThatCannotDoClientAuth(t *testing.T) { + f := newFixture(t) + + serverAuthOnly := f.newCA.ReissueWithExtKeyUsage(t, f.agent, agentCN, agentSerial, x509.ExtKeyUsageServerAuth) + + if _, err := certrenew.Apply(f.request(serverAuthOnly)); err == nil { + t.Fatal("Apply accepted a certificate that cannot be used for client authentication") + } + if got := f.onDisk(t); !bytes.Equal(got, f.originalPEM) { + t.Error("the working certificate was replaced by one the server would reject") + } +} + +// TestApplyAcceptsAnExplicitClientAuthCertificate is the control for the test +// above: tightening the check must not reject a certificate that names the +// usage explicitly, only one that excludes it. +func TestApplyAcceptsAnExplicitClientAuthCertificate(t *testing.T) { + f := newFixture(t) + + clientAuth := f.newCA.ReissueWithExtKeyUsage(t, f.agent, agentCN, agentSerial, x509.ExtKeyUsageClientAuth) + + if _, err := certrenew.Apply(f.request(clientAuth)); err != nil { + t.Fatalf("Apply rejected an explicit clientAuth certificate: %v", err) + } +} + +// TestApplyIsANoOpWhenTheCertificateIsAlreadyInstalled: a server that keeps +// answering RENEWED with the certificate the agent already presents must not +// push it through a connect-renew-reconnect loop forever. The comparison is on +// the DER, so re-encoded PEM does not read as a different certificate. +func TestApplyIsANoOpWhenTheCertificateIsAlreadyInstalled(t *testing.T) { + tests := []struct { + name string + candidate func(f *fixture) []byte + }{ + {"the exact bytes on disk", func(f *fixture) []byte { return f.originalPEM }}, + {"the same certificate, re-encoded", func(f *fixture) []byte { + block, _ := pem.Decode(f.originalPEM) + if block == nil { + t.Fatal("fixture certificate is not PEM") + } + // Same DER, different PEM framing: extra trailing newline and a + // header the original does not carry. + out := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Headers: map[string]string{"Comment": "re-encoded"}, + Bytes: block.Bytes, + }) + return append(out, '\n') + }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newFixture(t) + before, err := os.Stat(f.certPath) + if err != nil { + t.Fatal(err) + } + + candidate := tt.candidate(f) + if _, err := certrenew.Apply(f.request(candidate)); !errors.Is(err, certrenew.ErrUnchanged) { + t.Fatalf("Apply returned %v, want ErrUnchanged", err) + } + + if !bytes.Equal(f.onDisk(t), f.originalPEM) { + t.Error("certificate file was rewritten") + } + after, err := os.Stat(f.certPath) + if err != nil { + t.Fatal(err) + } + if !after.ModTime().Equal(before.ModTime()) { + t.Error("certificate file was touched") + } + if _, err := os.Stat(f.certPath + ".tmp"); err == nil { + t.Error("temporary file left behind") + } + }) + } +} + +// TestApplyLeavesFileUntouchedOnEveryRejection is the safety property that +// matters most: any failed check must leave the existing certificate byte for +// byte intact, because a broken agent.crt takes the agent offline behind a +// customer firewall where nobody can repair it. +func TestApplyLeavesFileUntouchedOnEveryRejection(t *testing.T) { + tests := []struct { + name string + candidate func(f *fixture) []byte + }{ + {"wrong private key", func(f *fixture) []byte { return f.wrongKey }}, + {"wrong serial number", func(f *fixture) []byte { return f.wrongSerial }}, + {"wrong subject", func(f *fixture) []byte { return f.wrongSubject }}, + {"unknown issuer", func(f *fixture) []byte { return f.unknownCA }}, + {"malformed pem", func(f *fixture) []byte { return []byte("-----BEGIN CERTIFICATE-----\nnope\n") }}, + {"not pem at all", func(f *fixture) []byte { return []byte("certainly not a certificate") }}, + {"empty", func(f *fixture) []byte { return nil }}, + {"pem block of the wrong type", func(f *fixture) []byte { + return []byte("-----BEGIN PRIVATE KEY-----\nZm9v\n-----END PRIVATE KEY-----\n") + }}, + {"corrupt certificate der", func(f *fixture) []byte { + return []byte("-----BEGIN CERTIFICATE-----\nZm9vYmFy\n-----END CERTIFICATE-----\n") + }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newFixture(t) + before := f.onDisk(t) + + installed, err := certrenew.Apply(f.request(tt.candidate(f))) + if err == nil { + t.Fatalf("Apply accepted a %s certificate", tt.name) + } + if installed != nil { + t.Error("Apply returned a certificate alongside an error") + } + t.Logf("rejected: %v", err) + + if after := f.onDisk(t); !bytes.Equal(before, after) { + t.Error("certificate file was modified despite the rejection") + } + if !bytes.Equal(f.onDisk(t), f.originalPEM) { + t.Error("certificate file no longer holds the original certificate") + } + + // The temporary file must not be left behind either. + if _, err := os.Stat(f.certPath + ".tmp"); err == nil { + t.Error("temporary file left behind after a rejected renewal") + } + }) + } +} + +// TestApplyRejectsUntrustedRoots covers the failure this whole project exists +// to prevent: a certificate whose own dates are fine but whose issuer this +// agent does not trust must not be installed. +func TestApplyRejectsUntrustedRoots(t *testing.T) { + f := newFixture(t) + + req := f.request(f.renewed) + req.Roots = x509.NewCertPool() // trusts nothing + + if _, err := certrenew.Apply(req); err == nil { + t.Fatal("Apply accepted a certificate that chains to no trusted CA") + } + if !bytes.Equal(f.onDisk(t), f.originalPEM) { + t.Error("certificate file was modified") + } +} + +// TestValidateRejectsMissingInputs guards the caller contract: without a +// current certificate or a root pool there is nothing to validate against, and +// silently accepting would be the worst possible default. +func TestValidateRejectsMissingInputs(t *testing.T) { + f := newFixture(t) + + t.Run("no current certificate", func(t *testing.T) { + req := f.request(f.renewed) + req.Current = nil + if _, err := certrenew.Validate(req); err == nil { + t.Error("expected an error") + } + }) + + t.Run("no roots", func(t *testing.T) { + req := f.request(f.renewed) + req.Roots = nil + if _, err := certrenew.Validate(req); err == nil { + t.Error("expected an error") + } + }) + + t.Run("missing private key file", func(t *testing.T) { + req := f.request(f.renewed) + req.KeyPath = filepath.Join(f.dir, "does-not-exist.key") + if _, err := certrenew.Validate(req); err == nil { + t.Error("expected an error") + } + }) +} + +// TestValidateHasNoSideEffects: Validate is the pure half of Apply and must +// never touch the certificate file, not even on success. +func TestValidateHasNoSideEffects(t *testing.T) { + f := newFixture(t) + + if _, err := certrenew.Validate(f.request(f.renewed)); err != nil { + t.Fatalf("Validate: %v", err) + } + if !bytes.Equal(f.onDisk(t), f.originalPEM) { + t.Error("Validate modified the certificate file") + } +} + +// TestApplyWithRSAKeys pins the key-pairing check against production's key +// algorithm. DeployHQ mints 4096-bit RSA keys; the size is irrelevant here but +// the algorithm is not, because the pairing check is algorithm-specific. +func TestApplyWithRSAKeys(t *testing.T) { + oldCA := testca.New(t, "old") + newCA := testca.New(t, "new") + agent := oldCA.IssueWithKey(t, agentCN, agentSerial, testca.KeyRSA) + impostor := newCA.IssueWithKey(t, agentCN, agentSerial, testca.KeyRSA) + + dir := t.TempDir() + certPath := filepath.Join(dir, "agent.crt") + keyPath := filepath.Join(dir, "agent.key") + if err := os.WriteFile(certPath, agent.CertPEM, 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyPath, agent.KeyPEM, 0600); err != nil { + t.Fatal(err) + } + + req := certrenew.Request{ + CertPath: certPath, + KeyPath: keyPath, + Roots: testca.Pool(oldCA, newCA), + Current: agent.Cert, + } + + req.CandidatePEM = impostor.CertPEM + if _, err := certrenew.Apply(req); err == nil { + t.Error("Apply accepted an RSA certificate for a different key") + } + + req.CandidatePEM = newCA.Reissue(t, agent, agentCN, agentSerial) + if _, err := certrenew.Apply(req); err != nil { + t.Fatalf("Apply rejected a valid RSA renewal: %v", err) + } + data, err := os.ReadFile(certPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(data, req.CandidatePEM) { + t.Error("RSA renewal was not written") + } +} + +// TestApplyWritesPrivateFile: the certificate itself is not secret, but the +// agent's ~/.deploy directory is 0700 and the renewal must not widen anything. +func TestApplyWritesPrivateFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix file modes do not apply on Windows") + } + f := newFixture(t) + + if _, err := certrenew.Apply(f.request(f.renewed)); err != nil { + t.Fatalf("Apply: %v", err) + } + + info, err := os.Stat(f.certPath) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0600 { + t.Errorf("mode = %#o, want 0600", perm) + } +} + +// TestApplyReplacesAStaleTempFile: a crash mid-renewal can leave agent.crt.tmp +// behind. The next attempt must overwrite it rather than fail forever. +func TestApplyReplacesAStaleTempFile(t *testing.T) { + f := newFixture(t) + + stale := f.certPath + ".tmp" + if err := os.WriteFile(stale, []byte("leftover from a crashed renewal"), 0600); err != nil { + t.Fatal(err) + } + + if _, err := certrenew.Apply(f.request(f.renewed)); err != nil { + t.Fatalf("Apply: %v", err) + } + if !bytes.Equal(f.onDisk(t), f.renewed) { + t.Error("renewed certificate was not installed over a stale temp file") + } + if _, err := os.Stat(stale); err == nil { + t.Error("temp file still present after a successful renewal") + } +} + +// TestLoadCurrent covers the helper the tunnel uses to learn which certificate +// it is presenting before asking the server to renew it. +func TestLoadCurrent(t *testing.T) { + f := newFixture(t) + + cert, err := certrenew.LoadCurrent(f.certPath) + if err != nil { + t.Fatalf("LoadCurrent: %v", err) + } + if cert.Subject.CommonName != agentCN || cert.SerialNumber.Int64() != agentSerial { + t.Errorf("loaded %q serial %s", cert.Subject.CommonName, cert.SerialNumber) + } + + t.Run("missing file", func(t *testing.T) { + if _, err := certrenew.LoadCurrent(filepath.Join(f.dir, "nope.crt")); err == nil { + t.Error("expected an error") + } + }) + + t.Run("not a certificate", func(t *testing.T) { + path := filepath.Join(f.dir, "garbage.crt") + if err := os.WriteFile(path, []byte("hello"), 0600); err != nil { + t.Fatal(err) + } + if _, err := certrenew.LoadCurrent(path); err == nil { + t.Error("expected an error") + } + }) +} diff --git a/internal/config/ca.go b/internal/config/ca.go new file mode 100644 index 0000000..44514c8 --- /dev/null +++ b/internal/config/ca.go @@ -0,0 +1,35 @@ +package config + +import ( + "fmt" + "os" +) + +// CAFileEnv names the environment variable that overrides the CA bundle used +// to verify the DeployHQ agent server. +const CAFileEnv = "DEPLOY_AGENT_CA_FILE" + +// CACert returns the CA bundle the agent should trust. +// +// By default this is the bundle compiled into the binary, passed in as +// embedded. Setting DEPLOY_AGENT_CA_FILE to a PEM file path replaces it — for +// staging, for a private agent server, and as the escape hatch that lets an +// operator trust a newly issued CA without waiting for a new release. +// +// An unreadable override is an error rather than a silent fall back to the +// embedded bundle: trusting a different CA than the operator asked for is worse +// than refusing to start. +func CACert(embedded []byte) ([]byte, error) { + path := os.Getenv(CAFileEnv) + if path == "" { + return embedded, nil + } + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading %s %q: %w", CAFileEnv, path, err) + } + if len(data) == 0 { + return nil, fmt.Errorf("reading %s %q: file is empty", CAFileEnv, path) + } + return data, nil +} diff --git a/internal/config/ca_test.go b/internal/config/ca_test.go new file mode 100644 index 0000000..3a38c6a --- /dev/null +++ b/internal/config/ca_test.go @@ -0,0 +1,83 @@ +package config_test + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/deployhq/network-agent/internal/config" + "github.com/deployhq/network-agent/internal/testca" +) + +func TestCACertDefaultsToTheEmbeddedBundle(t *testing.T) { + t.Setenv(config.CAFileEnv, "") + + embedded := testca.Bundle(testca.New(t, "embedded")) + got, err := config.CACert(embedded) + if err != nil { + t.Fatalf("CACert: %v", err) + } + if !bytes.Equal(got, embedded) { + t.Error("CACert did not return the embedded bundle") + } +} + +func TestCACertReadsTheOverrideFile(t *testing.T) { + embedded := testca.Bundle(testca.New(t, "embedded")) + override := testca.Bundle(testca.New(t, "operator supplied")) + + path := filepath.Join(t.TempDir(), "ca.pem") + if err := os.WriteFile(path, override, 0600); err != nil { + t.Fatal(err) + } + t.Setenv(config.CAFileEnv, path) + + got, err := config.CACert(embedded) + if err != nil { + t.Fatalf("CACert: %v", err) + } + if !bytes.Equal(got, override) { + t.Error("CACert did not return the override file's contents") + } + if bytes.Equal(got, embedded) { + t.Error("CACert returned the embedded bundle despite the override") + } +} + +// TestCACertFailsRatherThanFallingBack: silently trusting a different CA than +// the operator asked for is worse than refusing to start, so an unusable +// override must be an error and never a fallback to the embedded bundle. +func TestCACertFailsRatherThanFallingBack(t *testing.T) { + embedded := testca.Bundle(testca.New(t, "embedded")) + dir := t.TempDir() + + empty := filepath.Join(dir, "empty.pem") + if err := os.WriteFile(empty, nil, 0600); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + path string + }{ + {"missing file", filepath.Join(dir, "does-not-exist.pem")}, + {"a directory", dir}, + {"empty file", empty}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(config.CAFileEnv, tt.path) + + got, err := config.CACert(embedded) + if err == nil { + t.Fatal("expected an error, got none") + } + if got != nil { + t.Error("CACert returned a bundle alongside an error") + } + t.Logf("rejected: %v", err) + }) + } +} diff --git a/internal/config/tls.go b/internal/config/tls.go index 9f07128..707c0ac 100644 --- a/internal/config/tls.go +++ b/internal/config/tls.go @@ -1,16 +1,77 @@ package config import ( + "bytes" "crypto/tls" "crypto/x509" + "encoding/pem" "fmt" "os" "strings" ) +// pemBegin opens every PEM block; a bundle that does not start with it, or +// that carries more of them than parse, is malformed. +const pemBegin = "-----BEGIN " + +// NewCertPool parses a PEM bundle into a certificate pool. The bundle may hold +// more than one CA — that is how an agent trusts the old and the new DeployHQ +// CA simultaneously during a CA rotation. +// +// Every block has to parse. AppendCertsFromPEM would report success as soon as +// ONE did, silently dropping the rest: a bundle whose incoming CA is corrupt +// would start the agent trusting only the CA it already had, so the override +// looks like it worked and then fails the moment the server rotates. +func NewCertPool(caCert []byte) (*x509.CertPool, error) { + pool := x509.NewCertPool() + rest := caCert + added := 0 + + // pem.Decode SKIPS anything it cannot parse ahead of a block it can, so + // neither junk before the first block nor a BEGIN line with no matching END + // would be reported by decoding alone. Both are refused here: each chunk has + // to start with a BEGIN line, and the number of blocks decoded has to match + // the number of BEGIN lines in the file. + begins := bytes.Count(caCert, []byte(pemBegin)) + + for { + rest = bytes.TrimLeft(rest, " \t\r\n") + if len(rest) == 0 { + break + } + if !bytes.HasPrefix(rest, []byte(pemBegin)) { + return nil, fmt.Errorf("CA bundle has data that is not a PEM certificate, after %d certificate(s)", added) + } + + var block *pem.Block + block, rest = pem.Decode(rest) + if block == nil { + return nil, fmt.Errorf("CA bundle has an unreadable PEM block after %d certificate(s)", added) + } + if block.Type != "CERTIFICATE" { + return nil, fmt.Errorf("CA bundle holds a %q block, not a certificate", block.Type) + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("parsing certificate %d in the CA bundle: %w", added+1, err) + } + pool.AddCert(cert) + added++ + } + + if added == 0 { + return nil, fmt.Errorf("failed to parse CA certificate") + } + if added != begins { + return nil, fmt.Errorf("CA bundle declares %d certificates but only %d could be read", begins, added) + } + return pool, nil +} + // NewTLSConfig builds a mutual-TLS config for the agent: -// - Client certificate: ~/.deploy/agent.crt + agent.key -// - Server verification: embedded CA cert passed in via caCert +// - Client certificate: ~/.deploy/agent.crt + agent.key, re-read from disk on +// every handshake (see below) +// - Server verification: CA bundle passed in via caCert // // When verify is false, server certificate verification is skipped (dev/testing). // @@ -20,13 +81,27 @@ import ( // VerifyConnection callback that checks the CA chain and then falls back to CN // matching when no SANs are present. func NewTLSConfig(paths Paths, caCert []byte, verify bool) (*tls.Config, error) { - cert, err := tls.LoadX509KeyPair(paths.Certificate, paths.Key) - if err != nil { + // Load once up front so a missing, unreadable or mismatched key pair is a + // hard error at startup rather than a puzzling handshake failure later. + if _, err := tls.LoadX509KeyPair(paths.Certificate, paths.Key); err != nil { return nil, fmt.Errorf("loading agent certificate: %w", err) } cfg := &tls.Config{ - Certificates: []tls.Certificate{cert}, + // Certificates is deliberately left nil. A single *tls.Config is built + // once at start-up and reused for every reconnect, so a certificate + // cached here would keep being presented until the process restarted — + // which would defeat certificate renewal, where the agent replaces + // agent.crt in place and simply reconnects. Reading the key pair per + // handshake instead makes the renewed certificate take effect on the + // very next connection, with no customer-side restart. + GetClientCertificate: func(*tls.CertificateRequestInfo) (*tls.Certificate, error) { + cert, err := tls.LoadX509KeyPair(paths.Certificate, paths.Key) + if err != nil { + return nil, fmt.Errorf("loading agent certificate: %w", err) + } + return &cert, nil + }, } if !verify { @@ -34,9 +109,9 @@ func NewTLSConfig(paths Paths, caCert []byte, verify bool) (*tls.Config, error) return cfg, nil } - pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM(caCert) { - return nil, fmt.Errorf("failed to parse CA certificate") + pool, err := NewCertPool(caCert) + if err != nil { + return nil, err } serverHost := ServerHost() diff --git a/internal/config/tls_test.go b/internal/config/tls_test.go new file mode 100644 index 0000000..18a73f3 --- /dev/null +++ b/internal/config/tls_test.go @@ -0,0 +1,362 @@ +package config_test + +import ( + "crypto/tls" + "crypto/x509" + "errors" + "net" + "os" + "path/filepath" + "testing" + "time" + + "github.com/deployhq/network-agent/internal/caroot" + "github.com/deployhq/network-agent/internal/config" + "github.com/deployhq/network-agent/internal/testca" +) + +const serverCN = "agent.deployhq.com" + +// writeIdentity puts a key pair on disk the way `network-agent setup` does and +// returns the paths for it. +func writeIdentity(t *testing.T, id *testca.Identity) config.Paths { + t.Helper() + dir := t.TempDir() + paths := config.Paths{ + Config: dir, + Certificate: filepath.Join(dir, "agent.crt"), + Key: filepath.Join(dir, "agent.key"), + } + if err := os.WriteFile(paths.Certificate, id.CertPEM, 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(paths.Key, id.KeyPEM, 0600); err != nil { + t.Fatal(err) + } + return paths +} + +// TestVerifyConnectionAcceptsEitherCAInTheBundle is the property the whole CA +// rotation rests on: one binary carrying a two-CA bundle must accept an agent +// server presenting a leaf from either CA, and nothing else. +func TestVerifyConnectionAcceptsEitherCAInTheBundle(t *testing.T) { + t.Setenv("DEPLOY_AGENT_PROXY_IP", serverCN) + + oldCA := testca.New(t, "old") + newCA := testca.New(t, "new") + strangerCA := testca.New(t, "stranger") + + paths := writeIdentity(t, oldCA.Issue(t, "Deploy Agent #42", 42)) + + cfg, err := config.NewTLSConfig(paths, testca.Bundle(oldCA, newCA), true) + if err != nil { + t.Fatalf("NewTLSConfig: %v", err) + } + if cfg.VerifyConnection == nil { + t.Fatal("VerifyConnection was not installed") + } + + tests := []struct { + name string + leaf *x509.Certificate + wantErr bool + }{ + {"leaf from the outgoing CA", oldCA.Issue(t, serverCN, 1).Cert, false}, + {"leaf from the incoming CA", newCA.Issue(t, serverCN, 1).Cert, false}, + {"leaf from an untrusted CA", strangerCA.Issue(t, serverCN, 1).Cert, true}, + {"right CA, wrong common name", oldCA.Issue(t, "evil.example.com", 2).Cert, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := cfg.VerifyConnection(tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{tt.leaf}, + }) + if tt.wantErr && err == nil { + t.Error("expected verification to fail, it succeeded") + } + if !tt.wantErr && err != nil { + t.Errorf("expected verification to succeed, got: %v", err) + } + if err != nil { + t.Logf("rejected: %v", err) + } + }) + } + + t.Run("no certificate at all", func(t *testing.T) { + if err := cfg.VerifyConnection(tls.ConnectionState{}); err == nil { + t.Error("expected verification to fail when the server sends nothing") + } + }) +} + +// TestBundleOrderIsIrrelevant: nobody should have to think about which CA comes +// first in ca.crt when appending the new one. +func TestBundleOrderIsIrrelevant(t *testing.T) { + t.Setenv("DEPLOY_AGENT_PROXY_IP", serverCN) + + oldCA := testca.New(t, "old") + newCA := testca.New(t, "new") + paths := writeIdentity(t, oldCA.Issue(t, "Deploy Agent #42", 42)) + + leaves := map[string]*x509.Certificate{ + "old": oldCA.Issue(t, serverCN, 1).Cert, + "new": newCA.Issue(t, serverCN, 1).Cert, + } + + for name, bundle := range map[string][]byte{ + "old first": testca.Bundle(oldCA, newCA), + "new first": testca.Bundle(newCA, oldCA), + } { + t.Run(name, func(t *testing.T) { + cfg, err := config.NewTLSConfig(paths, bundle, true) + if err != nil { + t.Fatalf("NewTLSConfig: %v", err) + } + for which, leaf := range leaves { + if err := cfg.VerifyConnection(tls.ConnectionState{ + PeerCertificates: []*x509.Certificate{leaf}, + }); err != nil { + t.Errorf("%s leaf rejected: %v", which, err) + } + } + }) + } +} + +// TestGetClientCertificatePresentsRenewedCertificate is the regression test for +// the bug that made in-band renewal impossible: one *tls.Config is built at +// start-up and reused for every reconnect, so a client certificate cached in +// it would keep being presented until the process restarted. +// +// Replacing agent.crt on disk must change what the very next handshake +// presents, with no new config and no restart. +func TestGetClientCertificatePresentsRenewedCertificate(t *testing.T) { + oldCA := testca.New(t, "old") + newCA := testca.New(t, "new") + + agent := oldCA.Issue(t, "Deploy Agent #42", 42) + paths := writeIdentity(t, agent) + + server := newTestServer(t, oldCA.Issue(t, serverCN, 1), testca.Pool(oldCA, newCA)) + + // verify=false keeps the server's own certificate out of the picture; the + // client certificate is what is under test and it is loaded the same way + // on both paths. + cfg, err := config.NewTLSConfig(paths, testca.Bundle(oldCA, newCA), false) + if err != nil { + t.Fatalf("NewTLSConfig: %v", err) + } + // Structural checks first, but non-fatal: the handshakes below are the real + // evidence and should still run and report if these ever regress. + if cfg.Certificates != nil { + t.Error("Certificates is populated — a cached certificate cannot be renewed without a restart") + } + if cfg.GetClientCertificate == nil { + t.Error("GetClientCertificate was not installed") + } + + if got := server.issuerSeen(t, cfg); got != "Deploy Dev CA (old)" { + t.Fatalf("first handshake presented issuer %q, want the old CA", got) + } + + // The renewal: same key, same subject, same serial, new issuer. + renewed := newCA.Reissue(t, agent, "Deploy Agent #42", 42) + if err := os.WriteFile(paths.Certificate, renewed, 0600); err != nil { + t.Fatal(err) + } + + if got := server.issuerSeen(t, cfg); got != "Deploy Dev CA (new)" { + t.Errorf("second handshake presented issuer %q, want the new CA — the certificate was not reloaded", got) + } +} + +// TestNewTLSConfigRejectsBadInputs: every one of these must fail loudly at +// start-up rather than at handshake time behind a customer firewall. +func TestNewTLSConfigRejectsBadInputs(t *testing.T) { + ca := testca.New(t, "old") + good := ca.Issue(t, "Deploy Agent #42", 42) + other := ca.Issue(t, "Deploy Agent #42", 42) // same identity, different key + + t.Run("missing certificate", func(t *testing.T) { + paths := writeIdentity(t, good) + if err := os.Remove(paths.Certificate); err != nil { + t.Fatal(err) + } + if _, err := config.NewTLSConfig(paths, ca.CertPEM, true); err == nil { + t.Error("expected an error") + } + }) + + t.Run("missing key", func(t *testing.T) { + paths := writeIdentity(t, good) + if err := os.Remove(paths.Key); err != nil { + t.Fatal(err) + } + if _, err := config.NewTLSConfig(paths, ca.CertPEM, true); err == nil { + t.Error("expected an error") + } + }) + + t.Run("certificate and key do not match", func(t *testing.T) { + paths := writeIdentity(t, good) + if err := os.WriteFile(paths.Key, other.KeyPEM, 0600); err != nil { + t.Fatal(err) + } + if _, err := config.NewTLSConfig(paths, ca.CertPEM, true); err == nil { + t.Error("expected an error") + } + }) + + t.Run("unparsable CA bundle", func(t *testing.T) { + paths := writeIdentity(t, good) + if _, err := config.NewTLSConfig(paths, []byte("not a certificate"), true); err == nil { + t.Error("expected an error") + } + }) +} + +func TestNewCertPool(t *testing.T) { + oldCA := testca.New(t, "old") + newCA := testca.New(t, "new") + + if _, err := config.NewCertPool(testca.Bundle(oldCA, newCA)); err != nil { + t.Errorf("two-CA bundle: %v", err) + } + if _, err := config.NewCertPool(nil); err == nil { + t.Error("empty bundle should be an error") + } + if _, err := config.NewCertPool([]byte("garbage")); err == nil { + t.Error("unparsable bundle should be an error") + } +} + +// TestNewCertPoolRejectsAPartiallyMalformedBundle: DEPLOY_AGENT_CA_FILE is the +// escape hatch for trusting a new CA without waiting for a release, so it has +// to fail loudly when it is wrong. Go's AppendCertsFromPEM reports success as +// long as ONE block parsed, which would start the agent trusting only the CA it +// already had -- the override appears to have worked and then fails at the +// moment of rotation, which is the moment it exists for. +func TestNewCertPoolRejectsAPartiallyMalformedBundle(t *testing.T) { + good := testca.Bundle(testca.New(t, "old")) + corrupt := []byte("-----BEGIN CERTIFICATE-----\nbm90IGEgY2VydGlmaWNhdGU=\n-----END CERTIFICATE-----\n") + + if _, err := config.NewCertPool(append(append([]byte{}, good...), corrupt...)); err == nil { + t.Error("a bundle whose second certificate is malformed should be an error") + } + if _, err := config.NewCertPool(append(append([]byte{}, corrupt...), good...)); err == nil { + t.Error("a bundle whose first certificate is malformed should be an error") + } +} + +// TestNewCertPoolRejectsWhatPemDecodeWouldSkip: encoding/pem skips anything it +// cannot parse BEFORE a block it can -- leading junk, or a BEGIN line with no +// matching END. Either would leave the agent trusting whichever CA happened to +// parse, which is the same silent half-trust as a malformed block at the end of +// the file, just at the other end of it. +func TestNewCertPoolRejectsWhatPemDecodeWouldSkip(t *testing.T) { + good := testca.Bundle(testca.New(t, "old")) + + leadingGarbage := append([]byte("garbage before the block\n"), good...) + if _, err := config.NewCertPool(leadingGarbage); err == nil { + t.Error("data before the first certificate should be an error") + } + + unterminated := append([]byte("-----BEGIN CERTIFICATE-----\nbm90IGEgY2VydA==\n"), good...) + if _, err := config.NewCertPool(unterminated); err == nil { + t.Error("an unterminated certificate block should be an error") + } +} + +// TestNewCertPoolAcceptsTheShippedBundle: the stricter parsing above must not +// reject what this binary actually ships with -- the embedded bundle holds both +// DeployHQ CAs during the rotation, and it is read through this same function. +func TestNewCertPoolAcceptsTheShippedBundle(t *testing.T) { + if _, err := config.NewCertPool(caroot.CACert); err != nil { + t.Fatalf("the embedded CA bundle does not parse: %v", err) + } +} + +// TestNewCertPoolRejectsANonCertificateBlock: a PEM file of the right shape but +// the wrong contents -- a private key pasted in place of a certificate -- is a +// plausible operator mistake and must not read as an empty-but-valid bundle. +func TestNewCertPoolRejectsANonCertificateBlock(t *testing.T) { + notACert := []byte("-----BEGIN PRIVATE KEY-----\nMC4CAQAwBQYDK2VwBCIEIA==\n-----END PRIVATE KEY-----\n") + + if _, err := config.NewCertPool(notACert); err == nil { + t.Error("a bundle holding no certificate at all should be an error") + } +} + +// ── test server ────────────────────────────────────────────────────────────── + +// testServer accepts one mTLS connection at a time and reports the issuer of +// the client certificate it was shown. +type testServer struct { + ln net.Listener +} + +func newTestServer(t *testing.T, leaf *testca.Identity, clientCAs *x509.CertPool) *testServer { + t.Helper() + ln, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{ + Certificates: []tls.Certificate{leaf.TLS(t)}, + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: clientCAs, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { ln.Close() }) + return &testServer{ln: ln} +} + +// issuerSeen dials the server with cfg and returns the issuer common name of +// the client certificate the server was actually shown — the only authority on +// which certificate went on the wire. +func (s *testServer) issuerSeen(t *testing.T, cfg *tls.Config) string { + t.Helper() + + type result struct { + issuer string + err error + } + done := make(chan result, 1) + go func() { + conn, err := s.ln.Accept() + if err != nil { + done <- result{err: err} + return + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(10 * time.Second)) + tc := conn.(*tls.Conn) + if err := tc.Handshake(); err != nil { + done <- result{err: err} + return + } + peer := tc.ConnectionState().PeerCertificates + if len(peer) == 0 { + done <- result{err: errors.New("server saw no client certificate")} + return + } + done <- result{issuer: peer[0].Issuer.CommonName} + }() + + conn, err := tls.DialWithDialer(&net.Dialer{Timeout: 5 * time.Second}, "tcp", s.ln.Addr().String(), cfg) + if err != nil { + t.Fatalf("dialling test server: %v", err) + } + defer conn.Close() + + select { + case r := <-done: + if r.err != nil { + t.Fatalf("test server: %v", r.err) + } + return r.issuer + case <-time.After(10 * time.Second): + t.Fatal("timeout waiting for the test server") + return "" + } +} diff --git a/internal/protocol/commands.go b/internal/protocol/commands.go index a62afa5..fdb988a 100644 --- a/internal/protocol/commands.go +++ b/internal/protocol/commands.go @@ -9,4 +9,32 @@ const ( CmdReject byte = 5 CmdReconnect byte = 6 CmdKeepalive byte = 7 + + // CmdRenewRequest is sent by the agent once per connection, immediately + // after the TLS handshake. Its payload is a UTF-8 implementation/version + // identifier such as "go/0.3.0"; the server decides whether the agent's + // client certificate needs re-issuing under a newer CA. + CmdRenewRequest byte = 8 + + // CmdRenewResponse is the server's answer to CmdRenewRequest. Its payload + // is [status:1][body], see the RenewStatus* constants. The server never + // sends it unsolicited. + CmdRenewResponse byte = 9 +) + +// Renewal response statuses, carried in the first payload byte of a +// CmdRenewResponse frame. +const ( + // RenewStatusRenewed means the body holds a new client certificate in PEM + // form, re-issued for the agent's existing key pair. + RenewStatusRenewed byte = 0 + + // RenewStatusCurrent means the agent's certificate is already current. + // There is no body. + RenewStatusCurrent byte = 1 + + // RenewStatusError means renewal failed server-side. The body is a UTF-8 + // diagnostic message; the agent logs it and carries on with its current + // certificate. + RenewStatusError byte = 2 ) diff --git a/internal/protocol/framing.go b/internal/protocol/framing.go index af65259..e786916 100644 --- a/internal/protocol/framing.go +++ b/internal/protocol/framing.go @@ -79,6 +79,21 @@ func EncodeKeepalive() []byte { return EncodePacket(CmdKeepalive, nil) } +// EncodeRenewRequest encodes a RENEW_REQUEST packet. The payload is the UTF-8 +// implementation/version identifier of the agent, e.g. "go/0.3.0". +func EncodeRenewRequest(identifier string) []byte { + return EncodePacket(CmdRenewRequest, []byte(identifier)) +} + +// EncodeRenewResponse encodes a RENEW_RESPONSE packet. Only the server sends +// these; the agent side exists so tests can drive the full exchange. +func EncodeRenewResponse(status byte, body []byte) []byte { + payload := make([]byte, 1+len(body)) + payload[0] = status + copy(payload[1:], body) + return EncodePacket(CmdRenewResponse, payload) +} + // --- Per-command parse helpers --- // ParseCreateRequest parses a CREATE_REQUEST payload into connID, host, port. @@ -122,6 +137,24 @@ func ParseDestroy(payload []byte) (connID uint16, ok bool) { return } +// ParseRenewRequest parses a RENEW_REQUEST payload, returning the agent's +// implementation/version identifier. An empty payload is valid but yields "". +func ParseRenewRequest(payload []byte) (identifier string) { + return string(payload) +} + +// ParseRenewResponse parses a RENEW_RESPONSE payload into its status byte and +// body. ok is false when the payload is too short to carry a status. +func ParseRenewResponse(payload []byte) (status byte, body []byte, ok bool) { + if len(payload) < 1 { + return + } + status = payload[0] + body = payload[1:] + ok = true + return +} + // ParseData parses a DATA payload and returns connID + data bytes. func ParseData(payload []byte) (connID uint16, data []byte, ok bool) { if len(payload) < 2 { diff --git a/internal/protocol/renew_test.go b/internal/protocol/renew_test.go new file mode 100644 index 0000000..7fcebad --- /dev/null +++ b/internal/protocol/renew_test.go @@ -0,0 +1,82 @@ +package protocol_test + +import ( + "bytes" + "testing" + + "github.com/deployhq/network-agent/internal/protocol" +) + +// The renewal command bytes are part of a wire contract shared with the Ruby +// agent and the DeployHQ backend. Pin them so a reorder of the const block +// cannot silently change what goes on the wire. +func TestRenewCommandBytes(t *testing.T) { + if protocol.CmdRenewRequest != 8 { + t.Errorf("CmdRenewRequest = %d, want 8", protocol.CmdRenewRequest) + } + if protocol.CmdRenewResponse != 9 { + t.Errorf("CmdRenewResponse = %d, want 9", protocol.CmdRenewResponse) + } + if protocol.RenewStatusRenewed != 0 || protocol.RenewStatusCurrent != 1 || protocol.RenewStatusError != 2 { + t.Errorf("renew statuses = %d/%d/%d, want 0/1/2", + protocol.RenewStatusRenewed, protocol.RenewStatusCurrent, protocol.RenewStatusError) + } +} + +func TestEncodeRenewRequest(t *testing.T) { + frame := protocol.EncodeRenewRequest("go/1.2.3") + + want := []byte{0x00, 0x0b, 0x08, 'g', 'o', '/', '1', '.', '2', '.', '3'} + if !bytes.Equal(frame, want) { + t.Fatalf("frame = % x, want % x", frame, want) + } + + packets, remaining := protocol.DecodePackets(frame) + if len(packets) != 1 || len(remaining) != 0 { + t.Fatalf("decoded %d packets, %d remaining bytes", len(packets), len(remaining)) + } + if got := protocol.ParseRenewRequest(packets[0].Payload); got != "go/1.2.3" { + t.Errorf("identifier = %q, want %q", got, "go/1.2.3") + } +} + +func TestRenewResponseRoundtrip(t *testing.T) { + tests := []struct { + name string + status byte + body []byte + }{ + {"renewed", protocol.RenewStatusRenewed, []byte("-----BEGIN CERTIFICATE-----\n")}, + {"current", protocol.RenewStatusCurrent, nil}, + {"error", protocol.RenewStatusError, []byte("no certificate on file")}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + frame := protocol.EncodeRenewResponse(tt.status, tt.body) + packets, remaining := protocol.DecodePackets(frame) + if len(packets) != 1 || len(remaining) != 0 { + t.Fatalf("decoded %d packets, %d remaining bytes", len(packets), len(remaining)) + } + if packets[0].Cmd != protocol.CmdRenewResponse { + t.Fatalf("cmd = %d, want %d", packets[0].Cmd, protocol.CmdRenewResponse) + } + status, body, ok := protocol.ParseRenewResponse(packets[0].Payload) + if !ok { + t.Fatal("ParseRenewResponse reported a malformed payload") + } + if status != tt.status { + t.Errorf("status = %d, want %d", status, tt.status) + } + if !bytes.Equal(body, tt.body) && !(len(body) == 0 && len(tt.body) == 0) { + t.Errorf("body = %q, want %q", body, tt.body) + } + }) + } +} + +func TestParseRenewResponseRejectsEmptyPayload(t *testing.T) { + if _, _, ok := protocol.ParseRenewResponse(nil); ok { + t.Error("empty payload should not parse as a renew response") + } +} diff --git a/internal/testca/testca.go b/internal/testca/testca.go new file mode 100644 index 0000000..88dbf16 --- /dev/null +++ b/internal/testca/testca.go @@ -0,0 +1,229 @@ +// Package testca mints certificates shaped like DeployHQ's real ones, for use +// in tests. It is imported only from _test.go files and so is not linked into +// the released binary. +// +// The shapes matter. DeployHQ's CA, its agent-server leaf and its agent client +// certificates all predate SAN-only verification: +// +// - CA: CN "Deploy Dev CA ()" under O=aTech Media Ltd, serial 1, +// basicConstraints CA:TRUE, keyUsage certSign+crlSign. +// - Leaves (both the server's and the agent's): a Common Name and nothing +// else — no subjectAltName, no extendedKeyUsage, no keyUsage. The missing +// SAN is exactly why config.NewTLSConfig needs its CN fallback, and the +// missing EKU is why certificate verification has to ask for +// ExtKeyUsageAny. +// +// Keys are ECDSA P-256 by default because tests generate a lot of them. +// Production uses 4096-bit RSA; pass KeyRSA where the key type is the thing +// under test. +package testca + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "testing" + "time" +) + +// KeyType selects the key algorithm for a generated certificate. +type KeyType int + +const ( + // KeyECDSA is a P-256 key: fast to generate, used everywhere the key + // algorithm is irrelevant to what the test is proving. + KeyECDSA KeyType = iota + // KeyRSA is a 2048-bit RSA key, matching production's algorithm (production + // uses 4096 bits; the size changes nothing that is under test here). + KeyRSA +) + +// CA is a self-signed certificate authority that can issue leaves. +type CA struct { + Cert *x509.Certificate + CertPEM []byte + + key crypto.Signer +} + +// Identity is a key pair plus the certificate currently issued for it. +type Identity struct { + Cert *x509.Certificate + CertPEM []byte + KeyPEM []byte + + key crypto.Signer +} + +// New creates a CA named "Deploy Dev CA ()". +func New(t *testing.T, name string) *CA { + t.Helper() + return NewWithKey(t, name, KeyECDSA) +} + +// NewWithKey creates a CA with an explicit key algorithm. +func NewWithKey(t *testing.T, name string, kt KeyType) *CA { + t.Helper() + key := generateKey(t, kt) + + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: subject("Deploy Dev CA (" + name + ")"), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(10 * 365 * 24 * time.Hour), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + BasicConstraintsValid: true, + IsCA: true, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key) + if err != nil { + t.Fatalf("creating CA certificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("parsing CA certificate: %v", err) + } + return &CA{Cert: cert, CertPEM: certPEM(der), key: key} +} + +// Pool returns a certificate pool trusting just this CA. +func (ca *CA) Pool() *x509.CertPool { + pool := x509.NewCertPool() + pool.AddCert(ca.Cert) + return pool +} + +// Issue mints a CN-only leaf with a fresh key pair. +func (ca *CA) Issue(t *testing.T, commonName string, serial int64) *Identity { + t.Helper() + return ca.IssueWithKey(t, commonName, serial, KeyECDSA) +} + +// IssueWithKey mints a CN-only leaf with a fresh key pair of the given type. +func (ca *CA) IssueWithKey(t *testing.T, commonName string, serial int64, kt KeyType) *Identity { + t.Helper() + key := generateKey(t, kt) + certPEMBytes, cert := ca.sign(t, key.Public(), commonName, serial) + return &Identity{Cert: cert, CertPEM: certPEMBytes, KeyPEM: keyPEM(t, key), key: key} +} + +// Reissue signs an existing identity's public key again under this CA — the +// renewal operation the backend performs. Passing the identity's own common +// name and serial produces a valid renewal; passing different ones produces the +// certificates a correct agent must refuse. +func (ca *CA) Reissue(t *testing.T, id *Identity, commonName string, serial int64) []byte { + t.Helper() + certPEMBytes, _ := ca.sign(t, id.key.Public(), commonName, serial) + return certPEMBytes +} + +// ReissueWithExtKeyUsage is Reissue for a certificate that DOES carry an +// extended key usage extension. Real agent certificates carry none, so this +// exists to build the ones an agent must refuse -- a server-auth-only +// certificate that would be rejected by the server's client authentication. +func (ca *CA) ReissueWithExtKeyUsage(t *testing.T, id *Identity, commonName string, serial int64, eku ...x509.ExtKeyUsage) []byte { + t.Helper() + certPEMBytes, _ := ca.sign(t, id.key.Public(), commonName, serial, eku...) + return certPEMBytes +} + +func (ca *CA) sign(t *testing.T, pub crypto.PublicKey, commonName string, serial int64, eku ...x509.ExtKeyUsage) ([]byte, *x509.Certificate) { + t.Helper() + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(serial), + Subject: subject(commonName), + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(10 * 365 * 24 * time.Hour), + // No SANs, no KeyUsage, and no ExtKeyUsage unless a test asks for one: + // exactly as DeployHQ mints them. + ExtKeyUsage: eku, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, ca.Cert, pub, ca.key) + if err != nil { + t.Fatalf("signing certificate for %q: %v", commonName, err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatalf("parsing certificate for %q: %v", commonName, err) + } + return certPEM(der), cert +} + +// TLS returns the identity as a tls.Certificate. +func (id *Identity) TLS(t *testing.T) tls.Certificate { + t.Helper() + cert, err := tls.X509KeyPair(id.CertPEM, id.KeyPEM) + if err != nil { + t.Fatalf("building tls.Certificate: %v", err) + } + return cert +} + +// Bundle concatenates CA certificates into one PEM bundle, as internal/caroot's +// ca.crt holds them during a rotation. +func Bundle(cas ...*CA) []byte { + var out []byte + for _, ca := range cas { + out = append(out, ca.CertPEM...) + } + return out +} + +// Pool returns a pool trusting every given CA. +func Pool(cas ...*CA) *x509.CertPool { + pool := x509.NewCertPool() + for _, ca := range cas { + pool.AddCert(ca.Cert) + } + return pool +} + +// subject mirrors lib/certificate_authority.rb's distinguished name. +func subject(commonName string) pkix.Name { + return pkix.Name{ + Country: []string{"GB"}, + Province: []string{"Dorset"}, + Locality: []string{"Poole"}, + Organization: []string{"aTech Media Ltd"}, + OrganizationalUnit: []string{"Deploy"}, + CommonName: commonName, + } +} + +func generateKey(t *testing.T, kt KeyType) crypto.Signer { + t.Helper() + switch kt { + case KeyRSA: + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generating RSA key: %v", err) + } + return key + default: + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generating ECDSA key: %v", err) + } + return key + } +} + +func certPEM(der []byte) []byte { + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +func keyPEM(t *testing.T, key crypto.Signer) []byte { + t.Helper() + der, err := x509.MarshalPKCS8PrivateKey(key) + if err != nil { + t.Fatalf("marshalling private key: %v", err) + } + return pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: der}) +} diff --git a/internal/tunnel/agent.go b/internal/tunnel/agent.go index f62d820..d51a295 100644 --- a/internal/tunnel/agent.go +++ b/internal/tunnel/agent.go @@ -2,6 +2,7 @@ package tunnel import ( "crypto/tls" + "crypto/x509" "errors" "fmt" "io" @@ -19,22 +20,42 @@ const ( reconnectMaxSecs = 20 ) +// Options carries what the tunnel needs beyond the TLS connection itself. +// +// The zero value is valid and disables in-band certificate renewal, which is +// what tests that only exercise the proxy protocol want. +type Options struct { + // Version is this build's version, reported to the server as + // "go/" when asking whether the client certificate needs + // re-issuing. The server has no other way to tell a Go agent from a Ruby + // one, or to know which release it is talking to. + Version string + + // Paths locates agent.crt and agent.key. Renewal replaces the certificate + // in place; the key is never touched. + Paths config.Paths + + // CARoots are the trust anchors a renewed certificate must chain to. + // Leaving it nil disables certificate renewal entirely. + CARoots *x509.CertPool +} + // RunAgent connects to the DeployHQ server and keeps reconnecting on transient // failures. It blocks until an unrecoverable error occurs (e.g. REJECT from // server, or a TLS authentication error that persists after maxSSLRetries). -func RunAgent(tlsCfg *tls.Config, paths config.Paths, log *slog.Logger) error { +func RunAgent(tlsCfg *tls.Config, opts Options, log *slog.Logger) error { serverAddr := fmt.Sprintf("%s:%s", config.ServerHost(), config.ServerPort) sslRetries := 0 for { - access, err := acl.LoadFile(paths.Access) + access, err := acl.LoadFile(opts.Paths.Access) if err != nil { return fmt.Errorf("loading access list: %w", err) } log.Info("connecting to server", "addr", serverAddr) - sc, err := Connect(tlsCfg, serverAddr, access, log) + sc, err := Connect(tlsCfg, serverAddr, access, opts, log) if err != nil { // TLS-level failure: apply retry counter (matches Ruby's 4-retry limit) sslRetries++ @@ -57,6 +78,15 @@ func RunAgent(tlsCfg *tls.Config, paths config.Paths, log *slog.Logger) error { return runErr } + if errors.Is(runErr, ErrCertificateRenewed) { + // Not a failure: the certificate on disk has been replaced and a + // fresh handshake is what makes the agent present it. Reconnect + // immediately, and deliberately without touching sslRetries — this + // connection succeeded. + log.Info("reconnecting to present the renewed certificate") + continue + } + if runErr == nil || errors.Is(runErr, io.EOF) { // Clean disconnect (RECONNECT or EOF): reconnect immediately log.Info("server disconnected, reconnecting") diff --git a/internal/tunnel/integration_test.go b/internal/tunnel/integration_test.go index 596643d..99fac0d 100644 --- a/internal/tunnel/integration_test.go +++ b/internal/tunnel/integration_test.go @@ -188,16 +188,27 @@ func (r *serverReader) next() protocol.Packet { // connectAgent starts the agent in a goroutine and returns immediately. // The TLS handshake completes once the caller's fake server calls accept(). +// +// Options are left at their zero value, which disables certificate renewal — +// these tests exercise the proxy protocol. See renew_test.go for the renewal +// exchange. func connectAgent(t *testing.T, serverAddr string, certs *testCerts, access *acl.AccessList) <-chan error { t.Helper() cfg := &tls.Config{ Certificates: []tls.Certificate{certs.clientCert}, RootCAs: certs.caPool, } + return runAgentConn(t, cfg, serverAddr, access, tunnel.Options{}) +} + +// runAgentConn drives one ServerConn to completion and reports the error Run +// returned. +func runAgentConn(t *testing.T, cfg *tls.Config, serverAddr string, access *acl.AccessList, opts tunnel.Options) <-chan error { + t.Helper() log := slog.New(slog.NewTextHandler(io.Discard, nil)) errc := make(chan error, 1) go func() { - sc, err := tunnel.Connect(cfg, serverAddr, access, log) + sc, err := tunnel.Connect(cfg, serverAddr, access, opts, log) if err != nil { errc <- err return diff --git a/internal/tunnel/renew.go b/internal/tunnel/renew.go new file mode 100644 index 0000000..2886fcb --- /dev/null +++ b/internal/tunnel/renew.go @@ -0,0 +1,130 @@ +package tunnel + +import ( + "errors" + + "github.com/deployhq/network-agent/internal/certrenew" + "github.com/deployhq/network-agent/internal/protocol" +) + +// implementationPrefix identifies this agent to the server. The Ruby gem sends +// "ruby/" over the same command, which is the only way the backend can +// tell the two implementations apart — nothing else on the wire carries it. +const implementationPrefix = "go/" + +// devVersion is what an unreleased build reports, matching main.Version's +// default so a locally built agent is never mistaken for a release. +const devVersion = "dev" + +// ErrCertificateRenewed asks RunAgent for a fresh connection so the newly +// installed certificate is actually presented. It is a signal, not a failure, +// and must never count towards the TLS retry budget. +var ErrCertificateRenewed = errors.New("certificate renewed") + +// renewalEnabled reports whether this connection may install a new certificate. +// Renewal needs somewhere to write and something to verify against; without +// both, a RENEW_RESPONSE is ignored rather than acted on blindly. +func (sc *ServerConn) renewalEnabled() bool { + return sc.opts.CARoots != nil && + sc.opts.Paths.Certificate != "" && + sc.opts.Paths.Key != "" +} + +// sendRenewRequest asks the server, once per connection, whether this agent's +// client certificate needs re-issuing under a newer CA. The server decides; +// the agent only reports who it is. +func (sc *ServerConn) sendRenewRequest() { + if !sc.renewalEnabled() { + return + } + version := sc.opts.Version + if version == "" { + version = devVersion + } + identifier := implementationPrefix + version + + select { + case sc.serverSend <- protocol.EncodeRenewRequest(identifier): + sc.log.Debug("sent certificate renewal request", "agent", identifier) + default: + // Unreachable in practice — the queue is empty at this point — but a + // blocked send here would stall the whole connection, and renewal is + // never worth that. + sc.log.Warn("could not queue certificate renewal request") + } +} + +// handleRenewResponse acts on the server's answer to sendRenewRequest. +// +// It returns ErrCertificateRenewed when a new certificate has been installed, +// which tears this connection down so the next one presents it. Every other +// outcome — including every kind of failure — returns nil and leaves the +// connection running: a renewal that cannot happen is never a reason to take a +// working tunnel, or the process, down. +func (sc *ServerConn) handleRenewResponse(payload []byte) error { + status, body, ok := protocol.ParseRenewResponse(payload) + if !ok { + sc.log.Warn("malformed RENEW_RESPONSE") + return nil + } + + if !sc.renewalEnabled() { + sc.log.Warn("ignoring RENEW_RESPONSE: certificate renewal is not configured") + return nil + } + + switch status { + case protocol.RenewStatusCurrent: + sc.log.Debug("certificate is current, no renewal needed") + return nil + + case protocol.RenewStatusError: + sc.log.Warn("server could not renew certificate", "reason", string(body)) + return nil + + case protocol.RenewStatusRenewed: + return sc.installRenewedCertificate(body) + + default: + sc.log.Warn("unknown RENEW_RESPONSE status", "status", status) + return nil + } +} + +func (sc *ServerConn) installRenewedCertificate(certPEM []byte) error { + current, err := certrenew.LoadCurrent(sc.opts.Paths.Certificate) + if err != nil { + sc.log.Warn("certificate renewal skipped", "err", err) + return nil + } + + installed, err := certrenew.Apply(certrenew.Request{ + CertPath: sc.opts.Paths.Certificate, + KeyPath: sc.opts.Paths.Key, + Roots: sc.opts.CARoots, + Current: current, + CandidatePEM: certPEM, + }) + switch { + case errors.Is(err, certrenew.ErrUnchanged): + // The server offered the certificate the agent already presents. + // Reconnecting would change nothing, and a server that keeps answering + // this way would spin the agent through connect-renew-reconnect + // forever. Stay on this connection. + sc.log.Debug("server offered the certificate already installed, nothing to do") + return nil + + case err != nil: + // Nothing was written; the agent carries on with the certificate it + // has and will ask again on the next connection. + sc.log.Warn("certificate renewal rejected", "err", err) + return nil + } + + sc.log.Info("certificate renewed", + "issuer", installed.Issuer.CommonName, + "serial", installed.SerialNumber.String(), + "expires", installed.NotAfter.UTC().Format("2006-01-02")) + + return ErrCertificateRenewed +} diff --git a/internal/tunnel/renew_test.go b/internal/tunnel/renew_test.go new file mode 100644 index 0000000..ff838cc --- /dev/null +++ b/internal/tunnel/renew_test.go @@ -0,0 +1,446 @@ +package tunnel_test + +// In-band certificate renewal over the tunnel. +// +// The exchange under test: the agent sends RENEW_REQUEST once per connection +// carrying "go/"; the server answers RENEW_RESPONSE with status +// RENEWED (a re-signed certificate), CURRENT, or ERROR. Only RENEWED writes +// anything, and only after the new certificate has been fully validated. + +import ( + "bytes" + "crypto/tls" + "crypto/x509" + "errors" + "net" + "os" + "path/filepath" + "testing" + "time" + + "github.com/deployhq/network-agent/internal/acl" + "github.com/deployhq/network-agent/internal/certrenew" + "github.com/deployhq/network-agent/internal/config" + "github.com/deployhq/network-agent/internal/protocol" + "github.com/deployhq/network-agent/internal/testca" + "github.com/deployhq/network-agent/internal/tunnel" +) + +const ( + agentCN = "Deploy Agent #42" + agentSerial = 42 + testVersion = "1.2.3" +) + +// renewFixture wires a real mTLS connection between a fake DeployHQ server and +// an agent whose client certificate lives on disk, so a renewal that replaces +// that file is observable exactly as it would be in production. +type renewFixture struct { + certPath string + keyPath string + + originalPEM []byte // what agent.crt held before the exchange + renewedPEM []byte // a legitimate renewal: same key, subject and serial + untrustedPEM []byte // signed by a CA the agent does not trust + + ln net.Listener + opts tunnel.Options + cfg *tls.Config +} + +func newRenewFixture(t *testing.T) *renewFixture { + t.Helper() + + oldCA := testca.New(t, "old") + newCA := testca.New(t, "new") + otherCA := testca.New(t, "someone else") + + agent := oldCA.Issue(t, agentCN, agentSerial) + server := oldCA.Issue(t, "agent.deployhq.com", 1) + + dir := t.TempDir() + f := &renewFixture{ + certPath: filepath.Join(dir, "agent.crt"), + keyPath: filepath.Join(dir, "agent.key"), + originalPEM: agent.CertPEM, + renewedPEM: newCA.Reissue(t, agent, agentCN, agentSerial), + untrustedPEM: otherCA.Reissue(t, agent, agentCN, agentSerial), + } + if err := os.WriteFile(f.certPath, agent.CertPEM, 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(f.keyPath, agent.KeyPEM, 0600); err != nil { + t.Fatal(err) + } + + // The server trusts both CAs, as the backend does throughout a rotation. + ln, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{ + Certificates: []tls.Certificate{server.TLS(t)}, + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: testca.Pool(oldCA, newCA), + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { ln.Close() }) + f.ln = ln + + paths := config.Paths{Certificate: f.certPath, Key: f.keyPath} + + // verify=false: the server's own certificate is not what these tests are + // about, and it keeps the fixture free of hostname gymnastics. The client + // certificate is still loaded from disk per handshake either way. + cfg, err := config.NewTLSConfig(paths, testca.Bundle(oldCA, newCA), false) + if err != nil { + t.Fatal(err) + } + f.cfg = cfg + f.opts = tunnel.Options{ + Version: testVersion, + Paths: paths, + CARoots: testca.Pool(oldCA, newCA), + } + return f +} + +// connect starts the agent and returns the accepted server-side connection. +func (f *renewFixture) connect(t *testing.T, opts tunnel.Options) (net.Conn, <-chan error) { + t.Helper() + errc := runAgentConn(t, f.cfg, f.ln.Addr().String(), acl.Parse("127.0.0.1"), opts) + + connCh := make(chan net.Conn, 1) + go func() { + conn, err := f.ln.Accept() + if err == nil { + connCh <- conn + } + }() + select { + case conn := <-connCh: + conn.SetDeadline(time.Now().Add(10 * time.Second)) + t.Cleanup(func() { conn.Close() }) + // Accept returns before the handshake runs, so force it here: the + // client certificate the agent presented is the whole point of these + // tests and it is not observable until the handshake completes. + if err := conn.(*tls.Conn).Handshake(); err != nil { + t.Fatalf("server-side handshake: %v", err) + } + return conn, errc + case <-time.After(5 * time.Second): + t.Fatal("timeout accepting agent connection") + return nil, nil + } +} + +// peerIssuerCN reports the issuer of the client certificate the agent +// presented — the authoritative answer to "which certificate went on the wire". +func peerIssuerCN(t *testing.T, conn net.Conn) string { + t.Helper() + peer := conn.(*tls.Conn).ConnectionState().PeerCertificates + if len(peer) == 0 { + t.Fatal("server saw no client certificate") + } + return peer[0].Issuer.CommonName +} + +func (f *renewFixture) certOnDisk(t *testing.T) []byte { + t.Helper() + data, err := os.ReadFile(f.certPath) + if err != nil { + t.Fatalf("reading %s: %v", f.certPath, err) + } + return data +} + +// TestRenewRequestSentOnConnect: the request goes out once, immediately after +// the handshake, carrying the implementation and version the backend needs to +// tell Go agents from Ruby ones. +func TestRenewRequestSentOnConnect(t *testing.T) { + f := newRenewFixture(t) + serverConn, errc := f.connect(t, f.opts) + rd := newServerReader(t, serverConn) + + pkt := rd.next() + if pkt.Cmd != protocol.CmdRenewRequest { + t.Fatalf("first packet cmd = %d, want %d (RENEW_REQUEST)", pkt.Cmd, protocol.CmdRenewRequest) + } + if got := protocol.ParseRenewRequest(pkt.Payload); got != "go/"+testVersion { + t.Errorf("identifier = %q, want %q", got, "go/"+testVersion) + } + + stopAgent(t, serverConn, errc) +} + +// TestNoRenewRequestWhenRenewalDisabled: with zero Options — no CA roots, no +// paths — the agent must not ask. An agent that cannot safely install a +// certificate has no business requesting one. +func TestNoRenewRequestWhenRenewalDisabled(t *testing.T) { + f := newRenewFixture(t) + serverConn, errc := f.connect(t, tunnel.Options{}) + rd := newServerReader(t, serverConn) + + echoHost, echoPort := startEchoTarget(t) + if _, err := serverConn.Write(buildCreateRequest(1, echoHost, echoPort)); err != nil { + t.Fatal(err) + } + + // A renewal request would have been queued before anything else, so the + // first non-keepalive packet proves whether one was sent. + pkt := rd.next() + if pkt.Cmd == protocol.CmdRenewRequest { + t.Fatal("agent sent RENEW_REQUEST with renewal disabled") + } + if pkt.Cmd != protocol.CmdCreateResponse { + t.Fatalf("first packet cmd = %d, want %d (CREATE_RESPONSE)", pkt.Cmd, protocol.CmdCreateResponse) + } + + stopAgent(t, serverConn, errc) +} + +// TestRenewedCertificateIsInstalledAndConnectionRecycled is the happy path: +// the certificate on disk is replaced and Run returns the reconnect signal, so +// the next handshake presents the new certificate. +func TestRenewedCertificateIsInstalledAndConnectionRecycled(t *testing.T) { + f := newRenewFixture(t) + serverConn, errc := f.connect(t, f.opts) + + if got := peerIssuerCN(t, serverConn); got != "Deploy Dev CA (old)" { + t.Fatalf("first connection presented issuer %q, want the old CA", got) + } + + if _, err := serverConn.Write(protocol.EncodeRenewResponse(protocol.RenewStatusRenewed, f.renewedPEM)); err != nil { + t.Fatal(err) + } + + select { + case err := <-errc: + if !errors.Is(err, tunnel.ErrCertificateRenewed) { + t.Fatalf("Run returned %v, want ErrCertificateRenewed", err) + } + case <-time.After(5 * time.Second): + t.Fatal("timeout: agent did not recycle the connection after renewal") + } + + if got := f.certOnDisk(t); !bytes.Equal(got, f.renewedPEM) { + t.Fatal("agent.crt does not hold the renewed certificate") + } + + installed, err := certrenew.LoadCurrent(f.certPath) + if err != nil { + t.Fatal(err) + } + if installed.Issuer.CommonName != "Deploy Dev CA (new)" { + t.Errorf("issuer = %q, want the new CA", installed.Issuer.CommonName) + } + if installed.SerialNumber.Int64() != agentSerial || installed.Subject.CommonName != agentCN { + t.Errorf("renewal changed the agent's identity: %q serial %s", + installed.Subject.CommonName, installed.SerialNumber) + } + + // A reconnect is only useful if the renewed certificate is actually + // presented, which is what GetClientCertificate buys — prove it end to end. + serverConn.Close() + next, nextErrc := f.connect(t, f.opts) + if got := peerIssuerCN(t, next); got != "Deploy Dev CA (new)" { + t.Errorf("second connection presented issuer %q, want the new CA", got) + } + stopAgent(t, next, nextErrc) +} + +// TestRenewResponseStatusesThatWriteNothing: CURRENT and ERROR are normal +// answers, not failures. Neither may touch agent.crt and neither may drop the +// tunnel — the agent is mid-deployment as far as it knows. +func TestRenewResponseStatusesThatWriteNothing(t *testing.T) { + tests := []struct { + name string + status byte + body func(f *renewFixture) []byte + }{ + {"current", protocol.RenewStatusCurrent, func(*renewFixture) []byte { return nil }}, + {"error", protocol.RenewStatusError, func(*renewFixture) []byte { return []byte("agent has no certificate on file") }}, + // A server that answered RENEWED with the certificate the agent already + // presents would otherwise spin it through connect-renew-reconnect + // forever, so this must not recycle the connection either. + {"renewed with the certificate already installed", protocol.RenewStatusRenewed, func(f *renewFixture) []byte { return f.originalPEM }}, + {"renewed but untrusted issuer", protocol.RenewStatusRenewed, func(f *renewFixture) []byte { return f.untrustedPEM }}, + {"renewed but malformed", protocol.RenewStatusRenewed, func(*renewFixture) []byte { return []byte("not a certificate") }}, + {"renewed with an empty body", protocol.RenewStatusRenewed, func(*renewFixture) []byte { return nil }}, + {"unknown status", 200, func(*renewFixture) []byte { return nil }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newRenewFixture(t) + serverConn, errc := f.connect(t, f.opts) + rd := newServerReader(t, serverConn) + + if pkt := rd.next(); pkt.Cmd != protocol.CmdRenewRequest { + t.Fatalf("expected RENEW_REQUEST first, got cmd=%d", pkt.Cmd) + } + + if _, err := serverConn.Write(protocol.EncodeRenewResponse(tt.status, tt.body(f))); err != nil { + t.Fatal(err) + } + + // The connection must still be serving traffic afterwards. + echoHost, echoPort := startEchoTarget(t) + if _, err := serverConn.Write(buildCreateRequest(5, echoHost, echoPort)); err != nil { + t.Fatal(err) + } + resp := rd.next() + if resp.Cmd != protocol.CmdCreateResponse { + t.Fatalf("expected CREATE_RESPONSE, got cmd=%d — connection did not survive", resp.Cmd) + } + if _, status, reason, _ := protocol.ParseCreateResponse(resp.Payload); status != 0 { + t.Fatalf("CREATE_RESPONSE status=%d reason=%q", status, reason) + } + + if got := f.certOnDisk(t); !bytes.Equal(got, f.originalPEM) { + t.Error("agent.crt was modified") + } + if _, err := os.Stat(f.certPath + ".tmp"); err == nil { + t.Error("temporary file left behind") + } + + select { + case err := <-errc: + t.Fatalf("agent exited: %v", err) + default: + } + + stopAgent(t, serverConn, errc) + }) + } +} + +// TestMalformedRenewResponseIgnored: a zero-length payload cannot carry a +// status byte. It must be shrugged off, like any other frame the agent cannot +// make sense of. +func TestMalformedRenewResponseIgnored(t *testing.T) { + f := newRenewFixture(t) + serverConn, errc := f.connect(t, f.opts) + rd := newServerReader(t, serverConn) + + if pkt := rd.next(); pkt.Cmd != protocol.CmdRenewRequest { + t.Fatalf("expected RENEW_REQUEST first, got cmd=%d", pkt.Cmd) + } + + if _, err := serverConn.Write(protocol.EncodePacket(protocol.CmdRenewResponse, nil)); err != nil { + t.Fatal(err) + } + + echoHost, echoPort := startEchoTarget(t) + if _, err := serverConn.Write(buildCreateRequest(9, echoHost, echoPort)); err != nil { + t.Fatal(err) + } + if resp := rd.next(); resp.Cmd != protocol.CmdCreateResponse { + t.Fatalf("expected CREATE_RESPONSE, got cmd=%d", resp.Cmd) + } + if got := f.certOnDisk(t); !bytes.Equal(got, f.originalPEM) { + t.Error("agent.crt was modified") + } + + stopAgent(t, serverConn, errc) +} + +// TestRenewResponseIgnoredWhenRenewalDisabled: the server never sends one +// unsolicited, so this can only happen if something has gone wrong. Ignore it +// rather than writing a certificate that could not be verified. +func TestRenewResponseIgnoredWhenRenewalDisabled(t *testing.T) { + f := newRenewFixture(t) + serverConn, errc := f.connect(t, tunnel.Options{}) + rd := newServerReader(t, serverConn) + + if _, err := serverConn.Write(protocol.EncodeRenewResponse(protocol.RenewStatusRenewed, f.renewedPEM)); err != nil { + t.Fatal(err) + } + + echoHost, echoPort := startEchoTarget(t) + if _, err := serverConn.Write(buildCreateRequest(3, echoHost, echoPort)); err != nil { + t.Fatal(err) + } + if resp := rd.next(); resp.Cmd != protocol.CmdCreateResponse { + t.Fatalf("expected CREATE_RESPONSE, got cmd=%d", resp.Cmd) + } + if got := f.certOnDisk(t); !bytes.Equal(got, f.originalPEM) { + t.Error("agent.crt was modified with renewal disabled") + } + + stopAgent(t, serverConn, errc) +} + +// TestUnknownCommandsAreStillTolerated: adding commands 8 and 9 must not have +// narrowed what the agent shrugs off. Deployed v0.2.0 agents ignore unknown +// bytes, and this build has to keep doing the same for whatever comes next. +func TestUnknownCommandsAreStillTolerated(t *testing.T) { + certs := generateTestCerts(t) + srv := newFakeServer(t, certs) + errc := connectAgent(t, srv.addr(), certs, acl.Parse("127.0.0.1")) + + serverConn := srv.accept() + defer serverConn.Close() + rd := newServerReader(t, serverConn) + + for _, cmd := range []byte{10, 99, 200, 255} { + if _, err := serverConn.Write(protocol.EncodePacket(cmd, []byte("payload"))); err != nil { + t.Fatalf("writing cmd %d: %v", cmd, err) + } + } + + select { + case err := <-errc: + t.Fatalf("agent exited after unknown commands: %v", err) + case <-time.After(300 * time.Millisecond): + } + + echoHost, echoPort := startEchoTarget(t) + if _, err := serverConn.Write(buildCreateRequest(77, echoHost, echoPort)); err != nil { + t.Fatal(err) + } + resp := rd.next() + if resp.Cmd != protocol.CmdCreateResponse { + t.Fatalf("expected CREATE_RESPONSE after unknown commands, got cmd=%d", resp.Cmd) + } + + stopAgent(t, serverConn, errc) +} + +// TestRenewalDisabledWithoutRoots pins the individual preconditions: every one +// of them is required, so a half-configured agent never writes a certificate. +func TestRenewalRequiresCompleteOptions(t *testing.T) { + full := tunnel.Options{ + Version: testVersion, + Paths: config.Paths{Certificate: "/tmp/agent.crt", Key: "/tmp/agent.key"}, + CARoots: x509.NewCertPool(), + } + + tests := []struct { + name string + mutate func(o *tunnel.Options) + }{ + {"no CA roots", func(o *tunnel.Options) { o.CARoots = nil }}, + {"no certificate path", func(o *tunnel.Options) { o.Paths.Certificate = "" }}, + {"no key path", func(o *tunnel.Options) { o.Paths.Key = "" }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newRenewFixture(t) + opts := full + tt.mutate(&opts) + + serverConn, errc := f.connect(t, opts) + rd := newServerReader(t, serverConn) + + echoHost, echoPort := startEchoTarget(t) + if _, err := serverConn.Write(buildCreateRequest(1, echoHost, echoPort)); err != nil { + t.Fatal(err) + } + if pkt := rd.next(); pkt.Cmd == protocol.CmdRenewRequest { + t.Error("agent asked for renewal despite incomplete options") + } + + stopAgent(t, serverConn, errc) + }) + } +} diff --git a/internal/tunnel/server_conn.go b/internal/tunnel/server_conn.go index f71414b..c8e5169 100644 --- a/internal/tunnel/server_conn.go +++ b/internal/tunnel/server_conn.go @@ -26,6 +26,7 @@ type ServerConn struct { done chan struct{} // closed when Run() begins shutdown access *acl.AccessList + opts Options mu sync.Mutex dests map[uint16]*destConn @@ -34,7 +35,7 @@ type ServerConn struct { } // Connect dials the server and performs the mTLS handshake. -func Connect(tlsCfg *tls.Config, serverAddr string, access *acl.AccessList, log *slog.Logger) (*ServerConn, error) { +func Connect(tlsCfg *tls.Config, serverAddr string, access *acl.AccessList, opts Options, log *slog.Logger) (*ServerConn, error) { conn, err := tls.DialWithDialer( &net.Dialer{Timeout: 30 * time.Second}, "tcp", @@ -51,6 +52,7 @@ func Connect(tlsCfg *tls.Config, serverAddr string, access *acl.AccessList, log serverSend: make(chan []byte, 256), done: make(chan struct{}), access: access, + opts: opts, dests: make(map[uint16]*destConn), log: log, }, nil @@ -60,6 +62,7 @@ func Connect(tlsCfg *tls.Config, serverAddr string, access *acl.AccessList, log // It blocks until the connection is closed and returns ErrRejected on REJECT. func (sc *ServerConn) Run() error { go sc.writeLoop() + sc.sendRenewRequest() go sc.keepaliveLoop() err := sc.readLoop() @@ -176,6 +179,9 @@ func (sc *ServerConn) handlePacket(pkt protocol.Packet) error { case protocol.CmdKeepalive: // no-op + case protocol.CmdRenewResponse: + return sc.handleRenewResponse(pkt.Payload) + default: sc.log.Warn("unknown command", "cmd", pkt.Cmd) }