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
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
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
Add missing initialisers and fix order.
In some cases the member was actually unused and could just be removed.
Change-Id: I0f0c927eb8271c35dcfd371f225847f62bea2812
Warned-by: Coverity
Ignore SDP repeated in 183 and 200OK messages
in the late offer/answer, but only for the
`OA_OfferSent` OA state.
Change-Id: I136ad02182685d0158e2c7c4eee53833cd8e00f4
When OA generates own SDP body (e.g. case with
accepted invite in DSP), save it as established_body
for this leg, in order to be able to re-invite
this leg later.
It can happen there is still no SDP seen from the
callee side (other side), and in case we want to
send established re-invite towards this leg,
this will fail, since no SDP body seen yet.
Change-Id: Ifd9f0fb70d27deac871de4eed1648f7c152813f3
When replying with 200OK generated by SEMS to INVITE requests,
which have no SDP body included, just answer with empty 200OK.
This sets the `OAState` of the call leg to `OA_None`.
Change-Id: Ic28e80d670ba4ce98be6abac9d17689688f99f71
SDP session origin id and version are limited by it's size
to 64 bits (8 bytes), that is because we are using `unsigned long long`
type for both id and version. Hence the maximum value it can hold
is two sixty-forth power, so equals: 18,446,744,073,709,551,615
In case one intentionally sets it to something like:
"o=- 18446744073709551615 18446744073709551615 IN IP4 192.168.0.1"
SEMS gets overwhelmed with that conversion inside the code make
it just to a string literal 'F'.
To overcome it, just use __uint128 (which is of 128bits size)
for the session origin id and version.
Change-Id: I2ad9659aa81dad79969749053dc3fd0d69e2cbd2
Use `unsigned long long` for SDP session id and version
instead of unsigned int.
Refactor all usage of them accordingly.
Additionally intrdouce new utils functions for conversion:
- `ulonglong2str()` - converts `unsigned long long` to `string`
- `str2ull()` - converts `string` to `unsigned long long`
Change-Id: I4210349a5442d4173b14227497f4a01d68cad7a4
Backport from the upstream to add support for body content type
application/csta+xml in INVITE and INFO requests/responses.
Upstream commit: 6f67d15c3857c1f8ed5b33615b7f5ff0e12d3a92
Change-Id: I085eda4a10e73139bd4dce4d75def82bf6da1e8d
This fixes that:
AmOfferAnswer.cpp: In member function 'int AmOfferAnswer::onReplyOut(AmSipReply&)':
log.h:143:30: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
143 | _LOG(L_DBG, error_category " " fmt, ##args)
log.h:121:45: note: in definition of macro '_LOG'
121 | int n_ = snprintf(msg_, sizeof(msg_), fmt, ##args); \
| ^~~
log.h:166:29: note: in expansion of macro 'CAT_DBG'
166 | #define DBG(fmt, args...) CAT_DBG(ERROR_CATEGORY_DGENERAL, fmt, ##args)
| ^~~~~~~
AmOfferAnswer.cpp:367:9: note: in expansion of macro 'DBG'
367 | DBG("Forcing no OA state update (no SDP changes, same session version: was <%llu>, now is <%llu>).\n",
| ^~~
AmOfferAnswer.cpp:367:88: note: format string is defined here
367 | DBG("Forcing no OA state update (no SDP changes, same session version: was <%llu>, now is <%llu>).\n",
| ~~~^
| |
| long long unsigned int
| %u
In file included from AmArg.h:43,
from AmSipMsg.h:3,
from AmOfferAnswer.h:32,
from AmOfferAnswer.cpp:29:
log.h:143:30: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 5 has type 'unsigned int' [-Wformat=]
143 | _LOG(L_DBG, error_category " " fmt, ##args)
log.h:121:45: note: in definition of macro '_LOG'
121 | int n_ = snprintf(msg_, sizeof(msg_), fmt, ##args); \
| ^~~
log.h:166:29: note: in expansion of macro 'CAT_DBG'
166 | #define DBG(fmt, args...) CAT_DBG(ERROR_CATEGORY_DGENERAL, fmt, ##args)
| ^~~~~~~
AmOfferAnswer.cpp:367:9: note: in expansion of macro 'DBG'
367 | DBG("Forcing no OA state update (no SDP changes, same session version: was <%llu>, now is <%llu>).\n",
| ^~~
AmOfferAnswer.cpp:367:103: note: format string is defined here
367 | DBG("Forcing no OA state update (no SDP changes, same session version: was <%llu>, now is <%llu>).\n",
| ~~~^
| |
| long long unsigned int
| %u
Change-Id: I15c6e37756a55ea5c917b123030b30e0ddc55724
We don't have to change the SDP OA state, in case
the 183 response has exactly the same SDP content
(same SDP session version) as we already have seen
before in previous 183 message.
Change-Id: Ica008104c31e979cdcb352cefea39db4d1ff3c56
We have to improve AmOfferAnswer functions in terms of adding
more logging, to let the debug of the OA be more clear.
Also in parallel the refactoring is done in scope of AmOfferAnswer
in order to make reading more convenient.
Change-Id: If7353f1285057760dd5fbeada984a5ef96af9854
Re-use existing local sdp, in case there is a sequential 183 coming,
but it has no SDP.
This will fix the cases, when:
- first 183 has been sent, and provided to the system the SDP
- second/third etc. 183 is coming, but has no SDP body
To not confuse the recipient (end subscriber) of these 183,
the media description in the scenario with 183 having no SDP body,
mustn't be changed (session id, media and attributes).
Previously the behavior was to generate local SDP with
a new session id and add sems-b2b into the media processing by that
(new media port, codecs and other attributes).
However, this change doesn't have an influence on very first 183
sent with no SDP body, because there was essentially no local SDP
saved before. So in this situation behavior is not changed.
Change-Id: I53dd57ae8a4739d5356d94fff45fb290896fd777
Because of messy organizing of the code in the function,
it's not possible to read that and work with that.
Change-Id: I4a56000a6f33051f9267048dcb620fb1a4eba7c6
If an SDP UPDATE is received after the 183, but before the 200,
SEMS fails when it receive the final ACK. In fact SEMS tries to
find the SDP content inside the ACK message itself.
The solution is copied from sems-pbx module where the issue
doesn't happen.
(real ticket number: TT#43503)
Change-Id: I5a432dc57c701d7eb0d5306d6005508e3310e7ba
Bit-Savva Mikhail
Richard Fuchs
Donat Zenichev
Marco Capetta
Jozef Kenyeres
Victor Seva