From 00e4fc4dd37a0bdc9b7053917691e5c86f1e7100 Mon Sep 17 00:00:00 2001 From: fatedier Date: Mon, 19 Oct 2026 17:16:55 +0800 Subject: [PATCH] feat(client): reload plugin certificates on frpc reload (#5576) The https2http and https2https plugins read their certificate and key once when the proxy starts, so a certificate that an external tool renews on disk is only picked up after the proxy itself is recreated. frpc reload rebuilt proxies only when their configuration changed, and a rebuild closes the work connections that are already in use. Let the plugin own the refresh. The HTTPS plugin server keeps the active keypair and the last refresh error in one immutable snapshot published through an atomic pointer, so a TLS handshake only loads that pointer and never touches the disk. Reload rereads both files, and when their bytes match the published pair it returns without parsing or replacing anything; otherwise it parses the pair and swaps the snapshot in. A read or parse failure keeps the previous certificate serving and records the error next to it. frpc reload collects the reloadable plugins of the live proxies and applies the new configuration inside the same reload transaction that also serializes control handoff, so a reload that races a reconnect still refreshes the plugins of the published control. The certificate files are read only after that transaction has been released. The config store API keeps applying configuration without refreshing certificates, because only an explicit reload may reread files. Proxy status reports a refresh failure next to the still running proxy instead of dropping its remote address. No configuration field is added, and frps and the wire protocol are unchanged. Without a reload nothing changes: certificates are still read once at startup, generated self-signed certificates still never reload, and a proxy whose configuration changed still goes through the existing rebuild path. --- README.md | 9 + Release.md | 1 + client/config_manager.go | 21 +- client/config_manager_certificate_test.go | 316 ++++++++++++++++++ client/http/controller.go | 2 +- client/http/controller_test.go | 29 ++ client/proxy/plugin_reload_test.go | 148 ++++++++ client/proxy/proxy.go | 3 + client/proxy/proxy_manager.go | 35 ++ client/proxy/proxy_wrapper.go | 30 ++ client/service.go | 9 + pkg/plugin/client/http_common.go | 23 +- .../client/internal/httpsserver/server.go | 83 ++++- .../internal/httpsserver/server_test.go | 153 +++++++++ 14 files changed, 854 insertions(+), 8 deletions(-) create mode 100644 client/config_manager_certificate_test.go create mode 100644 client/proxy/plugin_reload_test.go create mode 100644 pkg/plugin/client/internal/httpsserver/server_test.go diff --git a/README.md b/README.md index 5e893ba3..5080e5e9 100644 --- a/README.md +++ b/README.md @@ -809,6 +809,15 @@ webServer.port = 7400 Then run command `frpc reload -c ./frpc.toml` and wait for about 10 seconds to let `frpc` create or update or remove proxies. +For unchanged proxies using `https2http` or `https2https`, `frpc reload` also rereads +certificate and key files so externally renewed certificates take effect on new +TLS handshakes without interrupting existing connections. Unchanged file contents +leave the certificate untouched. If either file cannot be read or the pair is +invalid, the proxy keeps its previous certificate and reports the error in its +status and logs; reload still succeeds. A successful retry clears the error. +Generated self-signed certificates are unchanged, and Store API updates do not +refresh certificates on unchanged proxies. + **Note that global client parameters won't be modified except 'start'.** `start` is a global allowlist evaluated after all sources are merged (config file/include/store). diff --git a/Release.md b/Release.md index b717568c..0e441d06 100644 --- a/Release.md +++ b/Release.md @@ -1,6 +1,7 @@ ## Features * frpc can verify server certificates using system CAs, optionally combined with CAs from `trustedCaFile`. +* `frpc reload` now refreshes certificate files for unchanged `https2http` and `https2https` proxies without interrupting existing connections. Failed refreshes keep the previous certificate and report the error in proxy status and logs while reload still succeeds. ## Fixes diff --git a/client/config_manager.go b/client/config_manager.go index 24512e9f..d7bc985e 100644 --- a/client/config_manager.go +++ b/client/config_manager.go @@ -45,10 +45,29 @@ func (m *serviceConfigManager) ReloadFromFile(strict bool) error { return fmt.Errorf("%w: %v", configmgmt.ErrInvalidArgument, err) } - if err := m.svr.UpdateConfigSource(result.Common, result.Proxies, result.Visitors); err != nil { + var reloadPlugins func() + err = func() error { + m.svr.reloadMu.Lock() + defer m.svr.reloadMu.Unlock() + + // Snapshot plugins and apply config in the same transaction as control handoff. + m.svr.ctlMu.RLock() + ctl := m.svr.ctl + m.svr.ctlMu.RUnlock() + if ctl != nil { + reloadPlugins = ctl.pm.PreparePluginReload() + } + return m.svr.updateConfigSourceLocked(result.Common, result.Proxies, result.Visitors) + }() + if err != nil { return fmt.Errorf("%w: %v", configmgmt.ErrApplyConfig, err) } + // Certificate file I/O must not hold the reload transaction lock. + if reloadPlugins != nil { + reloadPlugins() + } + log.Infof("success reload conf") return nil } diff --git a/client/config_manager_certificate_test.go b/client/config_manager_certificate_test.go new file mode 100644 index 00000000..da080681 --- /dev/null +++ b/client/config_manager_certificate_test.go @@ -0,0 +1,316 @@ +// Copyright 2026 The frp Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !frps + +package client + +import ( + "bufio" + "context" + "crypto/tls" + "crypto/x509" + "encoding/pem" + "fmt" + "io" + "net" + "net/http" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/fatedier/frp/client/proxy" + "github.com/fatedier/frp/client/visitor" + "github.com/fatedier/frp/pkg/config" + "github.com/fatedier/frp/pkg/config/source" + v1 "github.com/fatedier/frp/pkg/config/v1" + "github.com/fatedier/frp/pkg/msg" + "github.com/fatedier/frp/pkg/policy/security" + "github.com/fatedier/frp/pkg/transport" +) + +type certificateReloadSender struct{ started chan string } + +func (s *certificateReloadSender) Send(m msg.Message) error { + if start, ok := m.(*msg.NewProxy); ok { + s.started <- start.ProxyName + } + return nil +} + +func writeReloadCertificate(t *testing.T, crtPath, keyPath string) []byte { + t.Helper() + cfg, err := transport.NewServerTLSConfig("", "", "") + require.NoError(t, err) + cert := cfg.Certificates[0] + key, err := x509.MarshalPKCS8PrivateKey(cert.PrivateKey) + require.NoError(t, err) + require.NoError(t, os.WriteFile(crtPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Certificate[0]}), 0o600)) + require.NoError(t, os.WriteFile(keyPath, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: key}), 0o600)) + return cert.Certificate[0] +} + +func TestFileReloadCertificatesAndStoreMutations(t *testing.T) { + for _, pluginType := range []string{v1.PluginHTTPS2HTTP, v1.PluginHTTPS2HTTPS} { + t.Run(pluginType, func(t *testing.T) { + dir := t.TempDir() + crtPath, keyPath := filepath.Join(dir, "tls.crt"), filepath.Join(dir, "tls.key") + original := writeReloadCertificate(t, crtPath, keyPath) + configPath := filepath.Join(dir, "frpc.toml") + content := fmt.Sprintf(`[[proxies]] +name = "https" +type = "tcp" +remotePort = 8443 +[proxies.plugin] +type = %q +localAddr = "127.0.0.1:80" +crtPath = %q +keyPath = %q +`, pluginType, crtPath, keyPath) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0o600)) + result, err := config.LoadClientConfigResult(configPath, true) + require.NoError(t, err) + configSource := source.NewConfigSource() + storeSource, err := source.NewStoreSource(source.StoreSourceConfig{Path: filepath.Join(dir, "store.json")}) + require.NoError(t, err) + aggregator := source.NewAggregator(configSource) + aggregator.SetStoreSource(storeSource) + sender := &certificateReloadSender{started: make(chan string, 4)} + transporter := transport.NewMessageTransporter(sender) + ctx := context.Background() + pm := proxy.NewManager(ctx, result.Common, nil, transporter, nil, "") + vm := visitor.NewManager(ctx, "", result.Common, nil, transporter, nil) + t.Cleanup(pm.Close) + t.Cleanup(vm.Close) + svr := &Service{ + common: result.Common, + configFilePath: configPath, + unsafeFeatures: security.NewUnsafeFeatures(nil), + aggregator: aggregator, + configSource: configSource, + storeSource: storeSource, + ctl: &Control{pm: pm, vm: vm}, + } + require.NoError(t, svr.UpdateConfigSource(result.Common, result.Proxies, nil)) + select { + case name := <-sender.started: + require.Equal(t, "https", name) + case <-time.After(5 * time.Second): + t.Fatal("proxy did not request startup") + } + require.NoError(t, pm.StartProxy("https", ":8443", "")) + manager := &serviceConfigManager{svr: svr} + + connect := func(wantCert []byte) *tls.Conn { + t.Helper() + clientRaw, serverRaw := net.Pipe() + t.Cleanup(func() { _ = clientRaw.Close(); _ = serverRaw.Close() }) + require.NoError(t, clientRaw.SetDeadline(time.Now().Add(5*time.Second))) + pm.HandleWorkConn("https", serverRaw, &msg.StartWorkConn{}) + conn := tls.Client(clientRaw, &tls.Config{ServerName: "localhost", InsecureSkipVerify: true}) + require.NoError(t, conn.Handshake()) + require.Equal(t, wantCert, conn.ConnectionState().PeerCertificates[0].Raw) + return conn + } + oldConn := connect(original) + reader := bufio.NewReader(oldConn) + checkExistingConnection := func() { + t.Helper() + require.NoError(t, oldConn.SetDeadline(time.Now().Add(5*time.Second))) + // A mismatched Host is handled locally, so no upstream network is needed. + _, err := io.WriteString(oldConn, "GET / HTTP/1.1\r\nHost: other.example\r\n\r\n") + require.NoError(t, err) + resp, err := http.ReadResponse(reader, nil) + require.NoError(t, err) + require.Equal(t, http.StatusMisdirectedRequest, resp.StatusCode) + _, err = io.Copy(io.Discard, resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + } + checkExistingConnection() + require.NoError(t, manager.ReloadFromFile(true)) + require.NoError(t, connect(original).NetConn().Close()) + + next := writeReloadCertificate(t, crtPath, keyPath) + // Both store mutation helpers must leave unchanged proxies' certificates alone. + _, err = manager.CreateStoreProxy(result.Proxies[0].Clone()) + require.NoError(t, err) + require.NoError(t, connect(original).NetConn().Close()) + _, err = manager.UpdateStoreProxy("https", result.Proxies[0].Clone()) + require.NoError(t, err) + require.NoError(t, connect(original).NetConn().Close()) + require.NoError(t, manager.DeleteStoreProxy("https")) + require.NoError(t, connect(original).NetConn().Close()) + require.NoError(t, manager.ReloadFromFile(true)) + require.NoError(t, connect(next).NetConn().Close()) + checkExistingConnection() + + for _, badFile := range []string{"missing", "invalid"} { + saved, err := os.ReadFile(crtPath) + require.NoError(t, err) + if badFile == "missing" { + require.NoError(t, os.Remove(crtPath)) + } else { + require.NoError(t, os.WriteFile(crtPath, []byte("invalid"), 0o600)) + } + require.NoError(t, manager.ReloadFromFile(true), "certificate errors must not fail reload") + status, ok := pm.GetProxyStatus("https") + require.True(t, ok) + require.Equal(t, proxy.ProxyPhaseRunning, status.Phase) + require.Contains(t, status.Err, "reload plugin ") + require.NoError(t, connect(next).NetConn().Close()) + checkExistingConnection() + require.NoError(t, os.WriteFile(crtPath, saved, 0o600)) + require.NoError(t, manager.ReloadFromFile(true)) + status, _ = pm.GetProxyStatus("https") + require.Empty(t, status.Err, "restoring the active bytes must clear the reload error") + } + + // A config change still replaces the proxy and uses normal startup errors. + require.NoError(t, oldConn.NetConn().Close()) + require.NoError(t, os.WriteFile(crtPath, []byte("invalid"), 0o600)) + changedConfig := strings.Replace(content, "remotePort = 8443", "remotePort = 8444", 1) + require.NoError(t, os.WriteFile(configPath, []byte(changedConfig), 0o600)) + require.NoError(t, manager.ReloadFromFile(true)) + select { + case name := <-sender.started: + require.Equal(t, "https", name) + case <-time.After(5 * time.Second): + t.Fatal("replacement proxy did not request startup") + } + require.Error(t, pm.StartProxy("https", ":8444", "")) + status, ok := pm.GetProxyStatus("https") + require.True(t, ok) + require.Equal(t, proxy.ProxyPhaseStartErr, status.Phase) + require.NotEmpty(t, status.Err) + }) + } +} + +func TestFileReloadCertificatesDuringControlHandoff(t *testing.T) { + for _, pluginType := range []string{v1.PluginHTTPS2HTTP, v1.PluginHTTPS2HTTPS} { + t.Run(pluginType, func(t *testing.T) { + dir := t.TempDir() + crtPath, keyPath := filepath.Join(dir, "tls.crt"), filepath.Join(dir, "tls.key") + original := writeReloadCertificate(t, crtPath, keyPath) + configPath := filepath.Join(dir, "frpc.toml") + require.NoError(t, os.WriteFile(configPath, fmt.Appendf(nil, `[[proxies]] +name = "https" +type = "tcp" +remotePort = 8443 +[proxies.plugin] +type = %q +localAddr = "127.0.0.1:80" +crtPath = %q +keyPath = %q +`, pluginType, crtPath, keyPath), 0o600)) + result, err := config.LoadClientConfigResult(configPath, true) + require.NoError(t, err) + configSource := source.NewConfigSource() + svr := &Service{ + common: result.Common, + configFilePath: configPath, + unsafeFeatures: security.NewUnsafeFeatures(nil), + aggregator: source.NewAggregator(configSource), + configSource: configSource, + } + require.NoError(t, svr.UpdateConfigSource(result.Common, result.Proxies, nil)) + sender := &certificateReloadSender{started: make(chan string, 1)} + transporter := transport.NewMessageTransporter(sender) + newControl := func() *Control { + ctx := context.Background() + pm := proxy.NewManager(ctx, result.Common, nil, transporter, nil, "") + vm := visitor.NewManager(ctx, "", result.Common, nil, transporter, nil) + t.Cleanup(pm.Close) + t.Cleanup(vm.Close) + return &Control{pm: pm, vm: vm} + } + previous := newControl() + previous.pm.Close() + svr.ctl = previous + next := newControl() + + // Model the handoff transaction: start the new control's proxies while + // the published control is still the closed, empty previous manager. + svr.reloadMu.Lock() + releaseTransaction := sync.OnceFunc(svr.reloadMu.Unlock) + defer releaseTransaction() + next.pm.UpdateAll(svr.proxyCfgs) + select { + case name := <-sender.started: + require.Equal(t, "https", name) + case <-time.After(5 * time.Second): + t.Fatal("replacement proxy did not request startup") + } + require.NoError(t, next.pm.StartProxy("https", ":8443", "")) + checkCertificate := func(want []byte) { + t.Helper() + clientRaw, serverRaw := net.Pipe() + defer clientRaw.Close() + defer serverRaw.Close() + require.NoError(t, clientRaw.SetDeadline(time.Now().Add(5*time.Second))) + next.pm.HandleWorkConn("https", serverRaw, &msg.StartWorkConn{}) + conn := tls.Client(clientRaw, &tls.Config{ServerName: "localhost", InsecureSkipVerify: true}) + require.NoError(t, conn.Handshake()) + require.Equal(t, want, conn.ConnectionState().PeerCertificates[0].Raw) + } + checkCertificate(original) + renewed := writeReloadCertificate(t, crtPath, keyPath) + + svr.ctlMu.Lock() + releaseControl := sync.OnceFunc(svr.ctlMu.Unlock) + defer releaseControl() + reloadStarted, reloadDone := make(chan struct{}), make(chan struct{}) + var reloadErr error + go func() { + close(reloadStarted) + reloadErr = (&serviceConfigManager{svr: svr}).ReloadFromFile(true) + close(reloadDone) + }() + t.Cleanup(func() { + releaseTransaction() + releaseControl() + waitLifecycleSignal(t, reloadDone) + }) + waitLifecycleSignal(t, reloadStarted) + svr.ctl = next + releaseTransaction() + + // Keep ctlMu as a read barrier after publication. Reload must acquire + // reloadMu before selecting ctl, independently of goroutine scheduling. + // The deadline only detects a broken lock order; it does not stage the race. + deadline := time.NewTimer(5 * time.Second) + defer deadline.Stop() + for svr.reloadMu.TryLock() { + svr.reloadMu.Unlock() + select { + case <-deadline.C: + t.Fatal("reload selected ctl without first acquiring reloadMu") + default: + runtime.Gosched() + } + } + releaseControl() + waitLifecycleSignal(t, reloadDone) + require.NoError(t, reloadErr) + checkCertificate(renewed) + }) + } +} diff --git a/client/http/controller.go b/client/http/controller.go index 57e8165d..400e3cb5 100644 --- a/client/http/controller.go +++ b/client/http/controller.go @@ -149,7 +149,7 @@ func (c *Controller) buildProxyStatusResp(status *proxy.WorkingStatus) model.Pro } psr.Plugin = baseCfg.Plugin.Type - if status.Err == "" { + if status.Err == "" || status.Phase == proxy.ProxyPhaseRunning { psr.RemoteAddr = status.RemoteAddr if slices.Contains([]string{"tcp", "udp"}, status.Type) { psr.RemoteAddr = c.serverAddr + psr.RemoteAddr diff --git a/client/http/controller_test.go b/client/http/controller_test.go index 719fbcd4..f1cf572e 100644 --- a/client/http/controller_test.go +++ b/client/http/controller_test.go @@ -213,6 +213,35 @@ func TestBuildProxyStatusRespStoreSourceEnabled(t *testing.T) { } } +func TestBuildProxyStatusRespRunningWithErrorKeepsRemoteAddr(t *testing.T) { + const certificateErr = "reload plugin https2http: invalid certificate" + controller := &Controller{serverAddr: "127.0.0.1", manager: &fakeConfigManager{}} + for _, tc := range []struct { + name, phase, err, remoteAddr string + }{ + {"running", proxy.ProxyPhaseRunning, "", "127.0.0.1:43210"}, + {"running with error", proxy.ProxyPhaseRunning, certificateErr, "127.0.0.1:43210"}, + {"start error", proxy.ProxyPhaseStartErr, "start failed", ""}, + {"closed with error", proxy.ProxyPhaseClosed, certificateErr, ""}, + {"closed without error", proxy.ProxyPhaseClosed, "", "127.0.0.1:43210"}, + } { + t.Run(tc.name, func(t *testing.T) { + status := &proxy.WorkingStatus{ + Name: "https", Type: "tcp", Phase: tc.phase, Err: tc.err, + RemoteAddr: ":43210", + Cfg: newRawTCPProxyConfig("https"), // RemotePort remains zero (allocated by frps). + } + resp := controller.buildProxyStatusResp(status) + if resp.RemoteAddr != tc.remoteAddr { + t.Fatalf("unexpected remote addr: got %q, want %q", resp.RemoteAddr, tc.remoteAddr) + } + if resp.Err != tc.err || resp.Status != tc.phase { + t.Fatalf("status or error changed: %+v", resp) + } + }) + } +} + func TestReloadErrorMapping(t *testing.T) { tests := []struct { name string diff --git a/client/proxy/plugin_reload_test.go b/client/proxy/plugin_reload_test.go new file mode 100644 index 00000000..902c28b7 --- /dev/null +++ b/client/proxy/plugin_reload_test.go @@ -0,0 +1,148 @@ +// Copyright 2026 The frp Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !frps + +package proxy + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" + + plugin "github.com/fatedier/frp/pkg/plugin/client" + "github.com/fatedier/frp/pkg/util/xlog" +) + +type reloadError struct { + err error +} + +type testReloadPlugin struct { + calls atomic.Int32 + state atomic.Pointer[reloadError] + reload func() error +} + +func (p *testReloadPlugin) Name() string { return "test" } +func (p *testReloadPlugin) Handle(context.Context, *plugin.ConnectionInfo) {} +func (p *testReloadPlugin) Close() error { return nil } + +func (p *testReloadPlugin) Reload() error { + p.calls.Add(1) + err := p.reload() + p.state.Store(&reloadError{err: err}) + return err +} + +func (p *testReloadPlugin) ReloadError() error { + if s := p.state.Load(); s != nil { + return s.err + } + return nil +} + +func reloadFixture(p *testReloadPlugin) (*Manager, *Wrapper, *BaseProxy) { + base := &BaseProxy{proxyPlugin: p} + pw := &Wrapper{ + WorkingStatus: WorkingStatus{Name: "same", Phase: ProxyPhaseRunning}, + pxy: &GeneralTCPProxy{BaseProxy: base}, + xl: xlog.New(), + } + return &Manager{proxies: map[string]*Wrapper{"same": pw}}, pw, base +} + +func TestPluginSnapshotSkipsReplacedAndStopped(t *testing.T) { + p := &testReloadPlugin{reload: func() error { return nil }} + pm, pw, base := reloadFixture(p) + run := pm.PreparePluginReload() + q := &testReloadPlugin{reload: func() error { return nil }} + pw.mu.Lock() + base.proxyPlugin = q + pw.mu.Unlock() + run() + require.Zero(t, p.calls.Load()) + require.Zero(t, q.calls.Load()) + run = pm.PreparePluginReload() + pw.mu.Lock() + pw.Phase = ProxyPhaseClosed + pw.mu.Unlock() + run() + require.Zero(t, q.calls.Load()) +} + +func TestPluginErrorOwnershipAndLockIsolation(t *testing.T) { + started, release, done := make(chan struct{}), make(chan struct{}), make(chan struct{}) + p := &testReloadPlugin{reload: func() error { + close(started) + <-release + return errors.New("bad certificate") + }} + pm, pw, base := reloadFixture(p) + other := &testReloadPlugin{reload: func() error { return nil }} + pm2, _, _ := reloadFixture(other) + run := pm.PreparePluginReload() + releaseReload := sync.OnceFunc(func() { close(release) }) + t.Cleanup(func() { + releaseReload() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Error("reload did not finish") + } + }) + go func() { + run() + close(done) + }() + select { + case <-started: + case <-time.After(5 * time.Second): + t.Fatal("not started") + } + require.True(t, pm.mu.TryLock()) + pm.mu.Unlock() + require.True(t, pw.mu.TryLock()) + pw.mu.Unlock() + require.Empty(t, pw.GetStatus().Err) + require.Zero(t, other.calls.Load(), "same name in another manager must be isolated") + pm2.PreparePluginReload()() + require.EqualValues(t, 1, other.calls.Load()) + q := &testReloadPlugin{reload: func() error { return nil }} + pw.mu.Lock() + base.proxyPlugin = q + pw.mu.Unlock() + releaseReload() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("not done") + } + require.Error(t, p.ReloadError()) + require.Empty(t, pw.GetStatus().Err, "retired plugin errors must not attach to its replacement") + q.state.Store(&reloadError{err: errors.New("new plugin error")}) + require.Contains(t, pw.GetStatus().Err, "new plugin error") + require.Empty(t, pw.Err, "plugin state must not overwrite lifecycle errors") + pw.mu.Lock() + pw.Err = "lifecycle" + pw.mu.Unlock() + require.Contains(t, pw.GetStatus().Err, "lifecycle; reload plugin") + require.NoError(t, q.Reload()) + require.Equal(t, "lifecycle", pw.GetStatus().Err) +} diff --git a/client/proxy/proxy.go b/client/proxy/proxy.go index e982c03b..a09967c8 100644 --- a/client/proxy/proxy.go +++ b/client/proxy/proxy.go @@ -120,6 +120,9 @@ func (pxy *BaseProxy) Run() error { return nil } +// getPlugin is accessed while the owning Wrapper holds its lifecycle lock. +func (pxy *BaseProxy) getPlugin() plugin.Plugin { return pxy.proxyPlugin } + func (pxy *BaseProxy) Close() { if pxy.proxyPlugin != nil { pxy.proxyPlugin.Close() diff --git a/client/proxy/proxy_manager.go b/client/proxy/proxy_manager.go index 6bf8d4a8..6f525c24 100644 --- a/client/proxy/proxy_manager.go +++ b/client/proxy/proxy_manager.go @@ -139,6 +139,41 @@ func (pm *Manager) GetProxyStatus(name string) (*WorkingStatus, bool) { return nil, false } +// PreparePluginReload snapshots live plugin instances before applying config. +// Invoke the returned function only after config application succeeds and all +// configuration locks have been released. Replaced instances are skipped. +func (pm *Manager) PreparePluginReload() func() { + type target struct { + pw *Wrapper + p reloadablePlugin + } + pm.mu.RLock() + targets := make([]target, 0, len(pm.proxies)) + for _, pw := range pm.proxies { + pw.mu.RLock() + if pw.Phase == ProxyPhaseRunning { + if p := pw.reloadablePluginLocked(); p != nil { + targets = append(targets, target{pw: pw, p: p}) + } + } + pw.mu.RUnlock() + } + pm.mu.RUnlock() + return func() { + for _, t := range targets { + t.pw.mu.RLock() + current := t.pw.Phase == ProxyPhaseRunning && t.pw.reloadablePluginLocked() == t.p + t.pw.mu.RUnlock() + if !current { + continue + } + if err := t.p.Reload(); err != nil { + t.pw.xl.Warnf("reload plugin %s: %v", t.p.Name(), err) + } + } + } +} + func (pm *Manager) UpdateAll(proxyCfgs []v1.ProxyConfigurer) { xl := xlog.FromContextSafe(pm.ctx) proxyCfgsMap := lo.KeyBy(proxyCfgs, func(c v1.ProxyConfigurer) string { diff --git a/client/proxy/proxy_wrapper.go b/client/proxy/proxy_wrapper.go index 30ea5e33..bfe55504 100644 --- a/client/proxy/proxy_wrapper.go +++ b/client/proxy/proxy_wrapper.go @@ -30,6 +30,7 @@ import ( v1 "github.com/fatedier/frp/pkg/config/v1" "github.com/fatedier/frp/pkg/msg" "github.com/fatedier/frp/pkg/naming" + plugin "github.com/fatedier/frp/pkg/plugin/client" "github.com/fatedier/frp/pkg/transport" "github.com/fatedier/frp/pkg/util/xlog" "github.com/fatedier/frp/pkg/vnet" @@ -277,6 +278,25 @@ func (pw *Wrapper) InWorkConn(workConn net.Conn, m *msg.StartWorkConn) { } } +// reloadablePlugin owns its reload serialization and error snapshot. ReloadError +// must not block on I/O. Reload must be safe during Close, and retain usable +// resources on failure. Existing plugins need not implement this capability. +type reloadablePlugin interface { + plugin.Plugin + Reload() error + ReloadError() error +} + +// Caller holds pw.mu. Concrete reloadable plugins are pointer instances. +func (pw *Wrapper) reloadablePluginLocked() reloadablePlugin { + owner, ok := pw.pxy.(interface{ getPlugin() plugin.Plugin }) + if !ok { + return nil + } + p, _ := owner.getPlugin().(reloadablePlugin) + return p +} + func (pw *Wrapper) GetStatus() *WorkingStatus { pw.mu.RLock() defer pw.mu.RUnlock() @@ -288,5 +308,15 @@ func (pw *Wrapper) GetStatus() *WorkingStatus { Cfg: pw.Cfg, RemoteAddr: pw.RemoteAddr, } + if pw.Phase == ProxyPhaseRunning { + if p := pw.reloadablePluginLocked(); p != nil { + if err := p.ReloadError(); err != nil { + if ps.Err != "" { + ps.Err += "; " + } + ps.Err += fmt.Sprintf("reload plugin %s: %v", p.Name(), err) + } + } + } return ps } diff --git a/client/service.go b/client/service.go index e364dd11..bfeca8ae 100644 --- a/client/service.go +++ b/client/service.go @@ -424,6 +424,15 @@ func (svr *Service) UpdateConfigSource( svr.reloadMu.Lock() defer svr.reloadMu.Unlock() + return svr.updateConfigSourceLocked(common, proxyCfgs, visitorCfgs) +} + +// Caller must hold reloadMu. +func (svr *Service) updateConfigSourceLocked( + common *v1.ClientCommonConfig, + proxyCfgs []v1.ProxyConfigurer, + visitorCfgs []v1.VisitorConfigurer, +) error { cfgSource := svr.configSource if cfgSource == nil { return fmt.Errorf("config source is not available") diff --git a/pkg/plugin/client/http_common.go b/pkg/plugin/client/http_common.go index 5010a4e9..5e1eb0f5 100644 --- a/pkg/plugin/client/http_common.go +++ b/pkg/plugin/client/http_common.go @@ -51,8 +51,9 @@ func rewriteHTTPPluginRequest( } type httpBridgePlugin struct { - l *Listener - s *http.Server + l *Listener + s *http.Server + httpsServer *httpsserver.Server useSourceRemoteAddr bool } @@ -87,7 +88,8 @@ func newHTTPSBridgePluginServer( } p := &httpBridgePlugin{ l: listener, - s: server, + s: server.Server, + httpsServer: server, useSourceRemoteAddr: useSourceRemoteAddr, } go func() { @@ -124,3 +126,18 @@ func (p *httpBridgePlugin) Close() error { _ = p.l.Close() return err } + +// Reload refreshes only resources owned by this plugin; HTTP bridges are no-op. +func (p *httpBridgePlugin) Reload() error { + if p.httpsServer == nil { + return nil + } + return p.httpsServer.Reload() +} + +func (p *httpBridgePlugin) ReloadError() error { + if p.httpsServer == nil { + return nil + } + return p.httpsServer.ReloadError() +} diff --git a/pkg/plugin/client/internal/httpsserver/server.go b/pkg/plugin/client/internal/httpsserver/server.go index a2047b90..f3000c1a 100644 --- a/pkg/plugin/client/internal/httpsserver/server.go +++ b/pkg/plugin/client/internal/httpsserver/server.go @@ -17,9 +17,13 @@ package httpsserver import ( + "bytes" "crypto/tls" "fmt" "net/http" + "os" + "sync" + "sync/atomic" "time" "github.com/samber/lo" @@ -28,13 +32,38 @@ import ( httppkg "github.com/fatedier/frp/pkg/util/http" ) -func New(handler http.Handler, crtPath, keyPath string, enableHTTP2 *bool) (*http.Server, error) { - tlsConfig, err := transport.NewServerTLSConfig(crtPath, keyPath, "") +// snapshot is immutable after publication. Errors retain the last usable pair. +type snapshot struct { + cert *tls.Certificate + certPEM, keyPEM []byte + err error +} + +type Server struct { + *http.Server + + crtPath, keyPath string + reloadMu sync.Mutex + state atomic.Pointer[snapshot] +} + +func New(handler http.Handler, crtPath, keyPath string, enableHTTP2 *bool) (*Server, error) { + server := &Server{crtPath: crtPath, keyPath: keyPath} + var tlsConfig *tls.Config + var err error + if crtPath == "" || keyPath == "" { + tlsConfig, err = transport.NewServerTLSConfig(crtPath, keyPath, "") + } else { + err = server.Reload() + tlsConfig = &tls.Config{GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + return server.state.Load().cert, nil + }} + } if err != nil { return nil, fmt.Errorf("gen TLS config error: %v", err) } - server := &http.Server{ + server.Server = &http.Server{ Handler: withMisdirectedRequestCheck(handler), ReadHeaderTimeout: 60 * time.Second, TLSConfig: tlsConfig, @@ -58,3 +87,51 @@ func withMisdirectedRequestCheck(handler http.Handler) http.Handler { handler.ServeHTTP(w, r) }) } + +func (s *Server) Reload() (err error) { + if s.crtPath == "" || s.keyPath == "" { + return nil + } + s.reloadMu.Lock() + defer s.reloadMu.Unlock() + old := s.state.Load() + // Publish the error before releasing reloadMu; status never waits for I/O. + defer func() { + if err != nil { + next := snapshot{err: err} + if old != nil { + next.cert, next.certPEM, next.keyPEM = old.cert, old.certPEM, old.keyPEM + } + s.state.Store(&next) + } + }() + certPEM, err := os.ReadFile(s.crtPath) + if err != nil { + return fmt.Errorf("read certificate: %w", err) + } + keyPEM, err := os.ReadFile(s.keyPath) + if err != nil { + return fmt.Errorf("read private key: %w", err) + } + if old != nil && old.cert != nil && bytes.Equal(certPEM, old.certPEM) && bytes.Equal(keyPEM, old.keyPEM) { + if old.err != nil { + next := *old + next.err = nil + s.state.Store(&next) + } + return nil + } + cert, err := tls.X509KeyPair(certPEM, keyPEM) + if err != nil { + return fmt.Errorf("parse certificate pair: %w", err) + } + s.state.Store(&snapshot{cert: &cert, certPEM: certPEM, keyPEM: keyPEM}) + return nil +} + +func (s *Server) ReloadError() error { + if v := s.state.Load(); v != nil { + return v.err + } + return nil +} diff --git a/pkg/plugin/client/internal/httpsserver/server_test.go b/pkg/plugin/client/internal/httpsserver/server_test.go new file mode 100644 index 00000000..c42488ac --- /dev/null +++ b/pkg/plugin/client/internal/httpsserver/server_test.go @@ -0,0 +1,153 @@ +// Copyright 2026 The frp Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !frps + +package httpsserver + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "encoding/pem" + "io" + "math/big" + "net" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func certificatePEM(t *testing.T, serial int64) ([]byte, []byte) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + template := &x509.Certificate{ + SerialNumber: big.NewInt(serial), + DNSNames: []string{"localhost"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + require.NoError(t, err) + keyDER, err := x509.MarshalPKCS8PrivateKey(key) + require.NoError(t, err) + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), + pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyDER}) +} + +func handshake(t *testing.T, cfg *tls.Config) (*tls.Conn, *tls.Conn) { + t.Helper() + clientRaw, serverRaw := net.Pipe() + t.Cleanup(func() { _ = clientRaw.Close(); _ = serverRaw.Close() }) + require.NoError(t, clientRaw.SetDeadline(time.Now().Add(5*time.Second))) + require.NoError(t, serverRaw.SetDeadline(time.Now().Add(5*time.Second))) + client := tls.Client(clientRaw, &tls.Config{InsecureSkipVerify: true}) + server := tls.Server(serverRaw, cfg) + done := make(chan error, 1) + go func() { done <- server.Handshake() }() + require.NoError(t, client.Handshake()) + require.NoError(t, <-done) + return client, server +} + +func TestReloadCertificate(t *testing.T) { + dir := t.TempDir() + crtPath, keyPath := filepath.Join(dir, "tls.crt"), filepath.Join(dir, "tls.key") + certPEM, keyPEM := certificatePEM(t, 1) + require.NoError(t, os.WriteFile(crtPath, certPEM, 0o600)) + require.NoError(t, os.WriteFile(keyPath, keyPEM, 0o600)) + s, err := New(nil, crtPath, keyPath, nil) + require.NoError(t, err) + originalState := s.state.Load() + original := originalState.cert + oldClient, oldServer := handshake(t, s.TLSConfig) + require.Equal(t, int64(1), oldClient.ConnectionState().PeerCertificates[0].SerialNumber.Int64()) + + require.NoError(t, s.Reload()) + require.Same(t, originalState, s.state.Load(), "unchanged bytes must not parse or publish a new pair") + require.NoError(t, s.ReloadError()) + + for _, tc := range []struct { + name, path string + data []byte + }{ + {"missing certificate", crtPath, nil}, + {"missing key", keyPath, nil}, + {"invalid certificate", crtPath, []byte("invalid")}, + {"invalid key", keyPath, []byte("invalid")}, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.data == nil { + require.NoError(t, os.Remove(tc.path)) + } else { + require.NoError(t, os.WriteFile(tc.path, tc.data, 0o600)) + } + require.Error(t, s.Reload()) + require.Error(t, s.ReloadError()) + require.Same(t, original, s.state.Load().cert) + client, _ := handshake(t, s.TLSConfig) + require.Equal(t, int64(1), client.ConnectionState().PeerCertificates[0].SerialNumber.Int64()) + require.NoError(t, os.WriteFile(crtPath, certPEM, 0o600)) + require.NoError(t, os.WriteFile(keyPath, keyPEM, 0o600)) + require.NoError(t, s.Reload()) + require.NoError(t, s.ReloadError()) + require.Same(t, original, s.state.Load().cert) + }) + } + // Even a key-only byte change must be noticed, regardless of parsed identity. + require.NoError(t, os.WriteFile(keyPath, append(keyPEM, '\n'), 0o600)) + require.NoError(t, s.Reload()) + require.NotSame(t, original, s.state.Load().cert) + + nextCert, nextKey := certificatePEM(t, 2) + require.NoError(t, os.WriteFile(crtPath, nextCert, 0o600)) + beforeMismatch := s.state.Load().cert + require.Error(t, s.Reload(), "mismatched pair must retain the active certificate") + require.Same(t, beforeMismatch, s.state.Load().cert) + require.NoError(t, os.WriteFile(keyPath, nextKey, 0o600)) + beforeReload, _ := handshake(t, s.TLSConfig) + require.Equal(t, int64(1), beforeReload.ConnectionState().PeerCertificates[0].SerialNumber.Int64()) + require.NoError(t, s.Reload()) + newClient, _ := handshake(t, s.TLSConfig) + require.Equal(t, int64(2), newClient.ConnectionState().PeerCertificates[0].SerialNumber.Int64()) + + require.NoError(t, oldClient.SetDeadline(time.Now().Add(5*time.Second))) + require.NoError(t, oldServer.SetDeadline(time.Now().Add(5*time.Second))) + done := make(chan error, 1) + go func() { _, err := oldServer.Write([]byte("still connected")); done <- err }() + data := make([]byte, len("still connected")) + _, err = io.ReadFull(oldClient, data) + require.NoError(t, err) + require.NoError(t, <-done) + require.Equal(t, "still connected", string(data)) +} + +func TestReloadCertificateKeepsSelfSigned(t *testing.T) { + missing := filepath.Join(t.TempDir(), "missing") + for _, paths := range [][2]string{{"", ""}, {missing, ""}, {"", missing}} { + s, err := New(nil, paths[0], paths[1], nil) + require.NoError(t, err) + original := &s.TLSConfig.Certificates[0] + require.Nil(t, s.TLSConfig.GetCertificate) + require.NoError(t, s.Reload()) + require.Same(t, original, &s.TLSConfig.Certificates[0]) + } +}