uac_auth: fix 407 auth retry failing due to stuck OA state
When SEMS sent an INVITE with SDP to a peer requiring proxy
authentication, the 407 response was forwarded to the caller
instead of being handled transparently with a retry.
Root cause: AmOfferAnswer::onReplyIn() did not reset the OA state
for error replies (>= 300). After the initial INVITE, OA state
was OA_OfferSent. When the 407 arrived (no SDP body), the state
remained OA_OfferSent. UACAuth then tried to retry the INVITE
with the original SDP body, but onTxSdp() rejected it:
"There is already a pending offer".
Fix: add early return with clearTransitionalState() at the top of
AmOfferAnswer::onReplyIn() for replies with code >= 300, matching
the behavior already present in sems-pbx. This ensures the OA
state is reset to OA_None before UACAuth attempts the retry.
Additionally, store req.r_uri in SIPRequestInfo and use it for
digest URI computation instead of dlg->getRemoteUri(). This
ensures the Authorization uri field always matches the original
Request-URI, as required by RFC 2617, regardless of any dialog
state changes between the initial request and the 407 handler.
Change-Id: I780bc98f193421481156de3f69497d26b893c49d
(cherry picked from commit 6b0af315ee)
The following VSC have been added to sems CE:
* vsc_off
* clir
* colr
* dnd
The toggle version of the vsc codes is still NOT added because
reserved for PRO only.
Additionally the code has been reformated to align it with
the PRO version.
Change-Id: I0cf675651eaa13ddc5dc474a5a204686e9398962
Requesting removal of a timer that isn't present in the list indicates a
race condition. Something else has already removed/disarmed the timer
(possibly from firing the timer), and that something else is then in
charge of freeing the object. Therefore do not free the timer object at
removal if the timer wasn't armed.
Change-Id: I26b58b6dc3400acf3241375da3cde879a5968d41
For future usage with `AmRtpStream` introduce
a few things:
- typedef for SSRC in u_int32
- typedef for RTCP header
Change-Id: I1df9f7773fd9da55d9e9165fc59a22d13585b898
Introduce AmStunServer support and according
implementation of things it needs:
- ice candidates and utils
- stun packet and utils
- crc calc
Change-Id: I9a7f5552e0afc63638787ae39d744ebbb08e2c55
Don't check the SDP body length by NULL pointer,
but actually by this AmMimeBody where we currently are.
Change-Id: I86a65eec82c0f5c73eb3f2653c0c27aae7f1cb7f
If the Content-Length is empty, then it makes no sense
to parse and filter application/sdp body.
Otherwise a parser is going to fail and answer with 488.
Change-Id: I92cdd19319c6f4a4edcfbaeec7bad1be12c8a0b0
Add parsing of Content-Length, for cases
were this length matters (e.g. ct length > 0,
but suddenly there is no SDP body).
Also add a getter for the AmContentType's length.
Change-Id: I92cdd19319c6f4a4edcfbaeec7bad1be12c8a0b1
Use shared_ptr for automatic reference counting of sockets, instead of
doing it manually through atomic_ref_cnt.
Use shared_ptr instead of raw pointers in all appropriate places.
Add const qualifier for a few methods.
No functional change.
Change-Id: I7db07fd90a2398f0253290aa2d810f8599e3983e
Reverse inheritence between singleton class and its base.
Change singleton template so that the singleton class doesn't inherit
from its base, but rather that the singelton class inherits from the
singleton template. This removes the need to keep the singleton base
classes separate (with the underscore prefix) plus a `typedef` for the
singleton class, and instead makes the class itself become the
singleton. (The exception being intermediate classes that have multiple
other derived classes, which is only the wheeltimer).
This makes for cleaner typing, but requires use of listing the singleton
template as friend class.
No functional changes.
Change-Id: Ic45cb01f7870ce0ba97188e58340b10fdc0380cb
Use a container for the actual objects instead of manually managing
pointers. Switch from vector to deque as AmThread isn't
copy-constructible.
No functional changes.
Change-Id: I1bbcdcd961319b78612c6fa64dd95fdda8e43d2c
The destructor is good enough, and it removes ambiguity to the
singleton's own dispose method.
No functional changes.
Change-Id: I7bd2014cbeaab0eb5104c2674d39dd8d7e38affd
Rework the payload to be a string instead of the raw char array.
This eliminates cases when we actually need to track that this char array
is indeed null-terminated and deprives us from doing other tricks to
properly maintain this char array.
Also no need to return const of payload anymore, because simply copied
during return. Data protection ensured by not giving a reference to the payload,
modifiable still during the first time setting this payload.
Rework also other users, such as nested call into AmSdp provided `parse()`.
Now `parse()` takes a string, and already in a local scope handles a char array
copy of this given string. This is only because the `parse_sdp_line_ex()` still
requires a refactor to be working with strings instead of char array.
Additionally: move parser helpers of AmSdp to const char pointers,
instead of working on a plain char pointer. This is because the SDP body
parser has been moved from the referenced pointer `char *&`
to the `const char *` pointer. This allows to not have additionally
heap allocated SDP body C-string for parsing purposes.
No functional change.
Change-Id: Ic3ee6c349b62e7e5e0cd4de722f9ed923862a7cb
Preamble: no functional changes.
AmSdp::parse() mixes int values with bool,
what makes the behavior not really clear, not defined
and can lead to the unpredicted result (even though the
compiler *should* actually fix that, but one ought to
not rely on this).
Hence make the parse method only be working with bool values.
Then, reverse the return value so that it's clear
that wrongly parsed is `false`, and good parsed is `true`.
Refactor all users accordingly.
Also rework those users, who indeed need `int` value
be returned, e.g. when having -1.
Do the same thing with the `parse_sdp_line_ex()` helper,
whereas parse() is the only user of it. And also reverse
the interpretation of true/false.
Then, make `parse()` working with a plain `char*`, so not a const.
Because doing a tricks like:
cast `char*` to const (user level) -> cast `const char*` to non-const
in a function (in a old C-like manner), has really no sense.
Refactor everything accordingly.
Other than that, refactor AmMimeBody:
Refactor it to work with a plain `char *` pointer instead of working
with `unsigned char *`, which nowadays has really no sense and
rather is a rudiment of C-like code base coming from the past.
Convert `payload` from `unsigned char*` to `char *` accordingly.
Refactor everything in AmMimeBody implementation accordingly.
Remove rudiment C-like casting everywhere, where possible.
Update `parseMultipart()` to work with a plain `const char *`
instead of `const unsigned char*`
P.S.: leave a list of TODO's for further rework, which
is not directly related to this scope of rework.
Change-Id: Ie1e132429245e0d2cc740d5b1c1fc17cf037a820
Not even used in `AmOfferAnswer::onReplyIn()`.
Fixes:
*** CID 583407: (FORWARD_NULL)
/core/AmOfferAnswer.cpp: 217 in AmOfferAnswer::onReplyIn(const AmSipReply &)()
211 {
212 /* hack to handle 2xx with different tag than was in 183: accept the
213 * new SDP though we should accept the first one we received (without
214 * fork) */
215 ILOG_DLG(L_DBG, "overwriting SDP remembered within the same transaction\n");
216
>>> CID 583407: (FORWARD_NULL)
>>> Passing null pointer "sdp_body" to "getPayload", which dereferences it.
217 if (sdp_remote.parse((const char*)sdp_body->getPayload())){
218 err_code = 400;
219 err_txt = "session description parsing failed";
220 } else if(sdp_remote.media.empty()){
221 err_code = 400;
222 err_txt = "no media line found in SDP message";
/core/AmOfferAnswer.cpp: 243 in AmOfferAnswer::onReplyIn(const AmSipReply &)()
237 remote_port_seen = remote.port;
238 }
239
240 } else {
241 bool is_reliable = reply.code >= 200 || key_in_list(getHeader(reply.hdrs, SIP_HDR_REQUIRE), SIP_EXT_100REL);
242 saveState();
>>> CID 583407: (FORWARD_NULL)
>>> Passing null pointer "sdp_body" to "onRxSdp", which dereferences it.
243 err_code = onRxSdp(reply.cseq,reply.to_tag,is_reliable, *sdp_body,&err_txt);
244 checkStateChange();
245 }
246 }
247 }
248
Change-Id: I5298acf7ac54e3335a88cd3a18d1fea3d5436dda
When using that, the return value must always
be checked, because the parsing itself can indeed fail.
Fixes such things as:
** CID 583412: Error handling issues (CHECKED_RETURN)
/core/AmB2BSession.cpp: 340 in AmB2BSession::acceptPendingInvite(AmSipRequest *, const AmMimeBody *)()
_____________________________________________________________________________________________
*** CID 583412: Error handling issues (CHECKED_RETURN)
/core/AmB2BSession.cpp: 340 in AmB2BSession::acceptPendingInvite(AmSipRequest *, const AmMimeBody *)()
334 return;
335 }
336
337 /* port must reflect actual port in SDP offer coming in */
338 if (sdp) {
339 AmSdp fake_sdp;
>>> CID 583412: Error handling issues (CHECKED_RETURN)
>>> Calling "parse" without checking return value (as is done elsewhere 12 out of 15 times).
340 fake_sdp.parse((const char *)sdp->getPayload());
341 desired_port = getMediaPort(fake_sdp);
342 }
343
344 if (desired_port) {
345 ILOG_DLG(L_DBG, "Desired port for fake 200OK is '%d'\n", desired_port);
Change-Id: I7bd177a624ebe2df2629863eb31f97f99bd921bb
Set the port in fake SDP answer (to the other side)
coming from the leg, which now replaces the previously
existing one, to the port gotten from the replace INVITE.
Otherwise, if to use the port from the pending transaction
(towards pick-up'er) it confuses rtpengine with a case
when same port talks to himself, and then can later be
used to update the pick-uper's leg once again.
Change-Id: I4c001f7956a005b64285c05586839f01216cfd91
Force the leg being connected instead of the replaced one
to the rtp_relay_mode = RTP_Direct, because otherwise
legs can get SEMS involved into rtp relay after a transfer
is finished.
Change-Id: Ifb55feaa7813fc303a663ac132b0bfe175772ce0
Don't re-exploit the connection information and port
(taken from the coming SDP offer) for creation of faked
SDP answer (200OK). This is confusing and can break things
in rtpengine monologues handling.
Better to create fake SDP answer based on already learned
local SDP port and entirely connection information.
Change-Id: Id71453022fd6c93f6743f1046588447b52c38c07
Make the AmOfferAnswer model actual and introduce
according class API updates.
Accordingly update CallLeg, SBCCallLeg and AmSipDialog.
Change-Id: Iab26f4a7577af56df68ffe693859eaac08913b7f
In case of INVITE pending UAC transactions for the other leg,
and new coming in (like 200OK send to this leg, and just right
after that re-INVITE), just provide a fake SDP answer (200OK)
in the meanwhile.
And expect that a finished UAC transaction on the other side,
must trigger a consequent update for this leg, what will finish
these media negotiations.
Change-Id: I190dfcd5467c728e2192940dfac8454f708cecaf
Richard Fuchs
Bit-Savva Mikhail
Sipwise Jenkins Builder
Marco Capetta
Donat Zenichev