I just ran my first tests of both OsmoNITB and OsmoSGSN connecting to OsmoHLR.
It turns out OsmoHLR uses the ipaccess_unit data a.k.a. CCM (whatever CCM
means) to route GSUP replies back to its sender.

It looks like "NAME-00-00-00-00-00-00".

However, all of our GSUP client code simply sets the unit id to:
"SGSN-00-00-00-00-00-00".

The result is that the VLR never receives an UPDATE LOCATION RESULT, because it
is sent to the SGSN instead, since that was the first one to say that it is
"SGSN-00...".

I would extend the GSUP clients to allow setting an ID from VTY, or maybe a
random ID. At this point it would suffice to make the MSC side say it is
"MSC-00-00-00-00-00-00" but that's beside the point.

Do we really want to do that? Any peer could come along and say it is someone
else, very easy to misconfigure (and not very security sensitive when thinking
of OAP -- which we're not using but maybe we will one day).

For some messages, OsmoHLR uses the conn pointer from msg rx to route the
response back -- that works. AFAICT it just receives messages and replies to
them right away (but haven't seen / understood all of it). For the case of the
UPDATE LOCATION RESULT, it would also be possible to use the same conn pointer,
but the code chooses to resolve by id and then sends to the wrong peer. If it
used the peer's IP address and port instead, things would work out.

With the attached and possibly very stupid patch, OsmoHLR works for me with
both MSC and SGSN talking to it even though they have identical IPA IDs: I
bluntly store the conn in struct lu_operation and re-use it later.

Which way shall we resolve this?

~N
From ed3e60f309ef79f23a968baf9efe7c9e5adb0fe1 Mon Sep 17 00:00:00 2001
From: Neels Hofmeyr <[email protected]>
Date: Fri, 24 Feb 2017 06:39:15 +0100
Subject: [PATCH 1/2] RFC: add the osmo_gsup_conn directly to the lu_operation
 if known

Change-Id: I427b8e5ed2a6ce82231fe7e05d1b47e9f057d9cc
---
 src/luop.c | 23 ++++++++++++++++++++---
 src/luop.h |  6 ++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/luop.c b/src/luop.c
index ecf31b4..b0028da 100644
--- a/src/luop.c
+++ b/src/luop.c
@@ -54,9 +54,14 @@ static void _luop_tx_gsup(struct lu_operation *luop,
 	msg_out = msgb_alloc_headroom(1024+16, 16, "GSUP LUOP");
 	osmo_gsup_encode(msg_out, gsup);
 
-	osmo_gsup_addr_send(luop->gsup_server, luop->peer,
-			    talloc_total_size(luop->peer),
-			    msg_out);
+	if (luop->conn)
+		osmo_gsup_addr_send_direct(luop->gsup_server,
+					   luop->conn,
+					   msg_out);
+	else
+		osmo_gsup_addr_send(luop->gsup_server, luop->peer,
+				    talloc_total_size(luop->peer),
+				    msg_out);
 }
 
 static inline void fill_gsup_msg(struct osmo_gsup_message *out,
@@ -123,6 +128,7 @@ struct lu_operation *lu_op_alloc_conn(struct osmo_gsup_conn *conn)
 		return NULL;
 
 	luop->peer = talloc_memdup(luop, peer_addr, rc);
+	luop->conn = conn;
 
 	return luop;
 }
@@ -166,6 +172,17 @@ int osmo_gsup_addr_send(struct osmo_gsup_server *gs,
 		return -ENODEV;
 	}
 
+	return osmo_gsup_addr_send_direct(gs, conn, msg);
+}
+
+int osmo_gsup_addr_send_direct(struct osmo_gsup_server *gs,
+			       struct osmo_gsup_conn *conn, struct msgb *msg)
+{
+	OSMO_ASSERT(conn);
+	DEBUGP(DMAIN, "Tx to peer %s:%u  msg %s\n",
+	       conn->conn->addr, conn->conn->port,
+	       osmo_hexdump_nospc(msg->data, msg->len));
+
 	return osmo_gsup_conn_send(conn, msg);
 }
 
diff --git a/src/luop.h b/src/luop.h
index 7e2fbb0..659a09d 100644
--- a/src/luop.h
+++ b/src/luop.h
@@ -60,12 +60,18 @@ struct lu_operation {
 	struct hlr_subscriber subscr;
 	/*! peer VLR/SGSN starting the request */
 	uint8_t *peer;
+
+	/*! peer, if we already know it */
+	struct osmo_gsup_conn *conn;
 };
 
 int osmo_gsup_addr_send(struct osmo_gsup_server *gs,
 			const uint8_t *addr, size_t addrlen,
 			struct msgb *msg);
 
+int osmo_gsup_addr_send_direct(struct osmo_gsup_server *gs,
+			       struct osmo_gsup_conn *conn, struct msgb *msg);
+
 struct lu_operation *lu_op_alloc(struct osmo_gsup_server *srv);
 struct lu_operation *lu_op_alloc_conn(struct osmo_gsup_conn *conn);
 void lu_op_statechg(struct lu_operation *luop, enum lu_state new_state);
-- 
2.1.4

Attachment: signature.asc
Description: Digital signature

Reply via email to