diff --git a/README.md b/README.md index cdc8d7ea..28ad3b3e 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ the exact plan you reviewed. | Change review | A digest-bound plan shows the exact config, images, host state, rendered Compose, payloads, and operation graph before apply. | | Deployment | Health-gated rolling replacement drains traffic first and stops on failed readiness. | | Recovery | Every release records its predecessor; interrupted work can be resumed or aborted, and a failed deploy rolls back by default. | -| Host access | Agentless SSH, key authentication, and mandatory `known_hosts` verification. | +| Host access | Agentless SSH, key authentication, and mandatory `known_hosts` verification. An optional one-hop jump host reaches private targets, with both hops verified and the agent never forwarded. | | Runtime ownership | Generated Compose stays inspectable with `ob preview` and can be taken over permanently with `ob eject`. | | Automation | Human output, JSON envelopes, and NDJSON event streams come from the same lifecycle service. | diff --git a/cmd/ob/commands.go b/cmd/ob/commands.go index ad0ebd7d..79337d98 100644 --- a/cmd/ob/commands.go +++ b/cmd/ob/commands.go @@ -517,11 +517,11 @@ func newUI(cmd *cobra.Command, g *globalFlags) *ui.UI { // cliConnector is replaceable by in-package tests and honors OB_LOCAL for the // existing local-docker workflow. Production uses cancellable SSH dialing. -var cliConnector onebox.Connector = func(ctx context.Context, target string) (transport.Transport, error) { +var cliConnector onebox.Connector = func(ctx context.Context, route transport.Route) (transport.Transport, error) { if value := strings.TrimSpace(strings.ToLower(os.Getenv("OB_LOCAL"))); value == "1" || value == "true" { return transport.NewLocal(), nil } - return transport.NewSSHContext(ctx, target) + return transport.NewSSHRoute(ctx, route) } func attachTransportLogger(t transport.Transport, logger func(string, string)) { @@ -538,8 +538,8 @@ func operationsService(cmd *cobra.Command, g *globalFlags) *onebox.Service { } func operationsServiceWithUI(cmd *cobra.Command, g *globalFlags, u *ui.UI) *onebox.Service { - connector := func(ctx context.Context, target string) (transport.Transport, error) { - t, err := cliConnector(ctx, target) + connector := func(ctx context.Context, route transport.Route) (transport.Transport, error) { + t, err := cliConnector(ctx, route) if err == nil { attachTransportLogger(t, u.Cmd) } @@ -657,7 +657,7 @@ func connect(cmd *cobra.Command, g *globalFlags, cfg *app.Resolved, p *ctypes.Pr if err != nil { return nil, nil, err } - t, err := cliConnector(cmd.Context(), env.Destination()) + t, err := cliConnector(cmd.Context(), env.Route()) if err != nil { return nil, nil, err } diff --git a/cmd/ob/ops_contract_test.go b/cmd/ob/ops_contract_test.go index e9e43b95..105019ab 100644 --- a/cmd/ob/ops_contract_test.go +++ b/cmd/ob/ops_contract_test.go @@ -112,7 +112,7 @@ func TestSecretsEditRequiresIDWhenSeveralEntriesExist(t *testing.T) { func TestExecMissingReasonFailsBeforeTargetContact(t *testing.T) { previousConnector := cliConnector contacts := 0 - cliConnector = func(context.Context, string) (transport.Transport, error) { + cliConnector = func(context.Context, transport.Route) (transport.Transport, error) { contacts++ return nil, errors.New("target must not be contacted") } @@ -134,7 +134,7 @@ func TestDestroyConfirmationMismatchIsCancelledBeforeTargetContact(t *testing.T) config := writeOpsContractProject(t, dir, false) previousConnector := cliConnector contacts := 0 - cliConnector = func(context.Context, string) (transport.Transport, error) { + cliConnector = func(context.Context, transport.Route) (transport.Transport, error) { contacts++ return nil, errors.New("target must not be contacted") } @@ -195,7 +195,7 @@ services: {postgres: 17} } }} previousConnector := cliConnector - cliConnector = func(context.Context, string) (transport.Transport, error) { return fake, nil } + cliConnector = func(context.Context, transport.Route) (transport.Transport, error) { return fake, nil } t.Cleanup(func() { cliConnector = previousConnector }) for name, args := range map[string][]string{ diff --git a/cmd/ob/preflight.go b/cmd/ob/preflight.go index d061b561..2008a835 100644 --- a/cmd/ob/preflight.go +++ b/cmd/ob/preflight.go @@ -40,9 +40,9 @@ func addPreflightCommand(root *cobra.Command, g *globalFlags) { // User dropped a declared port and connected to 22 instead — a // silent success against the wrong server, which is worse than // any failure this command reports. - addr := env.Destination() + addr := env.Route().String() - t, err := transport.NewSSHContext(cmd.Context(), addr) + t, err := transport.NewSSHRoute(cmd.Context(), env.Route()) if err != nil { return writeStructuredReadFailure(cmd, g, fmt.Errorf("cannot reach %s: %w", addr, err)) } diff --git a/docs/onebox.run-v1.schema.json b/docs/onebox.run-v1.schema.json index 098148bf..3c4c8da5 100644 --- a/docs/onebox.run-v1.schema.json +++ b/docs/onebox.run-v1.schema.json @@ -641,6 +641,48 @@ }, "type": "array" }, + "jump": { + "anyOf": [ + { + "type": "string" + }, + { + "additionalProperties": false, + "description": "Optional SSH jump host tunnelling the connection to this server, written as user@host or as an object with host, user, and port. Onebox verifies and authenticates both hops and never forwards the SSH agent.", + "examples": [ + "deploy@bastion.example.com" + ], + "patternProperties": { + "^x-": {} + }, + "properties": { + "host": { + "description": "Jump host name or IP address.", + "examples": [ + "bastion.example.com" + ], + "type": "string" + }, + "port": { + "description": "SSH port on the jump host. The SSH default is used when omitted.", + "examples": [ + 2222 + ], + "type": "integer" + }, + "user": { + "description": "SSH user on the jump host. $USER is used when omitted; ob does not read ~/.ssh/config.", + "examples": [ + "deploy" + ], + "type": "string" + } + }, + "type": "object" + } + ], + "description": "Optional SSH jump host tunnelling the connection to this server, written as user@host or as an object with host, user, and port. Onebox verifies and authenticates both hops and never forwards the SSH agent. Also accepts user@host or user@host:port." + }, "overrides": { "additionalProperties": false, "description": "Environment-specific operational tuning. Overrides cannot change workload identity or data semantics.", @@ -772,7 +814,7 @@ "type": "integer" }, "user": { - "description": "SSH user. The local SSH configuration supplies it when omitted.", + "description": "SSH user. $USER is used when omitted; ob does not read ~/.ssh/config.", "examples": [ "root" ], diff --git a/internal/app/jsonschema.go b/internal/app/jsonschema.go index 56b46327..032963cf 100644 --- a/internal/app/jsonschema.go +++ b/internal/app/jsonschema.go @@ -185,6 +185,7 @@ var authoredForms = []struct { {[]string{"workloads", "*", "entrypoint"}, commandForms(), "an entrypoint or argument list"}, {[]string{"workloads", "*", "needs", "items"}, stringForm(), "the name of a prerequisite"}, {[]string{"environments", "*", "server"}, stringForm(), "user@host"}, + {[]string{"environments", "*", "jump"}, stringForm(), "user@host or user@host:port"}, {[]string{"runtime", "env_files", "items"}, stringForm(), "a path to an environment file"}, {[]string{"environments", "*", "env_files", "items"}, stringForm(), "a path to an environment file"}, {[]string{"workloads", "*", "env_files", "items"}, stringForm(), "a path to an environment file"}, diff --git a/internal/app/jump_config_test.go b/internal/app/jump_config_test.go new file mode 100644 index 00000000..04a29818 --- /dev/null +++ b/internal/app/jump_config_test.go @@ -0,0 +1,131 @@ +package app + +import ( + "strings" + "testing" +) + +func projectWithJump(jump string) string { + return "api_version: onebox.run/v1\napp: ledger\n" + + "environments: {production: {server: root@10.20.0.10, jump: " + jump + "}}\n" + + "image: nginx\ndomain: ledger.example.com\nport: 8080\n" +} + +func TestScalarJumpExpandsToUserHostAndPort(t *testing.T) { + resolved, err := LoadBytes([]byte(projectWithJump("deploy@bastion.example.com:2222")), "ob.yml") + if err != nil { + t.Fatal(err) + } + jump := resolved.Environments["production"].Jump + if jump == nil { + t.Fatal("jump = nil, want the declared bastion") + } + if jump.User != "deploy" || jump.Host != "bastion.example.com" || jump.Port != 2222 { + t.Fatalf("jump = %#v", jump) + } +} + +func TestScalarJumpWithoutUserOrPortKeepsThoseImplicit(t *testing.T) { + resolved, err := LoadBytes([]byte(projectWithJump("bastion.example.com")), "ob.yml") + if err != nil { + t.Fatal(err) + } + jump := resolved.Environments["production"].Jump + if jump == nil || jump.User != "" || jump.Host != "bastion.example.com" || jump.Port != 0 { + t.Fatalf("jump = %#v", jump) + } +} + +func TestObjectJumpDecodes(t *testing.T) { + resolved, err := LoadBytes([]byte(projectWithJump("{host: bastion.example.com, user: deploy, port: 2222}")), "ob.yml") + if err != nil { + t.Fatal(err) + } + jump := resolved.Environments["production"].Jump + if jump == nil || jump.User != "deploy" || jump.Host != "bastion.example.com" || jump.Port != 2222 { + t.Fatalf("jump = %#v", jump) + } +} + +func TestAbsentJumpLeavesTheEnvironmentDirect(t *testing.T) { + resolved, err := LoadBytes([]byte(min), "ob.yml") + if err != nil { + t.Fatal(err) + } + if jump := resolved.Environments["production"].Jump; jump != nil { + t.Fatalf("jump = %#v, want nil", jump) + } +} + +// A jump that only fails at dial time is a jump that fails after the operator +// has already been told the plan is sound, so every malformed form is rejected +// while the project is still being read. +func TestInvalidJumpIsRejectedAtLoad(t *testing.T) { + invalid := map[string]string{ + "port out of range": "deploy@bastion.example.com:99999", + "port not numeric": "deploy@bastion.example.com:ssh", + "object port high": "{host: bastion.example.com, port: 70000}", + "missing host": "{user: deploy}", + "empty scalar": "\"\"", + "unbracketed ipv6": "deploy@2001:db8::1", + "two at signs": "deploy@bastion@example.com", + "bad user character": "\"bad/user@bastion.example.com\"", + "port inside host": "{host: \"bastion.example.com:2222\"}", + "user inside host": "{host: \"deploy@bastion.example.com\"}", + "bad user object": "{host: bastion.example.com, user: \"bad/user\"}", + } + for name, jump := range invalid { + t.Run(name, func(t *testing.T) { + _, err := LoadBytes([]byte(projectWithJump(jump)), "ob.yml") + if err == nil { + t.Fatalf("jump %q was accepted", jump) + } + if !strings.Contains(err.Error(), "jump") { + t.Fatalf("error does not name the jump field: %v", err) + } + }) + } +} + +// An IPv6 bastion must be expressible: bracketed in the scalar form, where the +// grammar needs the brackets to find the port, and bare in the object form, +// where each part is already its own field. +func TestIPv6JumpIsAcceptedInBothForms(t *testing.T) { + forms := map[string]string{ + "scalar bracketed": `"deploy@[2001:db8::1]"`, + "scalar bracketed port": `"deploy@[2001:db8::1]:2222"`, + "object bare": `{host: "2001:db8::1", user: deploy}`, + // Brackets carried over from the scalar spelling are stripped rather + // than refused: the author meant the address, not a hostname with + // punctuation in it. + "object bracketed": `{host: "[2001:db8::1]", user: deploy}`, + } + for name, form := range forms { + t.Run(name, func(t *testing.T) { + resolved, err := LoadBytes([]byte(projectWithJump(form)), "ob.yml") + if err != nil { + t.Fatal(err) + } + if host := resolved.Environments["production"].Jump.Host; host != "2001:db8::1" { + t.Fatalf("jump host = %q, want the bare literal", host) + } + }) + } +} + +func TestIPv6JumpRoutesWithBracketsOnlyWhenAPortIsWritten(t *testing.T) { + route := func(form string) string { + t.Helper() + resolved, err := LoadBytes([]byte(projectWithJump(form)), "ob.yml") + if err != nil { + t.Fatal(err) + } + return resolved.Environments["production"].Route().String() + } + if got, want := route(`"deploy@[2001:db8::1]"`), "root@10.20.0.10 via deploy@2001:db8::1"; got != want { + t.Fatalf("route = %q, want %q", got, want) + } + if got, want := route(`"deploy@[2001:db8::1]:2222"`), "root@10.20.0.10 via deploy@[2001:db8::1]:2222"; got != want { + t.Fatalf("route = %q, want %q", got, want) + } +} diff --git a/internal/app/load.go b/internal/app/load.go index b7ef3058..c083cfa3 100644 --- a/internal/app/load.go +++ b/internal/app/load.go @@ -7,9 +7,12 @@ import ( "os" "path/filepath" "sort" + "strconv" "strings" "gopkg.in/yaml.v3" + + obtarget "github.com/labstack/onebox/internal/target" ) // APIVersion is the only authoring contract this package accepts. @@ -272,15 +275,17 @@ func expandTopLevelUnions(raw map[string]any) { } } if s, ok := em["server"].(string); ok { - host, user := s, "" - if at := strings.Index(s, "@"); at >= 0 { - user, host = s[:at], s[at+1:] - } - em["server"] = map[string]any{"host": host} - if user != "" { - em["server"].(map[string]any)["user"] = user - } + em["server"] = expandAddress(s) } + if s, ok := em["jump"].(string); ok { + em["jump"] = expandAddress(s) + } + // The object form is normalised too. Brackets belong to the + // scalar grammar, where they mark off an IPv6 address from its + // port; a host field has no port to mark off, and a bracketed + // value there reaches the dialler as part of the hostname. + unbracketHost(em["server"]) + unbracketHost(em["jump"]) } } if secrets, ok := raw["secrets"].(map[string]any); ok { @@ -735,3 +740,45 @@ func (p *Spec) checkDeclaredFilesExist() error { } return nil } + +// expandAddress turns a scalar `[user@]host[:port]` into the object form. +// +// It goes through the shared address grammar rather than splitting on "@" by +// hand, because a hand-rolled split leaves a written port inside the hostname: +// the value still round-trips through Destination(), so it looks correct +// everywhere except at the moment something dials it. A form the grammar +// rejects is left whole as the host, so validation reports it against the +// field the author wrote rather than as a decode error naming a Go type. +func expandAddress(scalar string) map[string]any { + expanded := map[string]any{"host": scalar} + parsed, err := obtarget.Parse(scalar) + if err != nil { + return expanded + } + expanded["host"] = parsed.Host + if parsed.User != "" { + expanded["user"] = parsed.User + } + if parsed.ExplicitPort { + port, _ := strconv.Atoi(parsed.Port) + expanded["port"] = port + } + return expanded +} + +// unbracketHost strips the brackets an author may have carried over from the +// scalar spelling of an IPv6 address. +func unbracketHost(value any) { + object, ok := value.(map[string]any) + if !ok { + return + } + host, ok := object["host"].(string) + if !ok || !strings.HasPrefix(host, "[") || !strings.HasSuffix(host, "]") { + return + } + inner := host[1 : len(host)-1] + if strings.Contains(inner, ":") && obtarget.ValidHost(inner) { + object["host"] = inner + } +} diff --git a/internal/app/route.go b/internal/app/route.go new file mode 100644 index 00000000..8ac60279 --- /dev/null +++ b/internal/app/route.go @@ -0,0 +1,31 @@ +package app + +import ( + "strconv" + + obtarget "github.com/labstack/onebox/internal/target" +) + +// Route is the whole connection this environment needs: the server, and the +// jump host it is reached through when one is declared. It is built from the +// declared fields rather than by reparsing Destination(), so a route without a +// jump renders byte-for-byte what Destination() has always rendered. +func (e Environment) Route() obtarget.Route { + route := obtarget.Route{Target: address(e.Server.User, e.Server.Host, e.Server.Port)} + if e.Jump != nil { + jump := address(e.Jump.User, e.Jump.Host, e.Jump.Port) + route.Jump = &jump + } + return route +} + +// address carries the authored port through as explicit and otherwise leaves +// the SSH default implicit, which is the distinction Destination() draws and +// every sealed plan already spells. +func address(user, host string, port int) obtarget.Address { + a := obtarget.Address{User: user, Host: host, Port: "22"} + if port != 0 { + a.Port, a.ExplicitPort = strconv.Itoa(port), true + } + return a +} diff --git a/internal/app/route_test.go b/internal/app/route_test.go new file mode 100644 index 00000000..93a63300 --- /dev/null +++ b/internal/app/route_test.go @@ -0,0 +1,160 @@ +package app + +import ( + "strings" + "testing" +) + +// A route without a jump must render exactly what Destination() renders: +// plans and approvals are sealed against that spelling, so any drift would +// invalidate every artifact created before jump hosts existed. +func TestRouteWithoutAJumpRendersTheDestinationVerbatim(t *testing.T) { + servers := map[string]Server{ + "user and port": {User: "deploy", Host: "example.com", Port: 2222}, + "port only": {Host: "example.com", Port: 2222}, + "no port": {User: "root", Host: "example.com"}, + "ipv6 with port": {User: "root", Host: "2a01:4ff::1", Port: 2222}, + "ipv6 no port": {User: "root", Host: "2a01:4ff::1"}, + } + for name, server := range servers { + t.Run(name, func(t *testing.T) { + e := Environment{Server: server} + if got, want := e.Route().String(), e.Destination(); got != want { + t.Fatalf("route = %q, destination = %q", got, want) + } + }) + } +} + +func TestRouteWithoutAJumpCarriesNoJumpAddress(t *testing.T) { + route := Environment{Server: Server{Host: "example.com"}}.Route() + if route.Jump != nil { + t.Fatalf("route.Jump = %#v, want nil", route.Jump) + } + if route.Target.Port != "22" || route.Target.ExplicitPort { + t.Fatalf("target = %#v, want the implicit default port", route.Target) + } +} + +func TestRouteNamesTheDeclaredJump(t *testing.T) { + e := Environment{ + Server: Server{User: "root", Host: "10.20.0.10"}, + Jump: &Jump{User: "deploy", Host: "bastion.example.com", Port: 2222}, + } + if route := e.Route(); route.Jump == nil { + t.Fatal("route.Jump = nil, want the declared bastion") + } + want := "root@10.20.0.10 via deploy@bastion.example.com:2222" + if got := e.Route().String(); got != want { + t.Fatalf("route = %q, want %q", got, want) + } +} + +func TestRouteJumpDefaultsToPort22(t *testing.T) { + e := Environment{ + Server: Server{User: "root", Host: "10.20.0.10"}, + Jump: &Jump{Host: "bastion.example.com"}, + } + route := e.Route() + if route.Jump.Port != "22" || route.Jump.ExplicitPort { + t.Fatalf("jump = %#v, want the implicit default port", route.Jump) + } + if got, want := route.String(), "root@10.20.0.10 via bastion.example.com"; got != want { + t.Fatalf("route = %q, want %q", got, want) + } +} + +// `server: root@host:2222` is a scalar the loader has always accepted. The +// port has to survive into the route, or the connection is attempted against +// a hostname with a colon in it. +func TestScalarServerPortReachesTheRoute(t *testing.T) { + resolved, err := LoadBytes([]byte("api_version: onebox.run/v1\napp: ledger\n"+ + "environments: {production: {server: root@10.20.0.10:2222}}\n"+ + "image: nginx\ndomain: d.example.com\nport: 8080\n"), "ob.yml") + if err != nil { + t.Fatal(err) + } + environment := resolved.Environments["production"] + route := environment.Route() + if route.Target.Host != "10.20.0.10" || route.Target.Port != "2222" { + t.Fatalf("target = %#v, want host 10.20.0.10 port 2222", route.Target) + } + if got, want := route.String(), environment.Destination(); got != want { + t.Fatalf("route = %q, destination = %q", got, want) + } + if want := "root@10.20.0.10:2222"; route.String() != want { + t.Fatalf("route = %q, want %q", route.String(), want) + } +} + +// A bracketed IPv6 scalar now normalises to the same address the object form +// produces: brackets belong to the written grammar, not to the hostname. The +// rendered destination therefore drops them when no port is written, exactly +// as `{host: "2001:db8::1"}` always has. +func TestBracketedIPv6ScalarNormalisesLikeTheObjectForm(t *testing.T) { + load := func(server string) Environment { + t.Helper() + resolved, err := LoadBytes([]byte("api_version: onebox.run/v1\napp: ledger\n"+ + "environments: {production: {server: "+server+"}}\n"+ + "image: nginx\ndomain: d.example.com\nport: 8080\n"), "ob.yml") + if err != nil { + t.Fatal(err) + } + return resolved.Environments["production"] + } + scalar := load(`"root@[2001:db8::1]"`) + object := load(`{host: "2001:db8::1", user: root}`) + if scalar.Server != object.Server { + t.Fatalf("scalar = %#v, object = %#v", scalar.Server, object.Server) + } + if got := scalar.Route().String(); got != "root@2001:db8::1" { + t.Fatalf("route = %q, want %q", got, "root@2001:db8::1") + } + withPort := load(`"root@[2001:db8::1]:2222"`) + if got := withPort.Route().String(); got != "root@[2001:db8::1]:2222" { + t.Fatalf("route = %q, want the bracketed form when a port is written", got) + } +} + +// A bracketed IPv6 `server.host` was dialable before routes existed, because +// every connection re-parsed the rendered destination. Nothing re-parses it +// now, so the brackets have to come off while the project is read or +// JoinHostPort builds [[2001:db8::1]]:22. +func TestBracketedIPv6ServerHostNormalises(t *testing.T) { + resolved, err := LoadBytes([]byte("api_version: onebox.run/v1\napp: ledger\n"+ + "environments: {production: {server: {host: \"[2001:db8::1]\", user: root}}}\n"+ + "image: nginx\ndomain: d.example.com\nport: 8080\n"), "ob.yml") + if err != nil { + t.Fatal(err) + } + environment := resolved.Environments["production"] + if environment.Server.Host != "2001:db8::1" { + t.Fatalf("server host = %q, want the bare literal", environment.Server.Host) + } + if got := environment.Route().Target.Host; got != "2001:db8::1" { + t.Fatalf("route host = %q, want the bare literal", got) + } +} + +// A server address the transport could never dial should be reported against +// the field the author wrote, not as a DNS lookup failure at connect time. +func TestInvalidServerAddressIsRejectedAtLoad(t *testing.T) { + invalid := map[string]string{ + "host with a port": `{host: "example.com:2222"}`, + "host with a user": `{host: "root@example.com"}`, + "bad user": `{host: example.com, user: "bad/user"}`, + } + for name, server := range invalid { + t.Run(name, func(t *testing.T) { + _, err := LoadBytes([]byte("api_version: onebox.run/v1\napp: ledger\n"+ + "environments: {production: {server: "+server+"}}\n"+ + "image: nginx\ndomain: d.example.com\nport: 8080\n"), "ob.yml") + if err == nil { + t.Fatalf("server %q was accepted", server) + } + if !strings.Contains(err.Error(), "server") { + t.Fatalf("error does not name the server field: %v", err) + } + }) + } +} diff --git a/internal/app/types.go b/internal/app/types.go index 58d6ce5c..f3026bea 100644 --- a/internal/app/types.go +++ b/internal/app/types.go @@ -66,7 +66,12 @@ type Spec struct { } type Environment struct { - Server Server `json:"server" description:"SSH server, written as user@host or as an object with host, user, and port." example:"root@203.0.113.10"` + Server Server `json:"server" description:"SSH server, written as user@host or as an object with host, user, and port." example:"root@203.0.113.10"` + // Jump sits beside Server rather than inside it so the one-line + // `server: root@host` form survives adding a bastion. It is a distinct + // type, not another Server: a Jump has no jump of its own, which is how + // "exactly one hop" is enforced by the model instead of by validation. + Jump *Jump `json:"jump,omitempty" description:"Optional SSH jump host tunnelling the connection to this server, written as user@host or as an object with host, user, and port. Onebox verifies and authenticates both hops and never forwards the SSH agent." example:"deploy@bastion.example.com"` BasePath string `json:"base_path,omitempty" description:"Environment-specific replacement for the project base_path." example:"/srv/ob"` // EnvFiles is this environment's default list. It sits on the environment // rather than in an environment-scoped `runtime` block for the same reason @@ -81,10 +86,19 @@ type Environment struct { // Server is a scalar `user@host` or an object. Both decode here. type Server struct { Host string `json:"host" description:"SSH hostname or IP address." example:"203.0.113.10"` - User string `json:"user,omitempty" description:"SSH user. The local SSH configuration supplies it when omitted." example:"root"` + User string `json:"user,omitempty" description:"SSH user. $USER is used when omitted; ob does not read ~/.ssh/config." example:"root"` Port int `json:"port,omitempty" description:"SSH port. The SSH default is used when omitted." example:"2222"` } +// Jump is a scalar `user@host` or an object, decoding exactly as Server does. +// Nothing is ever deployed to a jump host: it forwards one TCP channel to the +// server and runs no commands. +type Jump struct { + Host string `json:"host" description:"Jump host name or IP address." example:"bastion.example.com"` + User string `json:"user,omitempty" description:"SSH user on the jump host. $USER is used when omitted; ob does not read ~/.ssh/config." example:"deploy"` + Port int `json:"port,omitempty" description:"SSH port on the jump host. The SSH default is used when omitted." example:"2222"` +} + type Policy struct { RequireApproval bool `json:"require_approval" description:"Require a plan-bound local confirmation before mutating this environment." default:"true"` AllowAgentProposals bool `json:"allow_agent_proposals" description:"Declared permission for agent-authored proposals. The current CLI does not distinguish agent identity; execution remains approval-gated." default:"true"` diff --git a/internal/app/validate.go b/internal/app/validate.go index 34ae86ec..f2299ef9 100644 --- a/internal/app/validate.go +++ b/internal/app/validate.go @@ -1,6 +1,10 @@ package app -import "strings" +import ( + "strings" + + obtarget "github.com/labstack/onebox/internal/target" +) // Applying the constraints is explicit and typed, so the compiler knows when a // field moves and a reader can see exactly which rule a field is held to. The @@ -178,10 +182,11 @@ func validateEnvironment(e Environment, path string) error { if e.Server.Host == "" { return errf("project_invalid", path+".server", "", "an environment must name a server") } - if e.Server.Port != 0 { - if err := checkPort(path+".server.port", e.Server.Port); err != nil { - return err - } + if err := validateAddress("server", path+".server", e.Server.Host, e.Server.User, e.Server.Port); err != nil { + return err + } + if err := validateJump(e.Jump, path+".jump"); err != nil { + return err } if err := gAbsPath.checkOptional(path+".base_path", e.BasePath); err != nil { return err @@ -592,3 +597,41 @@ func validateChecks(c Checks) error { } return nil } + +// validateJump holds the jump to the same address grammar the transport dials +// with, so a bastion that cannot be reached is reported while reading the +// project rather than after the operator has approved a plan built around it. +func validateJump(jump *Jump, path string) error { + if jump == nil { + return nil + } + if jump.Host == "" { + return errf("project_invalid", path, "", "a jump must name a host") + } + return validateAddress("jump", path, jump.Host, jump.User, jump.Port) +} + +// validateAddress holds an authored SSH endpoint to the grammar the transport +// dials with. Each part is checked as it was written: recomposing them into one +// string and parsing that cannot tell a bad host from a bad user, and lets a +// host smuggle in a port or a user that only fails once something dials it. +// +// Nothing re-parses these fields on the way to the dialler, so an address that +// is only checked at connect time is an address checked after the operator has +// already approved a plan built around it. +func validateAddress(kind, path, host, user string, port int) error { + if port != 0 { + if err := checkPort(path+".port", port); err != nil { + return err + } + } + if !obtarget.ValidHost(host) { + return errf("project_invalid", path+".host", "", + "%s host %q must be a DNS name, an IPv4 address, or an unbracketed IPv6 address; write the port as `port` and the user as `user`", kind, host) + } + if user != "" && !obtarget.ValidUser(user) { + return errf("project_invalid", path+".user", "", + "%s user %q must start with a letter, digit, or underscore and contain only letters, digits, dot, underscore, or hyphen", kind, user) + } + return nil +} diff --git a/internal/engine/recreate.go b/internal/engine/recreate.go index c746efc6..81a3ad5f 100644 --- a/internal/engine/recreate.go +++ b/internal/engine/recreate.go @@ -139,6 +139,7 @@ func (e *Engine) runLocalHook(ctx context.Context, name, run, remoteReleaseDir s "OB_SERVER="+e.T.Destination(), // OpenSSH user@host (IPv6 unbracketed) "OB_SSH_USER="+e.T.SSHUser(), "OB_SSH_PORT="+e.T.SSHPort(), + "OB_SSH_JUMP="+e.T.SSHJump(), // empty when the target is reached directly "OB_RELEASE_DIR="+remoteReleaseDir, "OB_RELEASE_ID="+filepath.Base(remoteReleaseDir), ) diff --git a/internal/engine/recreate_test.go b/internal/engine/recreate_test.go index bd9ede08..77b28411 100644 --- a/internal/engine/recreate_test.go +++ b/internal/engine/recreate_test.go @@ -133,3 +133,35 @@ func TestRunHookNoopWhenAbsent(t *testing.T) { t.Fatalf("absent hook must run nothing: %v", f.Commands) } } + +// A local hook runs on the operator's machine, which has no tunnel of its own, +// so a hook that reaches the host itself needs the bastion named. Empty on a +// direct connection, so `ssh ${OB_SSH_JUMP:+-J $OB_SSH_JUMP}` works either way. +func TestLocalHookGetsTheJumpHostInEnv(t *testing.T) { + f := &transport.Fake{ + HostName: "10.20.0.10", TargetName: "root@10.20.0.10", + SSHUserName: "root", SSHPortName: "22", SSHJumpName: "deploy@bastion.example.com:2222", + } + cfg := testConfig() + cfg.Hooks["pre_release"] = app.Command{ + Run: `test "$OB_SSH_JUMP" = "deploy@bastion.example.com:2222" || { echo "got jump=[$OB_SSH_JUMP]" >&2; exit 1; }`, + Local: true, + } + e := New(cfg, testProject(t), f, Options{Out: &bytes.Buffer{}, Sleep: noSleep, LocalDir: t.TempDir()}) + if err := e.RunHook(context.Background(), "pre_release", "/r", "/r/compose.yaml"); err != nil { + t.Fatalf("hook must receive the jump host: %v", err) + } +} + +func TestLocalHookGetsAnEmptyJumpOnADirectConnection(t *testing.T) { + f := &transport.Fake{HostName: "10.20.0.10", TargetName: "root@10.20.0.10", SSHUserName: "root", SSHPortName: "22"} + cfg := testConfig() + cfg.Hooks["pre_release"] = app.Command{ + Run: `test -z "$OB_SSH_JUMP" || { echo "got jump=[$OB_SSH_JUMP]" >&2; exit 1; }`, + Local: true, + } + e := New(cfg, testProject(t), f, Options{Out: &bytes.Buffer{}, Sleep: noSleep, LocalDir: t.TempDir()}) + if err := e.RunHook(context.Background(), "pre_release", "/r", "/r/compose.yaml"); err != nil { + t.Fatalf("a direct connection must leave OB_SSH_JUMP empty: %v", err) + } +} diff --git a/internal/onebox/backup_evidence_test.go b/internal/onebox/backup_evidence_test.go index cd636844..95a22069 100644 --- a/internal/onebox/backup_evidence_test.go +++ b/internal/onebox/backup_evidence_test.go @@ -377,7 +377,7 @@ func TestPlanDerivesMigrationBackupRequirementAndExecuteRejectsMissingReportBefo service := New(Options{ ConfigPath: configPath, Now: func() time.Time { return now }, - Connect: func(context.Context, string) (transport.Transport, error) { + Connect: func(context.Context, transport.Route) (transport.Transport, error) { connects++ return fake, nil }, diff --git a/internal/onebox/binding.go b/internal/onebox/binding.go index 8e34dc86..66a82765 100644 --- a/internal/onebox/binding.go +++ b/internal/onebox/binding.go @@ -51,7 +51,7 @@ func (s *Service) executionBinding(lp *loadedProject) (ExecutionBinding, error) return ExecutionBinding{}, err } return ExecutionBinding{ - Application: lp.resolved.Name, Environment: s.environment, Server: environment.Destination(), + Application: lp.resolved.Name, Environment: s.environment, Server: environment.Route().String(), ConfigDigest: engine.HashBytes(lp.configBytes), ComposeDigest: engine.HashBytes(lp.composeBytes), }, nil } diff --git a/internal/onebox/bootstrap_test.go b/internal/onebox/bootstrap_test.go index c1cdd586..355cdf66 100644 --- a/internal/onebox/bootstrap_test.go +++ b/internal/onebox/bootstrap_test.go @@ -56,7 +56,7 @@ func TestBootstrapAcceptsBuildSourceWithoutStagingApplicationPayload(t *testing. service := New(Options{ ConfigPath: writeBootstrapBuildProject(t), Environment: "production", - Connect: func(context.Context, string) (transport.Transport, error) { + Connect: func(context.Context, transport.Route) (transport.Transport, error) { return fake, nil }, }) diff --git a/internal/onebox/exec_test.go b/internal/onebox/exec_test.go index 389abd59..1fecad41 100644 --- a/internal/onebox/exec_test.go +++ b/internal/onebox/exec_test.go @@ -45,7 +45,7 @@ func execService(t *testing.T, connect Connector) *Service { func TestExecRefusesInvalidReasonBeforeConnecting(t *testing.T) { connected := false - service := execService(t, func(context.Context, string) (transport.Transport, error) { + service := execService(t, func(context.Context, transport.Route) (transport.Transport, error) { connected = true return nil, errors.New("must not connect") }) @@ -64,7 +64,7 @@ func TestExecRefusesIncompleteRequestBeforeConnecting(t *testing.T) { {Target: "api", Reason: "inspect a stuck request"}, } { connected := false - service := execService(t, func(context.Context, string) (transport.Transport, error) { + service := execService(t, func(context.Context, transport.Route) (transport.Transport, error) { connected = true return nil, errors.New("must not connect") }) @@ -89,7 +89,7 @@ func TestExecEnforcesEnvironmentAndRunnerPolicyBeforeConnecting(t *testing.T) { } { t.Run(test.name, func(t *testing.T) { connected := false - service := execService(t, func(context.Context, string) (transport.Transport, error) { + service := execService(t, func(context.Context, transport.Route) (transport.Transport, error) { connected = true return nil, errors.New("must not connect") }) @@ -129,7 +129,7 @@ func TestExecLocksFencesAndJournalsTheExactContainer(t *testing.T) { } }, } - service := execService(t, func(context.Context, string) (transport.Transport, error) { return fake, nil }) + service := execService(t, func(context.Context, transport.Route) (transport.Transport, error) { return fake, nil }) var stdout bytes.Buffer const command = "printf secret-output" result, err := service.Exec(context.Background(), ExecRequest{ @@ -185,7 +185,7 @@ func TestExecClassifiesCancellation(t *testing.T) { return nil }, } - service := execService(t, func(context.Context, string) (transport.Transport, error) { return fake, nil }) + service := execService(t, func(context.Context, transport.Route) (transport.Transport, error) { return fake, nil }) result, err := service.Exec(context.Background(), ExecRequest{ Target: "api", Command: "true", Reason: "stop a hung diagnostic", }, &bytes.Buffer{}, &bytes.Buffer{}) diff --git a/internal/onebox/execution_boundary_test.go b/internal/onebox/execution_boundary_test.go index 908def35..0fd8ed66 100644 --- a/internal/onebox/execution_boundary_test.go +++ b/internal/onebox/execution_boundary_test.go @@ -225,7 +225,7 @@ func TestSchemaLessDeployPlansAreRejectedBeforeConnecting(t *testing.T) { svc := New(Options{ ConfigPath: filepath.Join(t.TempDir(), "must-not-be-read.yml"), Now: func() time.Time { return base }, - Connect: func(context.Context, string) (transport.Transport, error) { + Connect: func(context.Context, transport.Route) (transport.Transport, error) { connected = true return fake, nil }, @@ -269,7 +269,7 @@ func TestExecuteRejectsExpiredDeployBeforeConnecting(t *testing.T) { svc := New(Options{ ConfigPath: filepath.Join(t.TempDir(), "must-not-be-read.yml"), Now: func() time.Time { return now }, - Connect: func(context.Context, string) (transport.Transport, error) { + Connect: func(context.Context, transport.Route) (transport.Transport, error) { connected = true return serviceFake(), nil }, @@ -314,7 +314,7 @@ func TestExecuteRejectsFutureDeployBeforeConnecting(t *testing.T) { svc := New(Options{ ConfigPath: filepath.Join(t.TempDir(), "must-not-be-read.yml"), Now: func() time.Time { return now }, - Connect: func(context.Context, string) (transport.Transport, error) { + Connect: func(context.Context, transport.Route) (transport.Transport, error) { connected = true return serviceFake(), nil }, @@ -1011,7 +1011,7 @@ func savedPlanCarriesImage(t *testing.T, built string) { tick++ return base.Add(time.Duration(tick) * time.Minute) }, - Connect: func(_ context.Context, _ string) (transport.Transport, error) { return fake, nil }, + Connect: func(_ context.Context, _ transport.Route) (transport.Transport, error) { return fake, nil }, }) } diff --git a/internal/onebox/job_execute.go b/internal/onebox/job_execute.go index da3e5578..a1f1f077 100644 --- a/internal/onebox/job_execute.go +++ b/internal/onebox/job_execute.go @@ -55,7 +55,7 @@ func (s *Service) executeJob( if lp.resolved.Name != binding.Application || s.environment != binding.Environment { return "", nil, errors.New("job plan application or environment changed — re-plan") } - if environmentConfig.Destination() != binding.Server { + if environmentConfig.Route().String() != binding.Server { return "", nil, errors.New("job plan target changed — re-plan") } if engine.HashBytes(lp.configBytes) != binding.ConfigDigest { diff --git a/internal/onebox/job_plan_test.go b/internal/onebox/job_plan_test.go index d2fc3b4a..8f88effe 100644 --- a/internal/onebox/job_plan_test.go +++ b/internal/onebox/job_plan_test.go @@ -56,10 +56,10 @@ func newManualJobService(t *testing.T, effect string, requireBackup bool, fake * return New(Options{ ConfigPath: writeManualJobProject(t, effect, requireBackup), Now: func() time.Time { return *now }, - Connect: func(_ context.Context, target string) (transport.Transport, error) { + Connect: func(_ context.Context, route transport.Route) (transport.Transport, error) { *connects++ - if target != "deploy@example.invalid" { - t.Fatalf("connector target = %q", target) + if route.String() != "deploy@example.invalid" { + t.Fatalf("connector target = %q", route) } return fake, nil }, diff --git a/internal/onebox/jump_route_test.go b/internal/onebox/jump_route_test.go new file mode 100644 index 00000000..171f738d --- /dev/null +++ b/internal/onebox/jump_route_test.go @@ -0,0 +1,115 @@ +package onebox + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/labstack/onebox/internal/transport" +) + +func writeJumpProject(t *testing.T) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "ob.yml") + body := ` +api_version: onebox.run/v1 +app: demo +environments: + production: + server: deploy@example.invalid + jump: bastion@jump.invalid:2222 +image: ghcr.io/example/app:v1 +domain: demo.example.com +port: 8080 +` + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + return path +} + +// The route is the trust boundary, so it is what the operator confirms: a +// binding naming only the private target would let the bastion be swapped +// under an approval that never mentioned it. +func TestExecutionBindingNamesTheJumpRoute(t *testing.T) { + service := New(Options{ + ConfigPath: writeJumpProject(t), + Connect: func(context.Context, transport.Route) (transport.Transport, error) { + t.Fatal("resolving a binding must not connect") + return nil, nil + }, + }) + binding, err := service.ResolveExecutionBinding(context.Background(), KindDestroy) + if err != nil { + t.Fatal(err) + } + want := "deploy@example.invalid via bastion@jump.invalid:2222" + if binding.Server != want { + t.Fatalf("binding.Server = %q, want %q", binding.Server, want) + } +} + +func TestConnectorReceivesTheDeclaredJump(t *testing.T) { + var got string + service := New(Options{ + ConfigPath: writeJumpProject(t), + Connect: func(_ context.Context, route transport.Route) (transport.Transport, error) { + got = route.String() + if route.Jump == nil { + t.Fatal("connector route carries no jump") + } + return nil, errStopAfterConnect + }, + }) + _, _ = service.PlanDeploy(context.Background(), PlanDeployRequest{}) + if !strings.Contains(got, "via bastion@jump.invalid:2222") { + t.Fatalf("connector route = %q, want the declared jump", got) + } +} + +// errStopAfterConnect ends the operation at the connector: the route the +// connector was handed is the whole subject of the test. +var errStopAfterConnect = errors.New("stop after connect") + +// The bastion is part of what was approved, so swapping it must invalidate the +// confirmation the operator already gave. +func TestChangingOnlyTheJumpChangesTheBinding(t *testing.T) { + binding := func(jump string) ExecutionBinding { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "ob.yml") + body := ` +api_version: onebox.run/v1 +app: demo +environments: + production: + server: deploy@example.invalid + jump: ` + jump + ` +image: ghcr.io/example/app:v1 +domain: demo.example.com +port: 8080 +` + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + resolved, err := New(Options{ConfigPath: path}).ResolveExecutionBinding(context.Background(), KindDestroy) + if err != nil { + t.Fatal(err) + } + return resolved + } + first, second := binding("bastion@jump.invalid:2222"), binding("bastion@other.invalid:2222") + // The digests differ because the file differs, which would pass even if + // the route were dropped from the binding — so the server field is + // asserted on its own. + if first.Server == second.Server { + t.Fatalf("binding server is unchanged by a different jump host: %q", first.Server) + } + if first == second { + t.Fatalf("binding is unchanged by a different jump host: %#v", first) + } +} diff --git a/internal/onebox/load.go b/internal/onebox/load.go index 34b904f7..f2c7f3a8 100644 --- a/internal/onebox/load.go +++ b/internal/onebox/load.go @@ -121,7 +121,7 @@ func (s *Service) observeServiceRuntimeStates(ctx context.Context, resolved *app if err != nil { return nil, err } - target, err := s.connect(ctx, environment.Destination()) + target, err := s.connect(ctx, environment.Route()) if err != nil { return nil, fmt.Errorf("observe service lifecycle state: %w", err) } diff --git a/internal/onebox/load_service_runtime_test.go b/internal/onebox/load_service_runtime_test.go index 7cbc3520..765ba60a 100644 --- a/internal/onebox/load_service_runtime_test.go +++ b/internal/onebox/load_service_runtime_test.go @@ -63,7 +63,7 @@ func TestProductionLoadInjectsLifecycleStateBeforeServiceRendering(t *testing.T) }} service := New(Options{ ConfigPath: protectedRuntimeProject(t), Environment: "production", - Connect: func(context.Context, string) (transport.Transport, error) { return fake, nil }, + Connect: func(context.Context, transport.Route) (transport.Transport, error) { return fake, nil }, }) lp, err := service.loadProject(context.Background(), false) if err != nil { @@ -101,7 +101,7 @@ func TestProductionLoadRefusesProtectedImageAbsentFromRegistryAndCache(t *testin }} service := New(Options{ ConfigPath: protectedRuntimeProject(t), Environment: "production", - Connect: func(context.Context, string) (transport.Transport, error) { return fake, nil }, + Connect: func(context.Context, transport.Route) (transport.Transport, error) { return fake, nil }, }) if _, err := service.loadProject(context.Background(), false); err == nil || !strings.Contains(err.Error(), "service_image_digest_unavailable") { t.Fatalf("unavailable protected image error = %v", err) diff --git a/internal/onebox/secrets_push_test.go b/internal/onebox/secrets_push_test.go index 14b7ad7c..868e4fac 100644 --- a/internal/onebox/secrets_push_test.go +++ b/internal/onebox/secrets_push_test.go @@ -129,7 +129,7 @@ func pushService(t *testing.T, f *transport.Fake) *Service { tick++ return base.Add(time.Duration(tick) * time.Minute) }, - Connect: func(_ context.Context, target string) (transport.Transport, error) { + Connect: func(_ context.Context, route transport.Route) (transport.Transport, error) { return f, nil }, }) @@ -184,7 +184,7 @@ workloads: s := New(Options{ ConfigPath: filepath.Join(dir, "ob.yml"), Now: func() time.Time { return time.Date(2026, 7, 12, 18, 0, 0, 0, time.UTC) }, - Connect: func(_ context.Context, _ string) (transport.Transport, error) { + Connect: func(_ context.Context, _ transport.Route) (transport.Transport, error) { return f, nil }, }) diff --git a/internal/onebox/service.go b/internal/onebox/service.go index 3c049d90..1f150b84 100644 --- a/internal/onebox/service.go +++ b/internal/onebox/service.go @@ -21,7 +21,7 @@ import ( "github.com/labstack/onebox/internal/transport" ) -type Connector func(context.Context, string) (transport.Transport, error) +type Connector func(context.Context, transport.Route) (transport.Transport, error) type Options struct { ConfigPath string @@ -79,8 +79,8 @@ func New(opts Options) *Service { opts.Now = time.Now } if opts.Connect == nil { - opts.Connect = func(ctx context.Context, target string) (transport.Transport, error) { - return transport.NewSSHContext(ctx, target) + opts.Connect = func(ctx context.Context, route transport.Route) (transport.Transport, error) { + return transport.NewSSHRoute(ctx, route) } } if opts.Entropy == nil { @@ -123,8 +123,9 @@ func (s *Service) engineWith(ctx context.Context, lp *loadedProject, environment if err != nil { return nil, nil, "", err } - target := env.Destination() - t, err := s.connect(ctx, target) + route := env.Route() + target := route.String() + t, err := s.connect(ctx, route) if err != nil { return nil, nil, "", err } diff --git a/internal/onebox/service_test.go b/internal/onebox/service_test.go index 1356ba50..6fbfbd11 100644 --- a/internal/onebox/service_test.go +++ b/internal/onebox/service_test.go @@ -118,9 +118,9 @@ func newTestService(t *testing.T, f *transport.Fake) *Service { tick++ return base.Add(time.Duration(tick) * time.Minute) }, - Connect: func(_ context.Context, target string) (transport.Transport, error) { - if target != "deploy@example.invalid" { - t.Fatalf("connector target = %q", target) + Connect: func(_ context.Context, route transport.Route) (transport.Transport, error) { + if route.String() != "deploy@example.invalid" { + t.Fatalf("connector target = %q", route) } return f, nil }, @@ -180,7 +180,7 @@ func composeBuildService(t *testing.T, images app.Images) *Service { t.Helper() return New(Options{ ConfigPath: writeComposeBuildProject(t), Environment: "production", Images: images, - Connect: func(context.Context, string) (transport.Transport, error) { return serviceFake(), nil }, + Connect: func(context.Context, transport.Route) (transport.Transport, error) { return serviceFake(), nil }, }) } diff --git a/internal/target/address.go b/internal/target/address.go index a6434065..1f89b83c 100644 --- a/internal/target/address.go +++ b/internal/target/address.go @@ -81,6 +81,17 @@ func Parse(raw string) (Address, error) { return address, nil } +// ValidUser reports whether user is a legal SSH user in this grammar. It is +// exported so config validation can hold an authored field to the same rule the +// transport dials by, without recomposing a string to parse. +func ValidUser(user string) bool { return validUser(user) } + +// ValidHost reports whether host is a legal host: a DNS name, an IPv4 address, +// or an *unbracketed* IPv6 literal. Brackets belong to the scalar grammar, +// where they separate the address from the port; a host field has no port to +// separate. +func ValidHost(host string) bool { return validHost(host) } + func validUser(user string) bool { if user == "" || !isUserStart(user[0]) { return false diff --git a/internal/target/route.go b/internal/target/route.go new file mode 100644 index 00000000..744417c6 --- /dev/null +++ b/internal/target/route.go @@ -0,0 +1,37 @@ +package target + +import "net" + +// String renders the canonical `[user@]host[:port]` form. The port appears +// only when the author wrote one, so a target that never named a port keeps +// the exact spelling every plan and approval already carries. An IPv6 literal +// is bracketed only alongside a port, where its own colons would otherwise be +// read as the port separator. +func (a Address) String() string { + host := a.Host + if a.ExplicitPort { + host = net.JoinHostPort(host, a.Port) + } + if a.User == "" { + return host + } + return a.User + "@" + host +} + +// Route is one deployment target and, optionally, the single jump host the +// connection is tunnelled through. One hop is structural: a Route holds an +// Address, not another Route, so no configuration can describe a chain. +type Route struct { + Target Address + Jump *Address +} + +// String names the whole connection. Without a jump it is the target alone, +// so a direct route reads exactly as it did before jump hosts existed and +// approvals sealed against the old spelling still verify. +func (r Route) String() string { + if r.Jump == nil { + return r.Target.String() + } + return r.Target.String() + " via " + r.Jump.String() +} diff --git a/internal/target/route_test.go b/internal/target/route_test.go new file mode 100644 index 00000000..afdcb1b1 --- /dev/null +++ b/internal/target/route_test.go @@ -0,0 +1,42 @@ +package target + +import "testing" + +func TestAddressString(t *testing.T) { + tests := map[string]struct { + address Address + want string + }{ + "user and explicit port": {Address{User: "deploy", Host: "example.com", Port: "2222", ExplicitPort: true}, "deploy@example.com:2222"}, + "user without port": {Address{User: "root", Host: "example.com", Port: "22"}, "root@example.com"}, + "port without user": {Address{Host: "example.com", Port: "2222", ExplicitPort: true}, "example.com:2222"}, + "bare host": {Address{Host: "example.com", Port: "22"}, "example.com"}, + "ipv6 with port": {Address{User: "root", Host: "2a01:4ff::1", Port: "2222", ExplicitPort: true}, "root@[2a01:4ff::1]:2222"}, + "ipv6 without port": {Address{User: "root", Host: "2a01:4ff::1", Port: "22"}, "root@2a01:4ff::1"}, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + if got := test.address.String(); got != test.want { + t.Fatalf("String() = %q, want %q", got, test.want) + } + }) + } +} + +func TestRouteStringNamesOnlyTheTargetWithoutAJump(t *testing.T) { + route := Route{Target: Address{User: "root", Host: "10.20.0.10", Port: "22"}} + if got := route.String(); got != "root@10.20.0.10" { + t.Fatalf("String() = %q, want %q", got, "root@10.20.0.10") + } +} + +func TestRouteStringNamesTheJumpAfterTheTarget(t *testing.T) { + route := Route{ + Target: Address{User: "root", Host: "10.20.0.10", Port: "22"}, + Jump: &Address{User: "deploy", Host: "bastion.example.com", Port: "2222", ExplicitPort: true}, + } + want := "root@10.20.0.10 via deploy@bastion.example.com:2222" + if got := route.String(); got != want { + t.Fatalf("String() = %q, want %q", got, want) + } +} diff --git a/internal/transport/fake.go b/internal/transport/fake.go index 510a66cf..7cae7a62 100644 --- a/internal/transport/fake.go +++ b/internal/transport/fake.go @@ -32,6 +32,7 @@ type Fake struct { Uploads []string HostName string TargetName string // full user@host; falls back to HostName + SSHJumpName string SSHUserName string SSHPortName string // falls back to 22 when TargetName is set state map[string]string @@ -181,6 +182,7 @@ func (f *Fake) Destination() string { } func (f *Fake) SSHUser() string { return f.SSHUserName } +func (f *Fake) SSHJump() string { return f.SSHJumpName } func (f *Fake) SSHPort() string { if f.SSHPortName != "" { diff --git a/internal/transport/ssh.go b/internal/transport/ssh.go index 55a21e5e..71bb30e0 100644 --- a/internal/transport/ssh.go +++ b/internal/transport/ssh.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "strings" + "sync" "time" "golang.org/x/crypto/ssh" @@ -29,6 +30,15 @@ type SSH struct { target string // user@host — valid as an OpenSSH destination port string // separate because user@host:port is invalid for both tools Logger func(host, cmd string) + + // jumpClient and jumpTCP are the bastion hop, nil on a direct connection. + // The raw TCP conn is kept because closing the target client behind a jump + // only writes a channel-close message *through* the bastion: if the + // bastion is what is wedged, that write blocks and nothing is released. + // Closing this conn is what actually unblocks the stack. + jumpClient *ssh.Client + jumpTCP net.Conn + jump string // [user@]host[:port] of the bastion, empty when direct } // ParseAddr splits [user@]host[:port]; port defaults to 22. @@ -44,25 +54,35 @@ func ParseAddr(addr string) (user, host, port string) { // cancellation/deadline. A bounded fallback prevents an MCP tool from hanging // indefinitely when its target is unreachable or stops during handshake. func NewSSHContext(ctx context.Context, addr string) (*SSH, error) { - if err := ctx.Err(); err != nil { - return nil, err - } parsed, err := obtarget.Parse(addr) if err != nil { return nil, fmt.Errorf("target %q: %w", addr, err) } - user, host, port := parsed.User, parsed.Host, parsed.Port - if user == "" { - user = os.Getenv("USER") + return NewSSHRoute(ctx, obtarget.Route{Target: parsed}) +} + +// NewSSHRoute connects to the route's target, tunnelling through its jump host +// when one is declared. Both hops are verified against known_hosts and +// authenticated independently; the local agent may sign for either, but its +// socket is never forwarded, so a compromised bastion cannot borrow the +// operator's identity. Exactly one hop is possible — a Route holds an address, +// not another route. +func NewSSHRoute(ctx context.Context, route obtarget.Route) (*SSH, error) { + if err := ctx.Err(); err != nil { + return nil, err } home, err := os.UserHomeDir() if err != nil { return nil, err } - hk, err := knownhosts.New(filepath.Join(home, ".ssh", "known_hosts")) + hostKeys, err := knownhosts.New(filepath.Join(home, ".ssh", "known_hosts")) if err != nil { return nil, fmt.Errorf("known_hosts (required — ob never skips host verification): %w", err) } + // Gathered once for the whole route rather than per hop: sshAuths leaves + // the agent connection open for the handshake callback to re-list, so a + // second call would open a second agent connection and duplicate every + // diagnostic. auths, diag := sshAuths(ctx, home, os.Getenv("SSH_AUTH_SOCK")) if len(auths) == 0 { msg := "no usable SSH auth found (need an ssh-agent identity or ~/.ssh/id_ed25519|id_rsa)" @@ -71,53 +91,219 @@ func NewSSHContext(ctx context.Context, addr string) (*SSH, error) { } return nil, errors.New(msg) } + + target := resolveUser(route.Target) + if route.Jump == nil { + conn, err := dialTCP(ctx, target) + if err != nil { + return nil, hopError(stageDirect, target, phaseDial, err) + } + client, err := sshHandshake(ctx, conn, target, auths, hostKeys, stageDirect, nil) + if err != nil { + return nil, err + } + return newSSHFromClient(client, target, nil, nil, nil), nil + } + + jump := resolveUser(*route.Jump) + jumpTCP, err := dialTCP(ctx, jump) + if err != nil { + return nil, hopError(stageJump, jump, phaseDial, err) + } + jumpClient, err := sshHandshake(ctx, jumpTCP, jump, auths, hostKeys, stageJump, nil) + if err != nil { + return nil, err + } + // The tunnel is opened by the bastion's sshd, which applies its own + // policy and its own connect timeout; neither is ours to trust, so the + // dial carries an explicit bound of its own. + tunnelCtx, cancelTunnel := context.WithTimeout(ctx, dialTimeout) + defer cancelTunnel() + tunnel, err := jumpClient.DialContext(tunnelCtx, "tcp", net.JoinHostPort(target.Host, target.Port)) + if err != nil { + _ = jumpTCP.Close() + _ = jumpClient.Close() + return nil, hopError(stageTarget, target, phaseTunnel, err) + } + client, err := sshHandshake(ctx, tunnel, target, auths, hostKeys, stageTarget, func() { _ = jumpTCP.Close() }) + if err != nil { + _ = tunnel.Close() + _ = jumpTCP.Close() + _ = jumpClient.Close() + return nil, err + } + return newSSHFromClient(client, target, &jump, jumpClient, jumpTCP), nil +} + +func newSSHFromClient(client *ssh.Client, address obtarget.Address, jump *obtarget.Address, jumpClient *ssh.Client, jumpTCP net.Conn) *SSH { + s := &SSH{ + client: client, user: address.User, host: address.Host, port: address.Port, + target: address.Destination(address.User), + jumpClient: jumpClient, jumpTCP: jumpTCP, + } + if jump != nil { + s.jump = jump.String() + } + return s +} + +// resolveUser fills the SSH user the same way OpenSSH's default does when the +// author named none. +func resolveUser(address obtarget.Address) obtarget.Address { + if address.User == "" { + address.User = os.Getenv("USER") + } + return address +} + +const dialTimeout = 10 * time.Second + +// handshakeTimeout bounds a hop that connects but never completes its +// handshake. It is a variable so tests can shorten it: this bound is the only +// thing that stops such a hop behind a jump host, because an SSH channel +// refuses SetDeadline outright. +var handshakeTimeout = 15 * time.Second + +func dialTCP(ctx context.Context, address obtarget.Address) (net.Conn, error) { + return (&net.Dialer{Timeout: dialTimeout}).DialContext(ctx, "tcp", net.JoinHostPort(address.Host, address.Port)) +} + +// sshHandshake authenticates one hop over an already-established connection and +// verifies its host key. conn is a TCP connection for a direct target or a +// bastion, and an SSH channel for a target behind one; the handshake is +// identical either way, which is what makes both hops verified rather than one +// inheriting the other's trust. +func sshHandshake(ctx context.Context, conn net.Conn, address obtarget.Address, auths []ssh.AuthMethod, hostKeys ssh.HostKeyCallback, stage string, hardClose func()) (*ssh.Client, error) { + dialed := net.JoinHostPort(address.Host, address.Port) config := &ssh.ClientConfig{ - User: user, + User: address.User, Auth: auths, - HostKeyCallback: hk, + HostKeyCallback: hostKeys, // Ask the server for the host-key TYPE we actually have pinned. Without // this the client negotiates its own default (often ecdsa/rsa) which may // differ from what known_hosts holds (OpenSSH's TOFU writes a single // ed25519 line), and knownhosts then reports a spurious "key mismatch". - HostKeyAlgorithms: knownHostKeyAlgos(hk, net.JoinHostPort(host, port)), - } - address := net.JoinHostPort(host, port) - conn, err := (&net.Dialer{Timeout: 10 * time.Second}).DialContext(ctx, "tcp", address) - if err != nil { - return nil, fmt.Errorf("ssh %s@%s:%s: %w", user, host, port, err) + HostKeyAlgorithms: knownHostKeyAlgos(hostKeys, dialed), } - handshakeDeadline := time.Now().Add(15 * time.Second) + handshakeDeadline := time.Now().Add(handshakeTimeout) if deadline, ok := ctx.Deadline(); ok && deadline.Before(handshakeDeadline) { handshakeDeadline = deadline } + // Best effort: a TCP conn reports a clean i/o timeout this way, but an SSH + // channel refuses deadlines outright ("ssh: tcpChan: deadline not + // supported"), so the timer below is what actually bounds the second hop. _ = conn.SetDeadline(handshakeDeadline) + timeout := time.NewTimer(time.Until(handshakeDeadline)) + defer timeout.Stop() + // The watcher and the handshake race to decide the connection's fate, so + // they settle it under one lock rather than by signalling after the fact. + // Signalling after closing leaves a window in which the handshake has + // already returned a client whose connection is being torn down — the + // failure this guard exists to prevent, arriving opaquely at first use. + var settle sync.Mutex + var abandoned, established bool cancelWatch := make(chan struct{}) go func() { select { case <-ctx.Done(): - _ = conn.Close() + case <-timeout.C: case <-cancelWatch: + return + } + settle.Lock() + defer settle.Unlock() + if established { + return + } + abandoned = true + _ = conn.Close() + // Closing an SSH channel only sends a message through the hop below + // it, so a channel whose bastion has stopped answering stays blocked + // on a read that will never complete. hardClose drops the connection + // that message would have travelled on, which is the only close that + // still lands. + if hardClose != nil { + hardClose() } }() - sshConn, channels, requests, err := ssh.NewClientConn(conn, address, config) + sshConn, channels, requests, err := ssh.NewClientConn(conn, dialed, config) close(cancelWatch) + settle.Lock() + established = err == nil + gaveUp := abandoned + settle.Unlock() if err != nil { _ = conn.Close() if ctxErr := ctx.Err(); ctxErr != nil { - return nil, ctxErr + return nil, hopError(stage, address, phaseNone, ctxErr) } - return nil, fmt.Errorf("ssh %s@%s:%s: %w", user, host, port, err) + return nil, hopError(stage, address, phaseOf(err), err) } if ctxErr := ctx.Err(); ctxErr != nil { _ = sshConn.Close() - return nil, ctxErr + return nil, hopError(stage, address, phaseNone, ctxErr) + } + if gaveUp { + _ = sshConn.Close() + return nil, hopError(stage, address, phaseNone, errHandshakeAbandoned) } _ = conn.SetDeadline(time.Time{}) - client := ssh.NewClient(sshConn, channels, requests) - return &SSH{ - client: client, user: user, host: host, port: port, - target: parsed.Destination(user), - }, nil + return ssh.NewClient(sshConn, channels, requests), nil +} + +var errHandshakeAbandoned = errors.New("connection closed while completing the handshake") + +// Connection failures are reported by the hop they happened on and the phase +// that failed, so an operator can tell a bastion they cannot reach from one +// they cannot authenticate to, and either from a target the bastion refused to +// forward them to. +const ( + stageDirect = "ssh" + stageJump = "jump ssh" + stageTarget = "target ssh" +) + +const ( + phaseNone = "" + phaseDial = "" + phaseTunnel = "not reachable from the jump host" + phaseHostKey = "host key" + phaseAuth = "authenticate" +) + +// phaseOf names a handshake failure only when it can identify one. A timeout, +// a cancelled dial, or a peer that hung up are none of the named phases, and +// labelling them "authenticate" sends the operator to check a key that is +// fine. +func phaseOf(err error) string { + var keyErr *knownhosts.KeyError + var revoked *knownhosts.RevokedError + switch { + case errors.As(err, &keyErr), errors.As(err, &revoked): + return phaseHostKey + case isTransportFailure(err): + return phaseNone + default: + return phaseAuth + } +} + +func isTransportFailure(err error) bool { + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return true + } + return errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) || + errors.Is(err, net.ErrClosed) || errors.Is(err, context.Canceled) || + errors.Is(err, context.DeadlineExceeded) || errors.Is(err, errHandshakeAbandoned) +} + +func hopError(stage string, address obtarget.Address, phase string, err error) error { + where := fmt.Sprintf("%s %s@%s:%s", stage, address.User, address.Host, address.Port) + if phase == "" { + return fmt.Errorf("%s: %w", where, err) + } + return fmt.Errorf("%s: %s: %w", where, phase, err) } // sshAuths gathers publickey auth methods from the ssh-agent and on-disk keys. @@ -282,7 +468,7 @@ func (s *SSH) Upload(ctx context.Context, localDir, remoteDir string) error { if s.Logger != nil { s.Logger(s.host, "upload "+localDir+" -> "+remoteDir) } - return uploadWithClient(ctx, &sshUploadClient{client: s.client}, localDir, remoteDir) + return uploadWithClient(ctx, &sshUploadClient{client: s.client, closeAll: s.Close}, localDir, remoteDir) } type uploadClient interface { @@ -292,6 +478,10 @@ type uploadClient interface { type sshUploadClient struct { client *ssh.Client + // closeAll releases the jump hop too. Cancellation relies on closing the + // connection to unblock a wedged transfer, and behind a bastion the target + // client alone is not that connection. + closeAll func() error } func (c *sshUploadClient) NewSession() (uploadSession, error) { @@ -302,7 +492,12 @@ func (c *sshUploadClient) NewSession() (uploadSession, error) { return &sshUploadSession{Session: sess}, nil } -func (c *sshUploadClient) Close() error { return c.client.Close() } +func (c *sshUploadClient) Close() error { + if c.closeAll != nil { + return c.closeAll() + } + return c.client.Close() +} type uploadSession interface { StdinPipe() (io.WriteCloser, error) @@ -512,5 +707,54 @@ func uploadCause(ctx context.Context, err error) error { func (s *SSH) Host() string { return s.host } func (s *SSH) Destination() string { return s.target } func (s *SSH) SSHUser() string { return s.user } +func (s *SSH) SSHJump() string { return s.jump } func (s *SSH) SSHPort() string { return s.port } -func (s *SSH) Close() error { return s.client.Close() } + +// Close releases the whole connection in reverse order. The jump's TCP conn is +// closed last and unconditionally: it is the only close that cannot be +// swallowed by a wedged bastion. +func (s *SSH) Close() error { + if s.jumpTCP == nil { + return s.client.Close() + } + return closeStack(s.jumpTCP, closeGrace, s.client, s.jumpClient) +} + +// closeGrace bounds how long a graceful shutdown may take before the jump's +// connection is dropped underneath it. A clean close is one round trip; only a +// bastion that has stopped answering takes longer. +const closeGrace = 2 * time.Second + +// closeStack closes each graceful closer in order, then the jump host's raw +// connection. Every graceful close writes through that connection, so if the +// bastion has stopped moving bytes they block; dropping the connection is what +// unblocks them, which makes it a backstop rather than a formality. An +// already-closed connection is the normal case — closing the jump client +// closes it — so that is not reported as a failure. +func closeStack(raw io.Closer, grace time.Duration, graceful ...io.Closer) error { + done := make(chan error, 1) + go func() { + errs := make([]error, 0, len(graceful)) + for _, closer := range graceful { + errs = append(errs, closer.Close()) + } + done <- errors.Join(errs...) + }() + timer := time.NewTimer(grace) + defer timer.Stop() + select { + case err := <-done: + return errors.Join(err, ignoreClosed(raw.Close())) + case <-timer.C: + // The goroutine is released by this close and is not waited for: it + // cannot be, since waiting is the thing that was blocked. + return ignoreClosed(raw.Close()) + } +} + +func ignoreClosed(err error) error { + if errors.Is(err, net.ErrClosed) { + return nil + } + return err +} diff --git a/internal/transport/ssh_close_test.go b/internal/transport/ssh_close_test.go new file mode 100644 index 00000000..6f45b28f --- /dev/null +++ b/internal/transport/ssh_close_test.go @@ -0,0 +1,88 @@ +package transport + +import ( + "errors" + "io" + "testing" + "time" +) + +type blockingCloser struct { + release chan struct{} + closed chan struct{} +} + +func newBlockingCloser() *blockingCloser { + return &blockingCloser{release: make(chan struct{}), closed: make(chan struct{}, 1)} +} + +func (c *blockingCloser) Close() error { + c.closed <- struct{}{} + <-c.release + return nil +} + +type recordingCloser struct { + closed chan struct{} + err error +} + +func newRecordingCloser(err error) *recordingCloser { + return &recordingCloser{closed: make(chan struct{}, 1), err: err} +} + +func (c *recordingCloser) Close() error { + c.closed <- struct{}{} + return c.err +} + +// Closing an SSH client behind a jump host writes through the jump's own +// connection. When that connection has stopped moving bytes the write blocks, +// so the graceful close must not be the only close: dropping the jump's TCP +// connection is what actually releases everything. +func TestCloseStackDropsTheJumpConnectionWhenGracefulCloseBlocks(t *testing.T) { + graceful := newBlockingCloser() + raw := newRecordingCloser(nil) + defer close(graceful.release) + + done := make(chan error, 1) + go func() { done <- closeStack(raw, 100*time.Millisecond, graceful) }() + + select { + case <-raw.closed: + case <-time.After(5 * time.Second): + t.Fatal("the jump connection was never dropped") + } + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("close never returned while the graceful close was blocked") + } +} + +func TestCloseStackReportsGracefulErrorsWhenNothingBlocks(t *testing.T) { + first := newRecordingCloser(errors.New("target went away")) + second := newRecordingCloser(nil) + raw := newRecordingCloser(nil) + + err := closeStack(raw, time.Second, first, second) + if err == nil || !errors.Is(err, first.err) { + t.Fatalf("close error = %v, want the target's failure", err) + } + for _, closer := range []*recordingCloser{first, second, raw} { + select { + case <-closer.closed: + default: + t.Fatal("a closer was skipped") + } + } +} + +// An already-closed connection is the expected case: closing the jump client +// closes its connection, so the backstop close finds it gone. +func TestCloseStackIgnoresAnAlreadyClosedJumpConnection(t *testing.T) { + raw := newRecordingCloser(io.ErrClosedPipe) + if err := closeStack(raw, time.Second, newRecordingCloser(nil)); err == nil { + t.Fatal("an unexpected close error was swallowed") + } +} diff --git a/internal/transport/ssh_jump_test.go b/internal/transport/ssh_jump_test.go new file mode 100644 index 00000000..09774091 --- /dev/null +++ b/internal/transport/ssh_jump_test.go @@ -0,0 +1,438 @@ +package transport + +import ( + "context" + "errors" + "net" + "strings" + "testing" + "time" + + obtarget "github.com/labstack/onebox/internal/target" +) + +func obtargetRoute(target, jump obtarget.Address) obtarget.Route { + return obtarget.Route{Target: target, Jump: &jump} +} + +func TestRouteWithAJumpReachesTheTargetThroughIt(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, jump, target) + + route := obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy")) + conn, err := NewSSHRoute(context.Background(), route) + if err != nil { + t.Fatalf("connect through jump: %v", err) + } + defer conn.Close() + + if connections, forwarded := jump.stats(); connections != 1 || len(forwarded) != 1 { + t.Fatalf("jump saw %d connections and forwards %v, want one of each", connections, forwarded) + } + if forwarded := mustForward(t, jump); forwarded != target.addr() { + t.Fatalf("jump forwarded to %q, want %q", forwarded, target.addr()) + } + if connections, _ := target.stats(); connections != 1 { + t.Fatalf("target saw %d connections, want 1", connections) + } +} + +func mustForward(t *testing.T, server *testSSHServer) string { + t.Helper() + _, forwarded := server.stats() + if len(forwarded) != 1 { + t.Fatalf("forwards = %v, want exactly one", forwarded) + } + return forwarded[0] +} + +func TestDirectRouteStillConnectsWithoutAJump(t *testing.T) { + home, clientKey := sshTestHome(t) + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, target) + + conn, err := NewSSHContext(context.Background(), "root@"+target.addr()) + if err != nil { + t.Fatalf("direct connect: %v", err) + } + defer conn.Close() + if connections, _ := target.stats(); connections != 1 { + t.Fatalf("target saw %d connections, want 1", connections) + } +} + +// An untrusted bastion must be refused before it is ever told which private +// address to reach: the forwarding request itself discloses the target. +func TestUnknownJumpHostKeyFailsBeforeTheTargetIsNamed(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, target) // the jump is deliberately unpinned + + _, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))) + if err == nil { + t.Fatal("connected through an unpinned jump host") + } + assertStage(t, err, "jump ssh", "host key") + if _, forwarded := jump.stats(); len(forwarded) != 0 { + t.Fatalf("jump was asked to forward to %v before its key was trusted", forwarded) + } + if connections, _ := target.stats(); connections != 0 { + t.Fatalf("target saw %d connections, want 0", connections) + } +} + +func TestJumpAuthenticationFailureIsReportedAgainstTheJump(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, generateSigner(t).PublicKey(), true) // authorizes a key we do not hold + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, jump, target) + + _, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))) + if err == nil { + t.Fatal("authenticated to a jump host that rejects our key") + } + assertStage(t, err, "jump ssh", "authenticate") +} + +// A bastion that will not forward is a target-reachability problem. Reporting +// it against the jump sends the operator to fix credentials that already work. +func TestUnreachableTargetIsReportedAgainstTheTarget(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, false) // accepts us, refuses to forward + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, jump, target) + + _, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))) + if err == nil { + t.Fatal("connected through a jump host that refuses to forward") + } + assertStage(t, err, "target ssh", "not reachable from the jump host") + jump.waitForClosed(t, 1) +} + +func TestUnknownTargetHostKeyFailsThroughATrustedJump(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, jump) // the target is deliberately unpinned + + _, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))) + if err == nil { + t.Fatal("a trusted jump host let an unpinned target through") + } + assertStage(t, err, "target ssh", "host key") + jump.waitForClosed(t, 1) +} + +func TestTargetAuthenticationFailureIsReportedAgainstTheTarget(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + target := newTestSSHServer(t, generateSigner(t).PublicKey(), false) + writeKnownHosts(t, home, jump, target) + + _, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))) + if err == nil { + t.Fatal("authenticated to a target that rejects our key") + } + assertStage(t, err, "target ssh", "authenticate") + jump.waitForClosed(t, 1) +} + +func TestClosingAJumpedConnectionReleasesBothHops(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, jump, target) + + conn, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))) + if err != nil { + t.Fatal(err) + } + if err := conn.Close(); err != nil { + t.Fatalf("close: %v", err) + } + target.waitForClosed(t, 1) + jump.waitForClosed(t, 1) +} + +func TestCancelledContextStopsAJumpedConnection(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, jump, target) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := NewSSHRoute(ctx, obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))); !errors.Is(err, context.Canceled) { + t.Fatalf("connect with a cancelled context = %v, want context.Canceled", err) + } +} + +func assertStage(t *testing.T, err error, stage, phase string) { + t.Helper() + if !strings.HasPrefix(err.Error(), stage+" ") { + t.Fatalf("error %q does not name the %s hop", err, stage) + } + if !strings.Contains(err.Error(), phase) { + t.Fatalf("error %q does not name the %q phase", err, phase) + } +} + +func TestJumpedConnectionReportsTheBastionAndTargetSeparately(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, jump, target) + + conn, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + + host, port, _ := net.SplitHostPort(target.addr()) + if conn.Host() != host || conn.SSHPort() != port || conn.SSHUser() != "root" { + t.Fatalf("target identity = %s@%s:%s, want root@%s", conn.SSHUser(), conn.Host(), conn.SSHPort(), target.addr()) + } + jumpHost, jumpPort, _ := net.SplitHostPort(jump.addr()) + if want := "deploy@" + net.JoinHostPort(jumpHost, jumpPort); conn.SSHJump() != want { + t.Fatalf("SSHJump() = %q, want %q", conn.SSHJump(), want) + } +} + +func TestDirectConnectionReportsNoJump(t *testing.T) { + home, clientKey := sshTestHome(t) + target := newTestSSHServer(t, clientKey, false) + writeKnownHosts(t, home, target) + + conn, err := NewSSHContext(context.Background(), "root@"+target.addr()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if conn.SSHJump() != "" { + t.Fatalf("SSHJump() = %q, want empty", conn.SSHJump()) + } +} + +// A target that accepts the forwarded connection and then says nothing cannot +// be timed out with SetDeadline: an SSH channel rejects deadlines. Without an +// independent bound the CLI would wait on it forever, since an interactive +// context carries no deadline of its own. +func TestSilentTargetBehindAJumpIsBoundedByTheHandshakeTimeout(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + silent := newSilentListener(t) + writeKnownHosts(t, home, jump) + + previous := handshakeTimeout + handshakeTimeout = 200 * time.Millisecond + t.Cleanup(func() { handshakeTimeout = previous }) + + host, port, err := net.SplitHostPort(silent.Addr().String()) + if err != nil { + t.Fatal(err) + } + route := obtargetRoute(obtarget.Address{User: "root", Host: host, Port: port, ExplicitPort: true}, addressOf(t, jump, "deploy")) + + done := make(chan error, 1) + go func() { + _, err := NewSSHRoute(context.Background(), route) + done <- err + }() + select { + case err := <-done: + if err == nil { + t.Fatal("a silent target completed a handshake") + } + case <-time.After(10 * time.Second): + t.Fatal("connect to a silent target never returned") + } +} + +// newSilentListener accepts connections and never speaks. +func newSilentListener(t *testing.T) net.Listener { + t.Helper() + listener, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = listener.Close() }) + go func() { + for { + conn, err := listener.Accept() + if err != nil { + return + } + t.Cleanup(func() { _ = conn.Close() }) + } + }() + return listener +} + +// An IPv6 target is bracketed in known_hosts and in the dialled address but +// never in the SSH destination, and the two hops must agree on that. +func TestJumpAndTargetOnIPv6Loopback(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServerOn(t, "[::1]:0", clientKey, true) + target := newTestSSHServerOn(t, "[::1]:0", clientKey, false) + writeKnownHosts(t, home, jump, target) + + conn, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressOf(t, jump, "deploy"))) + if err != nil { + t.Fatalf("connect over IPv6: %v", err) + } + defer conn.Close() + if conn.Host() != "::1" { + t.Fatalf("Host() = %q, want the unbracketed literal", conn.Host()) + } + if forwarded := mustForward(t, jump); forwarded != target.addr() { + t.Fatalf("jump forwarded to %q, want %q", forwarded, target.addr()) + } +} + +// When the bastion itself stops answering, releasing the tunnel is only a +// message sent through the connection that has stopped answering. The hop-2 +// handshake must still be bounded, or `ob` waits on it forever. +func TestHandshakeIsBoundedWhenTheJumpStopsAnswering(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + proxy := newWedgeProxy(t, jump.addr()) + silent := newSilentListener(t) + writeKnownHostsFor(t, home, proxy.addr(), jump.hostKey.PublicKey()) + + previous := handshakeTimeout + handshakeTimeout = 500 * time.Millisecond + t.Cleanup(func() { handshakeTimeout = previous }) + + route := obtargetRoute(addressAt(t, silent.Addr().String(), "root"), addressAt(t, proxy.addr(), "deploy")) + done := make(chan error, 1) + go func() { + _, err := NewSSHRoute(context.Background(), route) + done <- err + }() + // The forward request proves hop 1 finished, so wedging now strands hop 2. + waitForForward(t, jump) + proxy.wedge() + + select { + case err := <-done: + if err == nil { + t.Fatal("connected through a jump host that stopped answering") + } + case <-time.After(15 * time.Second): + t.Fatal("connect never returned once the jump stopped answering") + } +} + +// The same connection, already established, must still be releasable: an +// operator pressing Ctrl-C has to get their terminal back. +func TestCloseReturnsWhenTheJumpStopsAnswering(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + target := newTestSSHServer(t, clientKey, false) + proxy := newWedgeProxy(t, jump.addr()) + writeKnownHostsFor(t, home, proxy.addr(), jump.hostKey.PublicKey()) + appendKnownHostsFor(t, home, target.addr(), target.hostKey.PublicKey()) + + conn, err := NewSSHRoute(context.Background(), obtargetRoute(addressOf(t, target, "root"), addressAt(t, proxy.addr(), "deploy"))) + if err != nil { + t.Fatal(err) + } + proxy.wedge() + + done := make(chan error, 1) + go func() { done <- conn.Close() }() + select { + case <-done: + case <-time.After(15 * time.Second): + t.Fatal("close never returned once the jump stopped answering") + } +} + +// A timeout is not an authentication failure, and the guide tells operators +// that "authenticate" means the server refused their key. +func TestTimeoutIsNotReportedAsAnAuthenticationFailure(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + silent := newSilentListener(t) + writeKnownHosts(t, home, jump) + + previous := handshakeTimeout + handshakeTimeout = 200 * time.Millisecond + t.Cleanup(func() { handshakeTimeout = previous }) + + _, err := NewSSHRoute(context.Background(), obtargetRoute(addressAt(t, silent.Addr().String(), "root"), addressOf(t, jump, "deploy"))) + if err == nil { + t.Fatal("a silent target completed a handshake") + } + if strings.Contains(err.Error(), "authenticate") { + t.Fatalf("a timeout is reported as an authentication failure: %v", err) + } + if !strings.HasPrefix(err.Error(), "target ssh ") { + t.Fatalf("error %q does not name the target hop", err) + } +} + +// A cancel that arrives mid-handshake must unblock it, not only one that +// arrives before the connection is attempted. +func TestCancelDuringTheTargetHandshakeReturns(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + silent := newSilentListener(t) + writeKnownHosts(t, home, jump) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { + _, err := NewSSHRoute(ctx, obtargetRoute(addressAt(t, silent.Addr().String(), "root"), addressOf(t, jump, "deploy"))) + done <- err + }() + waitForForward(t, jump) + cancel() + select { + case err := <-done: + if !errors.Is(err, context.Canceled) { + t.Fatalf("cancel mid-handshake = %v, want context.Canceled", err) + } + case <-time.After(15 * time.Second): + t.Fatal("cancel mid-handshake never returned") + } +} + +func waitForForward(t *testing.T, server *testSSHServer) { + t.Helper() + for range 400 { + if _, forwarded := server.stats(); len(forwarded) > 0 { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("the jump host was never asked to forward") +} + +// A deadline that expires mid-handshake must still say which hop was being +// established. The guide tells operators to read the hop off the front of the +// error, and a wedged bastion makes this the likeliest failure of all. +func TestExpiredDeadlineNamesTheHop(t *testing.T) { + home, clientKey := sshTestHome(t) + jump := newTestSSHServer(t, clientKey, true) + silent := newSilentListener(t) + writeKnownHosts(t, home, jump) + + ctx, cancel := context.WithTimeout(context.Background(), 750*time.Millisecond) + defer cancel() + _, err := NewSSHRoute(ctx, obtargetRoute(addressAt(t, silent.Addr().String(), "root"), addressOf(t, jump, "deploy"))) + if err == nil { + t.Fatal("a silent target completed a handshake") + } + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("error = %v, want a deadline failure", err) + } + if !strings.HasPrefix(err.Error(), "target ssh ") { + t.Fatalf("error %q does not name the target hop", err) + } +} diff --git a/internal/transport/sshserver_test.go b/internal/transport/sshserver_test.go new file mode 100644 index 00000000..638b2e97 --- /dev/null +++ b/internal/transport/sshserver_test.go @@ -0,0 +1,342 @@ +package transport + +import ( + "crypto/ed25519" + "crypto/rand" + "encoding/pem" + "errors" + "fmt" + "io" + "net" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/knownhosts" + + obtarget "github.com/labstack/onebox/internal/target" +) + +// testSSHServer is an in-process sshd: enough of one to exercise host-key +// verification, publickey auth, and direct-tcpip forwarding without depending +// on anything installed on the machine running the tests. +type testSSHServer struct { + t *testing.T + listener net.Listener + hostKey ssh.Signer + authorized ssh.PublicKey + + // forwardTo, when set, is dialled for every direct-tcpip channel instead + // of the address the client asked for — the fake bastion. + forward bool + + mu sync.Mutex + connections int + closed int + forwarded []string + rejectForwards bool +} + +func (s *testSSHServer) addr() string { return s.listener.Addr().String() } + +func (s *testSSHServer) stats() (connections int, forwarded []string) { + s.mu.Lock() + defer s.mu.Unlock() + return s.connections, append([]string(nil), s.forwarded...) +} + +// waitForClosed blocks until the server has seen want connections end, which is +// how a test observes that the client actually released the hop rather than +// leaving it open for the process to reap. +func (s *testSSHServer) waitForClosed(t *testing.T, want int) { + t.Helper() + for range 200 { + s.mu.Lock() + closed := s.closed + s.mu.Unlock() + if closed >= want { + return + } + time.Sleep(5 * time.Millisecond) + } + s.mu.Lock() + defer s.mu.Unlock() + t.Fatalf("server closed %d connections, want %d", s.closed, want) +} + +// newTestSSHServer starts a server that accepts only authorized and answers +// direct-tcpip when forward is set. +func newTestSSHServer(t *testing.T, authorized ssh.PublicKey, forward bool) *testSSHServer { + t.Helper() + return newTestSSHServerOn(t, "127.0.0.1:0", authorized, forward) +} + +// newTestSSHServerOn binds an explicit address so a test can exercise the IPv6 +// spelling the address grammar brackets. +func newTestSSHServerOn(t *testing.T, address string, authorized ssh.PublicKey, forward bool) *testSSHServer { + t.Helper() + hostKey := generateSigner(t) + listener, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", address) + if err != nil { + t.Skipf("cannot listen on %s: %v", address, err) + } + s := &testSSHServer{t: t, listener: listener, hostKey: hostKey, authorized: authorized, forward: forward} + t.Cleanup(func() { _ = listener.Close() }) + go s.serve() + return s +} + +func (s *testSSHServer) serve() { + for { + conn, err := s.listener.Accept() + if err != nil { + return + } + s.mu.Lock() + s.connections++ + s.mu.Unlock() + go s.handle(conn) + } +} + +func (s *testSSHServer) handle(conn net.Conn) { + defer func() { + _ = conn.Close() + s.mu.Lock() + s.closed++ + s.mu.Unlock() + }() + config := &ssh.ServerConfig{ + PublicKeyCallback: func(_ ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { + if s.authorized != nil && string(key.Marshal()) == string(s.authorized.Marshal()) { + return &ssh.Permissions{}, nil + } + return nil, errors.New("unauthorized key") + }, + } + config.AddHostKey(s.hostKey) + serverConn, channels, requests, err := ssh.NewServerConn(conn, config) + if err != nil { + return + } + defer serverConn.Close() + go ssh.DiscardRequests(requests) + for channel := range channels { + switch channel.ChannelType() { + case "direct-tcpip": + s.handleForward(channel) + default: + _ = channel.Reject(ssh.UnknownChannelType, channel.ChannelType()) + } + } +} + +func (s *testSSHServer) handleForward(request ssh.NewChannel) { + var payload struct { + Host string + Port uint32 + Orig string + Oport uint32 + } + if err := ssh.Unmarshal(request.ExtraData(), &payload); err != nil { + _ = request.Reject(ssh.ConnectionFailed, "bad payload") + return + } + destination := net.JoinHostPort(payload.Host, fmt.Sprint(payload.Port)) + s.mu.Lock() + s.forwarded = append(s.forwarded, destination) + reject := s.rejectForwards || !s.forward + s.mu.Unlock() + if reject { + _ = request.Reject(ssh.ConnectionFailed, "administratively prohibited") + return + } + upstream, err := (&net.Dialer{}).DialContext(s.t.Context(), "tcp", destination) + if err != nil { + _ = request.Reject(ssh.ConnectionFailed, err.Error()) + return + } + channel, requests, err := request.Accept() + if err != nil { + upstream.Close() + return + } + go ssh.DiscardRequests(requests) + // Either direction ending tears down both, the way a real sshd propagates + // a channel close to the forwarded connection. Closing only the direction + // that ended would leave the far end blocked on a read that never returns. + teardown := func() { + _ = channel.Close() + _ = upstream.Close() + } + go func() { + defer teardown() + _, _ = io.Copy(upstream, channel) + }() + go func() { + defer teardown() + _, _ = io.Copy(channel, upstream) + }() +} + +func generateSigner(t *testing.T) ssh.Signer { + t.Helper() + _, key, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + signer, err := ssh.NewSignerFromKey(key) + if err != nil { + t.Fatal(err) + } + return signer +} + +// sshTestHome points the transport's fixed ~/.ssh lookups at a temporary +// directory holding the client key and the known_hosts under test. +func sshTestHome(t *testing.T) (home string, clientKey ssh.PublicKey) { + t.Helper() + home = t.TempDir() + if err := os.MkdirAll(filepath.Join(home, ".ssh"), 0o700); err != nil { + t.Fatal(err) + } + _, private, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + der, err := ssh.MarshalPrivateKey(private, "") + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(home, ".ssh", "id_ed25519"), pem.EncodeToMemory(der), 0o600); err != nil { + t.Fatal(err) + } + signer, err := ssh.NewSignerFromKey(private) + if err != nil { + t.Fatal(err) + } + t.Setenv("HOME", home) + t.Setenv("SSH_AUTH_SOCK", "") + return home, signer.PublicKey() +} + +// writeKnownHostsFor pins one key at an arbitrary address, for a server reached +// through something other than its own listener. +func writeKnownHostsFor(t *testing.T, home, address string, key ssh.PublicKey) { + t.Helper() + line := knownhosts.Line([]string{knownhosts.Normalize(address)}, key) + "\n" + if err := os.WriteFile(filepath.Join(home, ".ssh", "known_hosts"), []byte(line), 0o600); err != nil { + t.Fatal(err) + } +} + +func appendKnownHostsFor(t *testing.T, home, address string, key ssh.PublicKey) { + t.Helper() + path := filepath.Join(home, ".ssh", "known_hosts") + existing, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + line := knownhosts.Line([]string{knownhosts.Normalize(address)}, key) + "\n" + if err := os.WriteFile(path, append(existing, line...), 0o600); err != nil { + t.Fatal(err) + } +} + +func addressAt(t *testing.T, address, user string) obtarget.Address { + t.Helper() + host, port, err := net.SplitHostPort(address) + if err != nil { + t.Fatal(err) + } + return obtarget.Address{User: user, Host: host, Port: port, ExplicitPort: true} +} + +// writeKnownHosts pins each server at its own listening address, which is what +// the transport verifies against. +func writeKnownHosts(t *testing.T, home string, servers ...*testSSHServer) { + t.Helper() + var lines string + for _, server := range servers { + lines += knownhosts.Line([]string{knownhosts.Normalize(server.addr())}, server.hostKey.PublicKey()) + "\n" + } + if err := os.WriteFile(filepath.Join(home, ".ssh", "known_hosts"), []byte(lines), 0o600); err != nil { + t.Fatal(err) + } +} + +func addressOf(t *testing.T, server *testSSHServer, user string) obtarget.Address { + t.Helper() + host, port, err := net.SplitHostPort(server.addr()) + if err != nil { + t.Fatal(err) + } + return obtarget.Address{User: user, Host: host, Port: port, ExplicitPort: true} +} + +// wedgeProxy forwards TCP to an upstream until it is wedged, after which it +// silently stops moving bytes in both directions without closing anything. +// That is what an unresponsive bastion looks like from the client: writes are +// accepted by the kernel, reads never complete, and no close is ever +// acknowledged. It is the only way to exercise the paths where releasing an +// SSH channel is not enough, because a channel close is just a message sent +// through the very connection that has stopped answering. +type wedgeProxy struct { + listener net.Listener + wedged chan struct{} + once sync.Once +} + +func newWedgeProxy(t *testing.T, upstream string) *wedgeProxy { + t.Helper() + listener, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + p := &wedgeProxy{listener: listener, wedged: make(chan struct{})} + t.Cleanup(func() { _ = listener.Close() }) + go func() { + for { + downstream, err := listener.Accept() + if err != nil { + return + } + up, err := (&net.Dialer{}).DialContext(t.Context(), "tcp", upstream) + if err != nil { + _ = downstream.Close() + return + } + go p.pump(downstream, up) + go p.pump(up, downstream) + } + }() + return p +} + +func (p *wedgeProxy) addr() string { return p.listener.Addr().String() } + +func (p *wedgeProxy) wedge() { p.once.Do(func() { close(p.wedged) }) } + +func (p *wedgeProxy) pump(from, to net.Conn) { + buf := make([]byte, 32*1024) + for { + n, err := from.Read(buf) + if n > 0 { + select { + case <-p.wedged: + var never chan struct{} + <-never // parked: these bytes, and every later one, are never delivered + default: + } + if _, werr := to.Write(buf[:n]); werr != nil { + return + } + } + if err != nil { + return + } + } +} diff --git a/internal/transport/transport.go b/internal/transport/transport.go index 2a734675..5534d576 100644 --- a/internal/transport/transport.go +++ b/internal/transport/transport.go @@ -18,8 +18,14 @@ import ( "strings" "github.com/labstack/onebox/internal/shellquote" + obtarget "github.com/labstack/onebox/internal/target" ) +// Route is the connection a transport dials: a target and, optionally, the one +// jump host it is reached through. Aliased here so callers that only speak +// "transport" need not import the address grammar to name a destination. +type Route = obtarget.Route + type Result struct { Stdout string Stderr string @@ -45,6 +51,10 @@ type Transport interface { // SSHUser is the resolved SSH username, exposed separately for tools whose // remote-spec grammar differs across implementations (notably IPv6 rsync). SSHUser() string + // SSHJump names the jump host the connection is tunnelled through, empty + // on a direct connection. Local hooks run without that tunnel, so they are + // given the bastion rather than a target they cannot reach. + SSHJump() string // SSHPort is the server's SSH port, or empty when SSH is not applicable. SSHPort() string Close() error @@ -125,6 +135,7 @@ func (l *Local) Upload(ctx context.Context, localDir, remoteDir string) error { func (l *Local) Host() string { return "local" } func (l *Local) Destination() string { return "local" } +func (l *Local) SSHJump() string { return "" } func (l *Local) SSHUser() string { return "" } func (l *Local) SSHPort() string { return "" } func (l *Local) Close() error { return nil } diff --git a/site/astro.config.mjs b/site/astro.config.mjs index 14bd371a..6391dbec 100644 --- a/site/astro.config.mjs +++ b/site/astro.config.mjs @@ -93,6 +93,7 @@ export default defineConfig({ { label: "Run migrations safely", slug: "guides/run-migrations" }, { label: "Schedule a job", slug: "guides/schedule-a-job" }, { label: "Roll back a release", slug: "guides/roll-back" }, + { label: "Deploy through a jump host", slug: "guides/deploy-through-a-jump-host" }, { label: "Adopt an existing Compose file", slug: "guides/adopt-compose" }, { label: "Eject", slug: "guides/eject" }, ], diff --git a/site/public/onebox.run-v1.schema.json b/site/public/onebox.run-v1.schema.json index 098148bf..3c4c8da5 100644 --- a/site/public/onebox.run-v1.schema.json +++ b/site/public/onebox.run-v1.schema.json @@ -641,6 +641,48 @@ }, "type": "array" }, + "jump": { + "anyOf": [ + { + "type": "string" + }, + { + "additionalProperties": false, + "description": "Optional SSH jump host tunnelling the connection to this server, written as user@host or as an object with host, user, and port. Onebox verifies and authenticates both hops and never forwards the SSH agent.", + "examples": [ + "deploy@bastion.example.com" + ], + "patternProperties": { + "^x-": {} + }, + "properties": { + "host": { + "description": "Jump host name or IP address.", + "examples": [ + "bastion.example.com" + ], + "type": "string" + }, + "port": { + "description": "SSH port on the jump host. The SSH default is used when omitted.", + "examples": [ + 2222 + ], + "type": "integer" + }, + "user": { + "description": "SSH user on the jump host. $USER is used when omitted; ob does not read ~/.ssh/config.", + "examples": [ + "deploy" + ], + "type": "string" + } + }, + "type": "object" + } + ], + "description": "Optional SSH jump host tunnelling the connection to this server, written as user@host or as an object with host, user, and port. Onebox verifies and authenticates both hops and never forwards the SSH agent. Also accepts user@host or user@host:port." + }, "overrides": { "additionalProperties": false, "description": "Environment-specific operational tuning. Overrides cannot change workload identity or data semantics.", @@ -772,7 +814,7 @@ "type": "integer" }, "user": { - "description": "SSH user. The local SSH configuration supplies it when omitted.", + "description": "SSH user. $USER is used when omitted; ob does not read ~/.ssh/config.", "examples": [ "root" ], diff --git a/site/src/content/docs/guides/deploy-through-a-jump-host.mdx b/site/src/content/docs/guides/deploy-through-a-jump-host.mdx new file mode 100644 index 00000000..7f9190cc --- /dev/null +++ b/site/src/content/docs/guides/deploy-through-a-jump-host.mdx @@ -0,0 +1,143 @@ +--- +title: Deploy through a jump host +description: Reach a private server through one bastion, with both hops verified. +summary: How the optional jump field works, what Onebox verifies on each hop, and how to read a failure by the stage it happened in. +sidebar: + order: 8 +read_when: + - "The deploy target has no public SSH port" + - "SSH access to the server goes through a bastion or management host" +--- + +A deployment target does not need a public SSH port. Name the bastion it is +reached through, and Onebox tunnels the connection through it: + +```yaml +environments: + production: + server: root@10.20.0.10 + jump: deploy@bastion.example.com +``` + +`jump` sits beside `server` and takes the same two forms it does — the one-line +`user@host` above, or an object when you want to be explicit: + +```yaml +environments: + production: + server: root@10.20.0.10 + jump: + host: bastion.example.com + user: deploy + port: 2222 +``` + +The user and port are optional in both forms: port 22 is used when you name +none, and `$USER` supplies the user — `ob` does not read `~/.ssh/config`, so a +`User` or `ProxyJump` written there has no effect on it. An environment with no +`jump` connects directly, exactly as before. + +An IPv6 bastion is bracketed in the one-line form, where the brackets are what +separate the address from the port, and bare in the object form, where `host` +and `port` are already separate fields. Brackets carried over into `host` are +stripped rather than refused: + +```yaml + jump: "deploy@[2001:db8::1]:2222" + # or + jump: { host: "2001:db8::1", user: deploy, port: 2222 } +``` + +Nothing is deployed to the jump host. It forwards one TCP connection to the +server and runs no commands, holds no releases, and needs no Docker. Onebox +remains one application on one host; the bastion is only how that host is +reached. + +## What Onebox verifies + +Both hops are verified and authenticated separately: + +1. The bastion's host key is checked against `known_hosts`. +2. Onebox authenticates to the bastion. +3. The bastion opens a forwarded connection to the server. +4. The **server's** host key is checked against `known_hosts`, independently. +5. Onebox authenticates to the server. + +A trusted bastion does not vouch for the server. Trusting one is not trusting +the other, and neither key is ever accepted implicitly. + +The SSH agent is never forwarded. Your local agent may sign for either hop, but +its socket is not exposed to the bastion, so a compromised jump host cannot +borrow your identity to reach anything else. + +Exactly one hop is supported. There is no `ProxyCommand`, and Onebox does not +read `~/.ssh/config` — a `ProxyJump` there has no effect on `ob`, because the +transport dials and verifies on its own rather than shelling out to `ssh`. + +## Enrolling both host keys + +Because each hop is verified, `known_hosts` needs an entry for each. The +bastion is reachable from your machine: + +```sh +ssh-keyscan -H bastion.example.com >> ~/.ssh/known_hosts +``` + +The server is not, so scan it from the bastion and append the result locally: + +```sh +ssh deploy@bastion.example.com 'ssh-keyscan -H 10.20.0.10' >> ~/.ssh/known_hosts +``` + +Read that second key the way you would any key you did not fetch yourself: it +arrives over a connection the bastion mediates, so it is only as trustworthy as +the bastion at the moment you enrolled it. Comparing it against the key printed +on the server's own console is the stronger move where you can. + +If the server listens on a non-default port, `known_hosts` must be keyed with +it — `ssh-keyscan -p 2222` writes the `[host]:port` form Onebox looks up. + +## The route is part of the plan + +Plans and approvals name the whole route, not just the server: + +``` +target root@10.20.0.10 via deploy@bastion.example.com:2222 +``` + +Changing the bastion changes what you approved, so an approval issued for one +route does not execute against another. Confirm the route on that line before +approving, exactly as you would the server. + +## Reading a failure + +Errors name the hop and the stage, so a failure points at one thing to fix: + +| Error begins | What failed | +|---|---| +| `jump ssh …:` (no stage) | The bastion could not be resolved, connected to, or finished a handshake with — including a timeout | +| `jump ssh …: host key:` | The bastion's key is missing from, or disagrees with, `known_hosts` | +| `jump ssh …: authenticate:` | The bastion refused your key | +| `target ssh …: not reachable from the jump host:` | The bastion connected but could not reach the server — wrong private address, or the bastion's own policy forbids the forward | +| `target ssh …: host key:` | The **server's** key is missing or mismatched, even though the bastion is trusted | +| `target ssh …: authenticate:` | The server refused your key | + +A failure that names no stage is a transport problem — a timeout, a cancelled +command, or a peer that hung up — never a rejected key. + +The distinction between the last four is the one worth internalising: a working +bastion tells you nothing about the server, and Onebox will not let a trusted +first hop paper over a problem with the second. + +## Local hooks + +A hook declared `local: true` runs on your machine, which has no tunnel of its +own. Such a hook receives `OB_SSH_JUMP` alongside `OB_SERVER`, empty when the +connection is direct: + +```sh +ssh ${OB_SSH_JUMP:+-J "$OB_SSH_JUMP"} "$OB_SERVER" -p "$OB_SSH_PORT" 'uptime' +``` + +A local hook written before you added a bastion will otherwise try to reach a +server it can no longer see. diff --git a/site/src/content/docs/reference/fields/environments.mdx b/site/src/content/docs/reference/fields/environments.mdx index 66d93866..06207b38 100644 --- a/site/src/content/docs/reference/fields/environments.mdx +++ b/site/src/content/docs/reference/fields/environments.mdx @@ -23,7 +23,7 @@ cannot drift from what `ob validate` accepts. ## Fields on this page -`allow_agent_proposals` · `backup_key_material` · `backup_max_age` · `base_path` · `env_files` · `file` · `host` · `migrations` · `min_onebox_version` · `min_plan_schema` · `overrides` · `policy` · `port` · `provider` · `require_approval` · `require_backup` · `require_restore_test` · `server` · `services` · `user` · `workloads` +`allow_agent_proposals` · `backup_key_material` · `backup_max_age` · `base_path` · `env_files` · `file` · `host` · `jump` · `migrations` · `min_onebox_version` · `min_plan_schema` · `overrides` · `policy` · `port` · `provider` · `require_approval` · `require_backup` · `require_restore_test` · `server` · `services` · `user` · `workloads` ## Reference @@ -33,6 +33,10 @@ cannot drift from what `ob validate` accepts. | `.env_files` | list | — | Default ordered environment-file list for application, worker, and job workloads in this environment. | | `.env_files[].file` `*` | string | — | Repository-relative environment file path. Expects a path inside the repository, with no control character or shell metacharacter. | | `.env_files[].provider` | `sops` | — | Decryptor used before staging the file. The supported encrypted provider is sops. | +| `.jump` | object | — | Optional SSH jump host tunnelling the connection to this server, written as user@host or as an object with host, user, and port. Onebox verifies and authenticates both hops and never forwards the SSH agent. Also accepts user@host or user@host:port. | +| `.jump.host` | string | — | Jump host name or IP address. | +| `.jump.port` | integer | — | SSH port on the jump host. The SSH default is used when omitted. | +| `.jump.user` | string | — | SSH user on the jump host. $USER is used when omitted; ob does not read ~/.ssh/config. | | `.overrides` | object | — | Environment-specific operational tuning. Overrides cannot change workload identity or data semantics. | | `.overrides.services` | map | — | Allowed service tuning keyed by service name: resources and settings. | | `.overrides.workloads` | map | — | Allowed workload tuning keyed by workload name: replicas, resources, env, env_files, strategy, and routes. | @@ -49,6 +53,6 @@ cannot drift from what `ob validate` accepts. | `.server` | object | — | SSH server, written as user@host or as an object with host, user, and port. Also accepts user@host. | | `.server.host` | string | — | SSH hostname or IP address. | | `.server.port` | integer | — | SSH port. The SSH default is used when omitted. | -| `.server.user` | string | — | SSH user. The local SSH configuration supplies it when omitted. | +| `.server.user` | string | — | SSH user. $USER is used when omitted; ob does not read ~/.ssh/config. | `*` marks a field that is required within its own object. diff --git a/site/src/content/docs/reference/project-file.mdx b/site/src/content/docs/reference/project-file.mdx index 82713fb2..28873c60 100644 --- a/site/src/content/docs/reference/project-file.mdx +++ b/site/src/content/docs/reference/project-file.mdx @@ -63,6 +63,7 @@ not convenience that might be withdrawn. | `image: nginx` | `image: {reference: nginx}` | | `health: /healthz` | `health: {http: /healthz}` | | `server: root@203.0.113.10` | `server: {user: root, host: 203.0.113.10}` | +| `jump: deploy@bastion.example.com:2222` | `jump: {user: deploy, host: bastion.example.com, port: 2222}` | | `needs: [postgres]` | `needs: [{name: postgres}]` | | `services: {postgres: 17}` | `services: {postgres: {version: 17}}` | | `env_files: [.env]` | `env_files: [{file: .env}]` | diff --git a/site/src/content/docs/start/first-deploy.mdx b/site/src/content/docs/start/first-deploy.mdx index 3aa6c35f..b88b32b3 100644 --- a/site/src/content/docs/start/first-deploy.mdx +++ b/site/src/content/docs/start/first-deploy.mdx @@ -23,6 +23,9 @@ You need: - one Linux server reachable over SSH, with Docker available to that SSH account - the server's host key already recorded in `known_hosts` +If the server has no public SSH port, name the bastion it is reached through — +see [Deploy through a jump host](/guides/deploy-through-a-jump-host/). + The first three steps are local and contact nothing. `ob bootstrap` is the first command that changes the server; the plan after it is read-only.