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 <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <lafo...@osmocom.org>
Gerrit-CC: pespin <pes...@sysmocom.de>
Gerrit-Attention: laforge <lafo...@osmocom.org>
Gerrit-Attention: pespin <pes...@sysmocom.de>
Gerrit-Comment-Date: Mon, 04 Dec 2023 23:43:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <lafo...@osmocom.org>
Comment-In-Reply-To: pespin <pes...@sysmocom.de>
Gerrit-MessageType: comment

Reply via email to