The initial idea of the SMS expiry threshold was to avoid storing
SMS messages with too long validity time (e.g. 63 weeks).
Unfortunately, neither this feature was properly documented, nor
the expiry threshold is configurable. Moreover, it has been
implemented in a wrong way, so instead of deleting the oldest
expired message, it would delete the youngest one or nothing:
SELECT ... FROM SMS ORDER BY created LIMIT 1;
while it should be sorted by 'valid_until' in ascending order:
SELECT .. FROM SMS ORDER BY valid_until LIMIT 1;
Thus, if the oldest message is expired, it gets deleted. If the
oldest message is not expired yet, there is nothing to delete.
Change-Id: I0ce6b1ab50986dc69a2be4ea62b6a24c7f3f8f0a
In general, neither TP-User-Data nor decoded text should be
truncated. If the SMSC's database for some reason does contain
such weird messages, let's at least let the user know about it.
Change-Id: I75e852ebe44ba4784572cbffa029e13f0d3c430c
The following functions:
- sms_from_result(),
- sms_from_result_v3(),
- sms_from_result_v4(),
do retrieve the TP-UD, TP-UDL and text in the same way.
A consequence of such duplication is [1], which fixed potential
NULL-pointer dereference for sms_from_result(), but not for two
other functions: sms_from_result_v3() and sms_from_result_v4().
[1] I545967464c406348b8505d1729213cfb4afcd3e2
Change-Id: If67dfb9f7d2a55fa3d45dc4689a2acff9909faf6
The value of 'sms->user_data_len' is fetched from the database:
sms->user_data_len = dbi_result_get_field_length(result, "user_data");
and this is where the problem is. As per the libdbi's documentation
(see 3.5.3), dbi_result_get_field_length() returns the length in
bytes of the value stored in the specified field:
unsigned int dbi_result_get_field_length(dbi_result Result,
const char *fieldname)
so 'unsigned int' is assigned to 'uint8_t', what could lead to an
integer overflow if the value is grather than 0xff. As a result,
if the database for some reason does contain such odd TP-UD,
the truncation of 'user_data' would be done incorrectly.
Let's avoid such direct assignment, and use a separate variable.
Also, let's warn user if TP-UDL value is grether than 140, as
per 3GPP TS 03.40.
Change-Id: Ibbd588545e1a4817504c806a3d02cf59d5938ee2
Related: OS#3684
Catched by ASan on db_sms_test unit test:
DDB NOTICE test_db_sms_get('Empty TP-UD'): osmo-msc/src/libmsc/db.c:796:2: runtime error: null pointer passed as argument 2, which is declared to never be null
That happens on empty PDU because dbi_result_get_binary returns NULL,
and sms->user_data_len is 0, so it's harmless but we can avoid calling
mempcy and make ASan happy.
Change-Id: I545967464c406348b8505d1729213cfb4afcd3e2
Thanks to db_sms_test, it was discovered that storing an SMS with
empty TP-User-Data (TP-UDL=1) causes buffer overruns in libdbi
and it's SQLite3 driver (libdbdsqlite3):
DDB NOTICE test_db_sms_store('Empty TP-UD'): ==7791== Invalid write of size 2
==7791== at 0x857DC60: dbd_quote_binary (in /usr/lib/x86_64-linux-gnu/dbd/libdbdsqlite3.so)
==7791== by 0x5B2B321: dbi_conn_quote_binary_copy (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
==7791== by 0x4073B1: db_sms_store (db.c:701)
==7791== by 0x405BB5: test_db_sms_store (db_sms_test.c:310)
==7791== by 0x405BB5: main (db_sms_test.c:546)
==7791== Address 0x7ed1cf0 is 0 bytes after a block of size 0 alloc'd
==7791== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7791== by 0x857DC4B: dbd_quote_binary (in /usr/lib/x86_64-linux-gnu/dbd/libdbdsqlite3.so)
==7791== by 0x5B2B321: dbi_conn_quote_binary_copy (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
==7791== by 0x4073B1: db_sms_store (db.c:701)
==7791== by 0x405BB5: test_db_sms_store (db_sms_test.c:310)
==7791== by 0x405BB5: main (db_sms_test.c:546)
...
DDB NOTICE test_db_sms_get('Empty TP-UD'): ==8051== Invalid read of size 1
==8051== at 0x5B30510: _dbd_decode_binary (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
==8051== by 0x857D957: dbd_fetch_row (in /usr/lib/x86_64-linux-gnu/dbd/libdbdsqlite3.so)
==8051== by 0x5B2C86E: dbi_result_seek_row (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
==8051== by 0x40828F: next_row (db.c:188)
==8051== by 0x40828F: db_sms_get (db.c:805)
==8051== by 0x406C29: test_db_sms_get (db_sms_test.c:390)
==8051== by 0x405C14: main (db_sms_test.c:547)
==8051== Address 0x8f74641 is 0 bytes after a block of size 1 alloc'd
==8051== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8051== by 0x5DBEB49: strdup (strdup.c:42)
==8051== by 0x857D93C: dbd_fetch_row (in /usr/lib/x86_64-linux-gnu/dbd/libdbdsqlite3.so)
==8051== by 0x5B2C86E: dbi_result_seek_row (in /usr/lib/x86_64-linux-gnu/libdbi.so.1.1.0)
==8051== by 0x40828F: next_row (db.c:188)
==8051== by 0x40828F: db_sms_get (db.c:805)
==8051== by 0x406C29: test_db_sms_get (db_sms_test.c:390)
==8051== by 0x405C14: main (db_sms_test.c:547)
==8051==
success, as expected
DDB NOTICE verify_sms('Empty TP-UD'): user_data_len mismatch: E0 vs A3
Apparently, dbi_conn_quote_binary_copy() doesn't properly handle
zero-length input. Let's guard against this.
Observed with:
- libdbi-dev 0.9.0-1
- libdbd-sqlite3:amd64 0.9.0-2ubuntu2
Change-Id: If0b2bb557118c5f0e520a2e6c2816336f6028661
Since OsmoMSC has built-in SMSC, it needs to store the messages
somewhere. Currently we use libdbi and SQLite3 back-end for that.
For a long time, the db_sms_* API remained uncovered by unit tests.
This change aims to fix that, and does cover the following calls:
- db_sms_store(),
- db_sms_get(),
- db_sms_get_next_unsent(),
- db_sms_mark_delivered(),
- db_sms_delete_sent_message_by_id(),
- db_sms_delete_by_msisdn(),
- db_sms_delete_oldest_expired_message().
Due to performance reasons, the test database is initialized in
RAM using the magic filename ':memory:'. This is a feature of
SQLite3 (and not libdbi), see:
https://www.sqlite.org/inmemorydb.html
Of course, this unit test helped to discover some problems:
1) Storing an SMS with empty TP-User-Data (TP-UDL=0) causes
buffer overruns in both db_sms_store() and db_sms_get().
2) TP-User-Data-Length is always being interpreted in octets,
regardless of DCS (Data Coding Scheme). This results in
storing garbage in the database if the default 7-bit
encoding is used. Fortunately, the 'user_data' buffer
in structure 'gsm_sms' is large emough, so we don't
experience buffer overruns.
3) db_sms_delete_oldest_expired_message() doesn't work
as expected. Instead of removing the *oldest* expired
message, it tries to remove the *newest* one.
The current test expectations do reflect these problems.
All of them will be fixed in the follow-up patches.
Change-Id: Id94ad35b6f78f839137db2e17010fbf9b40111a3
DB counters has been used to save osmo_counters & osmo_rate_ctr to a local
sqlite databases every 60 seconds.
This is quite slow e.g. 1000 subscriber might slow the msc down.
Change-Id: Id64f1839a55b5326f74ec04b7a5dbed9d269b89c
Using following semantic patch:
@@ expression A, B, C; @@
- osmo_strlcpy(A, B, sizeof(A));
+ OSMO_STRLCPY_ARRAY(A, B);
Which was applied using following command:
spatch --dir src -I src --sp-file strlcpy.spatch --in-place --recursive-includes
All the calls to osmo_strlcpy() which use destination buffer obtained
via sizeof() were replaced with the corresponding wrapper macro.
Change-Id: I67b482dedfa11237ac21894fc5930039e12434ab
Related: OS#2864
Delete expired SMS whenever we are done processing an SMS-related signal.
In order to minimize additional latency only one SMS is removed at a time.
Change-Id: I56cbe716e52b679c4b94f6cbb4a171306975be2e
Related: OS#2354
We already delete SMS which have been sent successfully. However, there
are plans to accept SMS for any subscriber in order to fix the problem
described in https://osmocom.org/issues/2354 ("SMSC: Store&Forward not
working for subscribed but unregistered MS").
This means we may end up storing SMS which never get sent, e.g. because
the B subscriber doesn't actually exist. This could lead to a higher
degree of SMS database growth over time, and therefore we need a way
to keep database size under control.
As a first step, introduce a DB function which removes an expired SMS,
and add a VTY command which removes all expired SMS from the DB.
Later commits will build upon this to remove expired SMS automatically.
The SMS expiry time period is currently hard-coded to 2 weeks.
We could make this configurable in the future if desired.
Change-Id: Icd6093b7b5d8db84b19a0aa47c68182566113ee2
Related: OS#2354
Quote the argument to sqlite's datetime(). Otherwise, the timestamp
stored in the database reads back as a negative value for some reason.
Before:
1032 validity_timestamp = dbi_result_get_datetime(result, "valid_until");
(gdb) p validity_timestamp
$2 = -1516814654
After:
1032 validity_timestamp = dbi_result_get_datetime(result, "valid_until");
(gdb) p validity_timestamp
$2 = 1516814654
Change-Id: Icf786f9b1efabfe7407fb6414ec0d326d8f7244a
Compute a validity timestamp based on SMS validity time.
Store the computed value in the database and recompute the validity
time when an SMS is read from the database.
Change-Id: Id27c250d3a64cd109416450e8ca155b18a8b9568
Currently the SMS database keeps accumulating entries for each SMS.
These entries are never deleted automatically. With this change, we
start deleting SMS which have successfully been sent to subscriber B.
Change-Id: I3749855fe25d9d4e37ec96b0c2bffbc692b66a78
If we cannot open a connection to the sqlite3 database, show the name of the
database we failed to access, and also hint at the fact that a likely reason
for the problem is a missing sqlite3 driver for libdbi.
Change-Id: If1c0026e882984b4358ce116ec4a7ad40340517c
This avoids potential licensing incompatibility and makes integration of
Debian packaging patches easier.
Related: OS#1694
Change-Id: I71cd631704a4dc155c6c752fee2a42cd6e2fa336
When the subscriber has no MSISDN, we might construct an invalid SQL statement
such as
... AND dest_addr= AND ...
Instead, don't even query for empty MSISDNs.
Related: OS#2706
Change-Id: I7d6169d774b2da04b3051957e364fe620feed51e
Set the time on the status report to the time the message was delivered, as
this may not be the same as the time when we are delivering the report to the
originating MS.
Change-Id: I9056429d40bf02731f004b7833f1de45a0d1add8
The change-id I7276d356d805a83ebeec72b02c8563b7135ea0b6 added msg_ref to
the databse but forgot to remove the comment stating it's not being
stored.
Change-Id: I204f098c8f2a480405446113e2181b2c53700cf3
SMPP DELIVER_SM messages with esm_class = Delivery Receipt need to send
this message reference (that the mobile phone allocates) to the ESME.
Thus, the ESME propagates it via SUBMIT_SM with esm_class = Delivery
Acknoledgment so that the SMSC sends the GSM 03.40 status-report to the
origin including this. Given this field is useful for status-reports, we
need to store it in the HLR database.
Moreover, we need a new field that specifies if the entry represents a
SMS status-report, to do the right handling from the gsm411_send_sms() -
such new handling comes in a follow up patch entitled "libmsc: handle
delivery ack via SMPP SUBMIT SM / send GSM 03.40 status report".
This patch includes the migration routines to the new database schema
revision 5, it's quite a bit of dbi boilerplate code - copied-pasted and
adapted.
Change-Id: I7276d356d805a83ebeec72b02c8563b7135ea0b6
Original libvlr code is by Harald Welte <laforge@gnumonks.org>,
polished and tweaked by Neels Hofmeyr <nhofmeyr@sysmocom.de>.
This is a long series of trial-and-error development collapsed in one patch.
This may be split in smaller commits if reviewers prefer that. If we can keep
it as one, we have saved ourselves the additional separation work.
SMS:
The SQL based lookup of SMS for attached subscribers no longer works since the
SQL database no longer has the subscriber data. Replace with a round-robin on
the SMS recipient MSISDNs paired with a VLR subscriber RAM lookup whether the
subscriber is currently attached.
If there are many SMS for not-attached subscribers in the SMS database, this
will become inefficient: a DB hit returns a pending SMS, the RAM lookup will
reveal that the subscriber is not attached, after which the DB is hit for the
next SMS. It would become more efficient e.g. by having an MSISDN based hash
list for the VLR subscribers and by marking non-attached SMS recipients in the
SMS database so that they can be excluded with the SQL query already.
There is a sanity limit to do at most 100 db hits per attempt to find a pending
SMS. So if there are more than 100 stored SMS waiting for their recipients to
actually attach to the MSC, it may take more than one SMS queue trigger to
deliver SMS for subscribers that are actually attached.
This is not very beautiful, but is merely intended to carry us over to a time
when we have a proper separate SMSC entity.
Introduce gsm_subscriber_connection ref-counting in libmsc.
Remove/Disable VTY and CTRL commands to create subscribers, which is now a task
of the OsmoHLR. Adjust the python tests accordingly.
Remove VTY cmd subscriber-keep-in-ram.
Use OSMO_GSUP_PORT = 4222 instead of 2222. See
I4222e21686c823985be8ff1f16b1182be8ad6175.
So far use the LAC from conn->bts, will be replaced by conn->lac in
Id3705236350d5f69e447046b0a764bbabc3d493c.
Related: OS#1592 OS#1974
Change-Id: I639544a6cdda77a3aafc4e3446a55393f60e4050
This is the first step in creating this repository from the legacy openbsc.git.
Like all other Osmocom repositories, keep the autoconf and automake files in
the repository root. openbsc.git has been the sole exception, which ends now.
Change-Id: I9c6f2a448d9cb1cc088cf1cf6918b69d7e69b4e7