14 Mar
2016
14 Mar
'16
8:46 p.m.
Hello all!
I just quickly looked at the gsm48_mi_to_string() method in gsm48.c.
I found that it is used in several projects like OsmocomBB and OpenBSC,
but it seems a bit strange to me that the TMSI represented in decimal...
...
uint32_t tmsi;
...
return snprintf(string, str_len, "%u", tmsi);
...
Can we move to use a hex representation instead of decimal? And why if not?
...
uint32_t tmsi;
...
return snprintf(string, str_len, "%x", tmsi);
...
С наилучшими пожеланиями,
Яницкий Вадим.
16 Mar
16 Mar
11:22 a.m.
Hi Vadim,
On Tue, Mar 15, 2016 at 01:46:31AM +0600, Вадим Яницкий wrote:
I found that it is used in several projects like
OsmocomBB and OpenBSC,
but it seems a bit strange to me that the TMSI represented in decimal...
yes, indeed this seems highly unusual. It reflects on the fact how
little we knew about the usual representation of TMSIs back when we
started to write that code.
Can we move to use a hex representation instead of
decimal? And why if not?
yes, I would merge an associated patch. Preferrably together with
patches for other locations where we print an IMSI and use the decimal
representation at this point.
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
4:12 p.m.
New subject: [PATCH] gsm48: move to hex TMSI representation
From: Vadim Yanitskiy <axilirator(a)gmail.com>
Signed-off-by: Vadim Yanitskiy <axilirator(a)gmail.com>
---
src/gsm/gsm48.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c
index d0a2286..e4a98d6 100644
--- a/src/gsm/gsm48.c
+++ b/src/gsm/gsm48.c
@@ -394,7 +394,7 @@ int gsm48_mi_to_string(char *string, const int str_len, const uint8_t
*mi,
if (mi_len == GSM48_TMSI_LEN && mi[0] == (0xf0 | GSM_MI_TYPE_TMSI)) {
memcpy(&tmsi, &mi[1], 4);
tmsi = ntohl(tmsi);
- return snprintf(string, str_len, "%u", tmsi);
+ return snprintf(string, str_len, "0x%08x", tmsi);
}
break;
case GSM_MI_TYPE_IMSI:
--
1.9.1
4:14 p.m.
New subject: [PATCH] move to hex TMSI representation
From: Vadim Yanitskiy <axilirator(a)gmail.com>
Signed-off-by: Vadim Yanitskiy <axilirator(a)gmail.com>
---
openbsc/include/openbsc/gsm_subscriber.h | 2 +-
openbsc/src/libmsc/db.c | 15 ++++++++-------
openbsc/tests/db/db_test.c | 4 ++--
openbsc/tests/gsm0408/gsm0408_test.c | 2 +-
4 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/openbsc/include/openbsc/gsm_subscriber.h
b/openbsc/include/openbsc/gsm_subscriber.h
index 7d6c776..785dc36 100644
--- a/openbsc/include/openbsc/gsm_subscriber.h
+++ b/openbsc/include/openbsc/gsm_subscriber.h
@@ -14,7 +14,7 @@
#define GSM_SUBSCRIBER_FIRST_CONTACT 0x00000001
/* gprs_sgsn.h defines additional flags including and above bit 16 (0x10000) */
-#define tmsi_from_string(str) strtoul(str, NULL, 10)
+#define tmsi_from_string(str) strtoul(str + 2, NULL, 16)
#define GSM_SUBSCRIBER_NO_EXPIRATION 0x0
diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 0935fc5..952151e 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -893,9 +893,10 @@ struct gsm_subscriber *db_get_subscriber(enum gsm_subscriber_field
field,
subscr->id = dbi_result_get_ulonglong(result, "id");
db_set_from_query(subscr, result);
- DEBUGP(DDB, "Found Subscriber: ID %llu, IMSI %s, NAME '%s', TMSI %u, EXTEN
'%s', LAC %hu, AUTH %u\n",
- subscr->id, subscr->imsi, subscr->name, subscr->tmsi,
subscr->extension,
- subscr->lac, subscr->authorized);
+ DEBUGP(DDB, "Found Subscriber: ID %llu, IMSI %s, NAME '%s', "
+ "TMSI 0x%08x, EXTEN '%s', LAC %hu, AUTH %u\n",
+ subscr->id, subscr->imsi, subscr->name, subscr->tmsi,
+ subscr->extension, subscr->lac, subscr->authorized);
dbi_result_free(result);
get_equipment_by_subscr(subscr);
@@ -935,7 +936,7 @@ int db_subscriber_update(struct gsm_subscriber *subscr)
int db_sync_subscriber(struct gsm_subscriber *subscriber)
{
dbi_result result;
- char tmsi[14];
+ char tmsi[11];
char *q_tmsi, *q_name, *q_extension;
dbi_conn_quote_string_copy(conn,
@@ -944,7 +945,7 @@ int db_sync_subscriber(struct gsm_subscriber *subscriber)
subscriber->extension, &q_extension);
if (subscriber->tmsi != GSM_RESERVED_TMSI) {
- sprintf(tmsi, "%u", subscriber->tmsi);
+ sprintf(tmsi, "0x%08x", subscriber->tmsi);
dbi_conn_quote_string_copy(conn,
tmsi,
&q_tmsi);
@@ -1194,7 +1195,7 @@ int db_subscriber_expire(void *priv, void (*callback)(void *priv,
long long unsi
int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber)
{
dbi_result result = NULL;
- char tmsi[14];
+ char tmsi[11];
char *tmsi_quoted;
for (;;) {
@@ -1205,7 +1206,7 @@ int db_subscriber_alloc_tmsi(struct gsm_subscriber *subscriber)
if (subscriber->tmsi == GSM_RESERVED_TMSI)
continue;
- sprintf(tmsi, "%u", subscriber->tmsi);
+ sprintf(tmsi, "0x%08x", subscriber->tmsi);
dbi_conn_quote_string_copy(conn, tmsi, &tmsi_quoted);
result = dbi_conn_queryf(conn,
"SELECT * FROM Subscriber "
diff --git a/openbsc/tests/db/db_test.c b/openbsc/tests/db/db_test.c
index a02d1f8..faea820 100644
--- a/openbsc/tests/db/db_test.c
+++ b/openbsc/tests/db/db_test.c
@@ -200,7 +200,7 @@ int main()
alice->lac=42;
db_sync_subscriber(alice);
/* Get by TMSI */
- snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi);
+ snprintf(scratch_str, sizeof(scratch_str), "0x%08x", alice->tmsi);
alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str);
COMPARE(alice, alice_db);
SUBSCR_PUT(alice_db);
@@ -227,7 +227,7 @@ int main()
db_subscriber_assoc_imei(alice, "1234567890");
db_subscriber_assoc_imei(alice, "6543560920");
/* Get by TMSI */
- snprintf(scratch_str, sizeof(scratch_str), "%"PRIu32, alice->tmsi);
+ snprintf(scratch_str, sizeof(scratch_str), "0x%08x", alice->tmsi);
alice_db = db_get_subscriber(GSM_SUBSCRIBER_TMSI, scratch_str);
COMPARE(alice, alice_db);
SUBSCR_PUT(alice_db);
diff --git a/openbsc/tests/gsm0408/gsm0408_test.c b/openbsc/tests/gsm0408/gsm0408_test.c
index 781ef61..8ed57ca 100644
--- a/openbsc/tests/gsm0408/gsm0408_test.c
+++ b/openbsc/tests/gsm0408/gsm0408_test.c
@@ -93,7 +93,7 @@ static void test_mi_functionality(void)
/* tmsi code */
mi_len = gsm48_generate_mid_from_tmsi(mi, tmsi);
gsm48_mi_to_string(mi_parsed, sizeof(mi_parsed), mi + 2, mi_len - 2);
- COMPARE((uint32_t)strtoul(mi_parsed, NULL, 10), ==, tmsi);
+ COMPARE((uint32_t)tmsi_from_string(mi_parsed), ==, tmsi);
/* imsi code */
mi_len = gsm48_generate_mid_from_imsi(mi, imsi_odd);
--
1.9.1
17 Mar
17 Mar
2:17 p.m.
New subject: [PATCH] move to hex TMSI representation
Hi Vadim,
I've merged both of your patches, thanks!
If you have time, please check if there are other occurrences in OpenBSC
or OsmocomBB where the TMSI is printed as integer. Thanks!
Regards,
Harald
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
2:29 p.m.
New subject: [PATCH] move to hex TMSI representation
On 17 Mar 2016, at 14:17, Harald Welte
<laforge(a)gnumonks.org> wrote:
Hi Vadim,
Hi Guys,
I've merged both of your patches, thanks!
I think it was a bit too quick. I foresee one problem. Let's assume someone is using
TMSIs and now upgrade the sourcecode. All CM Service Requests will fail because the TMSI
is not known. What is the migration/mitigation plan?
-#define tmsi_from_string(str) strtoul(str, NULL, 10)
+#define tmsi_from_string(str) strtoul(str + 2, NULL, 16)
is problematic too:
1.) E.g.
gsm48_mi_to_string(mi_string, sizeof(mi_string), idi->mi, idi->mi_len);
...
subscr = subscr_get_by_tmsi(bts->network->subscr_group,
tmsi_from_string(mi_string));
tmsi_from_string will not work for this anymore.
2.) there is no length check but that doesn't seem to be a big issue right now.
* I think the above hunk should be reverted
* a DB schema upgrade and store the TMSI as uint32_t
* Use hex presentation in VTY
What do you think?
2:43 p.m.
New subject: [PATCH] move to hex TMSI representation
Hi Holger,
On Thu, Mar 17, 2016 at 02:29:33PM +0100, Holger Freyther wrote:
I think it was a bit too quick. I foresee one problem.
Let's assume
someone is using TMSIs and now upgrade the sourcecode. All CM Service
Requests will fail because the TMSI is not known. What is the
migration/mitigation plan?
ugh. I failed to see that for some strange reason we store the TMSI as
"TEXT" in the database. That's really odd, especailly as we use %u
formatting when searching for TMSIs in the database.
* I think the above hunk should be reverted
done.
* a DB schema upgrade and store the TMSI as uint32_t
correct.
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
5:21 p.m.
New subject: [PATCH] move to hex TMSI representation
Hi Guys!
If you have time, please check if there are other
occurrences in OpenBSC
or OsmocomBB where the TMSI is printed as integer. Thanks!
No problem! :)
I think it was a bit too quick. I foresee one problem.
Let's assume
someone
is using TMSIs and now upgrade the sourcecode. All CM
Service Requests
will fail because the TMSI is not known. What is the migration/mitigation
plan?
I absolutely agree with Holger. Maybe we can add some code that will check
if database still stores TMSIs in old representation style and convert them
to
uint32_t? We can change the libmsc/db.c:db_prepare() for this purpose.
tmsi_from_string will not work for this anymore.
Yes, I forgot to change the gsm_subscriber.c ... Sorry.
there is no length check but that doesn't seem to
be a big issue right
now.
We can just write a function that will do this check instead of using
#define.
* a DB schema upgrade and store the TMSI as uint32_t
* Use hex presentation in VTY
+1
С наилучшими пожеланиями,
Яницкий Вадим.
2016-03-17 19:43 GMT+06:00 Harald Welte <laforge(a)gnumonks.org>rg>:
> Hi Holger,
>
> On Thu, Mar 17, 2016 at 02:29:33PM +0100, Holger Freyther wrote:
> I think it was a bit too quick. I foresee one problem.
Let's assume
> > someone is using TMSIs and now upgrade the sourcecode.
All CM Service
> > Requests will fail because the TMSI is not known. What is the
> > migration/mitigation plan?
>
> ugh. I failed to see that for some strange reason we store the TMSI as
> "TEXT" in the database. That's really odd, especailly as we use %u
> formatting when searching for TMSIs in the database.
>
> > * I think the above hunk should be reverted
>
> done.
>
> > * a DB schema upgrade and store the TMSI as uint32_t
>
> correct.
>
> --
> - Harald Welte <laforge(a)gnumonks.org>
> http://laforge.gnumonks.org/
>
> ============================================================================
> "Privacy in residential applications is a desirable marketing option."
> (ETSI EN 300 175-7 Ch.
> A6)
>
5:34 p.m.
New subject: [PATCH] move to hex TMSI representation
On 17 Mar 2016, at 17:21, Вадим Яницкий
<axilirator(a)gmail.com> wrote:
Hi Guys!
yes, we have a schema version and can just increase it. E.g. have a look at how we migrate
SMS.
If you have time, please check if there are other
occurrences in OpenBSC
or OsmocomBB where the TMSI is printed as integer. Thanks!
No problem! :)
I think it was a bit too quick. I foresee one
problem. Let's assume someone
is using TMSIs and now upgrade the sourcecode. All CM Service Requests
will fail because the TMSI is not known. What is the migration/mitigation plan?
I absolutely agree with Holger. Maybe we can add some code that will check
if database still stores TMSIs in old representation style and convert them to
uint32_t? We can change the libmsc/db.c:db_prepare() for this purpose. tmsi_from_string will not work for this anymore.
Yes, I forgot to change the gsm_subscriber.c ... Sorry. there is no length check but that doesn't
seem to be a big issue right now.
We can just write a function that will do this check instead of using #define. * a DB schema upgrade and store the TMSI as
uint32_t
* Use hex presentation in VTY
+1
16 Mar
16 Mar
4:26 p.m.
Hello, Harald!
There are two patches. One for libosmocore, another one for OpenBSC
(sorry for a little bit of mess). Right now I have no opportunity to test
it, but
the output of 'make check' seems ok.
С наилучшими пожеланиями,
Яницкий Вадим.
2016-03-16 16:22 GMT+06:00 Harald Welte <laforge(a)gnumonks.org>rg>:
Hi Vadim,
On Tue, Mar 15, 2016 at 01:46:31AM +0600, Вадим Яницкий wrote:
I found that it is used in several projects like
OsmocomBB and OpenBSC,
but it seems a bit strange to me that the TMSI represented in decimal...
yes, indeed this seems highly unusual. It reflects on the fact how
little we knew about the usual representation of TMSIs back when we
started to write that code.
Can we move to use a hex representation instead
of decimal? And why if
not?
yes, I would merge an associated patch. Preferrably together with
patches for other locations where we print an IMSI and use the decimal
representation at this point.
--
- Harald Welte <laforge(a)gnumonks.org>
http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch.
A6)
2968
days inactive
2971
days old
9 comments
4 participants
participants (4)
-
axilirator＠gmail.com -
Harald Welte -
Holger Freyther -
Вадим Яницкий