Follow same-host https redirects when dialing the TrueNAS websocket

The v6.1.2 JSON-RPC migration (b81ba7dd0) broke http-configured appliances sitting behind TrueNAS's HTTP -> HTTPS redirect: the REST transport followed the redirect transparently, but a websocket handshake cannot, so every poll failed with status=302 bad handshake. When the plaintext handshake answers with a 3xx whose Location is an https URL on the same host, retry once over TLS (honouring the configured skip-verify/fingerprint settings) and keep the upgraded wss endpoint for the client's lifetime. Cross-host and downgrade redirects are refused with an actionable error naming the target, as is a redirect whose TLS retry fails. Fixes #1631 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-10 02:25:56 +00:00 · 2026-07-27 09:07:08 +01:00
parent fe33d23210
commit bf24a9a9cf
3 changed files with 220 additions and 4 deletions
@@ -2120,10 +2120,49 @@ func (c *Client) dialRPC(ctx context.Context) (*websocket.Conn, error) {
 	if c == nil {
 		return nil, fmt.Errorf("truenas client is nil")
 	}
+	conn, err := c.dialRPCEndpoint(ctx, c.rpcURL, c.config.UseHTTPS)
+	if err == nil {
+		return conn, nil
+	}
+
+	// A plaintext handshake that answers with a redirect is almost always
+	// TrueNAS's HTTP -> HTTPS redirect (or an equivalent proxy) in front of
+	// the middleware. The REST transport used before v6.1.2 followed that
+	// redirect transparently, so http-configured appliances worked; a
+	// websocket handshake cannot follow it (issue #1631). When the redirect
+	// stays on the same host and upgrades to https, retry once over TLS and
+	// keep the upgraded endpoint for the rest of the client's lifetime.
+	var handshake *RPCHandshakeError
+	if !errors.As(err, &handshake) || !isRedirectStatus(handshake.StatusCode) {
+		return nil, err
+	}
+	upgraded, ok := httpsUpgradeTarget(c.rpcURL, handshake.Location)
+	if !ok {
+		if strings.TrimSpace(handshake.Location) != "" {
+			return nil, fmt.Errorf("truenas websocket endpoint redirected to %q; update the configured TrueNAS host to the address it redirects to: %w", handshake.Location, err)
+		}
+		return nil, fmt.Errorf("truenas websocket endpoint answered the handshake with a redirect; update the configured TrueNAS host to its https address: %w", err)
+	}
+
+	conn, retryErr := c.dialRPCEndpoint(ctx, upgraded, true)
+	if retryErr != nil {
+		return nil, fmt.Errorf("truenas endpoint redirects to https but the TLS retry against %s failed; configure the https endpoint explicitly and pin the certificate fingerprint or enable skip-verify for self-signed certificates: %w", upgraded, retryErr)
+	}
+
+	c.rpcURL = upgraded
+	c.config.UseHTTPS = true
+	c.updateTransportStatus(func(status *TransportStatus) {
+		status.Endpoint = upgraded
+		status.TLS = true
+	})
+	return conn, nil
+}
+
+func (c *Client) dialRPCEndpoint(ctx context.Context, rpcURL string, useTLS bool) (*websocket.Conn, error) {
 	dialer := websocket.Dialer{
 		Proxy: http.ProxyFromEnvironment,
 	}
-	if c.config.UseHTTPS {
+	if useTLS {
 		tlsConfig, err := buildTLSConfig(c.config.InsecureSkipVerify, c.config.Fingerprint)
 		if err != nil {
 			return nil, err
@@ -2137,17 +2176,55 @@ func (c *Client) dialRPC(ctx context.Context) (*websocket.Conn, error) {
 		}
 	}

-	conn, response, err := dialer.DialContext(ctx, c.rpcURL, nil)
+	conn, response, err := dialer.DialContext(ctx, rpcURL, nil)
 	if err != nil {
 		statusCode := 0
+		location := ""
 		if response != nil {
 			statusCode = response.StatusCode
+			location = response.Header.Get("Location")
 		}
-		return nil, &RPCHandshakeError{StatusCode: statusCode, Err: err}
+		return nil, &RPCHandshakeError{StatusCode: statusCode, Location: location, Err: err}
 	}
 	return conn, nil
 }

+func isRedirectStatus(statusCode int) bool {
+	switch statusCode {
+	case http.StatusMovedPermanently, http.StatusFound, http.StatusSeeOther, http.StatusTemporaryRedirect, http.StatusPermanentRedirect:
+		return true
+	default:
+		return false
+	}
+}
+
+// httpsUpgradeTarget maps a redirect answered to a plaintext ws:// handshake
+// onto the equivalent wss:// endpoint. Only a same-host upgrade to https is
+// accepted: scheme downgrades and cross-host redirects never are, so a
+// redirect can move the connection to TLS but never to another appliance.
+func httpsUpgradeTarget(currentURL, location string) (string, bool) {
+	current, err := url.Parse(currentURL)
+	if err != nil || current.Scheme != "ws" {
+		return "", false
+	}
+	target, err := url.Parse(strings.TrimSpace(location))
+	if err != nil || target.Hostname() == "" {
+		return "", false
+	}
+	scheme := strings.ToLower(target.Scheme)
+	if scheme != "https" && scheme != "wss" {
+		return "", false
+	}
+	if !strings.EqualFold(target.Hostname(), current.Hostname()) {
+		return "", false
+	}
+	port := target.Port()
+	if port == "" {
+		port = "443"
+	}
+	return fmt.Sprintf("wss://%s/api/current", net.JoinHostPort(target.Hostname(), port)), true
+}
+
 func (c *trueNASRPCClient) authenticate(ctx context.Context, config ClientConfig) (string, error) {
 	if apiKey := strings.TrimSpace(config.APIKey); apiKey != "" {
 		if username := strings.TrimSpace(config.Username); username != "" {
@@ -0,0 +1,136 @@
+package truenas
+
+// Pins the issue #1631 regression: the v6.1.2 JSON-RPC migration broke
+// http-configured appliances behind TrueNAS's HTTP -> HTTPS redirect. The
+// REST transport used before followed the redirect transparently; the
+// websocket handshake fails with status=302 instead, so the client must
+// upgrade a same-host https redirect to wss itself, and refuse anything
+// that would move the connection to another host.
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+)
+
+func newHTTPRedirectServer(t *testing.T, location func() string, status int) *httptest.Server {
+	t.Helper()
+	server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+		http.Redirect(writer, request, location(), status)
+	}))
+	t.Cleanup(server.Close)
+	return server
+}
+
+func redirectFixtureClient(t *testing.T, redirectURL string) *Client {
+	t.Helper()
+	parsed, err := url.Parse(redirectURL)
+	if err != nil {
+		t.Fatalf("parse redirect server URL %q: %v", redirectURL, err)
+	}
+	port, err := strconv.Atoi(parsed.Port())
+	if err != nil {
+		t.Fatalf("parse redirect server port from %q: %v", redirectURL, err)
+	}
+	client, err := NewClient(ClientConfig{
+		Host:               "http://" + parsed.Hostname(),
+		Port:               port,
+		Username:           "pulse-readonly",
+		APIKey:             "readonly-key",
+		InsecureSkipVerify: true,
+		Timeout:            5 * time.Second,
+	})
+	if err != nil {
+		t.Fatalf("NewClient() error = %v", err)
+	}
+	t.Cleanup(client.Close)
+	return client
+}
+
+func TestIssue1631HTTPRedirectUpgradesHandshakeToTLS(t *testing.T) {
+	fixture := newProtocolFixture(t, func(_ int, request trueNASRPCRequest) protocolFixtureReply {
+		if request.Method == "auth.login_ex" {
+			return protocolFixtureReply{result: map[string]any{"response_type": "SUCCESS", "user_info": nil}}
+		}
+		t.Errorf("unexpected rpc method %q", request.Method)
+		return protocolFixtureReply{close: true}
+	}, nil)
+
+	redirect := newHTTPRedirectServer(t, func() string {
+		return fixture.server.URL + "/api/current"
+	}, http.StatusFound)
+	client := redirectFixtureClient(t, redirect.URL)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	mode, err := client.ensureTransport(ctx)
+	if err != nil {
+		t.Fatalf("ensureTransport() error = %v", err)
+	}
+	if mode != TransportJSONRPC {
+		t.Fatalf("ensureTransport() mode = %s, want %s", mode, TransportJSONRPC)
+	}
+
+	status := client.TransportStatus()
+	if !status.TLS {
+		t.Errorf("TransportStatus().TLS = false, want true after https upgrade")
+	}
+	if !strings.HasPrefix(status.Endpoint, "wss://") {
+		t.Errorf("TransportStatus().Endpoint = %q, want wss:// endpoint", status.Endpoint)
+	}
+	if !client.config.UseHTTPS {
+		t.Errorf("config.UseHTTPS = false, want true after https upgrade")
+	}
+	if fixture.sessions.Load() == 0 {
+		t.Errorf("TLS fixture saw no websocket sessions; upgrade retry never landed")
+	}
+}
+
+func TestIssue1631CrossHostRedirectIsRefused(t *testing.T) {
+	redirect := newHTTPRedirectServer(t, func() string {
+		return "https://other-appliance.example/api/current"
+	}, http.StatusFound)
+	client := redirectFixtureClient(t, redirect.URL)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	_, err := client.ensureTransport(ctx)
+	if err == nil {
+		t.Fatal("ensureTransport() error = nil, want cross-host redirect refusal")
+	}
+	if !strings.Contains(err.Error(), "redirected to") || !strings.Contains(err.Error(), "other-appliance.example") {
+		t.Errorf("ensureTransport() error = %q, want actionable message naming the redirect target", err)
+	}
+	if status := client.TransportStatus(); status.TLS {
+		t.Errorf("TransportStatus().TLS = true, want false when redirect is refused")
+	}
+}
+
+func TestIssue1631HTTPSUpgradeTargetRules(t *testing.T) {
+	cases := []struct {
+		name     string
+		current  string
+		location string
+		want     string
+		ok       bool
+	}{
+		{"same host default port", "ws://nas.local:80/api/current", "https://nas.local/ui/", "wss://nas.local:443/api/current", true},
+		{"same host explicit port", "ws://nas.local:80/api/current", "https://nas.local:8443/", "wss://nas.local:8443/api/current", true},
+		{"cross host", "ws://nas.local:80/api/current", "https://evil.example/", "", false},
+		{"relative location", "ws://nas.local:80/api/current", "/ui/", "", false},
+		{"downgrade from wss", "wss://nas.local:443/api/current", "https://nas.local/", "", false},
+		{"http location", "ws://nas.local:80/api/current", "http://nas.local:8080/", "", false},
+		{"empty location", "ws://nas.local:80/api/current", "", "", false},
+	}
+	for _, tc := range cases {
+		got, ok := httpsUpgradeTarget(tc.current, tc.location)
+		if ok != tc.ok || got != tc.want {
+			t.Errorf("%s: httpsUpgradeTarget(%q, %q) = (%q, %v), want (%q, %v)", tc.name, tc.current, tc.location, got, ok, tc.want, tc.ok)
+		}
+	}
+}
@@ -106,7 +106,10 @@ func transportPhaseNoun(phase string) string {

 type RPCHandshakeError struct {
 	StatusCode int
-	Err        error
+	// Location is the redirect target from the handshake response, when the
+	// endpoint answered with a 3xx instead of upgrading the connection.
+	Location string
+	Err      error
 }

 func (e *RPCHandshakeError) Error() string {