Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 13: (3 comments) https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@469 PS13, Line 469: session_id > AFAIR, libosmocore doesn't encode the OSMO_GSUP_SESSION_ID_IE if > gsup_msg->session_state == 0. […] (So far none of the forwarded messages use a session id or session state. For MAP/DIAMETER compatibility, that might be necessary/nice-to-have in the future though) https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@485 PS13, Line 485: if (!msgb_l2(msg) || !msgb_l2len(msg)) > This check is redundant and not actually needed, since we do > osmo_gsup_decode(msgb_l2(msg), msgb_l2l […] I see it as an initial sanity check. If msgb_l2() is NULL, we may run into segfaults, if the len is 0, we should reject it. I'd rather keep this check. This comes from buggy code actually killing osmo-hlr, instead it should merely complain and continue to run. https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@501 PS13, Line 501: LOGP_GSUP_FWD > Here we would see 'OSMO_GSUP_MSGT_E_ROUTING_ERROR' as the message name, > because we don't store the o […] Ok, you are saying the logging fails to log the initial message, and instead logs the Routing Error reply. Nice catch. The problem is that at this point the initial msg has already been discarded... maybe LOG_GSUP_FWD should have a separate arg passing the original message type? -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 13 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Tue, 07 May 2019 13:38:52 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 13: (4 comments) https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@469 PS13, Line 469: session_id AFAIR, libosmocore doesn't encode the OSMO_GSUP_SESSION_ID_IE if gsup_msg->session_state == 0. Most likely, we should also store the gsup_msg->session_state here. https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@485 PS13, Line 485: if (!msgb_l2(msg) || !msgb_l2len(msg)) This check is redundant and not actually needed, since we do osmo_gsup_decode(msgb_l2(msg), msgb_l2len(msg), ) at line #525. https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@501 PS13, Line 501: LOGP_GSUP_FWD Here we would see 'OSMO_GSUP_MSGT_E_ROUTING_ERROR' as the message name, because we don't store the original value. This may be confusing. https://gerrit.osmocom.org/#/c/13006/13/src/hlr.c@503 PS13, Line 503: LOGP_GSUP_FWD Same as above. -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 13 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Tue, 07 May 2019 13:29:23 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Neels Hofmeyr has submitted this change and it was merged. (
https://gerrit.osmocom.org/13006 )
Change subject: hlr.c: forward GSUP messages between clients
..
hlr.c: forward GSUP messages between clients
Allow clients to forward any GSUP message between clients. Determine the
sender and receiver from the new {source,dest}_name{,_len} IEs. Reject
messages with a forged source name.
This will be used for the inter-MSC handover.
Depends: Ic00b0601eacff6d72927cea51767801142ee75db (libosmocore.git)
Related: OS#3793
Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
---
M src/hlr.c
1 file changed, 79 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
Neels Hofmeyr: Looks good to me, approved
diff --git a/src/hlr.c b/src/hlr.c
index 19cfebb..8078db0 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -441,6 +441,82 @@
return osmo_gsup_conn_send(conn, msg_out);
}
+static char namebuf[255];
+#define LOGP_GSUP_FWD(gsup, level, fmt, args ...) \
+ LOGP(DMAIN, level, "Forward %s (class=%s, IMSI=%s, %s->%s): " fmt, \
+osmo_gsup_message_type_name(gsup->message_type), \
+osmo_gsup_message_class_name(gsup->message_class), \
+gsup->imsi, \
+osmo_quote_str((const char *)gsup->source_name,
gsup->source_name_len), \
+osmo_quote_str_buf2(namebuf, sizeof(namebuf), (const char
*)gsup->destination_name, gsup->destination_name_len), \
+## args)
+
+static int read_cb_forward(struct osmo_gsup_conn *conn, struct msgb *msg,
const struct osmo_gsup_message *gsup)
+{
+ int ret = -EINVAL;
+ struct osmo_gsup_message *gsup_err;
+
+ /* FIXME: it would be better if the msgb never were deallocated
immediately by osmo_gsup_addr_send(), which a
+* select-loop volatile talloc context could facilitate. Then we would
still be able to access gsup-> members
+* (pointing into the msgb) even after sending failed, and we wouldn't
need to copy this data before sending: */
+ /* Prepare error message (before IEs get deallocated) */
+ gsup_err = talloc_zero(hlr_ctx, struct osmo_gsup_message);
+ OSMO_STRLCPY_ARRAY(gsup_err->imsi, gsup->imsi);
+ gsup_err->message_class = gsup->message_class;
+ gsup_err->destination_name = talloc_memdup(gsup_err,
gsup->destination_name, gsup->destination_name_len);
+ gsup_err->destination_name_len = gsup->destination_name_len;
+ gsup_err->message_type = OSMO_GSUP_MSGT_E_ROUTING_ERROR;
+ gsup_err->session_id = gsup->session_id;
+ gsup_err->source_name = talloc_memdup(gsup_err, gsup->source_name,
gsup->source_name_len);
+ gsup_err->source_name_len = gsup->source_name_len;
+
+ /* Check for routing IEs */
+ if (!gsup->source_name || !gsup->source_name_len ||
!gsup->destination_name || !gsup->destination_name_len) {
+ LOGP_GSUP_FWD(gsup, LOGL_ERROR, "missing routing IEs\n");
+ goto end;
+ }
+
+ /* Verify source name (e.g. "MSC-00-00-00-00-00-00") */
+ if (gsup_route_find(conn->server, gsup->source_name,
gsup->source_name_len) != conn) {
+ LOGP_GSUP_FWD(gsup, LOGL_ERROR, "mismatching source name\n");
+ goto end;
+ }
+
+ if (!msgb_l2(msg) || !msgb_l2len(msg)) {
+ LOGP_GSUP_FWD(gsup, LOGL_ERROR, "missing or empty l2 data\n");
+ goto end;
+ }
+
+ /* Forward message without re-encoding (so we don't remove unknown IEs)
*/
+ LOGP_GSUP_FWD(gsup, LOGL_INFO, "checks passed, forwarding\n");
+
+ /* Remove incoming IPA header to be able to prepend an outgoing IPA
header */
+ msgb_pull_to_l2(msg);
+ ret = osmo_gsup_addr_send(g_hlr->gs, gsup->destination_name,
gsup->destination_name_len, msg);
+ /* AT THIS POINT, THE msg MAY BE DEALLOCATED and the data like
gsup->imsi, gsup->source_name etc may all be
+* invalid and cause segfaults. */
+ msg = NULL;
+ gsup = NULL;
+ if (ret == -ENODEV)
+ LOGP_GSUP_FWD(gsup_err, LOGL_ERROR, "destination not
connected\n");
+ else if (ret)
+ LOGP_GSUP_FWD(gsup_err, LOGL_ERROR, "unknown error %i\n", ret);
+
+end:
+ /* Send error back to source */
+ if (ret) {
+ struct msgb *msg_err = msgb_alloc_headroom(1024+16, 16, "GSUP
forward ERR response");
+ OSMO_ASSERT(msg_err);
+ osmo_gsup_encode(msg_err, gsup_err);
+ LOGP_GSUP_FWD(gsup_err, LOGL_NOTICE, "Tx %s\n",
osmo_gsup_message_type_name(gsup_err->message_type));
+ osmo_gsup_conn_send(conn, msg_err);
+ }
+ talloc_free(gsup_err);
+ if (msg)
+ msgb_free(msg);
+ return ret;
+}
+
static int read_cb(struct osmo_gsup_conn *conn, struct msgb *msg)
{
static struct osmo_gsup_message gsup;
@@ -459,6 +535,9 @@
return
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 13: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 13 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Tue, 07 May 2019 13:08:29 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 13: few more fixes for this patch in new patch set: - logging tweaks - use "message_class", not "kind" -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 13 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Thu, 18 Apr 2019 13:42:19 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Neels Hofmeyr has uploaded a new patch set (#13) to the change originally
created by osmith. ( https://gerrit.osmocom.org/13006 )
Change subject: hlr.c: forward GSUP messages between clients
..
hlr.c: forward GSUP messages between clients
Allow clients to forward any GSUP message between clients. Determine the
sender and receiver from the new {source,dest}_name{,_len} IEs. Reject
messages with a forged source name.
This will be used for the inter-MSC handover.
Depends: Ic00b0601eacff6d72927cea51767801142ee75db (libosmocore.git)
Related: OS#3793
Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
---
M src/hlr.c
1 file changed, 79 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/13006/13
--
To view, visit https://gerrit.osmocom.org/13006
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 13
Gerrit-Owner: osmith
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: osmith
Gerrit-CC: Vadim Yanitskiy
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
osmith has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 12: > My apologies, but I'm infinitely way past my time, can I burden you with > resurrecting the parts I destroyed, while keeping the parts I might have > fixed? thanks! No problem, rebased on master and pushed patchset 10 again as patchset 12. It looks like your patchset 11 was the same as your patchset 7, except that it was rebased. (Patchset 10 was based on patchset 7 already, so all your fixes should be there.) -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 12 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Fri, 12 Apr 2019 08:16:30 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Hello Neels Hofmeyr, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/13006
to look at the new patch set (#12).
Change subject: hlr.c: forward GSUP messages between clients
..
hlr.c: forward GSUP messages between clients
Allow clients to forward any GSUP message between clients. Determine the
sender and receiver from the new {source,dest}_name{,_len} IEs. Reject
messages with a forged source name.
This will be used for the inter-MSC handover.
Depends: Ic00b0601eacff6d72927cea51767801142ee75db (libosmocore.git)
Related: OS#3793
Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
---
M src/hlr.c
1 file changed, 78 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/13006/12
--
To view, visit https://gerrit.osmocom.org/13006
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 12
Gerrit-Owner: osmith
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: osmith
Gerrit-CC: Vadim Yanitskiy
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 11: argh, @osmith, I think I undid some of your modifications by pushing another patch set. My apologies, but I'm infinitely way past my time, can I burden you with resurrecting the parts I destroyed, while keeping the parts I might have fixed? thanks! -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 11 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Fri, 12 Apr 2019 05:12:08 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Neels Hofmeyr has uploaded a new patch set (#11) to the change originally
created by osmith. ( https://gerrit.osmocom.org/13006 )
Change subject: hlr.c: forward GSUP messages between clients
..
hlr.c: forward GSUP messages between clients
Allow clients to forward any GSUP message between clients. Determine the
sender and receiver from the new {source,dest}_name{,_len} IEs. Reject
messages with a forged source name.
This will be used for the inter-MSC handover.
Depends: Ic00b0601eacff6d72927cea51767801142ee75db (libosmocore.git)
Related: OS#3793
Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
---
M src/hlr.c
1 file changed, 82 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/13006/11
--
To view, visit https://gerrit.osmocom.org/13006
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 11
Gerrit-Owner: osmith
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: osmith
Gerrit-CC: Vadim Yanitskiy
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Hello Neels Hofmeyr, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/13006
to look at the new patch set (#10).
Change subject: hlr.c: forward GSUP messages between clients
..
hlr.c: forward GSUP messages between clients
Allow clients to forward any GSUP message between clients. Determine the
sender and receiver from the new {source,dest}_name{,_len} IEs. Reject
messages with a forged source name.
This will be used for the inter-MSC handover.
Depends: Ic00b0601eacff6d72927cea51767801142ee75db (libosmocore.git)
Related: OS#3793
Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
---
M src/hlr.c
1 file changed, 78 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/13006/10
--
To view, visit https://gerrit.osmocom.org/13006
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 10
Gerrit-Owner: osmith
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: osmith
Gerrit-CC: Vadim Yanitskiy
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
osmith has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 8: (3 comments) https://gerrit.osmocom.org/#/c/13006/8/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/13006/8/src/hlr.c@478 PS8, Line 478: Cannot forward GSUP > All "GSUP message forwarding error" messages in this function look different. > […] Done https://gerrit.osmocom.org/#/c/13006/8/src/hlr.c@483 PS8, Line 483: LOGL_NOTICE > Let's rather use LOGL_INFO. […] Done https://gerrit.osmocom.org/#/c/13006/8/src/hlr.c@494 PS8, Line 494: gsup = NULL; > The argument is defined as follows: […] I think Neels put it there more of a reminder that accessing anything in gsup, like gsup->source_name may be deallocated at this point (since msg is deallocated). So we must not access it anymore below, and use the copy gsup_err instead. -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 8 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Thu, 11 Apr 2019 14:01:45 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Hello Neels Hofmeyr, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/13006
to look at the new patch set (#9).
Change subject: hlr.c: forward GSUP messages between clients
..
hlr.c: forward GSUP messages between clients
Allow clients to forward any GSUP message between clients. Determine the
sender and receiver from the new {source,dest}_name{,_len} IEs. Reject
messages with a forged source name.
This will be used for the inter-MSC handover.
Depends: Ic00b0601eacff6d72927cea51767801142ee75db (libosmocore.git)
Related: OS#3793
Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
---
M src/hlr.c
1 file changed, 78 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/13006/9
--
To view, visit https://gerrit.osmocom.org/13006
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 9
Gerrit-Owner: osmith
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: osmith
Gerrit-CC: Vadim Yanitskiy
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 8: (3 comments) https://gerrit.osmocom.org/#/c/13006/8/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/13006/8/src/hlr.c@478 PS8, Line 478: Cannot forward GSUP All "GSUP message forwarding error" messages in this function look different. This one is missing IMSI. Let's unify the message format for all LOGP() calls, for example: "GSUP message (IMSI-%imsi, kind=%kind, type=%type) forwarding error: REASON" This can be done using #define LOGP_GSUP(msg, fmt, ##args ...). https://gerrit.osmocom.org/#/c/13006/8/src/hlr.c@483 PS8, Line 483: LOGL_NOTICE Let's rather use LOGL_INFO. GSUP message forwarding is an expected event, and may happen quite often, so there is no need to distract log reader's attention. https://gerrit.osmocom.org/#/c/13006/8/src/hlr.c@494 PS8, Line 494: gsup = NULL; The argument is defined as follows: *const* struct osmo_gsup_message *gsup, so why do we need to forget this pointer? -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 8 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Thu, 11 Apr 2019 08:25:22 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Hello Neels Hofmeyr, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/13006
to look at the new patch set (#8).
Change subject: hlr.c: forward GSUP messages between clients
..
hlr.c: forward GSUP messages between clients
Allow clients to forward any GSUP message between clients. Determine the
sender and receiver from the new {source,dest}_name{,_len} IEs. Reject
messages with a forged source name.
This will be used for the inter-MSC handover.
Depends: Ic00b0601eacff6d72927cea51767801142ee75db (libosmocore.git)
Related: OS#3793
Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
---
M src/hlr.c
1 file changed, 82 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/13006/8
--
To view, visit https://gerrit.osmocom.org/13006
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 8
Gerrit-Owner: osmith
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Reviewer: osmith
Gerrit-CC: Vadim Yanitskiy
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
osmith has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 7: (2 comments) https://gerrit.osmocom.org/#/c/13006/7/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/13006/7/src/hlr.c@488 PS7, Line 488: and typo: and -> an (I'm quickly fixing this myself.) https://gerrit.osmocom.org/#/c/13006/7/src/hlr.c@491 PS7, Line 491: MAY BE DEALLOCATED Just to make sure I understand this right: msg is either free'd up directly (if an error occurs), or it gets added to the (struct ipa_server_conn) conn->tx_queue, and will eventually get sent and then deallocated in the main loop. Is that accurate? -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 7 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Thu, 11 Apr 2019 07:57:40 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Set Ready For Review -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 7 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: osmith Gerrit-CC: Vadim Yanitskiy Gerrit-Comment-Date: Wed, 10 Apr 2019 18:12:59 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
osmith has posted comments on this change. ( https://gerrit.osmocom.org/13006 ) Change subject: hlr.c: forward GSUP messages between clients .. Patch Set 1: (1 comment) Build fails, because of the dependency that isn't merged to master yet. https://gerrit.osmocom.org/#/c/13006/1/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/13006/1/src/hlr.c@485 PS1, Line 485: /* FIXME: send message back to sender on failure (new GSUP message type?) */ I could not find any MAP message type for "forward failed" or something similar. Can we just invent a new GSUP message type, which is not in MAP, for this use case? -- To view, visit https://gerrit.osmocom.org/13006 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5 Gerrit-Change-Number: 13006 Gerrit-PatchSet: 1 Gerrit-Owner: osmith Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: osmith Gerrit-Comment-Date: Fri, 22 Feb 2019 11:02:27 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-hlr[master]: hlr.c: forward GSUP messages between clients
osmith has uploaded this change for review. ( https://gerrit.osmocom.org/13006
Change subject: hlr.c: forward GSUP messages between clients
..
hlr.c: forward GSUP messages between clients
Allow clients to forward any GSUP message between clients. Determine the
sender and receiver from the new {source,dest}_name{,_len} IEs. Reject
messages with a forged source name or invalid source/dest name lengths.
This will be used for the inter-MSC handover.
Depends: Ic00b0601eacff6d72927cea51767801142ee75db (libosmocore.git)
Related: OS#3793
Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
---
M src/hlr.c
1 file changed, 50 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/13006/1
diff --git a/src/hlr.c b/src/hlr.c
index c544310..15ebc20 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -440,6 +440,53 @@
return osmo_gsup_conn_send(conn, msg_out);
}
+static int read_cb_forward(const struct osmo_gsup_conn *conn, struct msgb
*msg, const struct osmo_gsup_message *gsup)
+{
+ struct msgb *msg_out;
+ int ret = -EINVAL;
+
+ /* Check for routing IEs */
+ if (!gsup->source_name
+ || !gsup->source_name_len
+ || !gsup->destination_name
+ || !gsup->destination_name_len) {
+ LOGP(DMAIN, LOGL_ERROR, "Can't forward GSUP message for IMSI
%s: missing routing IEs\n", gsup->imsi);
+ goto end;
+ }
+
+ /* Verify source/dest string length */
+ if (gsup->source_name_len < 1
+ || gsup->destination_name_len < 1
+ || gsup->source_name[gsup->source_name_len - 1] != '\0'
+ || gsup->destination_name[gsup->destination_name_len - 1] != '\0')
+ {
+ LOGP(DMAIN, LOGL_ERROR, "Can't forward GSUP message for IMSI
%s: unexpected source/dest length\n",
+gsup->imsi);
+ goto end;
+ }
+
+ /* Verify source name (e.g. "MSC-00-00-00-00-00-00") */
+ if (gsup_route_find(conn->server, gsup->source_name,
gsup->source_name_len) != conn) {
+ LOGP(DMAIN, LOGL_ERROR, "Can't forward GSUP message for IMSI
%s: invalid source name '%s'\n",
+gsup->imsi, gsup->source_name);
+ goto end;
+ }
+
+ /* Forward message without re-encoding (so we don't remove unknown IEs)
*/
+ LOGP(DMAIN, LOGL_DEBUG, "Forwarding GSUP message for IMSI %s from %s to
%s\n", gsup->imsi, gsup->source_name,
+gsup->destination_name);
+ msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP FORWARD");
+ ret = osmo_gsup_addr_send(g_hlr->gs, gsup->destination_name,
gsup->destination_name_len, msg_out);
+ if (ret)
+ LOGP(DMAIN, LOGL_ERROR, "Can't forward GSUP message for IMSI %s
from %s to %s: destination not"
+ " connected?\n", gsup->imsi,
gsup->source_name, gsup->destination_name);
+
+end:
+ /* FIXME: send message back to sender on failure (new GSUP message
type?) */
+ msgb_free(msg);
+ return ret;
+}
+
static int read_cb(struct osmo_gsup_conn *conn, struct msgb *msg)
{
static struct osmo_gsup_message gsup;
@@ -456,6 +503,9 @@
if (strlen(gsup.imsi) < 5)
return gsup_send_err_reply(conn, gsup.imsi, gsup.message_type,
GMM_CAUSE_INV_MAND_INFO);
+ if (gsup.destination_name_len)
+ return read_cb_forward(conn, msg, );
+
switch (gsup.message_type) {
/* requests sent to us */
case OSMO_GSUP_MSGT_SEND_AUTH_INFO_REQUEST:
--
To view, visit https://gerrit.osmocom.org/13006
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia4f345abc877baaf0a8f73b8988e6514d9589bf5
Gerrit-Change-Number: 13006
Gerrit-PatchSet: 1
Gerrit-Owner: osmith
