The TypeBoolean case in xmlrpcval2amarg() has no break, so a boolean
parameter is immediately overwritten with AmArg() by the TypeInvalid
case. The defect is latent in the tree: no DI method takes boolean
parameters yet. Introduced in 6e306e95.
Closes#31
Change-Id: I630889d91d67e514170dfe576bf13d74ddb8bca9
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
During a pickup/reconnect SEMS answers the picking leg's INVITE with a
(fake) 200 OK and, driven by the reconnect/AA media update, immediately
re-INVITEs the same leg - but the leg has not ACKed the 200 OK yet, so
it gets a re-INVITE on top of an unfinished transaction (overlapping
transactions on its side; the pickup ends up with no audio).
The INVITE transaction is already gone from uas_trans the moment the 2xx
is sent (2xx terminates it, the ACK goes to the TU), so getUASPendingInv()
no longer sees it and both re-INVITE paths fire too early:
- applyPendingUpdate() - the reconnect update queued "for later", gated
by canUpdateSession();
- sendEstablishedReInvite() - the refresh after the other leg's SDP
changed, fired from AmB2BSession::onB2BEvent().
Remember in AmSipDialog the CSeq of the INVITE we answered with a 2xx as
UAS (uas_2xx_ack_cseq) and clear it on the ACK carrying that CSeq, or when
the ACK times out. A plain flag would not be enough: pending_invites is
already decremented and the O/A is complete by the time the 2xx is sent,
so a remote sending its next INVITE before it ACKs is not rejected with
491, and the ACK of the previous INVITE must not open the leg up while
that one is still unacknowledged.
Gate both paths on it - canUpdateSession() returns false while an ACK is
outstanding, and the established refresh is deferred
(pending_established_reinvite) and fired from onTransFinished() once the
ACK finishes the transaction.
Change-Id: I9356c013c813059a2f2c46dcf4cfb9480f3d6f83
This commit concerns the case, when a transaction stored in the
trans_bucket::trans_list does not have a To tag previously saved
as the 't->to_tag', which leads to problems when matching ACK.
Then try to use as a fall back the transacation's
message to match to To tag value 't->msg->to'.
Change-Id: I41790a1f0581a3917ad5acff79b63591cb3cc2f6
AmSdp::sdp_parse_rtcp() fills in the RTCP port of media which advertised
neither a=rtcp nor rtcp-mux. That branch is only entered when rtcp_address
holds no port, and then derives the port from that very field:
} else if (!it->rtcp_address.getPort()) {
...
it->rtcp_address.setPort(it->rtcp_address.getPort() + 1);
}
so the result is always 1. The media port was lost in 5eda9bb0e, where
rtcp_port became an RtcpAddress and the right hand side got rewritten
along with the left one.
AmRtpStream::init() then points the remote RTCP address at port 1, so RTCP
goes nowhere - unless the stream is in passive mode and the peer sends RTCP
first, in which case handleSymmetricRtp() learns the real address. Nothing
of this shows in the SDP we send out: setPort() leaves the port unflagged
and print() only emits a=rtcp for an explicitly given one, which is why it
stayed unnoticed.
Take the port from the media line, and leave it zeroed for a rejected one
instead of pointing it at port 1.
Change-Id: I844d379789958d8734d68159e39adc3428477bdb
When SEMS is the media endpoint - on a CloudPBX call that happens when it
plays music on hold - AmB2BMedia::replaceConnectionAddress() puts its own
port into m=audio, but leaves the a=rtcp taken over from the received SDP
untouched. AmSdp::print() prints that attribute back out verbatim, so the
offer SEMS sends towards the other leg pairs its own RTP port with the
RTCP port of the party the SDP came from:
m=audio 15000 RTP/AVP 8 101
a=rtcp:34505
RFC 3605 expects a=rtcp to carry the RTCP port of whoever sends the SDP,
so the attribute describes the wrong endpoint. Its value is also the port
rtpengine listens on towards SEMS, which is thereby told to send RTCP to
itself.
State our own RTCP port whenever the media port is replaced, no matter
whether the remote party advertised one. AmRtpSocketPair binds the RTCP
socket to RTP+1 and gives up rather than fall back to another port, so the
advertised value cannot disagree with the socket we listen on.
Only relayed media is affected. Pass-through SDP does not have its port
replaced and keeps the attribute as received.
Change-Id: I4b06236eea9d5ac26bbdfa12c4b34ed9f2562e2e
A blind transfer made by a party which took the call over by a
supervision pickup makes every party see a wrong connected number and
name: the caller is presented with the originally called extension
instead of the one it is really talking to, and the picking party is
presented with a number nobody in the call has.
cc_xfer describes the call being transferred to the proxy with the
P-Xfer-Firstcall-* headers, built from the initial INVITE of the leg the
new call is created from. After a pickup that INVITE still names the
party which was replaced, so the headers describe a call which no longer
exists. The proxy decides whether a transfer comes from the caller or
from the callee side by comparing Referred-By with
P-Xfer-Firstcall-Callee-To (kamailio-config, ROUTE_FIND_BLIND_TRANSFER_CALLER);
with the replaced party in that header neither side matches, so it takes
the calling identity of the new call from the wrong leg. That identity
then propagates into the From/P-Asserted-Identity of the transferred
call, into the dialog-info presentity of its ringing dialog and from
there into the COLP of the party which picks it up.
Remember the identity of the new remote party when a leg gets reconnected
to one (P-Caller-UUID and P-NGCP-Caller-Device-Id of its INVITE, which
the proxy provides), and let cc_xfer describe the first call with it.
Nothing changes for a transfer without a preceding pickup, where no
reconnect happened and the stored identity is empty.
A new virtual CallLeg::onPeerReplaced() carries the reconnect to the SBC
layer, which is where the call profile - and with it the cc_vars used by
the call control modules - lives.
Change-Id: I3ec4b1504f0c1e5843718c61b18c85efadac54ff
The `g729_create()` was previously declared with three arguments:
`(format_parameters, format_parameters_out, format_description**)`
and the mismatch was hidden by casting it to `amci_codec_init_t`.
The core itslef still calls the codec init with two arguments only:
`(format_parameters, fmt_i)`
As a result, the codec never fills the caller's `fmt_i`,
and `*format_description = ...` writes 8 bytes to whatever address
was left in the third argument register.
If that register points to `fmt_i`,
the `AmAudioRtpFormat::initCodec()` then reads uninitialized stack
and may pick a bogus frame size from it
(e.g. seen as `frame_size = 131074`, which then overflowed
the media processor buffer). Otherwise it is a stray write
into an arbitrary memory location on every G.729 stream setup.
Use the two-argument signature, fill the caller's array
(as other codecs do) and drop the cast, so the compiler
validates the signature. Also fix the encoded frame size:
G.729 at 20ms is 20 bytes (33 is the GSM one).
Change-Id: I791c300892bd905b72314c6c84aa3077e71c64fd
Introduce AmRtpAudio stored guard that rejects frame
sizes above 4096 samples (which is 8192 bytes of PCM16).
PCM16 is an internal compression algo used for buffering.
This guard prevents the daemon from crashing on overflowing
when writing streams.
Additionally, add bounds in the list of getters:
- `AmPlayList::get()`
- `AmRtpAudio::get()`
- `AmAudio::get()`
right before writting into the shared buffer.
This is to prevent the cases where a valid frame size still
grows too much after resampling.
Change-Id: I1201d7ae0ceea37c66881d317907fe4f530664f0
Synchronize db_reg_agent with the latest updates
in the project:
- support a separate registrar server (R-URI and From/To domains),
read from the new `registrar_server` DB column or a DI argument,
falling back to the realm
- add outbound_proxy_set/outbound_proxy_set_port to pick an
outbound proxy by the contact's IP
- drop the `username_with_domain` option
- remove the subscribers' event queues on shutdown
Core:
- AmSipRegistration: add `SIPRegistrationInfo::registrar_server` and
use it for the R-URI and From/To
- AmBasicSipDialog: add `SIP_FLAGS_KEEP_CONTACT`
- registrar_client: pass the domain as the registrar server
Change-Id: I506c48a1e06747fe970720f57ac264550fb45af8
When faking the SDP body (the rapid 200OK answer)
make sure about proper parsed given (requeste's) SDP body,
before to try to prepare a new faked SDP based on that.
Change-Id: I57d1010051adccc941d485b3c6178d34154c2709
Arm rtp_timer in init(), next to the transport attachment, and make the
existing monitor_rtp_timeout flag effective so an application can opt
out per stream via the DSM monitorRTPTimeout() action.
Change-Id: Iad193ec2554fed53f91237d9cd9e1f2f5521b0f0
Now as more states for DSM app is supported
(new state `connected` has been introduced), add
more flexible handling of states within the DSM core.
Now states can be comined and look like: `;state=updating,connected`.
Meanwhile each state his own scope. So they are rather option tokens.
Change-Id: I64f18e8c1c6f016aae0a46ea00eb3fea25acf432
An outgoing connection gets its events only in the `check_connection()`,
`after connect()`.
If `socket()` or setting it non-blocking fails, the
socket is removed from the worker map and destroyed with read_ev and
write_ev still NULL and `event_free(NULL)` dereferences it in `event_del()`.
Only free the events that were created.
Change-Id: I1b3d9903ce2cc47c39812c6e428a4b3ab394ab70
`add_connection()` replaces an existing map entry with the same peer
ip:port, dropping the old socket's reference without `close()`.
An outgoing connection still connecting to that peer is then destroyed
with its queued messages, which only the `close()` used to release.
The fd itself is closed by `~trsp_socket()`.
Free whatever is left in send_q in the destructor.
Change-Id: I63c636f7547490094e361e1a10382da791aeaca1
A connection replaced in the worker map by `add_connection()` keeps its
events registered while other references
(e.g. a transaction's retr_socket) keep it alive.
If the last of those is dropped on another thread while one of its callbacks
is in flight, the destructor waits in `event_free()` for the callback,
and the callback's `shared_from_this()` throws `std::bad_weak_ptr` through
libevent's C frames `std::terminate`.
Use `weak_from_this().lock()` and just return when the socket is already
being destroyed.
Change-Id: I26768aa3984be0e1667629c5cc08498676884233
On EOF or timeout, close() removes the socket from the worker
connection map, which holds its only shared_ptr. The destructor frees
the libevent events while still inside the read callback, and close()
goes on touching them. A peer quickly connecting and closing to the
SIP TCP port crashes SEMS within seconds.
Hold a shared_from_this() reference in the read/write callbacks, so
the socket outlives the callback that closes it. Also make
remove_connection() erase only its own map entry: the peer ip:port key
gets reused by new connections and must not be removed by the old
socket.
Closes#26
Change-Id: I3d6a3764470520d6124bf6e04158869a94dd98dd
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
A test answering one of SEMS' INVITEs later than T1 finds a timer-A
retransmission of it in front of the ACK, and the blind drain took that
for the ACK. The next in-dialog re-INVITE then went out right behind our
2xx, got read by another UDP receiver thread first and was rightly 491'd
as glare (RFC 3261 14.2). Wait for the ACK with the INVITE's CSeq
instead, skipping its retransmissions.
The 491 the glare test provokes was never ACKed, so SEMS kept resending
it (timer G) and a copy could be read in place of the relayed 200 OK.
ACK it (RFC 3261 17.1.1.3).
Fake 200s keep being resent for a while even after we have ACKed them,
so read final replies by CSeq rather than taking whatever comes next,
and answer INFOs relayed by DSM that show up meanwhile (late copies, or
ones an earlier read missed).
The renegotiation following the fake 200 to collide1 was waited for one
second only and then given up on; when it came later, collide2 met it
still pending and got 491'd at the dialog layer - the "491 instead of
200" on collide2. SEMS always sends it, so wait for it for real.
Change-Id: I0f28734ee384cb3ca9e900051e30c208f053c567
When the message completes in the incremental read branch, msgbuf is
dispatched as a string, but nothing writes the terminating NUL. On a
reused connection the scan then runs into leftover bytes of an earlier,
longer message. Put the NUL right after the ',' separator, as it is
already done in the first-read branch.
Change-Id: I1f6deb98b2e521b5d376292592c6c2a48ab3a6e2
read() returns ssize_t, storing it into the unsigned rcvd_size turns -1 into UINT_MAX.
A client can trigger this by sending '5:' and pausing: the body has not arrived yet,
read() fails with EAGAIN, the later 'rcvd_size < 0' checks are then never true,
and the next read continues at a wrapped offset into msgbuf.
Keep the result in the signed r across the checks
and assign rcvd_size only once it is known to be sane,
resetting it when switching to body mode.
Change-Id: I00e76af2bac46d972c25d54e27ef52d31c7bd9e5
The function mixed tabs and spaces and carried trailing whitespace;
re-indent it to two spaces, as the rest of the file. No functional
change -- git diff -w is empty.
Closes#27
Change-Id: Iee4b9ae723698536a59ad38407ebe9f7c07cf066
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
A peer announces the message length in the netstring header and the
reader trusts it: msg_size, up to nearly 4 GB, is used directly as the
read() size into the fixed 20 MB connection buffer. One frame with an
oversized length overwrites adjacent heap and crashes the server.
Reject the connection when the announced length does not fit into
msgbuf together with the trailing comma and the terminating NUL.
Closes#27
Change-Id: Idf3ab6d29f85e21599b0ced5df16ecd54868b517
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
event2/event.h defines EV_READ/EV_WRITE compatibility macros that
shadow libev's enumerators in any TU including them. Since the RTP
transport overhaul, AmRtpSocket.h pulled this header into every
plug-in's include chain, silently breaking libev-based plug-ins:
jsonrpc registered its accept watcher for EV_WRITE and never accepted
connections.
Headers only hold pointers to struct event/event_base, so replace the
include with forward declarations there (event2/util.h stays in
AmRtpSocket.h for evutil_socket_t, it has no EV_* macros) and add the
explicit event2/event.h include to the .cpp files that relied on the
transitive one.
Closes#25
Change-Id: Ic1655219a4969453b1c68075391061246d4830d8
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
Undef the libevent compatibility macros that core headers may expose,
restoring libev's enumerator values for the watcher code in this TU.
Defense in depth: the root cause is fixed on the core side, but the
plug-in must not depend on core header hygiene.
Change-Id: Idace5df3d04b10be16e713e89b672b2be4f76f6f
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
An older thing in the `AmSession`, we still by default always
have the `srtp` set to false, but should any time in the future
this be changed, passing the srtp in the place of the port param
(what currently is being done) would lead to setting the port value
to 1. Now that works only because the `srtp` is false, and hence
this gets converted to 0. Otherwise SEMS would try to bind the
rtp port to 1 and the new rtcp retry mechanism would quitely
move to a pool port.
Change-Id: I183ac8dae963aff61b679e1f7a0ad6893aa60791
Without rtcp-mux, an even RTP port is drawn from the pool and the
RTCP socket is then bound to port+1. Under high load the neighbour
port may still be occupied by a session being torn down - already
released from the active session range, but not yet free at OS
level - and the whole session setup fails. Retry binding the
even/odd pair up to 5 times, drawing a fresh port from the pool on
each attempt. With rtcp-mux only a single socket is allocated, so
no retry is needed.
Closes#21
Change-Id: I83589083a3065c3d0d23d5e16cec86511b898d65
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
AmPlugIn::loadPlugIn() wrapped the dlopen() result directly into a
shared_ptr<dlhandle> and tested the shared pointer for null, which
never holds even when dlopen() failed: the failure went unnoticed and
loading continued with a NULL handle. Check the raw dlopen() handle
and report dlerror() before wrapping it.
Closes#20
Change-Id: Ie27c229a0dc5d768003b474c631c7d0149f8f099
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
Before it actually clears the `rtp_streams`.
AmRtpTransport destructor run cleans the rtp streams with
`rtp_streams.clear()` while the socket is still registered
with the receiver.
Meanwhile the receiver thread can potentially be walking
trhough that same list in `recvRtp()` or `getStream()`.
Change-Id: I1b9df8a412a758ce8f0d8fb7e9213fdf7378b8d3
Don't close the socket before calling the `event_free()`.
If suddenly another thread would open the new RTP socket
in this gap of time and by coincidence get the same fd number,
libevent would believe that this very fd is being watched.
Then this leads to skip registering in the kernel,
thus no rtp data will be received and processed.
Change-Id: I0ff7c1a73b53bb948b2eeb6921e47847ce2ed93b
doUnregister() overwrote the "Expires: 0" header with the Contact
header instead of appending to it, so any de-registration for a
subscriber with a contact set left the outgoing REGISTER without
an Expires header at all. The registrar then treated it as a normal
refresh instead of removing the binding, while SEMS locally deleted
its registration record, leaving the two sides out of sync.
Change-Id: I4a141db069f305cd539fab7913c64c8944a978f5
get_lowest_expire() uses LONG_MAX without including <climits>,
which fails to build with newer/stricter compilers.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Change-Id: Id431111dce96cb9df76d92d15467b08057d7353a
The constructor printed 'enabled builtin method ...' for calls,
get_cpsavg, get_cpsmax, get_callsavg, get_callsmax, get_cpslimit,
set_cpslimit and loglevel/shutdownmode methods, but never called
s->addMethod() for them, so all builtin methods returned
'unknown method name'. Register them (and the 'di' method when
export_di=yes) after the server object is created.
Closes#15
Change-Id: I4c34aea41b26b5adadc73f0a3c3d8626692e7165
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
To keep the route set from PRACK 200 OK
Closes#17
Change-Id: I0bc9454f9e26541863f4e240b0eedaca513dda6d
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
cancel(hdrs) ignored its argument and always re-sent the headers
stored in the INVITE transaction. Use the caller's headers when
non-empty, falling back to the stored ones otherwise.
Closes#14
Change-Id: I2d7fd41ad309be20a5c7364a655c035151d20323
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
The hdrs argument was accepted but never copied into the ACK
request, so custom headers provided by the application were
silently dropped when ACKing a 2xx response.
Closes#14
Change-Id: I6f6667712eac716b1792614b2341dde712603154
Signed-off-by: Vadim Saranov <vadim.saranov@gmail.com>
make the `run()` more accurately honor the threads stop
request when checking registrations in the inner-loop.
If the thread is on `wait_or_stop()` this should be managed
just quickly and gracefully.
However if we are on the `check_registration()` or
`create_registration()` doing a DI cal, threads stop request
cannot really interrupt it. So tighten it.
Change-Id: I31ef44445817d4eb8e11cf269c2bd4db2eff71ef
If the owner of the `RegThread` (`RegistrationAgentFactory`)
was asked to be destructed, make the destroy process
of the reg thread more ordered as well.
That makes the thread to stop as soon as the app factory
begins its own destruction.
This will help to avoid a possible way to end up with
a stale (or invalid) DI factory ptr.
E.g.: `RegThread` keeps on running, while all the plugins
are being torn down by that time already. In this case
the `RegThread` can simply call `getFactory4Di()` or `di_f->getInstance()`
way after the registrar_client is already partially destroyed.
Change-Id: I10e38174d3856e19f2696f8d874ea166e0a035a8
Instead of blindly sleeping, use the `run_cond.wait_for()`
for the case, when AmThread sends `.notify_all()` upon stopping.
Hence we don't have to block the whole thread for 10 seconds if
suddenly the `stop()` is requested by AmThread at the very beginning
of the sleep.
Add a dedicated `RegThread::wait_or_stop()` wrapper to handle this.
Also differentiate whether the `RegThread::wait_or_stop()` returns
due to timer exceed or there was an actual thread stop requested.
Act accordingly during the `run()` execution.
Change-Id: I1e603c462af983e633c811d10ed2d0ac506f46d3
Check whether the thread was asked to be stopped,
instead on infinitely cycling through the true statement,
relying on the fact `getFactory4Di()` will always return
a safe and valid pointer.
Change-Id: Ibc27b725eaa2c183c50bff603f7d3115cdc57b3d
If 183 has empty body, don't change the state,
because otherwise it will break the OA model exchange
in later updates during the continued early dialog stage.
Keep pretending to remain in the `Completed` state.
Change-Id: I1053740a179c8589b54a0c856caa438a92521fda
For the cases with option tags are being added/removed
from the headers (e.g. Require, Supported),
we have to cover corner cases.
Introduce according tests into existing core tests
covering headers, as well as python framework part of it.
Change-Id: Iad15add88473f9d64cc6a3d27fc6e996e4a81abf
Do not use the original iterator's position,
when removing the first token from something like:
`Supported: timer, path, replaces`
Instead build the remaining option list by appending
only those kept, trimmed tokens with commas between them.
(P.S.: this is not related to the latest SST changes)
Change-Id: I5a274072dbbc336494a0802ff7e80f0bf0b51813
For the case when ptime sample requests > than the DlbBuffer is.
Actually normal rates (e.g. PCMA/PCMU) have 20ms (ca. 160bytes),
even if the ptime would be 100ms, which is unusual, the size
requested per sample would be ca 1600bytes.
`AmAudio::samples` are `DblBuffer` based,
which has size of `AUDIO_BUFFER_SIZE * 2`, but it's not
contiguous, but exposes only half of it at a time through
the `unsigned char*` overloading operator.
Add the main guard in `AmAudio::get()`, who calculates the actual size
to be requested from the buffer to read into.
Additionally (actually less effective) prevent also overflow
in the implementation itself in `AmAudioFile::read()`, this is where
the `fread()` happens. Just for the case if the path towards it
is different in any other case.
Change-Id: Ifd381f4435f0a16ecee5612da41ea9046f6d22a4
Add a test that covers `UDP/TLS/RTP/SAVPF` transport
and particularly focuses on ICE and DTLS related attrs:
- `a=fingerprint`
- `a=setup`
- `a=tls-id`
- `a=ice-ufrag`
- `a=ice-pwd`
- `a=candidate`
Send the original INVITE with `UDP/TLS/RTP/SAVPF` SDP,
including those attrs, and then assert that the B-leg INVITE
still has them.
Also improtve the ` _make_invite()` so that it can accept
an optional SDP body so the new case can reuse existing INVITE builder.
Change-Id: Ib3ff54bec50f364b79e189bd8e61bd42bb3f6503
Vadim Saranov
Bit-Savva Mikhail
Donat Zenichev
Kéryl Thiry
Sipwise Jenkins Builder
Fedorov Volodymyr