diff mbox

OsmoHLR and IPA unit id

Message ID 20170224054326.GB1368@my.box
State New
Headers show

Commit Message

Neels Hofmeyr Feb. 24, 2017, 5:43 a.m. UTC
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

Comments

Harald Welte Feb. 24, 2017, 3:53 p.m. UTC | #1
Hi Neels,

IPA CCM made sense on the Abis interface, where each BTS reports with
its unit_id and MAC address.  I'm not quite sure how much sense that
makes in the core network.  I also don't know if existing
implementations of e.g. A interface over IPA multiplex actually use it.

In terms of future outlook, the MSC/VLR/SGSN should register at the HLR
with some kind of identity.  In MAP, it is the SCCP Address that is used
for this purpose.  In absence of SCCP on GSUP, I used the CCM identity
as a replacement.  However, it is used as an opaque string, so that any
GSUP/MAP gateway might simply translate the SCCP address into a string
of its choosing, and then talk to OsmoHLR.  OsmoHLR would then simply
do a strcmp() on the string to match the identity.

Sooner or later, OsmoHLR will have to store this identity in the
database, e.g. for imlpementing message-waiting-lists of SMSCs in case
of MT-SMS from real SMSCs.

All-in-all, I think the string approach is not too bad in terms of
keeping GSUP simple while having a clan approach to interworkng with
MAP.

On Fri, Feb 24, 2017 at 06:43:26AM +0100, Neels Hofmeyr wrote:
> 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...".

this is of course not good.

> 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.

A random ID would not be permissible, as it has to be persistent accross
MSC/VLR/HLR re-starts, in order to make above-mentioned mechanisms
working.

I would say, why not simply use the same approach as in OsmoBTS, i.e. use
the MAC address (together with the MSC or SGSN prefix). The MAC address
is unlikely to change frequently or on short notice.  For people who
know what they're doing, we can have a VTY command to override the
identity with a manually-specified string.  If that option is not set,
the default "(SGSN|MSC)-MAC" is used.

> 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).

I don't think we are aiming for anyone to use those protocols on
public[ly accessible] networks.  There's no message authentication or
other cryptographic protection anyway.  OAP seems like a funny but
futile minor obstacle, but nothing that can provide any reasonable level
of security.

> For some messages, OsmoHLR uses the conn pointer from msg rx to route the
> response back -- that works. 

This should be done in all request-response style procedures, I think.

> 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.

Looking up by ID would also work in case meanwhile the old connection
has died and a new connection has been established

> 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.

For synchronous responses (i.e. no asynchronous activity in between)
this will work.  So I think it's an optimization for those cases, and we
shouldn't rely on this to always work for all transactions at all time
in the future.  Rather, we should make sure it works even without that
optimization?
Neels Hofmeyr Feb. 25, 2017, 4:22 p.m. UTC | #2
On Fri, Feb 24, 2017 at 04:53:03PM +0100, Harald Welte wrote:
> All-in-all, I think the string approach is not too bad in terms of
> keeping GSUP simple while having a clan approach to interworkng with
> MAP.

Yes, no objections there.


> > 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.
> 
> A random ID would not be permissible, as it has to be persistent accross
> MSC/VLR/HLR re-starts, in order to make above-mentioned mechanisms
> working.
> 
> I would say, why not simply use the same approach as in OsmoBTS, i.e. use
> the MAC address (together with the MSC or SGSN prefix). The MAC address
> is unlikely to change frequently or on short notice.  For people who
> know what they're doing, we can have a VTY command to override the
> identity with a manually-specified string.  If that option is not set,
> the default "(SGSN|MSC)-MAC" is used.

MAC will work when we have exactly one MSC|SGSN per machine: on loopback, MACs
are 0* and if two of the same run on the same ethernet device we will get
collisions (i.e. it doesn't help to create new IP addresses).

But having two of the same on the very same box is very unusual, right?
For those cases, like test setups, the VTY command solves it.

So yes, sounds like a plan.

Next question, *which* MAC?  We have osmo_get_mac_addr(), yes, but it naturally
needs a device name. In osmo-bts, I find:

osmo-bts/src/common/abis.c:251: osmo_get_macaddr(bts_dev_info.mac_addr, "eth0");

o_O that looks really bad! Hardcoded "eth0"? Ok for sysmobts, but otherwise,
seems like pure coincidence that it has worked out for everyone before.
(I'll open up an issue for that, but first awaiting your responses.)

I'm on unfamiliar turf ... is it possible to find the MAC for a given bind
instead? At first glance I thought: nice, there's and fd in osmo_get_macaddr(),
so I could just plug an fd sort of like:


	int osmo_get_macaddr_from_fd(uint8_t *mac_out, int fd)
	{
		int rc;
		struct ifreq ifr = {};

		memcpy(&ifr.ifr_name, dev_name, sizeof(ifr.ifr_name)); /* but */
		rc = ioctl(fd, SIOCGIFHWADDR, &ifr);

		if (rc < 0)
			return rc;

		memcpy(mac_out, ifr.ifr_hwaddr.sa_data, 6);

		return 0;
	}

But shucks, there's still a dev_name involved. Can't I get the SIOCGIFHWADDR of
the device that socket will use? Wait a minute, that won't work because the
kernel decides on the device to use based on the routing table...

Go through the list of interfaces and pick the first non-NULL MAC?  But that
would be vulnerable to server reconfig of possibly completely unrelated
interfaces.

At the moment it seems to me picking a MAC isn't all that simple or portable
(also need a separate FreeBSD impl), and requiring a unique ID in the config
file is so much simpler.

Does anyone have better insights and ideas than me?


> OAP seems like a funny but futile minor obstacle, but nothing that can
> provide any reasonable level of security.

heh "funny but futile"
Seems to me OAP was basically my initiation ritual at Osmocom ;)

> > For some messages, OsmoHLR uses the conn pointer from msg rx to route the
> > response back -- that works. 
> 
> This should be done in all request-response style procedures, I think.
> 
[...]
> Looking up by ID would also work in case meanwhile the old connection
> has died and a new connection has been established
> 
> > 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.
> 
> For synchronous responses (i.e. no asynchronous activity in between)
> this will work.  So I think it's an optimization for those cases, and we
> shouldn't rely on this to always work for all transactions at all time
> in the future.  Rather, we should make sure it works even without that
> optimization?

Fine with me.

That patch of mine, if we apply it for optimization, needs to cleanup
lu_operation instances in case a conn is closed.

OsmoHLR should probably warn on the error log in case two conns are open
simultaneously for the same peer ID; even reject a second conn for the same ID
to help users do less foot shooting.

~N
diff mbox

Patch

From ed3e60f309ef79f23a968baf9efe7c9e5adb0fe1 Mon Sep 17 00:00:00 2001
From: Neels Hofmeyr <nhofmeyr@sysmocom.de>
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