Change in osmo-hlr[master]: disable recording of LU timestamps by default

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Stefan Sperling gerrit-no-reply at lists.osmocom.org
Mon Dec 10 14:03:11 UTC 2018


Stefan Sperling has uploaded this change for review. ( https://gerrit.osmocom.org/12228


Change subject: disable recording of LU timestamps by default
......................................................................

disable recording of LU timestamps by default

Add VTY commands which enable or disable recording of
Location Update timestamps in the HLR database.

Because this feature has implications for the privacy of
network users, it is now off by default. It needs to be
explicitly enabled by the administrator who will see a
warning about potential privacy concerns when doing so.

The new commands added to the hlr configuration space are:

  record-lu-timestamps
  no record-lu-timestamps

DB tests keep recording timestamps in the test database to
ensure that the corresponding code is being exercised.

Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Related: OS#2838
Depends: Ie180c434f02ffec0d4b2f651a73258a8126b2e1a
---
M src/db.h
M src/db_hlr.c
M src/hlr.c
M src/hlr.h
M src/hlr_vty.c
M tests/db/db_test.c
M tests/db/db_test.err
7 files changed, 63 insertions(+), 31 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/28/12228/1

diff --git a/src/db.h b/src/db.h
index ae592fb..2439196 100644
--- a/src/db.h
+++ b/src/db.h
@@ -131,7 +131,7 @@
 			struct hlr_subscriber *subscr);
 int db_subscr_nam(struct db_context *dbc, const char *imsi, bool nam_val, bool is_ps);
 int db_subscr_lu(struct db_context *dbc, int64_t subscr_id,
-		 const char *vlr_or_sgsn_number, bool is_ps);
+		 const char *vlr_or_sgsn_number, bool is_ps, bool record_timestamp);
 
 int db_subscr_purge(struct db_context *dbc, const char *by_imsi,
 		    bool purge_val, bool is_ps);
diff --git a/src/db_hlr.c b/src/db_hlr.c
index c6293f9..dd80aaf 100644
--- a/src/db_hlr.c
+++ b/src/db_hlr.c
@@ -591,11 +591,12 @@
  * \param[in] subscr_id  ID of the subscriber in the HLR db.
  * \param[in] vlr_or_sgsn_number  ASCII string of identifier digits.
  * \param[in] is_ps  when true, set sgsn_number, else set vlr_number.
+ * \param[in] record_timestamp  if true, then store LU timestamp in the HLR db
  * \returns 0 on success, -ENOENT when the given subscriber does not exist,
  *         -EIO on database errors.
  */
 int db_subscr_lu(struct db_context *dbc, int64_t subscr_id,
-		 const char *vlr_or_sgsn_number, bool is_ps)
+		 const char *vlr_or_sgsn_number, bool is_ps, bool record_timestamp)
 {
 	sqlite3_stmt *stmt;
 	int rc, ret = 0;
@@ -637,6 +638,9 @@
 
 	db_remove_reset(stmt);
 
+	if (!record_timestamp)
+		return 0;
+
 	if (osmo_clock_gettime(CLOCK_REALTIME, &localtime) != 0) {
 		LOGP(DAUC, LOGL_ERROR, "Cannot get the current time: (%d) %s\n", errno, strerror(errno));
 		ret = -errno;
diff --git a/src/hlr.c b/src/hlr.c
index 4873a66..5020eae 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -325,7 +325,7 @@
 	LOGP(DAUC, LOGL_DEBUG, "IMSI='%s': storing %s = %s\n",
 	     subscr->imsi, luop->is_ps ? "SGSN number" : "VLR number",
 	     osmo_quote_str((const char*)luop->peer, -1));
-	if (db_subscr_lu(g_hlr->dbc, subscr->id, (const char *)luop->peer, luop->is_ps))
+	if (db_subscr_lu(g_hlr->dbc, subscr->id, (const char *)luop->peer, luop->is_ps, g_hlr->record_lu_timestamps))
 		LOGP(DAUC, LOGL_ERROR, "IMSI='%s': Cannot update %s in the database\n",
 		     subscr->imsi, luop->is_ps ? "SGSN number" : "VLR number");
 
@@ -614,6 +614,9 @@
 	/* Init default (call independent) SS session guard timeout value */
 	g_hlr->ncss_guard_timeout = NCSS_GUARD_TIMEOUT_DEFAULT;
 
+	/* Do not record location update timestamps by default. */
+	g_hlr->record_lu_timestamps = false;
+
 	rc = osmo_init_logging2(hlr_ctx, &hlr_log_info);
 	if (rc < 0) {
 		fprintf(stderr, "Error initializing logging\n");
diff --git a/src/hlr.h b/src/hlr.h
index e9cc747..7add8b7 100644
--- a/src/hlr.h
+++ b/src/hlr.h
@@ -51,6 +51,9 @@
 	struct llist_head ussd_routes;
 
 	struct llist_head ss_sessions;
+
+	/* Shall we store Location Update timestamps in the database? */
+	bool record_lu_timestamps;
 };
 
 extern struct hlr *g_hlr;
diff --git a/src/hlr_vty.c b/src/hlr_vty.c
index 6706aa4..d3caad5 100644
--- a/src/hlr_vty.c
+++ b/src/hlr_vty.c
@@ -71,6 +71,8 @@
 static int config_write_hlr(struct vty *vty)
 {
 	vty_out(vty, "hlr%s", VTY_NEWLINE);
+	if (g_hlr->record_lu_timestamps)
+		vty_out(vty, " record-lu-timestamps%s", VTY_NEWLINE);
 	return CMD_SUCCESS;
 }
 
@@ -123,6 +125,24 @@
 	return CMD_SUCCESS;
 }
 
+DEFUN(cfg_record_lu_timestamps, cfg_record_lu_timestamps_cmd,
+	"record-lu-timestamps",
+	"Record timestamps of Location Updates in the HLR database (off by default)")
+{
+	g_hlr->record_lu_timestamps = true;
+	vty_out(vty, "Timestamps of Location Updates will be stored in the HLR database.%s", VTY_NEWLINE);
+	vty_out(vty, "Please carefully consider the privacy implications of recording the activity of your users!%s", VTY_NEWLINE);
+	return CMD_SUCCESS;
+}
+
+DEFUN(cfg_no_record_lu_timestamps, cfg_no_record_lu_timestamps_cmd,
+	"no record-lu-timestamps",
+	NO_STR "Record timestamps of Location Updates in the HLR database")
+{
+	g_hlr->record_lu_timestamps = false;
+	return CMD_SUCCESS;
+}
+
 /***********************************************************************
  * USSD Entity
  ***********************************************************************/
@@ -353,6 +373,8 @@
 
 	install_element(CONFIG_NODE, &cfg_hlr_cmd);
 	install_node(&hlr_node, config_write_hlr);
+	install_element(HLR_NODE, &cfg_record_lu_timestamps_cmd);
+	install_element(HLR_NODE, &cfg_no_record_lu_timestamps_cmd);
 
 	install_element(HLR_NODE, &cfg_gsup_cmd);
 	install_node(&gsup_node, config_write_hlr_gsup);
diff --git a/tests/db/db_test.c b/tests/db/db_test.c
index c4ed6ed..e90661d 100644
--- a/tests/db/db_test.c
+++ b/tests/db/db_test.c
@@ -337,39 +337,39 @@
 
 	comment("Record LU for PS and CS (SGSN and VLR names)");
 
-	ASSERT_RC(db_subscr_lu(dbc, id0, "5952", true), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "5952", true, true), 0);
 	ASSERT_SEL(id, id0, 0);
-	ASSERT_RC(db_subscr_lu(dbc, id0, "712", false), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "712", false, true), 0);
 	ASSERT_SEL(id, id0, 0);
 
 	comment("Record LU for PS and CS (SGSN and VLR names) *again*");
 
-	ASSERT_RC(db_subscr_lu(dbc, id0, "111", true), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "111", true, true), 0);
 	ASSERT_SEL(id, id0, 0);
-	ASSERT_RC(db_subscr_lu(dbc, id0, "111", true), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "111", true, true), 0);
 	ASSERT_SEL(id, id0, 0);
-	ASSERT_RC(db_subscr_lu(dbc, id0, "222", false), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "222", false, true), 0);
 	ASSERT_SEL(id, id0, 0);
-	ASSERT_RC(db_subscr_lu(dbc, id0, "222", false), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "222", false, true), 0);
 	ASSERT_SEL(id, id0, 0);
 
 	comment("Unset LU info for PS and CS (SGSN and VLR names)");
-	ASSERT_RC(db_subscr_lu(dbc, id0, "", true), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "", true, true), 0);
 	ASSERT_SEL(id, id0, 0);
-	ASSERT_RC(db_subscr_lu(dbc, id0, "", false), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "", false, true), 0);
 	ASSERT_SEL(id, id0, 0);
 
-	ASSERT_RC(db_subscr_lu(dbc, id0, "111", true), 0);
-	ASSERT_RC(db_subscr_lu(dbc, id0, "222", false), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "111", true, true), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, "222", false, true), 0);
 	ASSERT_SEL(id, id0, 0);
-	ASSERT_RC(db_subscr_lu(dbc, id0, NULL, true), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, NULL, true, true), 0);
 	ASSERT_SEL(id, id0, 0);
-	ASSERT_RC(db_subscr_lu(dbc, id0, NULL, false), 0);
+	ASSERT_RC(db_subscr_lu(dbc, id0, NULL, false, true), 0);
 	ASSERT_SEL(id, id0, 0);
 
 	comment("Record LU for non-existent ID");
-	ASSERT_RC(db_subscr_lu(dbc, 99999, "5952", true), -ENOENT);
-	ASSERT_RC(db_subscr_lu(dbc, 99999, "712", false), -ENOENT);
+	ASSERT_RC(db_subscr_lu(dbc, 99999, "5952", true, true), -ENOENT);
+	ASSERT_RC(db_subscr_lu(dbc, 99999, "712", false, true), -ENOENT);
 	ASSERT_SEL(id, 99999, -ENOENT);
 
 	comment("Purge and un-purge PS and CS");
diff --git a/tests/db/db_test.err b/tests/db/db_test.err
index 1d34045..d556102 100644
--- a/tests/db/db_test.err
+++ b/tests/db/db_test.err
@@ -373,7 +373,7 @@
 
 --- Record LU for PS and CS (SGSN and VLR names)
 
-db_subscr_lu(dbc, id0, "5952", true) --> 0
+db_subscr_lu(dbc, id0, "5952", true, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -383,7 +383,7 @@
   .sgsn_number = '5952',
 }
 
-db_subscr_lu(dbc, id0, "712", false) --> 0
+db_subscr_lu(dbc, id0, "712", false, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -397,7 +397,7 @@
 
 --- Record LU for PS and CS (SGSN and VLR names) *again*
 
-db_subscr_lu(dbc, id0, "111", true) --> 0
+db_subscr_lu(dbc, id0, "111", true, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -408,7 +408,7 @@
   .sgsn_number = '111',
 }
 
-db_subscr_lu(dbc, id0, "111", true) --> 0
+db_subscr_lu(dbc, id0, "111", true, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -419,7 +419,7 @@
   .sgsn_number = '111',
 }
 
-db_subscr_lu(dbc, id0, "222", false) --> 0
+db_subscr_lu(dbc, id0, "222", false, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -430,7 +430,7 @@
   .sgsn_number = '111',
 }
 
-db_subscr_lu(dbc, id0, "222", false) --> 0
+db_subscr_lu(dbc, id0, "222", false, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -444,7 +444,7 @@
 
 --- Unset LU info for PS and CS (SGSN and VLR names)
 
-db_subscr_lu(dbc, id0, "", true) --> 0
+db_subscr_lu(dbc, id0, "", true, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -454,7 +454,7 @@
   .vlr_number = '222',
 }
 
-db_subscr_lu(dbc, id0, "", false) --> 0
+db_subscr_lu(dbc, id0, "", false, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -463,9 +463,9 @@
   .msisdn = '543210123456789',
 }
 
-db_subscr_lu(dbc, id0, "111", true) --> 0
+db_subscr_lu(dbc, id0, "111", true, true) --> 0
 
-db_subscr_lu(dbc, id0, "222", false) --> 0
+db_subscr_lu(dbc, id0, "222", false, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -476,7 +476,7 @@
   .sgsn_number = '111',
 }
 
-db_subscr_lu(dbc, id0, NULL, true) --> 0
+db_subscr_lu(dbc, id0, NULL, true, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -486,7 +486,7 @@
   .vlr_number = '222',
 }
 
-db_subscr_lu(dbc, id0, NULL, false) --> 0
+db_subscr_lu(dbc, id0, NULL, false, true) --> 0
 
 db_subscr_get_by_id(dbc, id0, &g_subscr) --> 0
 struct hlr_subscriber {
@@ -498,10 +498,10 @@
 
 --- Record LU for non-existent ID
 
-db_subscr_lu(dbc, 99999, "5952", true) --> -ENOENT
+db_subscr_lu(dbc, 99999, "5952", true, true) --> -ENOENT
 DAUC Cannot update SGSN number for subscriber ID=99999: no such subscriber
 
-db_subscr_lu(dbc, 99999, "712", false) --> -ENOENT
+db_subscr_lu(dbc, 99999, "712", false, true) --> -ENOENT
 DAUC Cannot update VLR number for subscriber ID=99999: no such subscriber
 
 db_subscr_get_by_id(dbc, 99999, &g_subscr) --> -ENOENT

-- 
To view, visit https://gerrit.osmocom.org/12228
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: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling <stsp at stsp.name>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181210/321d3095/attachment.htm>


More information about the gerrit-log mailing list