diff --git a/client/iface/wgproxy/bind/proxy.go b/client/iface/wgproxy/bind/proxy.go index e986d6d7b..e0883715a 100644 --- a/client/iface/wgproxy/bind/proxy.go +++ b/client/iface/wgproxy/bind/proxy.go @@ -104,8 +104,8 @@ func (p *ProxyBind) proxyToLocal(ctx context.Context) { } }() - buf := make([]byte, 1500) for { + buf := make([]byte, 1500) n, err := p.remoteConn.Read(buf) if err != nil { if ctx.Err() != nil { diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index 56b772759..84a8c221f 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -309,6 +309,11 @@ func (conn *Conn) iCEConnectionIsReady(priority ConnPriority, iceConnInfo ICECon return } + if remoteConnNil(conn.log, iceConnInfo.RemoteConn) { + conn.log.Errorf("remote ICE connection is nil") + return + } + conn.log.Debugf("ICE connection is ready") if conn.currentConnPriority > priority { diff --git a/client/internal/peer/nilcheck.go b/client/internal/peer/nilcheck.go new file mode 100644 index 000000000..058fe9a26 --- /dev/null +++ b/client/internal/peer/nilcheck.go @@ -0,0 +1,21 @@ +package peer + +import ( + "net" + + log "github.com/sirupsen/logrus" +) + +func remoteConnNil(log *log.Entry, conn net.Conn) bool { + if conn == nil { + log.Errorf("ice conn is nil") + return true + } + + if conn.RemoteAddr() == nil { + log.Errorf("ICE remote address is nil") + return true + } + + return false +} diff --git a/client/server/panic_generic.go b/client/server/panic_generic.go new file mode 100644 index 000000000..f027b954b --- /dev/null +++ b/client/server/panic_generic.go @@ -0,0 +1,7 @@ +//go:build !windows + +package server + +func handlePanicLog() error { + return nil +} diff --git a/client/server/panic_windows.go b/client/server/panic_windows.go new file mode 100644 index 000000000..1d4ba4b75 --- /dev/null +++ b/client/server/panic_windows.go @@ -0,0 +1,83 @@ +package server + +import ( + "fmt" + "os" + "path/filepath" + "syscall" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/util" +) + +const ( + windowsPanicLogEnvVar = "NB_WINDOWS_PANIC_LOG" + // STD_ERROR_HANDLE ((DWORD)-12) = 4294967284 + stdErrorHandle = ^uintptr(11) +) + +var ( + kernel32 = syscall.NewLazyDLL("kernel32.dll") + + // https://learn.microsoft.com/en-us/windows/console/setstdhandle + setStdHandleFn = kernel32.NewProc("SetStdHandle") +) + +func handlePanicLog() error { + logPath := os.Getenv(windowsPanicLogEnvVar) + if logPath == "" { + return nil + } + + // Ensure the directory exists + logDir := filepath.Dir(logPath) + if err := os.MkdirAll(logDir, 0750); err != nil { + return fmt.Errorf("create panic log directory: %w", err) + } + if err := util.EnforcePermission(logPath); err != nil { + return fmt.Errorf("enforce permission on panic log file: %w", err) + } + + // Open log file with append mode + f, err := os.OpenFile(logPath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0644) + if err != nil { + return fmt.Errorf("open panic log file: %w", err) + } + + // Redirect stderr to the file + if err = redirectStderr(f); err != nil { + if closeErr := f.Close(); closeErr != nil { + log.Warnf("failed to close file after redirect error: %v", closeErr) + } + return fmt.Errorf("redirect stderr: %w", err) + } + + log.Infof("successfully configured panic logging to: %s", logPath) + return nil +} + +// redirectStderr redirects stderr to the provided file +func redirectStderr(f *os.File) error { + // Get the current process's stderr handle + if err := setStdHandle(f); err != nil { + return fmt.Errorf("failed to set stderr handle: %w", err) + } + + // Also set os.Stderr for Go's standard library + os.Stderr = f + + return nil +} + +func setStdHandle(f *os.File) error { + handle := f.Fd() + r0, _, e1 := setStdHandleFn.Call(stdErrorHandle, handle) + if r0 == 0 { + if e1 != nil { + return e1 + } + return syscall.EINVAL + } + return nil +} diff --git a/client/server/server.go b/client/server/server.go index a03322081..4d921851f 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -97,6 +97,10 @@ func (s *Server) Start() error { defer s.mutex.Unlock() state := internal.CtxGetState(s.rootCtx) + if err := handlePanicLog(); err != nil { + log.Warnf("failed to redirect stderr: %v", err) + } + if err := restoreResidualState(s.rootCtx); err != nil { log.Warnf(errRestoreResidualState, err) } diff --git a/management/server/peer.go b/management/server/peer.go index 19c673818..bd9ddb0d8 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -472,12 +472,12 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s } var newPeer *nbpeer.Peer - var groupsToAdd []string err = am.Store.ExecuteInTransaction(ctx, func(transaction Store) error { var setupKeyID string var setupKeyName string var ephemeral bool + var groupsToAdd []string if addedByUser { user, err := transaction.GetUserByUserID(ctx, LockingStrengthUpdate, userID) if err != nil { @@ -620,7 +620,7 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s unlock() unlock = nil - updateAccountPeers, err := am.areGroupChangesAffectPeers(ctx, accountID, groupsToAdd) + updateAccountPeers, err := am.isPeerInActiveGroup(ctx, accountID, newPeer.ID) if err != nil { return nil, nil, nil, err } diff --git a/management/server/status/error.go b/management/server/status/error.go index 1e53f4a42..f343f69e8 100644 --- a/management/server/status/error.go +++ b/management/server/status/error.go @@ -3,6 +3,7 @@ package status import ( "errors" "fmt" + "time" ) const ( @@ -186,3 +187,8 @@ func NewUnauthorizedToViewNSGroupsError() error { func NewUnauthorizedToViewRoutesError() error { return Errorf(PermissionDenied, "only users with admin power can view network routes") } + +// NewStoreContextCanceledError creates a new Error with Internal type for a canceled store context +func NewStoreContextCanceledError(duration time.Duration) error { + return Errorf(Internal, "store access: context canceled after %v", duration) +} diff --git a/relay/client/manager.go b/relay/client/manager.go index 3981415fc..b14a7701b 100644 --- a/relay/client/manager.go +++ b/relay/client/manager.go @@ -16,6 +16,7 @@ import ( var ( relayCleanupInterval = 60 * time.Second + keepUnusedServerTime = 5 * time.Second ErrRelayClientNotConnected = fmt.Errorf("relay client not connected") ) @@ -27,10 +28,13 @@ type RelayTrack struct { sync.RWMutex relayClient *Client err error + created time.Time } func NewRelayTrack() *RelayTrack { - return &RelayTrack{} + return &RelayTrack{ + created: time.Now(), + } } type OnServerCloseListener func() @@ -302,6 +306,18 @@ func (m *Manager) cleanUpUnusedRelays() { for addr, rt := range m.relayClients { rt.Lock() + // if the connection failed to the server the relay client will be nil + // but the instance will be kept in the relayClients until the next locking + if rt.err != nil { + rt.Unlock() + continue + } + + if time.Since(rt.created) <= keepUnusedServerTime { + rt.Unlock() + continue + } + if rt.relayClient.HasConns() { rt.Unlock() continue diff --git a/relay/client/manager_test.go b/relay/client/manager_test.go index e9cc2c581..bfc342f25 100644 --- a/relay/client/manager_test.go +++ b/relay/client/manager_test.go @@ -288,8 +288,9 @@ func TestForeginAutoClose(t *testing.T) { t.Fatalf("failed to close connection: %s", err) } - t.Logf("waiting for relay cleanup: %s", relayCleanupInterval+1*time.Second) - time.Sleep(relayCleanupInterval + 1*time.Second) + timeout := relayCleanupInterval + keepUnusedServerTime + 1*time.Second + t.Logf("waiting for relay cleanup: %s", timeout) + time.Sleep(timeout) if len(mgr.relayClients) != 0 { t.Errorf("expected 0, got %d", len(mgr.relayClients)) } diff --git a/relay/client/picker_test.go b/relay/client/picker_test.go index eb14581e0..4800e05ba 100644 --- a/relay/client/picker_test.go +++ b/relay/client/picker_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "testing" - "time" ) func TestServerPicker_UnavailableServers(t *testing.T) { @@ -13,7 +12,7 @@ func TestServerPicker_UnavailableServers(t *testing.T) { PeerID: "test", } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), connectionTimeout+1) defer cancel() go func() {