diff --git a/internal/web/service/node_mtls.go b/internal/web/service/node_mtls.go index 18bb1f77b..242eedce1 100644 --- a/internal/web/service/node_mtls.go +++ b/internal/web/service/node_mtls.go @@ -2,8 +2,6 @@ package service import ( "crypto/tls" - "crypto/x509" - "encoding/pem" "strings" "github.com/mhsanaei/3x-ui/v3/internal/util/common" @@ -42,19 +40,13 @@ func (s *NodeService) ReloadMasterMtlsClient() error { return nil } -// SetNodeMtlsTrustCA stores the CA certificate this panel trusts for incoming -// node-API client certificates. An empty value clears it (mTLS off). A -// non-empty value must be a PEM certificate (fail closed). Takes effect on the -// next panel restart, when the listener's ClientCAs is rebuilt. +// SetNodeMtlsTrustCA stores the CA certificate bundle trusted for incoming +// node-API clients. An empty value clears it; changes apply after restart. func (s *NodeService) SetNodeMtlsTrustCA(caPem string) error { caPem = strings.TrimSpace(caPem) if caPem != "" { - block, _ := pem.Decode([]byte(caPem)) - if block == nil || block.Type != "CERTIFICATE" { - return common.NewError("trust CA must be a PEM-encoded certificate") - } - if _, err := x509.ParseCertificate(block.Bytes); err != nil { - return common.NewError("invalid trust CA certificate: " + err.Error()) + if _, err := parseCertificateBundlePEM([]byte(caPem)); err != nil { + return common.NewError("invalid trust CA certificate bundle: ", err) } } return (&SettingService{}).setString(settingNodeMtlsClientCA, caPem) diff --git a/internal/web/service/setting_mtls.go b/internal/web/service/setting_mtls.go index e6c9abb48..d62812cd6 100644 --- a/internal/web/service/setting_mtls.go +++ b/internal/web/service/setting_mtls.go @@ -1,11 +1,14 @@ package service import ( + "bytes" "crypto/sha256" "crypto/tls" "crypto/x509" "encoding/hex" "encoding/pem" + "errors" + "fmt" "strings" "sync" @@ -189,9 +192,42 @@ func (s *SettingService) NodeMtlsClientCAPool() (*x509.CertPool, error) { if caPem == "" { return nil, nil } + certs, err := parseCertificateBundlePEM([]byte(caPem)) + if err != nil { + return nil, fmt.Errorf("nodeMtlsClientCAPem is not a valid certificate bundle: %w", err) + } pool := x509.NewCertPool() - if !pool.AppendCertsFromPEM([]byte(caPem)) { - return nil, common.NewError("nodeMtlsClientCAPem is not a valid certificate") + for _, cert := range certs { + pool.AddCert(cert) } return pool, nil } + +// parseCertificateBundlePEM avoids AppendCertsFromPEM because that helper can +// silently accept a bundle after parsing only its first certificate. +func parseCertificateBundlePEM(bundle []byte) ([]*x509.Certificate, error) { + rest := bytes.TrimSpace(bundle) + if len(rest) == 0 { + return nil, errors.New("certificate bundle is empty") + } + certs := make([]*x509.Certificate, 0, 1) + for len(rest) > 0 { + if !bytes.HasPrefix(rest, []byte("-----BEGIN CERTIFICATE-----")) { + return nil, errors.New("certificate bundle contains malformed or non-PEM data") + } + block, next := pem.Decode(rest) + if block == nil { + return nil, errors.New("certificate bundle contains malformed or non-PEM data") + } + if block.Type != "CERTIFICATE" { + return nil, errors.New("certificate bundle contains a non-certificate PEM block") + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, errors.New("certificate bundle contains an invalid certificate") + } + certs = append(certs, cert) + rest = bytes.TrimSpace(next) + } + return certs, nil +} diff --git a/internal/web/service/setting_mtls_bundle_test.go b/internal/web/service/setting_mtls_bundle_test.go new file mode 100644 index 000000000..c1dface5f --- /dev/null +++ b/internal/web/service/setting_mtls_bundle_test.go @@ -0,0 +1,74 @@ +package service + +import ( + "strings" + "testing" + + "github.com/mhsanaei/3x-ui/v3/internal/util/crypto" +) + +func mustNodeCAPEM(t *testing.T, name string) string { + t.Helper() + ca, err := crypto.GenerateNodeCA(name) + if err != nil { + t.Fatalf("GenerateNodeCA(%q): %v", name, err) + } + return string(ca.CertPEM) +} + +func TestParseCertificateBundlePEM(t *testing.T) { + first := mustNodeCAPEM(t, "bundle test CA one") + second := mustNodeCAPEM(t, "bundle test CA two") + + corrupt := strings.Replace(second, "-----BEGIN CERTIFICATE-----\n", "-----BEGIN CERTIFICATE-----\nAA", 1) + + tests := []struct { + name string + bundle string + wantCerts int + wantErr string + }{ + {name: "single certificate", bundle: first, wantCerts: 1}, + {name: "two certificates", bundle: first + second, wantCerts: 2}, + {name: "empty", bundle: "", wantErr: "certificate bundle is empty"}, + {name: "whitespace only", bundle: "\n\t \n", wantErr: "certificate bundle is empty"}, + {name: "leading non-PEM data", bundle: "junk\n" + first, wantErr: "certificate bundle contains malformed or non-PEM data"}, + {name: "interstitial non-PEM data", bundle: first + "junk\n" + second, wantErr: "certificate bundle contains malformed or non-PEM data"}, + {name: "second certificate corrupt", bundle: first + corrupt, wantErr: "certificate bundle contains malformed or non-PEM data"}, + {name: "trailing non-PEM data", bundle: first + "not a certificate\n", wantErr: "certificate bundle contains malformed or non-PEM data"}, + {name: "non-certificate block", bundle: first + "-----BEGIN PRIVATE KEY-----\nAAAA\n-----END PRIVATE KEY-----\n", wantErr: "certificate bundle contains malformed or non-PEM data"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + certs, err := parseCertificateBundlePEM([]byte(tt.bundle)) + if tt.wantErr != "" { + if err == nil || err.Error() != tt.wantErr { + t.Fatalf("parseCertificateBundlePEM() error = %v, want %q", err, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("parseCertificateBundlePEM(): %v", err) + } + if len(certs) != tt.wantCerts { + t.Fatalf("parseCertificateBundlePEM() = %d certs, want %d", len(certs), tt.wantCerts) + } + }) + } +} + +func TestNodeMtlsClientCAPoolRejectsPartiallyValidBundle(t *testing.T) { + s := setupSettingMtlsDB(t) + + valid := mustNodeCAPEM(t, "pool test CA") + if err := s.setString("nodeMtlsClientCAPem", valid+"-----BEGIN CERTIFICATE-----\nnot base64\n-----END CERTIFICATE-----\n"); err != nil { + t.Fatalf("setString: %v", err) + } + + pool, err := s.NodeMtlsClientCAPool() + want := "nodeMtlsClientCAPem is not a valid certificate bundle: certificate bundle contains malformed or non-PEM data" + if err == nil || err.Error() != want { + t.Fatalf("NodeMtlsClientCAPool() = %v, error = %v, want %q", pool, err, want) + } +}