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]) + } +}