mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-13 00:34:43 +00:00
Backfill monitoring contract for the TrueNAS handshake-redirect upgrade
bf24a9a9c changed truenas runtime transport behavior without the
canonical paperwork, so its governance run failed the completion guard.
Document the redirect rule in the monitoring subsystem contract — a
plaintext handshake redirect never wakes the REST bridge; a same-host
https redirect upgrades once to wss and the upgrade is scheme-only and
monotonic — and move the Issue1631 regression tests into
transport_test.go, which is a registered verification artifact for the
truenas runtime path policy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>This commit is contained in:
courtmanr@gmail.com
parent
13371a6b6c
commit
b200955ab8
3 changed files with 137 additions and 136 deletions
|
|
@ -605,6 +605,18 @@ must never prevent the remaining configured hosts from being suppressed.
|
|||
CORE/FreeNAS. Unknown or current versions fail closed. The decision and
|
||||
persistent socket belong to one configured client, so reconnects or
|
||||
legacy negotiation for one appliance cannot alter another appliance.
|
||||
A plaintext `ws://` handshake answered with a redirect is not an
|
||||
unsupported endpoint and must never wake the REST bridge. When the
|
||||
redirect target is an https URL on the same host — TrueNAS's HTTP to
|
||||
HTTPS redirect, or an equivalent proxy — the dial retries once over TLS
|
||||
with the entry's verification settings, and the upgraded `wss://`
|
||||
endpoint becomes the client's endpoint for its remaining lifetime
|
||||
(#1631). The upgrade is scheme-only and monotonic: cross-host redirects,
|
||||
downgrades to plaintext, and relative targets fail closed with the
|
||||
redirect target named in the error, so a redirect can move a connection
|
||||
to TLS but never to another appliance or back to plaintext. Regression
|
||||
coverage: the `Issue1631` handshake-redirect tests in
|
||||
`internal/truenas/transport_test.go`.
|
||||
Current API-key authentication uses `auth.login_ex` with the key owner's
|
||||
username and `API_KEY_PLAIN`; password authentication uses
|
||||
`PASSWORD_PLAIN`. Username-less stored API keys may use the deprecated
|
||||
|
|
|
|||
|
|
@ -1,136 +0,0 @@
|
|||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -6,10 +6,13 @@ import (
|
|||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/websocket"
|
||||
)
|
||||
|
|
@ -796,3 +799,125 @@ func TestTransportDiagnosticsAreConnectionLocalAndSecretFree(t *testing.T) {
|
|||
t.Fatalf("failed authentication selected a transport: %+v", status)
|
||||
}
|
||||
}
|
||||
|
||||
// Issue #1631 handshake-redirect coverage: the v6.1.2 JSON-RPC migration
|
||||
// broke http-configured appliances behind TrueNAS's HTTP -> HTTPS redirect.
|
||||
// The websocket dial must upgrade a same-host https redirect to wss itself
|
||||
// and refuse anything that would move the connection to another host.
|
||||
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue