Преглед на файлове

fix(nodes): honor TLS verify mode skip/pin for remote node operations (#5264)

The node probe honored the per-node TlsVerifyMode (skip/pin) but
runtime.Remote used a shared client with no TLSClientConfig, so traffic
sync and every other remote op fell back to system-CA verification and
failed against self-signed nodes even after the operator set skip/pin.

Move the TLS client builder into the runtime layer (HTTPClientForNode /
DecodeCertPin) as the single source of truth, have Remote build and cache
its per-node client through it, and delegate the service probe to the same
builder so the two paths can no longer diverge.
MHSanaei преди 2 дни
родител
ревизия
4c8d3cb625
променени са 4 файла, в които са добавени 271 реда и са изтрити 83 реда
  1. 20 10
      internal/web/runtime/remote.go
  2. 84 0
      internal/web/runtime/tls_client.go
  3. 165 0
      internal/web/runtime/tls_client_test.go
  4. 2 73
      internal/web/service/node.go

+ 20 - 10
internal/web/runtime/remote.go

@@ -23,15 +23,6 @@ import (
 
 const remoteHTTPTimeout = 10 * time.Second
 
-var remoteHTTPClient = &http.Client{
-	Transport: &http.Transport{
-		MaxIdleConns:        64,
-		MaxIdleConnsPerHost: 4,
-		IdleConnTimeout:     60 * time.Second,
-		DialContext:         netsafe.SSRFGuardedDialContext,
-	},
-}
-
 type envelope struct {
 	Success bool            `json:"success"`
 	Msg     string          `json:"msg"`
@@ -43,6 +34,12 @@ type Remote struct {
 
 	mu            sync.RWMutex
 	remoteIDByTag map[string]int
+
+	// Per-node client honoring the TLS verify mode, built once and reused; a
+	// node config change drops the cached Remote so the next one rebuilds it.
+	clientOnce sync.Once
+	client     *http.Client
+	clientErr  error
 }
 
 type RemoteInboundOption struct {
@@ -61,6 +58,15 @@ func NewRemote(n *model.Node) *Remote {
 
 func (r *Remote) Name() string { return "node:" + r.node.Name }
 
+// httpClient lazily builds and caches the per-node client honoring the TLS
+// verify mode, so Remote ops don't fall back to system CA on skip/pin (#5264).
+func (r *Remote) httpClient() (*http.Client, error) {
+	r.clientOnce.Do(func() {
+		r.client, r.clientErr = HTTPClientForNode(r.node)
+	})
+	return r.client, r.clientErr
+}
+
 func (r *Remote) baseURL() (string, error) {
 	addr, err := netsafe.NormalizeHost(r.node.Address)
 	if err != nil {
@@ -129,7 +135,11 @@ func (r *Remote) do(ctx context.Context, method, path string, body any) (*envelo
 		req.Header.Set("Content-Type", contentType)
 	}
 
-	resp, err := remoteHTTPClient.Do(req)
+	client, err := r.httpClient()
+	if err != nil {
+		return nil, err
+	}
+	resp, err := client.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("%s %s: %w", method, path, err)
 	}

+ 84 - 0
internal/web/runtime/tls_client.go

@@ -0,0 +1,84 @@
+package runtime
+
+import (
+	"crypto/sha256"
+	"crypto/subtle"
+	"crypto/tls"
+	"encoding/base64"
+	"encoding/hex"
+	"net/http"
+	"strings"
+	"time"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+	"github.com/mhsanaei/3x-ui/v3/internal/util/common"
+	"github.com/mhsanaei/3x-ui/v3/internal/util/netsafe"
+)
+
+// defaultNodeHTTPClient reaches nodes trusting the system CA store ("verify"
+// mode or plain http); shared so connections pool across nodes.
+var defaultNodeHTTPClient = &http.Client{
+	Transport: &http.Transport{
+		MaxIdleConns:        64,
+		MaxIdleConnsPerHost: 4,
+		IdleConnTimeout:     60 * time.Second,
+		DialContext:         netsafe.SSRFGuardedDialContext,
+	},
+}
+
+// HTTPClientForNode returns the node's HTTP client honoring its TLS verify mode
+// (verify→system CA, skip→no check, pin→leaf SHA-256). Used by both the probe
+// and every Remote op so they can't disagree on a self-signed node (#5264).
+func HTTPClientForNode(n *model.Node) (*http.Client, error) {
+	mode := n.TlsVerifyMode
+	if mode == "" {
+		mode = "verify"
+	}
+	if mode == "verify" || n.Scheme == "http" {
+		return defaultNodeHTTPClient, nil
+	}
+	tlsCfg := &tls.Config{InsecureSkipVerify: true} // lgtm[go/disabled-certificate-check]
+	if mode == "pin" {
+		want, err := DecodeCertPin(n.PinnedCertSha256)
+		if err != nil {
+			return nil, err
+		}
+		tlsCfg.VerifyConnection = func(cs tls.ConnectionState) error {
+			if len(cs.PeerCertificates) == 0 {
+				return common.NewError("node presented no certificate")
+			}
+			sum := sha256.Sum256(cs.PeerCertificates[0].Raw)
+			if subtle.ConstantTimeCompare(sum[:], want) != 1 {
+				return common.NewError("node certificate does not match pinned SHA-256")
+			}
+			return nil
+		}
+	}
+	return &http.Client{
+		Transport: &http.Transport{
+			MaxIdleConns:        64,
+			MaxIdleConnsPerHost: 4,
+			IdleConnTimeout:     60 * time.Second,
+			DialContext:         netsafe.SSRFGuardedDialContext,
+			TLSClientConfig:     tlsCfg,
+		},
+	}, nil
+}
+
+// DecodeCertPin decodes a SHA-256 cert pin given as base64 (Xray's
+// pinnedPeerCertSha256 form) or hex with optional colons into 32 raw bytes.
+func DecodeCertPin(s string) ([]byte, error) {
+	s = strings.TrimSpace(s)
+	if s == "" {
+		return nil, common.NewError("certificate pin is empty")
+	}
+	if b, err := hex.DecodeString(strings.ReplaceAll(s, ":", "")); err == nil && len(b) == sha256.Size {
+		return b, nil
+	}
+	for _, enc := range []*base64.Encoding{base64.StdEncoding, base64.RawStdEncoding, base64.URLEncoding, base64.RawURLEncoding} {
+		if b, err := enc.DecodeString(s); err == nil && len(b) == sha256.Size {
+			return b, nil
+		}
+	}
+	return nil, common.NewError("certificate pin must be a SHA-256 hash (base64 or hex)")
+}

+ 165 - 0
internal/web/runtime/tls_client_test.go

@@ -0,0 +1,165 @@
+package runtime
+
+import (
+	"context"
+	"crypto/sha256"
+	"encoding/base64"
+	"encoding/hex"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+)
+
+// nodeForServer builds a node pointing at a loopback test server (loopback is
+// SSRF-blocked, so AllowPrivateAddress is set for the guarded dialer).
+func nodeForServer(t *testing.T, srv *httptest.Server, mode, pin string) *model.Node {
+	t.Helper()
+	u, err := url.Parse(srv.URL)
+	if err != nil {
+		t.Fatalf("parse server url: %v", err)
+	}
+	port, err := strconv.Atoi(u.Port())
+	if err != nil {
+		t.Fatalf("parse server port: %v", err)
+	}
+	return &model.Node{
+		Id:                  1,
+		Name:                "n1",
+		Scheme:              "https",
+		Address:             u.Hostname(),
+		Port:                port,
+		BasePath:            "/",
+		ApiToken:            "token",
+		Enable:              true,
+		AllowPrivateAddress: true,
+		TlsVerifyMode:       mode,
+		PinnedCertSha256:    pin,
+	}
+}
+
+func leafPinBase64(srv *httptest.Server) string {
+	sum := sha256.Sum256(srv.Certificate().Raw)
+	return base64.StdEncoding.EncodeToString(sum[:])
+}
+
+// A self-signed node must be reachable by Remote ops under skip/pin and
+// rejected under verify — the split issue #5264 reported.
+func TestRemoteHonorsTLSVerifyMode(t *testing.T) {
+	srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"success":true,"obj":[]}`))
+	}))
+	defer srv.Close()
+
+	goodPin := leafPinBase64(srv)
+	wrongPin := base64.StdEncoding.EncodeToString(make([]byte, sha256.Size))
+
+	cases := []struct {
+		name    string
+		mode    string
+		pin     string
+		wantErr bool
+	}{
+		{"verify rejects self-signed", "verify", "", true},
+		{"skip accepts self-signed", "skip", "", false},
+		{"pin accepts matching cert", "pin", goodPin, false},
+		{"pin rejects mismatched cert", "pin", wrongPin, true},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			r := NewRemote(nodeForServer(t, srv, c.mode, c.pin))
+			_, err := r.ListInboundOptions(context.Background())
+			if c.wantErr && err == nil {
+				t.Fatalf("mode %q: expected error, got nil", c.mode)
+			}
+			if !c.wantErr && err != nil {
+				t.Fatalf("mode %q: unexpected error: %v", c.mode, err)
+			}
+		})
+	}
+}
+
+// The lazily-built client is cached for the Remote's lifetime so repeated
+// operations reuse one pooled transport rather than rebuilding TLS each call.
+func TestRemoteClientCached(t *testing.T) {
+	r := NewRemote(&model.Node{Scheme: "https", TlsVerifyMode: "skip"})
+	c1, err1 := r.httpClient()
+	c2, err2 := r.httpClient()
+	if err1 != nil || err2 != nil {
+		t.Fatalf("httpClient errors: %v %v", err1, err2)
+	}
+	if c1 != c2 {
+		t.Fatal("expected the same cached client across calls")
+	}
+}
+
+func TestHTTPClientForNodeVerifyShared(t *testing.T) {
+	// verify mode and plain http both reuse the shared default client.
+	for _, n := range []*model.Node{
+		{Scheme: "https", TlsVerifyMode: "verify"},
+		{Scheme: "https", TlsVerifyMode: ""},
+		{Scheme: "http", TlsVerifyMode: "skip"},
+	} {
+		c, err := HTTPClientForNode(n)
+		if err != nil {
+			t.Fatalf("HTTPClientForNode(%+v): %v", n, err)
+		}
+		if c != defaultNodeHTTPClient {
+			t.Fatalf("HTTPClientForNode(%+v) = %p, want shared default %p", n, c, defaultNodeHTTPClient)
+		}
+	}
+}
+
+func TestHTTPClientForNodePinInvalid(t *testing.T) {
+	if _, err := HTTPClientForNode(&model.Node{Scheme: "https", TlsVerifyMode: "pin", PinnedCertSha256: "not-a-pin"}); err == nil {
+		t.Fatal("expected error for invalid pin")
+	}
+}
+
+func TestDecodeCertPin(t *testing.T) {
+	raw := sha256.Sum256([]byte("cert"))
+	hexColon := strings.ToUpper(hex.EncodeToString(raw[:]))
+	// reinsert colons in openssl -fingerprint style
+	var withColons strings.Builder
+	for i := 0; i < len(hexColon); i += 2 {
+		if i > 0 {
+			withColons.WriteByte(':')
+		}
+		withColons.WriteString(hexColon[i : i+2])
+	}
+
+	cases := []struct {
+		name    string
+		in      string
+		wantErr bool
+	}{
+		{"base64 std", base64.StdEncoding.EncodeToString(raw[:]), false},
+		{"base64 raw url", base64.RawURLEncoding.EncodeToString(raw[:]), false},
+		{"hex bare", hex.EncodeToString(raw[:]), false},
+		{"hex colon openssl", withColons.String(), false},
+		{"empty", "", true},
+		{"garbage", "not-a-pin", true},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			got, err := DecodeCertPin(c.in)
+			if c.wantErr {
+				if err == nil {
+					t.Fatalf("expected error for %q", c.in)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error for %q: %v", c.in, err)
+			}
+			if string(got) != string(raw[:]) {
+				t.Fatalf("decoded bytes mismatch for %q", c.in)
+			}
+		})
+	}
+}

+ 2 - 73
internal/web/service/node.go

@@ -3,10 +3,8 @@ package service
 import (
 	"context"
 	"crypto/sha256"
-	"crypto/subtle"
 	"crypto/tls"
 	"encoding/base64"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -44,75 +42,6 @@ type HeartbeatPatch struct {
 
 type NodeService struct{}
 
-var nodeHTTPClient = &http.Client{
-	Transport: &http.Transport{
-		MaxIdleConns:        64,
-		MaxIdleConnsPerHost: 4,
-		IdleConnTimeout:     60 * time.Second,
-		DialContext:         netsafe.SSRFGuardedDialContext,
-	},
-}
-
-// nodeHTTPClientFor returns the HTTP client used to reach a node, honoring its
-// per-node TLS verification mode. "verify" (or any http node) uses the shared
-// client with default certificate validation. "skip" disables validation.
-// "pin" disables the default chain check but verifies the leaf certificate's
-// SHA-256 against the stored pin, keeping MITM protection for self-signed certs.
-func nodeHTTPClientFor(n *model.Node) (*http.Client, error) {
-	mode := n.TlsVerifyMode
-	if mode == "" {
-		mode = "verify"
-	}
-	if mode == "verify" || n.Scheme == "http" {
-		return nodeHTTPClient, nil
-	}
-	tlsCfg := &tls.Config{InsecureSkipVerify: true}
-	if mode == "pin" {
-		want, err := decodeCertPin(n.PinnedCertSha256)
-		if err != nil {
-			return nil, err
-		}
-		tlsCfg.VerifyConnection = func(cs tls.ConnectionState) error {
-			if len(cs.PeerCertificates) == 0 {
-				return common.NewError("node presented no certificate")
-			}
-			sum := sha256.Sum256(cs.PeerCertificates[0].Raw)
-			if subtle.ConstantTimeCompare(sum[:], want) != 1 {
-				return common.NewError("node certificate does not match pinned SHA-256")
-			}
-			return nil
-		}
-	}
-	return &http.Client{
-		Transport: &http.Transport{
-			MaxIdleConns:        64,
-			MaxIdleConnsPerHost: 4,
-			IdleConnTimeout:     60 * time.Second,
-			DialContext:         netsafe.SSRFGuardedDialContext,
-			TLSClientConfig:     tlsCfg,
-		},
-	}, nil
-}
-
-// decodeCertPin accepts a SHA-256 certificate hash as base64 (the format used
-// by Xray's pinnedPeerCertSha256) or hex with optional colons (the openssl
-// -fingerprint style) and returns the 32 raw bytes.
-func decodeCertPin(s string) ([]byte, error) {
-	s = strings.TrimSpace(s)
-	if s == "" {
-		return nil, common.NewError("certificate pin is empty")
-	}
-	if b, err := hex.DecodeString(strings.ReplaceAll(s, ":", "")); err == nil && len(b) == sha256.Size {
-		return b, nil
-	}
-	for _, enc := range []*base64.Encoding{base64.StdEncoding, base64.RawStdEncoding, base64.URLEncoding, base64.RawURLEncoding} {
-		if b, err := enc.DecodeString(s); err == nil && len(b) == sha256.Size {
-			return b, nil
-		}
-	}
-	return nil, common.NewError("certificate pin must be a SHA-256 hash (base64 or hex)")
-}
-
 // FetchCertFingerprint connects to the node over HTTPS without verifying the
 // certificate and returns the leaf certificate's SHA-256 as base64, so the UI
 // can offer a "fetch and pin current certificate" action.
@@ -367,7 +296,7 @@ func (s *NodeService) normalize(n *model.Node) error {
 		n.InboundTags = tags
 	}
 	if n.TlsVerifyMode == "pin" {
-		if _, err := decodeCertPin(n.PinnedCertSha256); err != nil {
+		if _, err := runtime.DecodeCertPin(n.PinnedCertSha256); err != nil {
 			return common.NewError(err.Error())
 		}
 	}
@@ -692,7 +621,7 @@ func (s *NodeService) Probe(ctx context.Context, n *model.Node) (HeartbeatPatch,
 	}
 	req.Header.Set("Accept", "application/json")
 
-	client, err := nodeHTTPClientFor(n)
+	client, err := runtime.HTTPClientForNode(n)
 	if err != nil {
 		patch.LastError = err.Error()
 		return patch, err