[reSIProcate] Considering sent-by value in transaction-id
Dear All,
We met with a situation in which Resip's stack considered an incoming
invite as a retransmit even though it belonged to a subsequent SIP
dialog. The sender UA generated the same branch-id which was recently
used for a previous invite transaction but the sent-by value in the
Via header was different. We discovered that Resip's stack disregards
the sent-by value to identify transactions even though RFC 3261
(https://tools.ietf.org/html/rfc3261#section-17.2.3) says:
The request matches a transaction if:
1. the branch parameter in the request is equal to the one in the
top Via header field of the request that created the
transaction, and
2. the sent-by value in the top Via of the request is equal to the
one in the request that created the transaction, and
3. the method of the request matches the one that created the
transaction, except for ACK, where the method of the request
that created the transaction is INVITE.
This matching rule applies to both INVITE and non-INVITE transactions
alike.
The sent-by value is used as part of the matching process because
there could be accidental or malicious duplication of branch
parameters from different clients.
Here is a patch proposal which is a solution for us. It is for
Resiprocate 1.7 but 1.10 still the same in this way. Sorry about that.
Possibly you can advise a more appropriate one.
Best Regards,
Tibor
diff -Naur resiprocate-1.7-orig/resip/stack/SipMessage.cxx resiprocate-1.7/resip/stack/SipMessage.cxx
--- resiprocate-1.7-orig/resip/stack/SipMessage.cxx 2011-03-30 04:48:35.000000000 +0000
+++ resiprocate-1.7/resip/stack/SipMessage.cxx 2015-12-03 17:14:53.590396995 +0000
@@ -35,6 +35,7 @@
mStartLine(0),
mContentsHfv(0),
mContents(0),
+ mTransactionId(),
mRFC2543TransactionId(),
mRequest(false),
mResponse(false),
@@ -318,14 +319,40 @@
InfoLog (<< "Bad message with no Vias: " << *this);
throw Exception("No Via in message", __FILE__,__LINE__);
}
-
+
assert(exists(h_Vias) && !header(h_Vias).empty());
- if( exists(h_Vias) && header(h_Vias).front().exists(p_branch)
- && header(h_Vias).front().param(p_branch).hasMagicCookie()
- && (!header(h_Vias).front().param(p_branch).getTransactionId().empty())
- )
+ if(!mTransactionId.empty()) // only in case of server transaction
+ {
+ return mTransactionId;
+ }
+ else if( exists(h_Vias) && header(h_Vias).front().exists(p_branch)
+ && header(h_Vias).front().param(p_branch).hasMagicCookie()
+ && (!header(h_Vias).front().param(p_branch).getTransactionId().empty())
+ )
{
- return header(h_Vias).front().param(p_branch).getTransactionId();
+ if(isClientTransaction())
+ {
+ // Do not store tid into mTransactionId in order to cache here!
+ // Client ACKs do not like it
+ return header(h_Vias).front().param(p_branch).getTransactionId();
+ }
+ else
+ {
+ // rfc3261 17.2.3 Matching Requests to Server Transactions
+ DataStream s(mTransactionId);
+ s << header(h_Vias).front().param(p_branch).getTransactionId();
+ s << header(h_Vias).front().sentHost();
+ s << header(h_Vias).front().sentPort();
+ if (method() != ACK)
+ {
+ s << method();
+ }
+ else
+ {
+ s << INVITE;
+ }
+ }
+ return mTransactionId;
}
else
{
diff -Naur resiprocate-1.7-orig/resip/stack/SipMessage.hxx resiprocate-1.7/resip/stack/SipMessage.hxx
--- resiprocate-1.7-orig/resip/stack/SipMessage.hxx 2011-03-30 04:48:35.000000000 +0000
+++ resiprocate-1.7/resip/stack/SipMessage.hxx 2015-12-03 17:14:53.586397001 +0000
@@ -409,6 +409,9 @@
// lazy parser for the contents
mutable Contents* mContents;
+ // cached value of transaction id. as per rfc3261 see 17.2.3
+ mutable Data mTransactionId;
+
// cached value of a hash of the transaction id for a message received
// from a 2543 sip element. as per rfc3261 see 17.2.3
mutable Data mRFC2543TransactionId;
diff -Naur resiprocate-1.7-orig/resip/stack/test/testSipMessage.cxx resiprocate-1.7/resip/stack/test/testSipMessage.cxx
--- resiprocate-1.7-orig/resip/stack/test/testSipMessage.cxx 2010-08-04 10:52:40.000000000 +0000
+++ resiprocate-1.7/resip/stack/test/testSipMessage.cxx 2015-12-03 17:14:53.590396995 +0000
@@ -832,7 +832,7 @@
assert(r3->header(h_AllowEvents).size() == 2);
assert(r3->header(h_AllowEvents).front().value() == "foo");
resipCerr << r3->brief() << endl;
- assert(Data::from(r3->brief()) == "SipResp: 489 tid=899769382 cseq=1 SUBSCRIBE / 1 from(tu)");
+ assert(Data::from(r3->brief()) == "SipResp: 489 tid=899769382RjS.localdomain50709 cseq=1 SUBSCRIBE / 1 from(tu)");
const char * txt4 = ("SIP/2.0 489 Bad Event" CRLF
"Via: SIP/2.0/UDP RjS.localdomain:5070;branch=z9hG4bK" RESIP_COOKIE "899769382-1--" RESIP_COOKIE "" CRLF