From 60453bf5238a1a8e00e31368da21336a9ac41559 Mon Sep 17 00:00:00 2001 From: n0ctal <4c866w5fn9@privaterelay.appleid.com> Date: Fri, 14 Aug 2026 23:03:54 +0500 Subject: [PATCH] fix(nodes): persist the master mTLS credential atomically and stop silent reissue (#6195) * fix: harden master mTLS credential persistence * fix(mtls): validate and serialize credential persistence --------- Co-authored-by: n0ctal <293235942+n0ctal@users.noreply.github.com> --- internal/web/service/setting.go | 1 + .../service/setting_factory_defaults_test.go | 1 + internal/web/service/setting_mtls.go | 76 ++++++++- internal/web/service/setting_mtls_test.go | 149 ++++++++++++++++++ 4 files changed, 225 insertions(+), 2 deletions(-) diff --git a/internal/web/service/setting.go b/internal/web/service/setting.go index 1cedef037..aea096b2f 100644 --- a/internal/web/service/setting.go +++ b/internal/web/service/setting.go @@ -59,6 +59,7 @@ var defaultValueMap = map[string]string{ "nodeMtlsCaKeyPem": "", "nodeMtlsClientCertPem": "", "nodeMtlsClientKeyPem": "", + "nodeMtlsClientCertSha256": "", "nodeMtlsClientCAPem": "", "webBasePath": normalizeBasePath(getEnv("XUI_INIT_WEB_BASE_PATH", "/")), "sessionMaxAge": "360", diff --git a/internal/web/service/setting_factory_defaults_test.go b/internal/web/service/setting_factory_defaults_test.go index 73c9f4bad..333c09673 100644 --- a/internal/web/service/setting_factory_defaults_test.go +++ b/internal/web/service/setting_factory_defaults_test.go @@ -52,6 +52,7 @@ func TestGetFactoryDefaultsOmitsSensitiveMaterial(t *testing.T) { "nodeMtlsCaKeyPem", "nodeMtlsClientCertPem", "nodeMtlsClientKeyPem", + "nodeMtlsClientCertSha256", "xrayTemplateConfig", "tgBotToken", "twoFactorToken", diff --git a/internal/web/service/setting_mtls.go b/internal/web/service/setting_mtls.go index 2bc9aa04d..65bd01ede 100644 --- a/internal/web/service/setting_mtls.go +++ b/internal/web/service/setting_mtls.go @@ -1,17 +1,30 @@ package service import ( + "crypto/sha256" + "crypto/tls" "crypto/x509" + "encoding/hex" + "encoding/pem" + "strings" + "sync" + "gorm.io/gorm" + + "github.com/mhsanaei/3x-ui/v3/internal/database" + "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/crypto" ) +var masterClientCredentialMu sync.Mutex + const ( settingNodeMtlsCaCert = "nodeMtlsCaCertPem" settingNodeMtlsCaKey = "nodeMtlsCaKeyPem" settingNodeMtlsClientCert = "nodeMtlsClientCertPem" settingNodeMtlsClientKey = "nodeMtlsClientKeyPem" + settingNodeMtlsClientPin = "nodeMtlsClientCertSha256" settingNodeMtlsClientCA = "nodeMtlsClientCAPem" ) @@ -49,10 +62,26 @@ func (s *SettingService) EnsureNodeMtlsCA() (crypto.CertKeyPEM, error) { return ca, nil } +func clientCertSHA256FromPEM(certPEM []byte) (string, error) { + block, rest := pem.Decode(certPEM) + if block == nil || block.Type != "CERTIFICATE" || len(strings.TrimSpace(string(rest))) != 0 { + return "", common.NewError("client certificate is not valid PEM") + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return "", err + } + sum := sha256.Sum256(cert.Raw) + return hex.EncodeToString(sum[:]), nil +} + // EnsureMasterClientCert returns the client certificate this panel presents when // calling its nodes over mTLS, issuing it from the node CA on first use and // reusing the stored pair thereafter. func (s *SettingService) EnsureMasterClientCert() (crypto.CertKeyPEM, error) { + masterClientCredentialMu.Lock() + defer masterClientCredentialMu.Unlock() + certPem, err := s.getString(settingNodeMtlsClientCert) if err != nil { return crypto.CertKeyPEM{}, err @@ -61,7 +90,27 @@ func (s *SettingService) EnsureMasterClientCert() (crypto.CertKeyPEM, error) { if err != nil { return crypto.CertKeyPEM{}, err } + storedPin, err := s.getString(settingNodeMtlsClientPin) + if err != nil { + return crypto.CertKeyPEM{}, err + } + storedPin = strings.ToLower(strings.TrimSpace(storedPin)) if certPem != "" && keyPem != "" { + if _, err := tls.X509KeyPair([]byte(certPem), []byte(keyPem)); err != nil { + return crypto.CertKeyPEM{}, common.NewError("stored master client certificate/key pair is invalid: ", err) + } + actualPin, err := clientCertSHA256FromPEM([]byte(certPem)) + if err != nil { + return crypto.CertKeyPEM{}, err + } + if storedPin != "" && storedPin != actualPin { + return crypto.CertKeyPEM{}, common.NewError("stored master client certificate does not match nodeMtlsClientCertSha256; refusing to rotate") + } + if storedPin == "" { + if err := s.saveSetting(settingNodeMtlsClientPin, actualPin); err != nil { + return crypto.CertKeyPEM{}, err + } + } return crypto.CertKeyPEM{CertPEM: []byte(certPem), KeyPEM: []byte(keyPem)}, nil } // Half a stored pair signals corrupted settings; reissuing would rotate the @@ -77,15 +126,38 @@ func (s *SettingService) EnsureMasterClientCert() (crypto.CertKeyPEM, error) { if err != nil { return crypto.CertKeyPEM{}, err } - if err := s.saveSetting(settingNodeMtlsClientCert, string(client.CertPEM)); err != nil { + pin, err := clientCertSHA256FromPEM(client.CertPEM) + if err != nil { return crypto.CertKeyPEM{}, err } - if err := s.saveSetting(settingNodeMtlsClientKey, string(client.KeyPEM)); err != nil { + if err := saveMasterClientCredential(client, pin); err != nil { return crypto.CertKeyPEM{}, err } return client, nil } +func saveMasterClientCredential(client crypto.CertKeyPEM, pin string) error { + values := map[string]string{ + settingNodeMtlsClientCert: string(client.CertPEM), + settingNodeMtlsClientKey: string(client.KeyPEM), + settingNodeMtlsClientPin: pin, + } + return database.GetDB().Transaction(func(tx *gorm.DB) error { + for key, value := range values { + result := tx.Model(&model.Setting{}).Where("key = ?", key).Update("value", value) + if result.Error != nil { + return result.Error + } + if result.RowsAffected == 0 { + if err := tx.Create(&model.Setting{Key: key, Value: value}).Error; err != nil { + return err + } + } + } + return nil + }) +} + // NodeMtlsClientCAPool builds the trust pool used as the panel listener's // ClientCAs for incoming node-API client certificates. It returns (nil, nil) // when no trust CA is configured, so mTLS stays off and the listener behaves diff --git a/internal/web/service/setting_mtls_test.go b/internal/web/service/setting_mtls_test.go index 69327913c..841f0e56b 100644 --- a/internal/web/service/setting_mtls_test.go +++ b/internal/web/service/setting_mtls_test.go @@ -6,8 +6,10 @@ import ( "encoding/pem" "path/filepath" "testing" + "time" "github.com/mhsanaei/3x-ui/v3/internal/database" + "github.com/mhsanaei/3x-ui/v3/internal/util/crypto" ) func setupSettingMtlsDB(t *testing.T) *SettingService { @@ -88,6 +90,153 @@ func TestEnsureMasterClientCert_VerifiesAndIdempotent(t *testing.T) { } } +func TestEnsureMasterClientCertRejectsMismatchedStoredKey(t *testing.T) { + s := setupSettingMtlsDB(t) + first, err := s.EnsureMasterClientCert() + if err != nil { + t.Fatal(err) + } + ca, err := s.EnsureNodeMtlsCA() + if err != nil { + t.Fatal(err) + } + other, err := crypto.IssueClientCert(ca, "other master") + if err != nil { + t.Fatal(err) + } + if err := s.setString(settingNodeMtlsClientCert, string(first.CertPEM)); err != nil { + t.Fatal(err) + } + if err := s.setString(settingNodeMtlsClientKey, string(other.KeyPEM)); err != nil { + t.Fatal(err) + } + if _, err := s.EnsureMasterClientCert(); err == nil { + t.Fatal("mismatched stored certificate and key were accepted") + } +} + +func TestEnsureMasterClientCert_ReissuesLeafWhenCAStillExists(t *testing.T) { + s := setupSettingMtlsDB(t) + + client, err := s.EnsureMasterClientCert() + if err != nil { + t.Fatalf("EnsureMasterClientCert: %v", err) + } + pin, err := clientCertSHA256FromPEM(client.CertPEM) + if err != nil { + t.Fatalf("clientCertSHA256FromPEM: %v", err) + } + if err := s.setString(settingNodeMtlsClientPin, pin); err != nil { + t.Fatalf("persist client pin: %v", err) + } + if err := s.setString(settingNodeMtlsClientCert, ""); err != nil { + t.Fatalf("clear client cert: %v", err) + } + if err := s.setString(settingNodeMtlsClientKey, ""); err != nil { + t.Fatalf("clear client key: %v", err) + } + + reissued, err := s.EnsureMasterClientCert() + if err != nil { + t.Fatalf("reissue with surviving CA: %v", err) + } + newPin, err := clientCertSHA256FromPEM(reissued.CertPEM) + if err != nil { + t.Fatalf("new pin: %v", err) + } + if newPin == pin { + t.Fatal("reissued credential kept the lost leaf identity") + } + stored, err := s.getString(settingNodeMtlsClientPin) + if err != nil || stored != newPin { + t.Fatalf("stored pin = %q, error = %v, want %q", stored, err, newPin) + } +} + +func TestEnsureMasterClientCertConcurrentFirstUseMintsOneCredential(t *testing.T) { + s := setupSettingMtlsDB(t) + blocked := make(chan struct{}) + masterClientCredentialMu.Lock() + go func() { + _, _ = s.EnsureMasterClientCert() + close(blocked) + }() + select { + case <-blocked: + masterClientCredentialMu.Unlock() + t.Fatal("EnsureMasterClientCert returned while its serialization lock was held") + case <-time.After(50 * time.Millisecond): + } + masterClientCredentialMu.Unlock() + select { + case <-blocked: + case <-time.After(time.Second): + t.Fatal("EnsureMasterClientCert remained blocked after serialization lock release") + } + const callers = 8 + start := make(chan struct{}) + results := make(chan crypto.CertKeyPEM, callers) + errs := make(chan error, callers) + for range callers { + go func() { + <-start + credential, err := s.EnsureMasterClientCert() + results <- credential + errs <- err + }() + } + close(start) + var first crypto.CertKeyPEM + for i := 0; i < callers; i++ { + credential := <-results + if err := <-errs; err != nil { + t.Fatalf("caller %d: %v", i, err) + } + if i == 0 { + first = credential + } else if !bytes.Equal(first.CertPEM, credential.CertPEM) || !bytes.Equal(first.KeyPEM, credential.KeyPEM) { + t.Fatalf("caller %d received a different credential", i) + } + } + storedCert, _ := s.getString(settingNodeMtlsClientCert) + storedKey, _ := s.getString(settingNodeMtlsClientKey) + if storedCert != string(first.CertPEM) || storedKey != string(first.KeyPEM) { + t.Fatal("persisted credential differs from concurrent callers") + } +} + +func TestEnsureMasterClientCert_PersistsCredentialAtomically(t *testing.T) { + s := setupSettingMtlsDB(t) + db := database.GetDB() + trigger := `CREATE TRIGGER fail_master_pin_insert + BEFORE INSERT ON settings + WHEN NEW.key = 'nodeMtlsClientCertSha256' + BEGIN SELECT RAISE(ABORT, 'injected pin failure'); END` + if err := db.Exec(trigger).Error; err != nil { + t.Fatalf("create failure trigger: %v", err) + } + + if _, err := s.EnsureMasterClientCert(); err == nil { + t.Fatal("injected persistence failure unexpectedly succeeded") + } + for _, key := range []string{settingNodeMtlsClientCert, settingNodeMtlsClientKey, settingNodeMtlsClientPin} { + got, err := s.getString(key) + if err != nil { + t.Fatalf("get %s after rollback: %v", key, err) + } + if got != "" { + t.Fatalf("%s persisted despite transaction rollback", key) + } + } + + if err := db.Exec("DROP TRIGGER fail_master_pin_insert").Error; err != nil { + t.Fatalf("drop failure trigger: %v", err) + } + if _, err := s.EnsureMasterClientCert(); err != nil { + t.Fatalf("retry after rollback: %v", err) + } +} + func TestNodeMtlsClientCAPool(t *testing.T) { s := setupSettingMtlsDB(t)