From bb0897dd857d28d27a459a9701d58d10697e2f8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Fri, 8 Aug 2025 01:00:18 +0200 Subject: [PATCH] Change session id from int to string --- client/internal/engine.go | 70 +++++++++++++++--------- client/internal/peer/handshaker.go | 5 +- client/internal/peer/session_id.go | 47 ++++++++++++++++ client/internal/peer/signaler.go | 10 +++- client/internal/peer/worker_ice.go | 20 +++++-- shared/signal/client/client.go | 4 +- shared/signal/proto/signalexchange.pb.go | 12 ++-- shared/signal/proto/signalexchange.proto | 2 +- 8 files changed, 125 insertions(+), 45 deletions(-) create mode 100644 client/internal/peer/session_id.go diff --git a/client/internal/engine.go b/client/internal/engine.go index 12909e827..197036ea9 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -55,11 +55,11 @@ import ( nbssh "github.com/netbirdio/netbird/client/ssh" "github.com/netbirdio/netbird/client/system" nbdns "github.com/netbirdio/netbird/dns" + "github.com/netbirdio/netbird/route" mgm "github.com/netbirdio/netbird/shared/management/client" mgmProto "github.com/netbirdio/netbird/shared/management/proto" auth "github.com/netbirdio/netbird/shared/relay/auth/hmac" relayClient "github.com/netbirdio/netbird/shared/relay/client" - "github.com/netbirdio/netbird/route" signal "github.com/netbirdio/netbird/shared/signal/client" sProto "github.com/netbirdio/netbird/shared/signal/proto" "github.com/netbirdio/netbird/util" @@ -1332,36 +1332,15 @@ func (e *Engine) receiveSignalEvents() { switch msg.GetBody().Type { case sProto.Body_OFFER, sProto.Body_ANSWER: - remoteCred, err := signal.UnMarshalCredential(msg) + offerAnswer, err := convertToOfferAnswer(msg) if err != nil { return err } - var ( - rosenpassPubKey []byte - rosenpassAddr string - ) - if cfg := msg.GetBody().GetRosenpassConfig(); cfg != nil { - rosenpassPubKey = cfg.GetRosenpassPubKey() - rosenpassAddr = cfg.GetRosenpassServerAddr() - } - offerAnswer := peer.OfferAnswer{ - IceCredentials: peer.IceCredentials{ - UFrag: remoteCred.UFrag, - Pwd: remoteCred.Pwd, - }, - WgListenPort: int(msg.GetBody().GetWgListenPort()), - Version: msg.GetBody().GetNetBirdVersion(), - RosenpassPubKey: rosenpassPubKey, - RosenpassAddr: rosenpassAddr, - RelaySrvAddress: msg.GetBody().GetRelayServerAddress(), - SessionID: msg.GetBody().SessionId, - } - if msg.Body.Type == sProto.Body_OFFER { - conn.OnRemoteOffer(offerAnswer) + conn.OnRemoteOffer(*offerAnswer) } else { - conn.OnRemoteAnswer(offerAnswer) + conn.OnRemoteAnswer(*offerAnswer) } case sProto.Body_CANDIDATE: candidate, err := ice.UnmarshalCandidate(msg.GetBody().Payload) @@ -2060,3 +2039,44 @@ func createFile(path string) error { } return file.Close() } + +func convertToOfferAnswer(msg *sProto.Message) (*peer.OfferAnswer, error) { + remoteCred, err := signal.UnMarshalCredential(msg) + if err != nil { + return nil, err + } + + var ( + rosenpassPubKey []byte + rosenpassAddr string + ) + if cfg := msg.GetBody().GetRosenpassConfig(); cfg != nil { + rosenpassPubKey = cfg.GetRosenpassPubKey() + rosenpassAddr = cfg.GetRosenpassServerAddr() + } + + // Handle optional SessionID + var sessionID *peer.ICESessionID + if sessionBytes := msg.GetBody().GetSessionId(); sessionBytes != nil { + if id, err := peer.ICESessionIDFromBytes(sessionBytes); err != nil { + log.Warnf("Invalid session ID in message: %v", err) + sessionID = nil // Set to nil if conversion fails + } else { + sessionID = &id + } + } + + offerAnswer := peer.OfferAnswer{ + IceCredentials: peer.IceCredentials{ + UFrag: remoteCred.UFrag, + Pwd: remoteCred.Pwd, + }, + WgListenPort: int(msg.GetBody().GetWgListenPort()), + Version: msg.GetBody().GetNetBirdVersion(), + RosenpassPubKey: rosenpassPubKey, + RosenpassAddr: rosenpassAddr, + RelaySrvAddress: msg.GetBody().GetRelayServerAddress(), + SessionID: sessionID, + } + return &offerAnswer, nil +} diff --git a/client/internal/peer/handshaker.go b/client/internal/peer/handshaker.go index 91b573515..3cbf74cfd 100644 --- a/client/internal/peer/handshaker.go +++ b/client/internal/peer/handshaker.go @@ -3,7 +3,6 @@ package peer import ( "context" "errors" - "strconv" "sync" log "github.com/sirupsen/logrus" @@ -41,14 +40,14 @@ type OfferAnswer struct { // relay server address RelaySrvAddress string // SessionID is the unique identifier of the session, used to discard old messages - SessionID *int64 + SessionID *ICESessionID } func (oa *OfferAnswer) SessionIDString() string { if oa.SessionID == nil { return "unknown" } - return strconv.FormatInt(*oa.SessionID, 10) + return oa.SessionID.String() } type Handshaker struct { diff --git a/client/internal/peer/session_id.go b/client/internal/peer/session_id.go new file mode 100644 index 000000000..4f630adc0 --- /dev/null +++ b/client/internal/peer/session_id.go @@ -0,0 +1,47 @@ +package peer + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "io" +) + +const sessionIDSize = 5 + +type ICESessionID string + +// NewICESessionID generates a new session ID for distinguishing sessions +func NewICESessionID() (ICESessionID, error) { + b := make([]byte, sessionIDSize) + if _, err := io.ReadFull(rand.Reader, b); err != nil { + return "", fmt.Errorf("failed to generate session ID: %w", err) + } + return ICESessionID(hex.EncodeToString(b)), nil +} + +func ICESessionIDFromBytes(b []byte) (ICESessionID, error) { + if len(b) != sessionIDSize { + return "", fmt.Errorf("invalid session ID length: %d", len(b)) + } + return ICESessionID(hex.EncodeToString(b)), nil +} + +// Bytes returns the raw bytes of the session ID for protobuf serialization +func (id ICESessionID) Bytes() ([]byte, error) { + if len(id) == 0 { + return nil, fmt.Errorf("ICE session ID is empty") + } + b, err := hex.DecodeString(string(id)) + if err != nil { + return nil, fmt.Errorf("invalid ICE session ID encoding: %w", err) + } + if len(b) != sessionIDSize { + return nil, fmt.Errorf("invalid ICE session ID length: expected %d bytes, got %d", sessionIDSize, len(b)) + } + return b, nil +} + +func (id ICESessionID) String() string { + return string(id) +} diff --git a/client/internal/peer/signaler.go b/client/internal/peer/signaler.go index ac65160a2..ca1d421a5 100644 --- a/client/internal/peer/signaler.go +++ b/client/internal/peer/signaler.go @@ -2,6 +2,7 @@ package peer import ( "github.com/pion/ice/v3" + log "github.com/sirupsen/logrus" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" signal "github.com/netbirdio/netbird/shared/signal/client" @@ -45,6 +46,10 @@ func (s *Signaler) Ready() bool { // SignalOfferAnswer signals either an offer or an answer to remote peer func (s *Signaler) signalOfferAnswer(offerAnswer OfferAnswer, remoteKey string, bodyType sProto.Body_Type) error { + sessionIDBytes, err := offerAnswer.SessionID.Bytes() + if err != nil { + log.Warnf("failed to get session ID bytes: %v", err) + } msg, err := signal.MarshalCredential( s.wgPrivateKey, offerAnswer.WgListenPort, @@ -57,13 +62,12 @@ func (s *Signaler) signalOfferAnswer(offerAnswer OfferAnswer, remoteKey string, offerAnswer.RosenpassPubKey, offerAnswer.RosenpassAddr, offerAnswer.RelaySrvAddress, - *offerAnswer.SessionID) + sessionIDBytes) if err != nil { return err } - err = s.signal.Send(msg) - if err != nil { + if err = s.signal.Send(msg); err != nil { return err } diff --git a/client/internal/peer/worker_ice.go b/client/internal/peer/worker_ice.go index 606f20aa1..afd71c253 100644 --- a/client/internal/peer/worker_ice.go +++ b/client/internal/peer/worker_ice.go @@ -47,12 +47,12 @@ type WorkerICE struct { agentConnecting bool // while it is true, drop all incoming offers lastSuccess time.Time // with this avoid the too frequent ICE agent recreation // remoteSessionID represents the peer's session identifier from the latest remote offer. - remoteSessionID int64 + remoteSessionID ICESessionID // sessionID is used to track the current session ID of the ICE agent // increase by one when disconnecting the agent // with it the remote peer can discard the already deprecated offer/answer // Without it the remote peer may recreate a workable ICE connection - sessionID int64 + sessionID ICESessionID muxAgent sync.Mutex StunTurn []*stun.URI @@ -67,6 +67,11 @@ type WorkerICE struct { } func NewWorkerICE(ctx context.Context, log *log.Entry, config ConnConfig, conn *Conn, signaler *Signaler, ifaceDiscover stdnet.ExternalIFaceDiscover, statusRecorder *Status, hasRelayOnLocally bool) (*WorkerICE, error) { + sessionID, err := NewICESessionID() + if err != nil { + return nil, err + } + w := &WorkerICE{ ctx: ctx, log: log, @@ -77,7 +82,7 @@ func NewWorkerICE(ctx context.Context, log *log.Entry, config ConnConfig, conn * statusRecorder: statusRecorder, hasRelayOnLocally: hasRelayOnLocally, lastKnownState: ice.ConnectionStateDisconnected, - sessionID: 0, + sessionID: sessionID, } localUfrag, localPwd, err := icemaker.GenerateICECredentials() @@ -208,7 +213,7 @@ func (w *WorkerICE) reCreateAgent(dialerCancel context.CancelFunc, candidates [] return agent, nil } -func (w *WorkerICE) SessionID() int64 { +func (w *WorkerICE) SessionID() ICESessionID { w.muxAgent.Lock() defer w.muxAgent.Unlock() @@ -282,7 +287,12 @@ func (w *WorkerICE) closeAgent(agent *ice.Agent, cancel context.CancelFunc) { } w.muxAgent.Lock() - w.sessionID++ + sessionID, err := NewICESessionID() + if err != nil { + w.log.Errorf("failed to create new session ID: %s", err) + } + w.sessionID = sessionID + if w.agent == agent { w.agent = nil w.agentConnecting = false diff --git a/shared/signal/client/client.go b/shared/signal/client/client.go index e29940e75..5347c80e9 100644 --- a/shared/signal/client/client.go +++ b/shared/signal/client/client.go @@ -52,7 +52,7 @@ func UnMarshalCredential(msg *proto.Message) (*Credential, error) { } // MarshalCredential marshal a Credential instance and returns a Message object -func MarshalCredential(myKey wgtypes.Key, myPort int, remoteKey string, credential *Credential, t proto.Body_Type, rosenpassPubKey []byte, rosenpassAddr string, relaySrvAddress string, sessionID int64) (*proto.Message, error) { +func MarshalCredential(myKey wgtypes.Key, myPort int, remoteKey string, credential *Credential, t proto.Body_Type, rosenpassPubKey []byte, rosenpassAddr string, relaySrvAddress string, sessionID []byte) (*proto.Message, error) { return &proto.Message{ Key: myKey.PublicKey().String(), RemoteKey: remoteKey, @@ -66,7 +66,7 @@ func MarshalCredential(myKey wgtypes.Key, myPort int, remoteKey string, credenti RosenpassServerAddr: rosenpassAddr, }, RelayServerAddress: relaySrvAddress, - SessionId: &sessionID, + SessionId: sessionID, }, }, nil } diff --git a/shared/signal/proto/signalexchange.pb.go b/shared/signal/proto/signalexchange.pb.go index cf488a8e1..d9c61a846 100644 --- a/shared/signal/proto/signalexchange.pb.go +++ b/shared/signal/proto/signalexchange.pb.go @@ -230,7 +230,7 @@ type Body struct { RosenpassConfig *RosenpassConfig `protobuf:"bytes,7,opt,name=rosenpassConfig,proto3" json:"rosenpassConfig,omitempty"` // relayServerAddress is url of the relay server RelayServerAddress string `protobuf:"bytes,8,opt,name=relayServerAddress,proto3" json:"relayServerAddress,omitempty"` - SessionId *int64 `protobuf:"varint,9,opt,name=sessionId,proto3,oneof" json:"sessionId,omitempty"` + SessionId []byte `protobuf:"bytes,10,opt,name=sessionId,proto3,oneof" json:"sessionId,omitempty"` } func (x *Body) Reset() { @@ -321,11 +321,11 @@ func (x *Body) GetRelayServerAddress() string { return "" } -func (x *Body) GetSessionId() int64 { - if x != nil && x.SessionId != nil { - return *x.SessionId +func (x *Body) GetSessionId() []byte { + if x != nil { + return x.SessionId } - return 0 + return nil } // Mode indicates a connection mode @@ -475,7 +475,7 @@ var file_signalexchange_proto_rawDesc = []byte{ 0x65, 0x72, 0x76, 0x65, 0x72, 0x41, 0x64, 0x64, 0x72, 0x65, 0x73, 0x73, 0x18, 0x08, 0x20, 0x01, 0x28, 0x09, 0x52, 0x12, 0x72, 0x65, 0x6c, 0x61, 0x79, 0x53, 0x65, 0x72, 0x76, 0x65, 0x72, 0x41, 0x64, 0x64, 0x72, 0x65, 0x73, 0x73, 0x12, 0x21, 0x0a, 0x09, 0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, - 0x6e, 0x49, 0x64, 0x18, 0x09, 0x20, 0x01, 0x28, 0x03, 0x48, 0x00, 0x52, 0x09, 0x73, 0x65, 0x73, + 0x6e, 0x49, 0x64, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x0c, 0x48, 0x00, 0x52, 0x09, 0x73, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x49, 0x64, 0x88, 0x01, 0x01, 0x22, 0x43, 0x0a, 0x04, 0x54, 0x79, 0x70, 0x65, 0x12, 0x09, 0x0a, 0x05, 0x4f, 0x46, 0x46, 0x45, 0x52, 0x10, 0x00, 0x12, 0x0a, 0x0a, 0x06, 0x41, 0x4e, 0x53, 0x57, 0x45, 0x52, 0x10, 0x01, 0x12, 0x0d, 0x0a, 0x09, 0x43, 0x41, 0x4e, 0x44, diff --git a/shared/signal/proto/signalexchange.proto b/shared/signal/proto/signalexchange.proto index 18fc008f8..0a33ad78b 100644 --- a/shared/signal/proto/signalexchange.proto +++ b/shared/signal/proto/signalexchange.proto @@ -65,7 +65,7 @@ message Body { // relayServerAddress is url of the relay server string relayServerAddress = 8; - optional int64 sessionId = 9; + optional bytes sessionId = 10; } // Mode indicates a connection mode