fix: remote CSEQ must be differentely validated #187

Author: emiago

dialog.go

@@ -35,7 +35,8 @@ type Dialog struct {
 	InviteRequest *sip.Request
 
 	// lastCSeqNo is set for every request within dialog except ACK CANCEL
-	lastCSeqNo atomic.Uint32
+	lastCSeqNo   atomic.Uint32
+	remoteCSeqNo atomic.Uint32
 
 	// InviteResponse is last response received or sent. It is not thread safe!
 	// Use it only as read only and do not change values
@@ -58,6 +59,7 @@ func (d *Dialog) Init() {
 	// We may have sequence number initialized
 	if cseq := d.InviteRequest.CSeq(); cseq != nil {
 		d.lastCSeqNo.Store(cseq.SeqNo)
+		d.remoteCSeqNo.Store(cseq.SeqNo)
 	}
 	d.onStatePointer = atomic.Pointer[DialogStateFn]{}
 }
@@ -144,3 +146,23 @@ func (d *Dialog) CSEQ() uint32 {
 func (d *Dialog) Context() context.Context {
 	return d.ctx
 }
+
+func (d *Dialog) validateRemoteRequest(req *sip.Request) (err error) {
+	// Make sure this is bye for this dialog
+	if req.CSeq().SeqNo < d.remoteCSeqNo.Load() {
+		return ErrDialogInvalidCseq
+	}
+	return nil
+}
+
+func (d *Dialog) ReadRequest(req *sip.Request, tx sip.ServerTransaction) error {
+	// UAS role of dialog SHOULD be
+	// prepared to receive and process requests with CSeq values more than
+	// one higher than the previous received request.
+	if err := d.validateRemoteRequest(req); err != nil {
+		return err
+	}
+
+	d.remoteCSeqNo.Store(req.CSeq().SeqNo)
+	return nil
+}

dialog_client.go

@@ -21,14 +21,6 @@ type DialogClientSession struct {
 	onClose func()
 }
 
-func (dt *DialogClientSession) validateRequest(req *sip.Request) (err error) {
-	// Make sure this is bye for this dialog
-	if req.CSeq().SeqNo < dt.lastCSeqNo.Load() {
-		return ErrDialogInvalidCseq
-	}
-	return nil
-}
-
 func (s *DialogClientSession) ReadBye(req *sip.Request, tx sip.ServerTransaction) error {
 	s.setState(sip.DialogStateEnded)
 
@@ -46,16 +38,6 @@ func (s *DialogClientSession) ReadBye(req *sip.Request, tx sip.ServerTransaction
 	return nil
 }
 
-// ReadRequest is generic func to validate new request in dialog and update seq. Use it if there are no predefined
-func (s *DialogClientSession) ReadRequest(req *sip.Request, tx sip.ServerTransaction) error {
-	if err := s.validateRequest(req); err != nil {
-		return err
-	}
-
-	s.lastCSeqNo.Store(req.CSeq().SeqNo)
-	return nil
-}
-
 // Do sends request and waits final response using Dialog rules
 // For more control use TransactionRequest
 //

dialog_server.go

@@ -51,16 +51,6 @@ func (s *DialogServerSession) ReadBye(req *sip.Request, tx sip.ServerTransaction
 	return nil
 }
 
-// ReadRequest is generic func to validate new request in dialog and update seq. Use it if there are no predefined
-func (s *DialogServerSession) ReadRequest(req *sip.Request, tx sip.ServerTransaction) error {
-	if err := s.validateRequest(req); err != nil {
-		return err
-	}
-
-	s.lastCSeqNo.Store(req.CSeq().SeqNo)
-	return nil
-}
-
 // Do does request response pattern. For more control over transaction use TransactionRequest
 func (s *DialogServerSession) Do(ctx context.Context, req *sip.Request) (*sip.Response, error) {
 	tx, err := s.TransactionRequest(ctx, req)
@@ -431,11 +421,7 @@ func (s *DialogServerSession) WriteBye(ctx context.Context, bye *sip.Request) er
 func (dt *DialogServerSession) validateRequest(req *sip.Request) (err error) {
 	// Make sure this is bye for this dialog
 
-	// UAS SHOULD be
-	// prepared to receive and process requests with CSeq values more than
-	// one higher than the previous received request.
-
-	if req.CSeq().SeqNo < dt.lastCSeqNo.Load() {
+	if req.CSeq().SeqNo < dt.InviteRequest.CSeq().SeqNo {
 		return ErrDialogInvalidCseq
 	}
 	return nil

dialog_server_test.go

@@ -112,6 +112,75 @@ func TestDialogServerTransactionCanceled(t *testing.T) {
 
 }
 
+func TestDialogServerRequestsWithinDialog(t *testing.T) {
+	// https://datatracker.ietf.org/doc/html/rfc3261#section-12.2.2
+
+	ua, _ := NewUA()
+	defer ua.Close()
+	cli, _ := NewClient(ua)
+
+	uasContact := sip.ContactHeader{
+		Address: sip.Uri{User: "test", Host: "127.0.0.200", Port: 5099},
+	}
+	dialogSrv := NewDialogServerCache(cli, uasContact)
+
+	invite, _, _ := createTestInvite(t, "sip:[email protected]", "udp", "127.0.0.1:5090")
+	invite.AppendHeader(&sip.ContactHeader{Address: sip.Uri{Host: "uas", Port: 1234}})
+
+	t.Run("InvalidCseq", func(t *testing.T) {
+		// This covers issue explained as
+		// https://github.com/emiago/sipgo/issues/187
+		conn := &sip.UDPConnection{
+			PacketConn: &fakes.UDPConn{
+				Writers: map[string]io.Writer{
+					"127.0.0.1:5090": bytes.NewBuffer(make([]byte, 0)),
+				},
+			},
+		}
+		tx := sip.NewServerTx("test", invite, conn, slog.Default())
+		tx.Init()
+
+		dialog, err := dialogSrv.ReadInvite(invite, tx)
+		require.NoError(t, err)
+		defer dialog.Close()
+
+		byeWrongCseq := newByeRequestUAC(invite, sip.NewResponseFromRequest(invite, 200, "OK", nil), nil)
+		byeWrongCseq.CSeq().SeqNo--
+		tx = sip.NewServerTx("test", byeWrongCseq, conn, slog.Default())
+		tx.Init()
+		err = dialog.ReadBye(byeWrongCseq, tx)
+		require.ErrorIs(t, err, ErrDialogInvalidCseq)
+	})
+
+	t.Run("TerminateAfterSentRequest", func(t *testing.T) {
+		// This covers issue explained as
+		// https://github.com/emiago/sipgo/issues/187
+		conn := &sip.UDPConnection{
+			PacketConn: &fakes.UDPConn{
+				Writers: map[string]io.Writer{
+					"127.0.0.1:5090": bytes.NewBuffer(make([]byte, 0)),
+				},
+			},
+		}
+		tx := sip.NewServerTx("test", invite, conn, slog.Default())
+		tx.Init()
+
+		dialog, err := dialogSrv.ReadInvite(invite, tx)
+		require.NoError(t, err)
+		defer dialog.Close()
+
+		reinvite := sip.NewRequest(sip.INVITE, invite.From().Address)
+		_, err = dialog.TransactionRequest(context.TODO(), reinvite)
+		require.NoError(t, err)
+
+		bye := newByeRequestUAC(invite, sip.NewResponseFromRequest(invite, 200, "OK", nil), nil)
+		tx = sip.NewServerTx("test-bye", bye, conn, slog.Default())
+		tx.Init()
+		err = dialog.ReadBye(bye, tx)
+		require.NoError(t, err)
+	})
+}
+
 func TestDialogServer2xxRetransmission(t *testing.T) {
 	// sip.T1 = 1
 	ua, _ := NewUA()

server_test.go

@@ -73,7 +73,7 @@ func createTestInvite(t testing.TB, targetSipUri string, transport, addr string)
 		"From: \"Alice\" <sip:alice@" + addr + ">;tag=" + ftag,
 		"To: \"Bob\" <" + targetSipUri + ">",
 		"Call-ID: " + callid,
-		"CSeq: 1 INVITE",
+		"CSeq: 10 INVITE",
 		"Content-Length: 0",
 		"",
 		"",
@@ -88,7 +88,7 @@ func createTestBye(t testing.TB, targetSipUri string, transport, addr string, ca
 		"From: \"Alice\" <sip:alice@" + addr + ">;tag=" + ftag,
 		"To: \"Bob\" <" + targetSipUri + ">;tag=" + totag,
 		"Call-ID: " + callid,
-		"CSeq: 1 INVITE",
+		"CSeq: 10 INVITE",
 		"Content-Length: 0",
 		"",
 		"",