From 2817f62c13a2bd51e332b796e23c4a468177580e Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Fri, 25 Apr 2025 09:26:18 +0200 Subject: [PATCH 1/3] [client] Fix error handling case of flow grpc error (#3727) When a gRPC error occurs in the Flow package, it will be propagated to the upper layers and handled similarly to a Management gRPC error. Always report a disconnected state in the event of any error Hide the underlying gRPC errors Force close the gRPC connection in the event of any error --- management/client/grpc.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/management/client/grpc.go b/management/client/grpc.go index d3aaffec0..956aaebb2 100644 --- a/management/client/grpc.go +++ b/management/client/grpc.go @@ -128,7 +128,13 @@ func (c *GrpcClient) Sync(ctx context.Context, sysInfo *system.Info, msgHandler return err } - return c.handleStream(ctx, *serverPubKey, sysInfo, msgHandler) + streamErr := c.handleStream(ctx, *serverPubKey, sysInfo, msgHandler) + if c.conn.GetState() != connectivity.Shutdown { + if err := c.conn.Close(); err != nil { + log.Warnf("failed closing connection to Management service: %s", err) + } + } + return streamErr } err := backoff.Retry(operation, defaultBackoff(ctx)) @@ -159,6 +165,7 @@ func (c *GrpcClient) handleStream(ctx context.Context, serverPubKey wgtypes.Key, // blocking until error err = c.receiveEvents(stream, serverPubKey, msgHandler) if err != nil { + c.notifyDisconnected(err) s, _ := gstatus.FromError(err) switch s.Code() { case codes.PermissionDenied: @@ -167,7 +174,6 @@ func (c *GrpcClient) handleStream(ctx context.Context, serverPubKey wgtypes.Key, log.Debugf("management connection context has been canceled, this usually indicates shutdown") return nil default: - c.notifyDisconnected(err) log.Warnf("disconnected from the Management service but will retry silently. Reason: %v", err) return err } @@ -258,10 +264,10 @@ func (c *GrpcClient) receiveEvents(stream proto.ManagementService_SyncClient, se return err } - err = msgHandler(decryptedResp) - if err != nil { + if err := msgHandler(decryptedResp); err != nil { log.Errorf("failed handling an update message received from Management Service: %v", err.Error()) - return err + // hide any grpc error code that is not relevant for management + return fmt.Errorf("msg handler error: %v", err.Error()) } } } From ef8b8a28912e7979098b9f94294c8f23fc3aa81b Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 25 Apr 2025 12:43:20 +0200 Subject: [PATCH 2/3] [client] Ensure dst-type local marks can overwrite nat marks (#3738) --- client/firewall/iptables/router_linux.go | 4 +++- client/firewall/nftables/router_linux.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/client/firewall/iptables/router_linux.go b/client/firewall/iptables/router_linux.go index b59c88580..bb799b99b 100644 --- a/client/firewall/iptables/router_linux.go +++ b/client/firewall/iptables/router_linux.go @@ -631,7 +631,9 @@ func (r *router) addNatRule(pair firewall.RouterPair) error { "-j", "MARK", "--set-mark", fmt.Sprintf("%#x", markValue), ) - if err := r.iptablesClient.Append(tableMangle, chainRTPRE, rule...); err != nil { + // Ensure nat rules come first, so the mark can be overwritten. + // Currently overwritten by the dst-type LOCAL rules for redirected traffic. + if err := r.iptablesClient.Insert(tableMangle, chainRTPRE, 1, rule...); err != nil { // TODO: rollback ipset counter return fmt.Errorf("error while adding marking rule for %s: %v", pair.Destination, err) } diff --git a/client/firewall/nftables/router_linux.go b/client/firewall/nftables/router_linux.go index c2ba2a072..0f6c5bdf6 100644 --- a/client/firewall/nftables/router_linux.go +++ b/client/firewall/nftables/router_linux.go @@ -666,7 +666,9 @@ func (r *router) addNatRule(pair firewall.RouterPair) error { } } - r.rules[ruleKey] = r.conn.AddRule(&nftables.Rule{ + // Ensure nat rules come first, so the mark can be overwritten. + // Currently overwritten by the dst-type LOCAL rules for redirected traffic. + r.rules[ruleKey] = r.conn.InsertRule(&nftables.Rule{ Table: r.workTable, Chain: r.chains[chainNameManglePrerouting], Exprs: exprs, From c0eaea938e3a640d22cf87ddfdc947955f8aacf6 Mon Sep 17 00:00:00 2001 From: Carlos Hernandez Date: Fri, 25 Apr 2025 06:41:57 -0600 Subject: [PATCH 3/3] [client] Fix macos privacy warning when checking static info (#3496) avoid checking static info with a init call --- client/cmd/login.go | 3 +++ client/cmd/service_controller.go | 5 +++++ client/system/info.go | 7 +++++++ client/system/static_info.go | 6 ------ client/system/static_info_stub.go | 8 ++++++++ 5 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 client/system/static_info_stub.go diff --git a/client/cmd/login.go b/client/cmd/login.go index c86d6c636..549eef40e 100644 --- a/client/cmd/login.go +++ b/client/cmd/login.go @@ -48,6 +48,9 @@ var loginCmd = &cobra.Command{ return err } + // update host's static platform and system information + system.UpdateStaticInfo() + // workaround to run without service if logFile == "console" { err = handleRebrand(cmd) diff --git a/client/cmd/service_controller.go b/client/cmd/service_controller.go index 0ddf6c4c8..5e3c63e57 100644 --- a/client/cmd/service_controller.go +++ b/client/cmd/service_controller.go @@ -16,12 +16,17 @@ import ( "github.com/netbirdio/netbird/client/proto" "github.com/netbirdio/netbird/client/server" + "github.com/netbirdio/netbird/client/system" "github.com/netbirdio/netbird/util" ) func (p *program) Start(svc service.Service) error { // Start should not block. Do the actual work async. log.Info("starting Netbird service") //nolint + + // Collect static system and platform information + system.UpdateStaticInfo() + // in any case, even if configuration does not exists we run daemon to serve CLI gRPC API. p.serv = grpc.NewServer() diff --git a/client/system/info.go b/client/system/info.go index 2a0343ca6..3a0c57156 100644 --- a/client/system/info.go +++ b/client/system/info.go @@ -185,3 +185,10 @@ func GetInfoWithChecks(ctx context.Context, checks []*proto.Checks) (*Info, erro return info, nil } + +// UpdateStaticInfo asynchronously updates static system and platform information +func UpdateStaticInfo() { + go func() { + _ = updateStaticInfo() + }() +} diff --git a/client/system/static_info.go b/client/system/static_info.go index fabe65a68..f178ec932 100644 --- a/client/system/static_info.go +++ b/client/system/static_info.go @@ -16,12 +16,6 @@ var ( once sync.Once ) -func init() { - go func() { - _ = updateStaticInfo() - }() -} - func updateStaticInfo() StaticInfo { once.Do(func() { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) diff --git a/client/system/static_info_stub.go b/client/system/static_info_stub.go new file mode 100644 index 000000000..faa3e700b --- /dev/null +++ b/client/system/static_info_stub.go @@ -0,0 +1,8 @@ +//go:build android || freebsd || ios + +package system + +// updateStaticInfo returns an empty implementation for unsupported platforms +func updateStaticInfo() StaticInfo { + return StaticInfo{} +}