Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions apps/ingest/internal/db/queries/terminal_sessions.sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,55 @@ func ValidateAndActivateTerminalSession(ctx context.Context, pool *pgxpool.Pool,
AND h.deleted_at IS NULL
RETURNING ts.instance_id, ts.host_id,
ts.user_id,
COALESCE(NULLIF(h.hostname, ''), h.ip_addresses->>0) AS host,
COALESCE(h.hostname, '') AS hostname,
COALESCE(h.ip_addresses, '[]'::jsonb)::text AS ip_addresses,
COALESCE(ts.username, '') AS username,
COALESCE((o.metadata->>'terminalLoggingEnabled')::boolean, false) AS logging_enabled
`
var info TerminalSessionInfo
var hostname string
var rawIPAddresses string
err := pool.QueryRow(ctx, q, sessionID, tokenHash).Scan(
&info.InstanceID,
&info.HostID,
&info.UserID,
&info.Host,
&hostname,
&rawIPAddresses,
&info.Username,
&info.LoggingEnabled,
)
if err != nil {
return nil, fmt.Errorf("terminal session not found or expired: %w", err)
}
info.Host = terminalSSHTarget(hostname, terminalIPAddressesFromJSON(rawIPAddresses))
return &info, nil
}

func terminalIPAddressesFromJSON(raw string) []string {
var ips []string
if err := json.Unmarshal([]byte(raw), &ips); err != nil {
return nil
}
return ips
}

func terminalSSHTarget(hostname string, ipAddresses []string) string {
if useful := FilterHostIdentityIPs(ipAddresses); len(useful) > 0 {
return useful[0]
Comment on lines +88 to +89
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve fallback targets when choosing SSH host

Returning only useful[0] makes terminal connection success depend on the first reported IP, with no retry to hostname or other candidate IPs. For multihomed hosts (for example IPv6 reported before IPv4, or VPN/LAN address ordering), the first IP can be unreachable from ingest, and openSSHSession then performs a single ssh.Dial to that literal address and fails even though another recorded address (or the hostname) would work. This is a regression from the previous hostname-first behavior and can block terminal access for affected hosts.

Useful? React with 👍 / 👎.

}

if hostname = strings.TrimSpace(hostname); hostname != "" {
return hostname
}

for _, ip := range ipAddresses {
if ip = strings.TrimSpace(ip); ip != "" {
return ip
}
}
return ""
}

const (
terminalAuthWindow = 15 * time.Minute
terminalAuthMaxFailures = 5
Expand Down
24 changes: 24 additions & 0 deletions apps/ingest/internal/db/queries/terminal_sessions_target_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package queries

import "testing"

func TestTerminalSSHTargetPrefersUsefulIPAddress(t *testing.T) {
got := terminalSSHTarget("ct-ops", []string{"172.17.0.1", "192.168.1.42"})
if got != "192.168.1.42" {
t.Fatalf("terminalSSHTarget() = %q, want first useful IP", got)
}
}

func TestTerminalSSHTargetFallsBackToHostname(t *testing.T) {
got := terminalSSHTarget("ct-ops", []string{"127.0.0.1", "172.17.0.1"})
if got != "ct-ops" {
t.Fatalf("terminalSSHTarget() = %q, want hostname fallback", got)
}
}

func TestTerminalSSHTargetUsesFirstIPWhenHostnameMissing(t *testing.T) {
got := terminalSSHTarget("", []string{"127.0.0.1", "172.17.0.1"})
if got != "127.0.0.1" {
t.Fatalf("terminalSSHTarget() = %q, want first recorded IP fallback", got)
}
}
21 changes: 21 additions & 0 deletions apps/ingest/internal/handlers/terminal_ws.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"strconv"
"strings"
"sync"
"syscall"
"time"

"github.com/coder/websocket"
Expand Down Expand Up @@ -378,6 +379,26 @@ func terminalSSHFailureDetails(err error) (reason string, message string) {
if isSSHAuthenticationFailure(err) {
return "ssh authentication failed", "SSH authentication failed"
}

var dnsErr *net.DNSError
if errors.As(err, &dnsErr) || strings.Contains(strings.ToLower(err.Error()), "no such host") {
return "ssh connection failed", "SSH connection failed: host name could not be resolved"
}

var netErr net.Error
if errors.Is(err, context.DeadlineExceeded) ||
errors.Is(err, syscall.ETIMEDOUT) ||
(errors.As(err, &netErr) && netErr.Timeout()) {
return "ssh connection failed", "SSH connection failed: connection timed out"
}

if errors.Is(err, syscall.ECONNREFUSED) {
return "ssh connection failed", "SSH connection failed: connection refused"
}
if errors.Is(err, syscall.EHOSTUNREACH) || errors.Is(err, syscall.ENETUNREACH) {
return "ssh connection failed", "SSH connection failed: host is unreachable"
}

return "ssh connection failed", "SSH connection failed"
}

Expand Down
16 changes: 14 additions & 2 deletions apps/ingest/internal/handlers/terminal_ws_security_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package handlers

import (
"context"
"errors"
"os"
"path/filepath"
"reflect"
"runtime"
"strings"
"syscall"
"testing"

"golang.org/x/crypto/ssh"
Expand Down Expand Up @@ -97,8 +99,18 @@ func TestTerminalSSHFailureDetails(t *testing.T) {
}

reason, message = terminalSSHFailureDetails(errors.New("dial tcp: lookup host.example.test: no such host"))
if reason != "ssh connection failed" || message != "SSH connection failed" {
t.Fatalf("terminalSSHFailureDetails(network) = %q, %q", reason, message)
if reason != "ssh connection failed" || message != "SSH connection failed: host name could not be resolved" {
t.Fatalf("terminalSSHFailureDetails(dns) = %q, %q", reason, message)
}

reason, message = terminalSSHFailureDetails(syscall.ECONNREFUSED)
if reason != "ssh connection failed" || message != "SSH connection failed: connection refused" {
t.Fatalf("terminalSSHFailureDetails(refused) = %q, %q", reason, message)
}

reason, message = terminalSSHFailureDetails(context.DeadlineExceeded)
if reason != "ssh connection failed" || message != "SSH connection failed: connection timed out" {
t.Fatalf("terminalSSHFailureDetails(timeout) = %q, %q", reason, message)
}
}

Expand Down
Loading