| Message ID | 20160128133239.GD1755@dub6 |
|---|---|
| State | Rejected |
| Headers | show |
> On 28 Jan 2016, at 14:32, Neels Hofmeyr <nhofmeyr@sysmocom.de> wrote: > > Would anyone like to prevent this commit from reaching master? ;) of course. I will even unplug your heated blanket! The silent_call handling doesn't fit from an architectural point of view. It is used to demonstrate a security implication of carrying a mobile phone. We simply open a channel and then don't do anything with it. At the same time one will receive measurement reports and could more easily triangulate or actively try to determine where the phone is. As such this feature needs to always "accept" a new "Complete layer3 information" (as of GSM 08.08). The original todo is indeed not a good one as it has absolutely no meta information. Thinking about the code my goal was to define a more clear ownership of the "MSC" side of an operation. So instead of starting a generic "close soon" timer (or the loc operation timer) have something that takes ownership of the first message. This would mean we dispatch "complete layer3 information" differently than follow up messages. As with many parts this part of the architecture has never been fixed/implemented/corrected as the current is working good enough(tm). We can either remove the comment, leave it as such or fix the above text and put it into the code as a fixme/todo. holger
On Thu, Feb 04, 2016 at 01:53:06PM +0100, Holger Freyther wrote: > > > On 28 Jan 2016, at 14:32, Neels Hofmeyr <nhofmeyr@sysmocom.de> wrote: > > > > Would anyone like to prevent this commit from reaching master? ;) > > The silent_call handling doesn't fit from an architectural point of view. It is used to demonstrate a security implication of carrying a mobile phone. We simply open a channel and then don't do anything with it. At the same time one will receive measurement reports and could more easily triangulate or actively try to determine where the phone is. > > > As such this feature needs to always "accept" a new "Complete layer3 information" (as of GSM 08.08). The original todo is indeed not a good one as it has absolutely no meta information. Thinking about the code my goal was to define a more clear ownership of the "MSC" side of an operation. So instead of starting a generic "close soon" timer (or the loc operation timer) have something that takes ownership of the first message. This would mean we dispatch "complete layer3 information" differently than follow up messages. As with many parts this part of the architecture has never been fixed/implemented/corrected as the current is working good enough(tm). > > We can either remove the comment, leave it as such or fix the above text and put it into the code as a fixme/todo. Most helpful for me would be if you just fixed the todo comment, or handed me a text I can copy-paste without having to reflect on it myself... Thx :) ~Neels
diff --git a/openbsc/src/libmsc/osmo_msc.c b/openbsc/src/libmsc/osmo_msc.c index b4facff..4bea699 100644 --- a/openbsc/src/libmsc/osmo_msc.c +++ b/openbsc/src/libmsc/osmo_msc.c @@ -48,7 +48,7 @@ static int msc_compl_l3(struct gsm_subscriber_connection *conn, struct msgb *msg gsm0408_new_conn(conn); gsm0408_dispatch(conn, msg); - /* TODO: do better */ + /* TODO: do better */ /* FIXME: do the TODO comment better in the first place! */ if (conn->silent_call) return BSC_API_CONN_POL_ACCEPT; if (conn->loc_operation || conn->sec_operation || conn->anch_operation)
Would anyone like to prevent this commit from reaching master? ;) Thanks! ~Neels commit 76af8ad075e7857983ef102f80440ee716958482 Author: Neels Hofmeyr <nhofmeyr@sysmocom.de> Date: Thu Jan 28 14:26:05 2016 +0100 add fixme comment for todo comment :/