[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
pespin has submitted this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email )
Change subject: IuUP: allow Initialization from any address if not yet set
..
IuUP: allow Initialization from any address if not yet set
Do not refuse IuUP Initialization messages coming in on an RTP port if
the remote IP address is not yet known.
If an IUFP conn is not yet configured (pre-Initialization), allow rx
from any remote address if the remote IP address is not yet known.
If we refuse the IuUP Initialization, a 3G RNC may fail to set up a RAB.
We will know the remote address only *after* assigning a RAB succeeded.
So the IuUP Initialization must be allowed before knowing all addresses.
At the time of writing, CRCX for IUFP are sent to osmo-mgw in either
LOOPBACK or in RECVONLY mode:
- current osmo-msc: recvonly
- osmo-msc <= v1.10.0: loopback
- osmo-hnbgw: loopback
IuUP Initialization should work regardless of that.
See also next patch I158dd046fdfcb10392cde3de8cc88dd095a05b40
IuUP is one layer below the loopback/send/recv decision for RTP; IuUP is
always terminated at the MGW, while the AMR payload carries through.
Related: alternative patch Idd833997abce46886e9664505b2776fa5dadc8db
Related: SYS#6657
Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 43 insertions(+), 0 deletions(-)
Approvals:
neels: Looks good to me, but someone else must approve
Jenkins Builder: Verified
osmith: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index b1bce97..6a726d4 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -835,6 +835,18 @@
char ipbuf[INET6_ADDRSTRLEN];
if (osmo_sockaddr_is_any(>end.addr) != 0) {
+ if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
+ /* Allow IuUP Initialization to get through even if we
don't have a remote address set yet.
+* This is needed because hNodeB doesn't announce its
IuUP remote IP addr to the MGCP client
+* (RAB Assignment Response at HNBGW) until it has gone
through IuUP Initialization against
+* this MGW here. Hence the MGW may not yet know the
remote IuUP address and port at the time
+* of receiving IuUP Initialization from the hNodeB.
+*/
+ LOGPCONN(conn->conn, DRTP, LOGL_INFO,
+"Rx RTP from %s: allowing unknown src for IuUP
Initialization\n",
+osmo_sockaddr_to_str(addr));
+ return 0;
+ }
switch (conn->conn->mode) {
case MGCP_CONN_LOOPBACK:
/* HACK: for IuUP, we want to reply with an IuUP
Initialization ACK upon the first RTP
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
Gerrit-Change-Number: 35205
Gerrit-PatchSet: 3
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-CC: laforge
Gerrit-MessageType: merged
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: dexter, laforge, neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email ) Change subject: IuUP: allow Initialization from any address if not yet set .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b Gerrit-Change-Number: 35205 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: laforge Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: dexter Gerrit-Comment-Date: Wed, 06 Dec 2023 14:03:12 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: dexter, laforge, neels, pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email ) Change subject: IuUP: allow Initialization from any address if not yet set .. Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: > This is an invitation to others to add a +1, and not be scared off by the > long discussions. ack, skipped reading the long open thread. the patch looks sane. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b Gerrit-Change-Number: 35205 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Wed, 06 Dec 2023 13:51:09 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: neels Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: laforge, neels.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email )
Change subject: IuUP: allow Initialization from any address if not yet set
..
Patch Set 3:
(2 comments)
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/f6fdee5b_ff892409
PS1, Line 838: if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
> Re the linked patch, I see only address and port related code in that patch,
> nothing about codecs no […]
1) Yes, we used to always add the codec in CRCX in that scenario, but with my
libosmo-mgcp-client in gerrit we didn't. I already changed again that patch to
include the codec.
2) Yes we can, but since we want to send a tentative remote address in the
first CRCX, then we need to add the SDP instead.
3) This is precisely how all this patchset started: we want to send a remote IP
address on the RAN-side conn during first CRCX so that osmo-mgw's "ip probing"
can potentially bind to the correct local IP address and provide it in CRCX
ACK. osmo-hnbgw will use the remote Iuh IP address (HNB's Iuh IP addr) as a
remote IuUP address in the first CRCX, which is usually correct. In the event
it wasn't, during rx RAB-Ass-Resp it will send an MDCX to the MGW, and the MGW
will update the address.
We can ofc only guess the IP address, not the port. And even the IP address may
be wrong, hence why we neeed to accept rx of IuUP Init from any source.
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/dcefeb85_7fe3e76f
PS2, Line 837: != 0)
> (... […]
in the resulting code execution yes, but this way also provides more info that
errors may occur and how are they handled, eg AF_UNSPEC.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
Gerrit-Change-Number: 35205
Gerrit-PatchSet: 3
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels
Gerrit-CC: laforge
Gerrit-CC: pespin
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Comment-Date: Wed, 06 Dec 2023 11:54:02 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email ) Change subject: IuUP: allow Initialization from any address if not yet set .. Patch Set 3: (1 comment) Patchset: PS3: This is an invitation to others to add a +1, and not be scared off by the long discussions. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b Gerrit-Change-Number: 35205 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 06 Dec 2023 02:06:52 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email )
Change subject: IuUP: allow Initialization from any address if not yet set
..
Patch Set 3: Code-Review+1
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/1192328b_6d1965a0
PS1, Line 28: Decided for now that it's not worth the extra effort to make this
more
: restrictive
> "we do allow any source address to send MGCP to the MGW and actually". […]
ack
(So for early IuUP Initialization ACK, we use the sender's address somehow, for
ip probing?)
This modified patch and the following ones do make the code more restrictive as
requested, so resolving this
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/d3d00eb9_a8b469c1
PS1, Line 838: if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
> I updated https://gerrit.osmocom. […]
Re the linked patch, I see only address and port related code in that patch,
nothing about codecs nor IUFP? wrong link / am i not seeing it?
i'm a bit confused with these aspects:
1) whether the conn is indicated to be IUFP from the first CRCX
2) whether SDP is sent in the first CRCX
3) whether the remote RTP IP address is known from the first CRCX
For 1), I'm pretty sure that we are always telling the MGW about IUFP codec
right from the first CRCX. In the MGCP header's "L:" line if no SDP is present,
and in the SDP if SDP is present. And not only since yesterday.
(May need to qualify this statement for {msc,hnbgw}x{nightly,latest})
So if you add SDP even if port=0, it does not add the IUFP information;
IUFP codec was already indicated before, only now it is in the SDP instead of
MGCP head.
For 2), we can send IUFP codec even when there is no SDP.
SDP is needed only for address and port.
For 3), I'm pretty sure we DO NOT send the remote RTP IP address right from the
start, nor can we always do that, AFAICT?
>From your patch I gather that it is sometimes possible to know the remote
>address right in the first CRCX, and would like to understand it...
hnbgw knows msc's address right from the start; but cannot know RNC's address?
osmo-msc MO cannot know MT's address right from the start.
AFAICT there will always be cases where we cannot include a remote IP address
in the first CRCX, because we don't know the remote address yet, right? I am
asking because, if we can teach all clients to always include a remote address,
then this patch is not needed (besides maybe for backwards compat).
We do still need/want to have that check_rtp() skipping for IuUP Init, right?
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/60546d0c_82fe0ab5
PS2, Line 837: != 0)
> It's not a bool, it's a tristate 1, 0, -1. It's a bool + error. […]
(...which is exactly the same as handling the return val as bool)
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
Gerrit-Change-Number: 35205
Gerrit-PatchSet: 3
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels
Gerrit-CC: laforge
Gerrit-CC: pespin
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-Comment-Date: Wed, 06 Dec 2023 02:05:10 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels
Comment-In-Reply-To: laforge
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: laforge, neels. pespin has uploaded a new patch set (#3) to the change originally created by neels. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email ) The following approvals got outdated and were removed: Code-Review+1 by neels, Verified+1 by Jenkins Builder Change subject: IuUP: allow Initialization from any address if not yet set .. IuUP: allow Initialization from any address if not yet set Do not refuse IuUP Initialization messages coming in on an RTP port if the remote IP address is not yet known. If an IUFP conn is not yet configured (pre-Initialization), allow rx from any remote address if the remote IP address is not yet known. If we refuse the IuUP Initialization, a 3G RNC may fail to set up a RAB. We will know the remote address only *after* assigning a RAB succeeded. So the IuUP Initialization must be allowed before knowing all addresses. At the time of writing, CRCX for IUFP are sent to osmo-mgw in either LOOPBACK or in RECVONLY mode: - current osmo-msc: recvonly - osmo-msc <= v1.10.0: loopback - osmo-hnbgw: loopback IuUP Initialization should work regardless of that. See also next patch I158dd046fdfcb10392cde3de8cc88dd095a05b40 IuUP is one layer below the loopback/send/recv decision for RTP; IuUP is always terminated at the MGW, while the AMR payload carries through. Related: alternative patch Idd833997abce46886e9664505b2776fa5dadc8db Related: SYS#6657 Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 43 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/05/35205/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b Gerrit-Change-Number: 35205 Gerrit-PatchSet: 3 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-Attention: laforge Gerrit-MessageType: newpatchset
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: laforge, neels.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email )
Change subject: IuUP: allow Initialization from any address if not yet set
..
Patch Set 2:
(1 comment)
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/de56d40a_437eaf7b
PS1, Line 838: if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
> With current master (at least from yesterday) yes, you are right. However, […]
I updated https://gerrit.osmocom.org/c/osmo-mgw/+/35152 to submit the codec
(IUFP) even if port is not set, so that MGW knows that it has to create an IuUP
conn.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
Gerrit-Change-Number: 35205
Gerrit-PatchSet: 2
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels
Gerrit-CC: laforge
Gerrit-CC: pespin
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Comment-Date: Tue, 05 Dec 2023 12:41:56 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: laforge, neels.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email )
Change subject: IuUP: allow Initialization from any address if not yet set
..
Patch Set 2:
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/158c6533_50887307
PS1, Line 28: Decided for now that it's not worth the extra effort to make this
more
: restrictive
> ok, I understand. […]
"we do allow any source address to send MGCP to the MGW and actually". This is
easily constraint by selecting a proper IP address like a localhost address
when configuring osmo-mgw local MGCP address, or binding to an IP address only
available on a given interface.
The problem with "security" from the RTP ports comes from the fact that the
remote IP address is selected by a 3rd entity/node connected to us, and
osmo-mgw uses ip probing to find out how to connect.
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/d09ff72c_2e8481ca
PS1, Line 838: if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
> (There are two places to indicate codecs, one in the MGCP header which we
> basically ignore, and the […]
With current master (at least from yesterday) yes, you are right. However,
as I mentioned I merged a patch in osmo-hnbgw to announce a "hnb IuUP address"
to osmo-mgw right from the first RAN-side MGCP CRCX, by using the Iuh remote IP
address at the hnbgw, so that osmo-mgw can guess correctly its binding IuUP
address in the assumed general case where Iuh IP address = IuUP IP address in
HNB. See
https://gitea.osmocom.org/cellular-infrastructure/osmo-hnbgw/commit/656d1d27788a000b93f00cf9cdf659e0dacadde7
Besides that, libosmo-mgcp-client needs to be adapted in order to allow
submitting an IP address on the wire (CRCX) even if the port is yet not known
(port=0). This is what this osmo-mgw.git patch is accomplishing:
https://gerrit.osmocom.org/c/osmo-mgw/+/35152
As you see, with this patch the generated CRCX now contains an SDP, and hence
it doesn't go through the add_lco() path, but through the add_sdp() (because
it's the only way to provide an IP address to osmo-mgw so it does the ip
probing).
Now that you processed all the above, read again my previous comment to
understand the modifications (going back more or less to version 1 of the patch
to send the codec in the CRCX even if the port=0).
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/6a20a4bb_df864304
PS2, Line 837: != 0)
> (i find this really hard to read. […]
It's not a bool, it's a tristate 1, 0, -1. It's a bool + error. So in here we
are handling error as "consider it as not set".
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
Gerrit-Change-Number: 35205
Gerrit-PatchSet: 2
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels
Gerrit-CC: laforge
Gerrit-CC: pespin
Gerrit-Attention: neels
Gerrit-Attention: laforge
Gerrit-Comment-Date: Tue, 05 Dec 2023 10:30:45 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels
Comment-In-Reply-To: laforge
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email ) Change subject: IuUP: allow Initialization from any address if not yet set .. Patch Set 2: Code-Review+1 (1 comment) File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/0fd6cabe_ade0e27e PS1, Line 843: return 0; > we should only avoid the check if the conn addr is not set (see > "osmo_sockaddr_is_any(>end. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b Gerrit-Change-Number: 35205 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: laforge Gerrit-Attention: pespin Gerrit-Comment-Date: Mon, 04 Dec 2023 23:49:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email )
Change subject: IuUP: allow Initialization from any address if not yet set
..
Patch Set 2:
(4 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/b8d972c3_5d581f06
PS1, Line 28: Decided for now that it's not worth the extra effort to make this
more
: restrictive
> I would beg to differ. […]
ok, I understand. I thought the paradigm would be to allow any path for the UDP
of RTP, but you are instead stressing the need to validate the sender for RTP.
One last spark of doubt: we do allow any source address to send MGCP to the MGW
and actually create and destroy entire endpoints, so it seems a bit silly to
restrict RTP? Is it because MGCP to MGW is lateral / easy to restrict locally,
while RTP travels up/down "the stack"?
Patchset:
PS2:
got me confused at first by uploading a new version of my patch...
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/8befca28_81155a88
PS1, Line 838: if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
> I see a problem with this, regarding SYS#6657. […]
(There are two places to indicate codecs, one in the MGCP header which we
basically ignore, and the SDP payload.)
In my tests, both osmo-msc and osmo-hnbgw indicate IUFP in the very first CRCX
they send out. That is done in the *MGCP* header's 'L:' line, *without* SDP.
That is how osmo-mgw can put the conn in IUFP mode right from the start. See
for example https://people.osmocom.org/neels/Iere6lei/3g3g.pcapng
It is actually a bit more complex in osmo-hnbgw:
osmo-hnbgw sends an 'L: ... a:VNG.3GPP.IUFP' in the first CRCX;
but for the endpoint's other conn, the second CRCX, there is no 'L:' header and
the contained SDP indicates IUFP.
So AFAICT indicating IUFP already happens with nightly osmo-cni, right from the
first CRCX, and does not require an IP address.
(Not so sure about latest or whatever the issue reporter has)
Which means we can safely check for conn->type == IUFP here.
Right?
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/5ef3644d_2f2de566
PS2, Line 837: != 0)
(i find this really hard to read. the function returns essentially a bool, so
i'd much rather just read 'if (osmo_sockaddr_is_any(..))' here. Seeing the "!=
0" makes me think that 0 might be the success-rc and we check for NOT any
instead.)
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
Gerrit-Change-Number: 35205
Gerrit-PatchSet: 2
Gerrit-Owner: neels
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge
Gerrit-CC: pespin
Gerrit-Attention: laforge
Gerrit-Attention: pespin
Gerrit-Comment-Date: Mon, 04 Dec 2023 23:43:06 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: allow Initialization from any address if not yet set
Attention is currently required from: neels. pespin has uploaded a new patch set (#2) to the change originally created by neels. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email ) The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: IuUP: allow Initialization from any address if not yet set .. IuUP: allow Initialization from any address if not yet set Do not refuse IuUP Initialization messages coming in on an RTP port if the remote IP address is not yet known. If an IUFP conn is not yet configured (pre-Initialization), allow rx from any remote address if the remote IP address is not yet known. If we refuse the IuUP Initialization, a 3G RNC may fail to set up a RAB. We will know the remote address only *after* assigning a RAB succeeded. So the IuUP Initialization must be allowed before knowing all addresses. At the time of writing, CRCX for IUFP are sent to osmo-mgw in either LOOPBACK or in RECVONLY mode: - current osmo-msc: recvonly - osmo-msc <= v1.10.0: loopback - osmo-hnbgw: loopback IuUP Initialization should work regardless of that. See also next patch I158dd046fdfcb10392cde3de8cc88dd095a05b40 IuUP is one layer below the loopback/send/recv decision for RTP; IuUP is always terminated at the MGW, while the AMR payload carries through. Related: alternative patch Idd833997abce46886e9664505b2776fa5dadc8db Related: SYS#6657 Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 43 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/05/35205/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6c365559a7bd197349f0ea99f7a13b56a4bb580b Gerrit-Change-Number: 35205 Gerrit-PatchSet: 2 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge Gerrit-CC: pespin Gerrit-Attention: neels Gerrit-MessageType: newpatchset
