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

2018-12-12 Thread Stefan Sperling
Stefan Sperling has abandoned this change. ( https://gerrit.osmocom.org/12228 )

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


Abandoned

Fair enough. I've made my point.
--
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: abandon
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


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

2018-12-12 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 4: Code-Review-2

I really don't like to disable this by default, nor the warning.  A 3GPP 
(compliant, compatible, inspired) network records privacy related information 
all over the place.


--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Wed, 12 Dec 2018 09:06:23 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


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

2018-12-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has removed a vote on this change.

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


Removed Code-Review+2 by Pau Espin Pedrol 
--
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: deleteVote
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


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

2018-12-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 4: Code-Review+1


--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:14:38 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


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

2018-12-10 Thread Stefan Sperling
Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 4:

Please don't merge this before Harald has had a chance to respond. In the issue 
he suggested that he does not agree with this approach but I wrote this patch 
anyway, hoping to convince him.


--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:12:27 +
Gerrit-HasComments: No
Gerrit-HasLabels: No


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

2018-12-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 4: Code-Review+2


--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:11:48 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes


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

2018-12-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
> I'll restore the previous version then. I liked it better.
I actually prefer this version, it's still more split between different parts.



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:11:13 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


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

2018-12-10 Thread Stefan Sperling
Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 4:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  * \param[in] by_imsi  ASCII string of IMSI digits.
> Ah good point about the goto :)
I'll restore the previous version then. I liked it better.



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:08:57 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


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

2018-12-10 Thread Stefan Sperling
Hello Pau Espin Pedrol, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/12228

to look at the new patch set (#4).

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
M tests/test_nodes.vty
8 files changed, 65 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/28/12228/4
--
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: newpatchset
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 


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

2018-12-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 3: Code-Review+2

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
> Maybe the previous version was better after all? Refactoring this block of 
> code actually adds a lot  […]
Ah good point about the goto :)



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 18:06:52 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


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

2018-12-10 Thread Stefan Sperling
Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
> Yes it looks weird, but it is correct. […]
Maybe the previous version was better after all? Refactoring this block of code 
actually adds a lot of line churn no matter what we do.



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:46:40 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


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

2018-12-10 Thread Stefan Sperling
Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 3:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
> This looks weird. […]
Yes it looks weird, but it is correct.

Is it OK to run other statements before the most recently used statement has 
been reset? If so we could simplify this code.
Otherwise, moving some code into a function has the downside that the 'goto 
out:' doesn't work anymore for the code which was moved to the function.



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:42:22 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


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

2018-12-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 3: Code-Review+1

(1 comment)

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/3/src/db_hlr.c@688
PS3, Line 688:  db_remove_reset(stmt);
This looks weird. Why not always first do db_remove_reset(stmt), then remove it 
second time inside the if  after calling db_subscr_lu_record_timestamp()? 
Doesn't look more sane to you?



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Pau Espin Pedrol 
Gerrit-Reviewer: Stefan Sperling 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:34:12 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes


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

2018-12-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c
File src/hlr_vty.c:

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c@133
PS2, Line 133:  vty_out(vty, "Timestamps of Location Updates will be stored in 
the HLR database.%s", VTY_NEWLINE);
> Can you elaborate what you're referring to when you say we're storing 
> subscriber names? […]
Hm I cannot find it anymore, so maybe it got remove, but I'm pretty sure at 
some point it was possible to attach some "human information" to subscriber 
through VTY in order to identify them.



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:30:18 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


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

2018-12-10 Thread Stefan Sperling
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/12228

to look at the new patch set (#3).

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
M tests/test_nodes.vty
8 files changed, 117 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/28/12228/3
--
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: newpatchset
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 3
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Pau Espin Pedrol 


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

2018-12-10 Thread Stefan Sperling
Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/2/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/2/src/db_hlr.c@644
PS2, Line 644:  if (osmo_clock_gettime(CLOCK_REALTIME, ) != 0) {
> Since starting from this line it seems to be a separate logic (updating VLR 
> subscriber timestamp), I […]
Yes, good idea. I'll roll that into this patch.



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:07:07 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


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

2018-12-10 Thread Stefan Sperling
Stefan Sperling has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 2:

(1 comment)

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c
File src/hlr_vty.c:

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c@133
PS2, Line 133:  vty_out(vty, "Timestamps of Location Updates will be stored in 
the HLR database.%s", VTY_NEWLINE);
> IMHO these vty_out lines can be dropped, but fine otherwise. […]
Can you elaborate what you're referring to when you say we're storing 
subscriber names?
Which field of the DB do you mean? Do you mean some other database then the HLR 
db?



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Stefan Sperling 
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 17:06:20 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


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

2018-12-10 Thread Pau Espin Pedrol
Pau Espin Pedrol has posted comments on this change. ( 
https://gerrit.osmocom.org/12228 )

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


Patch Set 2:

(2 comments)

https://gerrit.osmocom.org/#/c/12228/2/src/db_hlr.c
File src/db_hlr.c:

https://gerrit.osmocom.org/#/c/12228/2/src/db_hlr.c@644
PS2, Line 644:  if (osmo_clock_gettime(CLOCK_REALTIME, ) != 0) {
Since starting from this line it seems to be a separate logic (updating VLR 
subscriber timestamp), I think it'd be clearer to have it in a separate 
function. Can be done in a follow-up patch. What do you think?


https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c
File src/hlr_vty.c:

https://gerrit.osmocom.org/#/c/12228/2/src/hlr_vty.c@133
PS2, Line 133:  vty_out(vty, "Timestamps of Location Updates will be stored in 
the HLR database.%s", VTY_NEWLINE);
IMHO these vty_out lines can be dropped, but fine otherwise. We are storing 
subscriber data anyway, like "name", etc.



--
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: comment
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-CC: Pau Espin Pedrol 
Gerrit-Comment-Date: Mon, 10 Dec 2018 16:21:12 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No


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

2018-12-10 Thread Stefan Sperling
Hello Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/12228

to look at the new patch set (#2).

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
M tests/test_nodes.vty
8 files changed, 65 insertions(+), 31 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/28/12228/2
--
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: newpatchset
Gerrit-Change-Id: I2f18b5ddc9b4b8e1174c6dea71cddf1c8d2230df
Gerrit-Change-Number: 12228
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling 
Gerrit-Reviewer: Jenkins Builder (102)


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

2018-12-10 Thread Stefan Sperling
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, ) != 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, _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