From 36faffb44f0689d3e8ad55af4a35b9420c4bddc6 Mon Sep 17 00:00:00 2001 From: jagaimoworks Date: Sat, 3 Aug 2024 14:17:13 +0200 Subject: [PATCH 01/17] added capability to set forwarded ports at runtime --- internal/portforward/interfaces.go | 1 + internal/portforward/loop.go | 11 +++++++++ internal/portforward/service/service.go | 31 +++++++++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/internal/portforward/interfaces.go b/internal/portforward/interfaces.go index fb442d5eb..d56e5390a 100644 --- a/internal/portforward/interfaces.go +++ b/internal/portforward/interfaces.go @@ -10,6 +10,7 @@ type Service interface { Start(ctx context.Context) (runError <-chan error, err error) Stop() (err error) GetPortsForwarded() (ports []uint16) + SetPortsForwarded(ctx context.Context, ports []uint16) (err error) } type Routing interface { diff --git a/internal/portforward/loop.go b/internal/portforward/loop.go index 8f9431456..23fb657d6 100644 --- a/internal/portforward/loop.go +++ b/internal/portforward/loop.go @@ -166,6 +166,17 @@ func (l *Loop) GetPortsForwarded() (ports []uint16) { return l.service.GetPortsForwarded() } +func (l *Loop) SetPortsForwarded(ports []uint16) { + if l.service == nil { + return + } + + err := l.service.SetPortsForwarded(l.runCtx, ports) + if err != nil { + l.logger.Error(err.Error()) + } +} + func ptrTo[T any](value T) *T { return &value } diff --git a/internal/portforward/service/service.go b/internal/portforward/service/service.go index 579b37397..f2b8f4304 100644 --- a/internal/portforward/service/service.go +++ b/internal/portforward/service/service.go @@ -50,3 +50,34 @@ func (s *Service) GetPortsForwarded() (ports []uint16) { copy(ports, s.ports) return ports } + +func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err error) { + for _, port := range s.ports { + err := s.portAllower.RemoveAllowedPort(ctx, port) + if err != nil { + s.logger.Error(err.Error()) + } + } + + for _, port := range ports { + err := s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) + if err != nil { + s.logger.Error(err.Error()) + } + } + + err = s.writePortForwardedFile(ports) + if err != nil { + _ = s.cleanup() + return err + } + + s.portMutex.RLock() + defer s.portMutex.RUnlock() + s.ports = make([]uint16, len(ports)) + copy(s.ports, ports) + + s.logger.Info("Updated: " + portsToString(s.ports)) + + return nil +} From 101656ad2d11f82497b59afe7224cb0080cd00a6 Mon Sep 17 00:00:00 2001 From: jagaimoworks Date: Sat, 3 Aug 2024 14:18:24 +0200 Subject: [PATCH 02/17] added control server route for dynamic port forwarding --- internal/server/handler.go | 4 ++-- internal/server/interfaces.go | 3 ++- internal/server/openvpn.go | 32 +++++++++++++++++++++++++++++--- internal/server/server.go | 4 ++-- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/internal/server/handler.go b/internal/server/handler.go index 92cdf7b90..1a1f301b3 100644 --- a/internal/server/handler.go +++ b/internal/server/handler.go @@ -15,7 +15,7 @@ func newHandler(ctx context.Context, logger Logger, logging bool, authSettings auth.Settings, buildInfo models.BuildInformation, vpnLooper VPNLooper, - pfGetter PortForwardedGetter, + pf PortForwarded, dnsLooper DNSLoop, updaterLooper UpdaterLooper, publicIPLooper PublicIPLoop, @@ -25,7 +25,7 @@ func newHandler(ctx context.Context, logger Logger, logging bool, handler := &handler{} vpn := newVPNHandler(ctx, vpnLooper, storage, ipv6Supported, logger) - openvpn := newOpenvpnHandler(ctx, vpnLooper, pfGetter, logger) + openvpn := newOpenvpnHandler(ctx, vpnLooper, pf, logger) dns := newDNSHandler(ctx, dnsLooper, logger) updater := newUpdaterHandler(ctx, updaterLooper, logger) publicip := newPublicIPHandler(publicIPLooper, logger) diff --git a/internal/server/interfaces.go b/internal/server/interfaces.go index 5b4702510..711fcdf03 100644 --- a/internal/server/interfaces.go +++ b/internal/server/interfaces.go @@ -21,8 +21,9 @@ type DNSLoop interface { GetStatus() (status models.LoopStatus) } -type PortForwardedGetter interface { +type PortForwarded interface { GetPortsForwarded() (ports []uint16) + SetPortsForwarded(ports []uint16) } type PublicIPLoop interface { diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index 3e25c5feb..e49921bb1 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -11,12 +11,12 @@ import ( ) func newOpenvpnHandler(ctx context.Context, looper VPNLooper, - pfGetter PortForwardedGetter, w warner, + portForwarded PortForwarded, w warner, ) http.Handler { return &openvpnHandler{ ctx: ctx, looper: looper, - pf: pfGetter, + pf: portForwarded, warner: w, } } @@ -24,7 +24,7 @@ func newOpenvpnHandler(ctx context.Context, looper VPNLooper, type openvpnHandler struct { ctx context.Context //nolint:containedctx looper VPNLooper - pf PortForwardedGetter + pf PortForwarded warner warner } @@ -51,6 +51,8 @@ func (h *openvpnHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: h.getPortForwarded(w) + case http.MethodPut: + h.setPortForwarded(w, r) default: errMethodNotSupported(w, r.Method) } @@ -142,3 +144,27 @@ func (h *openvpnHandler) getPortForwarded(w http.ResponseWriter) { w.WriteHeader(http.StatusInternalServerError) } } + +func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request) { + decoder := json.NewDecoder(r.Body) + encoder := json.NewEncoder(w) + var data portsWrapper + + if err := decoder.Decode(&data); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + if len(data.Ports) == 0 { + http.Error(w, "invalid request", http.StatusBadRequest) + return + } + + h.pf.SetPortsForwarded(data.Ports) + + err := encoder.Encode(h.pf.GetPortsForwarded()) + if err != nil { + h.warner.Warn(err.Error()) + w.WriteHeader(http.StatusInternalServerError) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 3f50717b2..57da7ce1e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -11,7 +11,7 @@ import ( func New(ctx context.Context, address string, logEnabled bool, logger Logger, authConfigPath string, buildInfo models.BuildInformation, openvpnLooper VPNLooper, - pfGetter PortForwardedGetter, dnsLooper DNSLoop, + pf PortForwarded, dnsLooper DNSLoop, updaterLooper UpdaterLooper, publicIPLooper PublicIPLoop, storage Storage, ipv6Supported bool) ( server *httpserver.Server, err error, @@ -27,7 +27,7 @@ func New(ctx context.Context, address string, logEnabled bool, logger Logger, } handler, err := newHandler(ctx, logger, logEnabled, authSettings, buildInfo, - openvpnLooper, pfGetter, dnsLooper, updaterLooper, publicIPLooper, + openvpnLooper, pf, dnsLooper, updaterLooper, publicIPLooper, storage, ipv6Supported) if err != nil { return nil, fmt.Errorf("creating handler: %w", err) From 8a5b48145a6b9ab414a6048690a669e0617f0321 Mon Sep 17 00:00:00 2001 From: jagaimoworks Date: Mon, 5 Aug 2024 17:35:04 +0200 Subject: [PATCH 03/17] addressing renaming nits --- internal/portforward/service/service.go | 2 +- internal/server/handler.go | 2 +- internal/server/interfaces.go | 2 +- internal/server/openvpn.go | 6 +++--- internal/server/server.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/portforward/service/service.go b/internal/portforward/service/service.go index f2b8f4304..b4bc357b8 100644 --- a/internal/portforward/service/service.go +++ b/internal/portforward/service/service.go @@ -77,7 +77,7 @@ func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err er s.ports = make([]uint16, len(ports)) copy(s.ports, ports) - s.logger.Info("Updated: " + portsToString(s.ports)) + s.logger.Info("updated: " + portsToString(s.ports)) return nil } diff --git a/internal/server/handler.go b/internal/server/handler.go index 1a1f301b3..9508b9ca5 100644 --- a/internal/server/handler.go +++ b/internal/server/handler.go @@ -15,7 +15,7 @@ func newHandler(ctx context.Context, logger Logger, logging bool, authSettings auth.Settings, buildInfo models.BuildInformation, vpnLooper VPNLooper, - pf PortForwarded, + pf PortForwarding, dnsLooper DNSLoop, updaterLooper UpdaterLooper, publicIPLooper PublicIPLoop, diff --git a/internal/server/interfaces.go b/internal/server/interfaces.go index 711fcdf03..df1fe7ce1 100644 --- a/internal/server/interfaces.go +++ b/internal/server/interfaces.go @@ -21,7 +21,7 @@ type DNSLoop interface { GetStatus() (status models.LoopStatus) } -type PortForwarded interface { +type PortForwarding interface { GetPortsForwarded() (ports []uint16) SetPortsForwarded(ports []uint16) } diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index e49921bb1..49e526076 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -11,12 +11,12 @@ import ( ) func newOpenvpnHandler(ctx context.Context, looper VPNLooper, - portForwarded PortForwarded, w warner, + portForwarding PortForwarding, w warner, ) http.Handler { return &openvpnHandler{ ctx: ctx, looper: looper, - pf: portForwarded, + pf: portForwarding, warner: w, } } @@ -24,7 +24,7 @@ func newOpenvpnHandler(ctx context.Context, looper VPNLooper, type openvpnHandler struct { ctx context.Context //nolint:containedctx looper VPNLooper - pf PortForwarded + pf PortForwarding warner warner } diff --git a/internal/server/server.go b/internal/server/server.go index 57da7ce1e..e296cf313 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -11,7 +11,7 @@ import ( func New(ctx context.Context, address string, logEnabled bool, logger Logger, authConfigPath string, buildInfo models.BuildInformation, openvpnLooper VPNLooper, - pf PortForwarded, dnsLooper DNSLoop, + pf PortForwarding, dnsLooper DNSLoop, updaterLooper UpdaterLooper, publicIPLooper PublicIPLoop, storage Storage, ipv6Supported bool) ( server *httpserver.Server, err error, From 8dfad10229059c0e673e2f792bc848537f7273b5 Mon Sep 17 00:00:00 2001 From: jagaimoworks Date: Mon, 5 Aug 2024 19:19:54 +0200 Subject: [PATCH 04/17] try to silently undo changes when port set/remove fail --- internal/portforward/service/service.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/internal/portforward/service/service.go b/internal/portforward/service/service.go index b4bc357b8..8c9311311 100644 --- a/internal/portforward/service/service.go +++ b/internal/portforward/service/service.go @@ -52,17 +52,28 @@ func (s *Service) GetPortsForwarded() (ports []uint16) { } func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err error) { - for _, port := range s.ports { + for i, port := range s.ports { err := s.portAllower.RemoveAllowedPort(ctx, port) if err != nil { + for j := 0; j <= i; j++ { + _ = s.portAllower.SetAllowedPort(ctx, s.ports[j], s.settings.Interface) + } s.logger.Error(err.Error()) + return err } } - for _, port := range ports { + for i, port := range ports { err := s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) if err != nil { + for j := 0; j <= i; j++ { + _ = s.portAllower.RemoveAllowedPort(ctx, s.ports[j]) + } + for _, port := range s.ports { + _ = s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) + } s.logger.Error(err.Error()) + return err } } From d489f45dba15ad67a4604d1599ae82118e742311 Mon Sep 17 00:00:00 2001 From: jagaimoworks Date: Mon, 5 Aug 2024 23:18:42 +0200 Subject: [PATCH 05/17] added http error response for failing to set forwarded ports --- internal/portforward/loop.go | 7 +++++-- internal/server/interfaces.go | 2 +- internal/server/openvpn.go | 5 ++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/portforward/loop.go b/internal/portforward/loop.go index 23fb657d6..d39938c3d 100644 --- a/internal/portforward/loop.go +++ b/internal/portforward/loop.go @@ -166,15 +166,18 @@ func (l *Loop) GetPortsForwarded() (ports []uint16) { return l.service.GetPortsForwarded() } -func (l *Loop) SetPortsForwarded(ports []uint16) { +func (l *Loop) SetPortsForwarded(ports []uint16) (err error) { if l.service == nil { return } - err := l.service.SetPortsForwarded(l.runCtx, ports) + err = l.service.SetPortsForwarded(l.runCtx, ports) if err != nil { l.logger.Error(err.Error()) + return err } + + return nil } func ptrTo[T any](value T) *T { diff --git a/internal/server/interfaces.go b/internal/server/interfaces.go index df1fe7ce1..b49d28373 100644 --- a/internal/server/interfaces.go +++ b/internal/server/interfaces.go @@ -23,7 +23,7 @@ type DNSLoop interface { type PortForwarding interface { GetPortsForwarded() (ports []uint16) - SetPortsForwarded(ports []uint16) + SetPortsForwarded(ports []uint16) (err error) } type PublicIPLoop interface { diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index 49e526076..12eec9c0d 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -160,7 +160,10 @@ func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request return } - h.pf.SetPortsForwarded(data.Ports) + if err := h.pf.SetPortsForwarded(data.Ports); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } err := encoder.Encode(h.pf.GetPortsForwarded()) if err != nil { From 3a808ddc30d9b02ad98db74773338ed6b717fda9 Mon Sep 17 00:00:00 2001 From: jagaimoworks Date: Mon, 5 Aug 2024 23:29:11 +0200 Subject: [PATCH 06/17] changed http error response to accuratly depict the error --- internal/server/openvpn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index 12eec9c0d..5951563bc 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -156,7 +156,7 @@ func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request } if len(data.Ports) == 0 { - http.Error(w, "invalid request", http.StatusBadRequest) + http.Error(w, "no port specified", http.StatusBadRequest) return } From 44c0ee260c1c06862c993b41205af4eff2a611da Mon Sep 17 00:00:00 2001 From: jagaimoworks Date: Mon, 5 Aug 2024 22:28:42 +0000 Subject: [PATCH 07/17] moved statement to improve readability --- internal/server/openvpn.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index 5951563bc..887898169 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -146,10 +146,9 @@ func (h *openvpnHandler) getPortForwarded(w http.ResponseWriter) { } func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request) { - decoder := json.NewDecoder(r.Body) - encoder := json.NewEncoder(w) var data portsWrapper + decoder := json.NewDecoder(r.Body) if err := decoder.Decode(&data); err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return @@ -165,6 +164,7 @@ func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request return } + encoder := json.NewEncoder(w) err := encoder.Encode(h.pf.GetPortsForwarded()) if err != nil { h.warner.Warn(err.Error()) From 17e5dbfc6981963b3fc6e00f74538168b9e5cf6c Mon Sep 17 00:00:00 2001 From: jagaimoworks Date: Mon, 5 Aug 2024 22:40:56 +0000 Subject: [PATCH 08/17] =?UTF-8?q?fix:=20don=C2=B4t=20readd=20failed=20port?= =?UTF-8?q?=20forwardings=20that=20failed=20to=20be=20removed=20and=20vice?= =?UTF-8?q?=20versa?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/portforward/service/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/portforward/service/service.go b/internal/portforward/service/service.go index 8c9311311..5247d2c79 100644 --- a/internal/portforward/service/service.go +++ b/internal/portforward/service/service.go @@ -55,7 +55,7 @@ func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err er for i, port := range s.ports { err := s.portAllower.RemoveAllowedPort(ctx, port) if err != nil { - for j := 0; j <= i; j++ { + for j := 0; j < i; j++ { _ = s.portAllower.SetAllowedPort(ctx, s.ports[j], s.settings.Interface) } s.logger.Error(err.Error()) @@ -66,7 +66,7 @@ func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err er for i, port := range ports { err := s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) if err != nil { - for j := 0; j <= i; j++ { + for j := 0; j < i; j++ { _ = s.portAllower.RemoveAllowedPort(ctx, s.ports[j]) } for _, port := range s.ports { From 0817dc4946e749fdd36681d5e168a416aff87893 Mon Sep 17 00:00:00 2001 From: Rubyn Angelo Stark Date: Fri, 29 Nov 2024 14:55:47 +0000 Subject: [PATCH 09/17] addressing intrange nit --- internal/portforward/service/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/portforward/service/service.go b/internal/portforward/service/service.go index 5247d2c79..8edcbe524 100644 --- a/internal/portforward/service/service.go +++ b/internal/portforward/service/service.go @@ -55,7 +55,7 @@ func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err er for i, port := range s.ports { err := s.portAllower.RemoveAllowedPort(ctx, port) if err != nil { - for j := 0; j < i; j++ { + for j := range i { _ = s.portAllower.SetAllowedPort(ctx, s.ports[j], s.settings.Interface) } s.logger.Error(err.Error()) From 1afbcbd6582cd9a4fb98410ae3010d1af4711e76 Mon Sep 17 00:00:00 2001 From: Rubyn Angelo Stark Date: Fri, 29 Nov 2024 15:19:51 +0000 Subject: [PATCH 10/17] moved logging responsibility to caller --- internal/portforward/service/service.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/portforward/service/service.go b/internal/portforward/service/service.go index 8edcbe524..38c04382c 100644 --- a/internal/portforward/service/service.go +++ b/internal/portforward/service/service.go @@ -2,6 +2,7 @@ package service import ( "context" + "fmt" "net/http" "sync" ) @@ -58,8 +59,7 @@ func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err er for j := range i { _ = s.portAllower.SetAllowedPort(ctx, s.ports[j], s.settings.Interface) } - s.logger.Error(err.Error()) - return err + return fmt.Errorf("removing allowed port: %w", err) } } @@ -72,8 +72,7 @@ func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err er for _, port := range s.ports { _ = s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) } - s.logger.Error(err.Error()) - return err + return fmt.Errorf("setting allowed port: %w", err) } } From 0f56d12e3556d758e970135890b85b2d660c799c Mon Sep 17 00:00:00 2001 From: Rubyn Angelo Stark Date: Fri, 29 Nov 2024 15:24:22 +0000 Subject: [PATCH 11/17] reduced verbosity of the http responses and log the errors as a warnings instead --- internal/server/openvpn.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index 887898169..e4f40b3f1 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -3,6 +3,7 @@ package server import ( "context" "encoding/json" + "fmt" "net/http" "strings" @@ -150,7 +151,8 @@ func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request decoder := json.NewDecoder(r.Body) if err := decoder.Decode(&data); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) + h.warner.Warn(fmt.Sprintf("failed setting forwarded ports: %s", err)) + http.Error(w, "failed setting forwarded ports", http.StatusBadRequest) return } @@ -160,7 +162,8 @@ func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request } if err := h.pf.SetPortsForwarded(data.Ports); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + h.warner.Warn(fmt.Sprintf("failed setting forwarded ports: %s", err)) + http.Error(w, "failed setting forwarded ports", http.StatusInternalServerError) return } From a812bf2d871a0800c7a58b44d0cd9115d30f301c Mon Sep 17 00:00:00 2001 From: Rubyn Angelo Stark Date: Fri, 29 Nov 2024 19:51:09 +0000 Subject: [PATCH 12/17] allow setting NO port forwards => clears forwarded ports --- internal/server/openvpn.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index e4f40b3f1..30d7926f5 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -156,11 +156,6 @@ func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request return } - if len(data.Ports) == 0 { - http.Error(w, "no port specified", http.StatusBadRequest) - return - } - if err := h.pf.SetPortsForwarded(data.Ports); err != nil { h.warner.Warn(fmt.Sprintf("failed setting forwarded ports: %s", err)) http.Error(w, "failed setting forwarded ports", http.StatusInternalServerError) From c1de4bcb93b3743d7527ff2e3fc53d9edfc5cd78 Mon Sep 17 00:00:00 2001 From: Rubyn Angelo Stark Date: Fri, 29 Nov 2024 20:15:07 +0000 Subject: [PATCH 13/17] removed logging --- internal/portforward/loop.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/portforward/loop.go b/internal/portforward/loop.go index d39938c3d..b0d8d17f7 100644 --- a/internal/portforward/loop.go +++ b/internal/portforward/loop.go @@ -173,7 +173,6 @@ func (l *Loop) SetPortsForwarded(ports []uint16) (err error) { err = l.service.SetPortsForwarded(l.runCtx, ports) if err != nil { - l.logger.Error(err.Error()) return err } From 0d8daa454fdc3840a9fa3620c50e724e0249ab87 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Fri, 27 Dec 2024 20:49:45 +0000 Subject: [PATCH 14/17] nitpicky changes --- internal/portforward/loop.go | 9 ++------- internal/portforward/service/service.go | 4 ++-- internal/server/openvpn.go | 8 +++++--- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/internal/portforward/loop.go b/internal/portforward/loop.go index b0d8d17f7..b606ade26 100644 --- a/internal/portforward/loop.go +++ b/internal/portforward/loop.go @@ -168,15 +168,10 @@ func (l *Loop) GetPortsForwarded() (ports []uint16) { func (l *Loop) SetPortsForwarded(ports []uint16) (err error) { if l.service == nil { - return - } - - err = l.service.SetPortsForwarded(l.runCtx, ports) - if err != nil { - return err + return nil } - return nil + return l.service.SetPortsForwarded(l.runCtx, ports) } func ptrTo[T any](value T) *T { diff --git a/internal/portforward/service/service.go b/internal/portforward/service/service.go index 38c04382c..cd4db69a1 100644 --- a/internal/portforward/service/service.go +++ b/internal/portforward/service/service.go @@ -66,7 +66,7 @@ func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err er for i, port := range ports { err := s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) if err != nil { - for j := 0; j < i; j++ { + for j := range i { _ = s.portAllower.RemoveAllowedPort(ctx, s.ports[j]) } for _, port := range s.ports { @@ -79,7 +79,7 @@ func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err er err = s.writePortForwardedFile(ports) if err != nil { _ = s.cleanup() - return err + return fmt.Errorf("writing port forwarded file: %w", err) } s.portMutex.RLock() diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index 30d7926f5..8994b90b3 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -150,20 +150,22 @@ func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request var data portsWrapper decoder := json.NewDecoder(r.Body) - if err := decoder.Decode(&data); err != nil { + err := decoder.Decode(&data) + if err != nil { h.warner.Warn(fmt.Sprintf("failed setting forwarded ports: %s", err)) http.Error(w, "failed setting forwarded ports", http.StatusBadRequest) return } - if err := h.pf.SetPortsForwarded(data.Ports); err != nil { + err = h.pf.SetPortsForwarded(data.Ports) + if err != nil { h.warner.Warn(fmt.Sprintf("failed setting forwarded ports: %s", err)) http.Error(w, "failed setting forwarded ports", http.StatusInternalServerError) return } encoder := json.NewEncoder(w) - err := encoder.Encode(h.pf.GetPortsForwarded()) + err = encoder.Encode(h.pf.GetPortsForwarded()) if err != nil { h.warner.Warn(err.Error()) w.WriteHeader(http.StatusInternalServerError) From 802b809bf881e51866c03b685459cff8f89d30d5 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Fri, 27 Dec 2024 20:50:33 +0000 Subject: [PATCH 15/17] Only write status ok on success --- internal/server/openvpn.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/server/openvpn.go b/internal/server/openvpn.go index 8994b90b3..746c41694 100644 --- a/internal/server/openvpn.go +++ b/internal/server/openvpn.go @@ -164,10 +164,5 @@ func (h *openvpnHandler) setPortForwarded(w http.ResponseWriter, r *http.Request return } - encoder := json.NewEncoder(w) - err = encoder.Encode(h.pf.GetPortsForwarded()) - if err != nil { - h.warner.Warn(err.Error()) - w.WriteHeader(http.StatusInternalServerError) - } + w.WriteHeader(http.StatusOK) } From 19a007f5e51df628368181b0a7f569291c09f119 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Fri, 27 Dec 2024 21:06:29 +0000 Subject: [PATCH 16/17] Use common code for Start and setting ports from control server --- internal/portforward/service/service.go | 41 +++++--------- internal/portforward/service/start.go | 73 +++++++++++++++---------- 2 files changed, 57 insertions(+), 57 deletions(-) diff --git a/internal/portforward/service/service.go b/internal/portforward/service/service.go index cd4db69a1..10b0b2187 100644 --- a/internal/portforward/service/service.go +++ b/internal/portforward/service/service.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "slices" "sync" ) @@ -53,39 +54,25 @@ func (s *Service) GetPortsForwarded() (ports []uint16) { } func (s *Service) SetPortsForwarded(ctx context.Context, ports []uint16) (err error) { - for i, port := range s.ports { - err := s.portAllower.RemoveAllowedPort(ctx, port) - if err != nil { - for j := range i { - _ = s.portAllower.SetAllowedPort(ctx, s.ports[j], s.settings.Interface) - } - return fmt.Errorf("removing allowed port: %w", err) - } - } + s.startStopMutex.Lock() + defer s.startStopMutex.Unlock() - for i, port := range ports { - err := s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) - if err != nil { - for j := range i { - _ = s.portAllower.RemoveAllowedPort(ctx, s.ports[j]) - } - for _, port := range s.ports { - _ = s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) - } - return fmt.Errorf("setting allowed port: %w", err) - } + s.portMutex.Lock() + defer s.portMutex.Unlock() + slices.Sort(ports) + if slices.Equal(s.ports, ports) { + return nil } - err = s.writePortForwardedFile(ports) + err = s.cleanup() if err != nil { - _ = s.cleanup() - return fmt.Errorf("writing port forwarded file: %w", err) + return fmt.Errorf("cleaning up: %w", err) } - s.portMutex.RLock() - defer s.portMutex.RUnlock() - s.ports = make([]uint16, len(ports)) - copy(s.ports, ports) + err = s.onNewPorts(ctx, ports) + if err != nil { + return fmt.Errorf("handling new ports: %w", err) + } s.logger.Info("updated: " + portsToString(s.ports)) diff --git a/internal/portforward/service/start.go b/internal/portforward/service/start.go index a13ac0e41..2eefee622 100644 --- a/internal/portforward/service/start.go +++ b/internal/portforward/service/start.go @@ -3,6 +3,7 @@ package service import ( "context" "fmt" + "slices" "github.com/qdm12/gluetun/internal/netlink" "github.com/qdm12/gluetun/internal/provider/utils" @@ -47,18 +48,54 @@ func (s *Service) Start(ctx context.Context) (runError <-chan error, err error) return nil, fmt.Errorf("port forwarding for the first time: %w", err) } + err = s.onNewPorts(ctx, ports) + if err != nil { + return nil, err + } + + keepPortCtx, keepPortCancel := context.WithCancel(context.Background()) + s.keepPortCancel = keepPortCancel + runErrorCh := make(chan error) + keepPortDoneCh := make(chan struct{}) + s.keepPortDoneCh = keepPortDoneCh + + readyCh := make(chan struct{}) + go func(ctx context.Context, portForwarder PortForwarder, + obj utils.PortForwardObjects, readyCh chan<- struct{}, + runError chan<- error, doneCh chan<- struct{}, + ) { + defer close(doneCh) + close(readyCh) + err = portForwarder.KeepPortForward(ctx, obj) + crashed := ctx.Err() == nil + if !crashed { // stopped by Stop call + return + } + s.startStopMutex.Lock() + defer s.startStopMutex.Unlock() + _ = s.cleanup() + runError <- err + }(keepPortCtx, s.settings.PortForwarder, obj, readyCh, runErrorCh, keepPortDoneCh) + <-readyCh + + return runErrorCh, nil +} + +func (s *Service) onNewPorts(ctx context.Context, ports []uint16) (err error) { + slices.Sort(ports) + s.logger.Info(portsToString(ports)) for _, port := range ports { err = s.portAllower.SetAllowedPort(ctx, port, s.settings.Interface) if err != nil { - return nil, fmt.Errorf("allowing port in firewall: %w", err) + return fmt.Errorf("allowing port in firewall: %w", err) } if s.settings.ListeningPort != 0 { err = s.portAllower.RedirectPort(ctx, s.settings.Interface, port, s.settings.ListeningPort) if err != nil { - return nil, fmt.Errorf("redirecting port in firewall: %w", err) + return fmt.Errorf("redirecting port in firewall: %w", err) } } } @@ -66,11 +103,12 @@ func (s *Service) Start(ctx context.Context) (runError <-chan error, err error) err = s.writePortForwardedFile(ports) if err != nil { _ = s.cleanup() - return nil, fmt.Errorf("writing port file: %w", err) + return fmt.Errorf("writing port file: %w", err) } s.portMutex.Lock() - s.ports = ports + s.ports = make([]uint16, len(ports)) + copy(s.ports, ports) s.portMutex.Unlock() if s.settings.UpCommand != "" { @@ -81,30 +119,5 @@ func (s *Service) Start(ctx context.Context) (runError <-chan error, err error) } } - keepPortCtx, keepPortCancel := context.WithCancel(context.Background()) - s.keepPortCancel = keepPortCancel - runErrorCh := make(chan error) - keepPortDoneCh := make(chan struct{}) - s.keepPortDoneCh = keepPortDoneCh - - readyCh := make(chan struct{}) - go func(ctx context.Context, portForwarder PortForwarder, - obj utils.PortForwardObjects, readyCh chan<- struct{}, - runError chan<- error, doneCh chan<- struct{}, - ) { - defer close(doneCh) - close(readyCh) - err = portForwarder.KeepPortForward(ctx, obj) - crashed := ctx.Err() == nil - if !crashed { // stopped by Stop call - return - } - s.startStopMutex.Lock() - defer s.startStopMutex.Unlock() - _ = s.cleanup() - runError <- err - }(keepPortCtx, s.settings.PortForwarder, obj, readyCh, runErrorCh, keepPortDoneCh) - <-readyCh - - return runErrorCh, nil + return nil } From 3f29f16e70cca51fecf773d79d0b4319eeac590b Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Fri, 27 Dec 2024 21:10:08 +0000 Subject: [PATCH 17/17] Return an error if service is not running --- internal/portforward/loop.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/portforward/loop.go b/internal/portforward/loop.go index b606ade26..2e19a4bea 100644 --- a/internal/portforward/loop.go +++ b/internal/portforward/loop.go @@ -2,6 +2,7 @@ package portforward import ( "context" + "errors" "fmt" "net/http" "sync" @@ -166,9 +167,11 @@ func (l *Loop) GetPortsForwarded() (ports []uint16) { return l.service.GetPortsForwarded() } +var ErrServiceNotStarted = errors.New("port forwarding service not started") + func (l *Loop) SetPortsForwarded(ports []uint16) (err error) { if l.service == nil { - return nil + return fmt.Errorf("%w", ErrServiceNotStarted) } return l.service.SetPortsForwarded(l.runCtx, ports)