Commit Graph

29 Commits

Author SHA1 Message Date
Vadim Yanitskiy fbd736ef37 src/db.c: integrate SQLite3 with talloc allocator
This change introduces an optional feature that allows to make
SQLite3 use talloc for all internal allocations. This would
facilitate finding memleaks. OsmoHLR needs to be configured
with '--enable-sqlite-talloc'.

  full talloc report on 'OsmoHLR' (total 292168 bytes in 449 blocks)
    struct osmo_gsup_server        contains    162 bytes in   3 blocks (ref 0)
      ...
    struct db_context              contains 288407 bytes in 420 blocks (ref 0)
      hlr.db                       contains      7 bytes in   1 blocks (ref 0)
    SQLite3                        contains 288192 bytes in 418 blocks (ref 0)
      db.c:95                      contains     48 bytes in   1 blocks (ref 0)
      db.c:95                      contains      2 bytes in   1 blocks (ref 0)
      ...

Unfortunately, old SQLite3 versions (such as 3.8.2) run out
of memory when trying to initialize a new database:

  DDB ERROR  db.c:88 (7) statement aborts at 3: []
  DDB ERROR  db.c:420 Unable to set Write-Ahead Logging: out of memory
  DDB ERROR  db.c:88 (7) statement aborts at 3: []
  DDB ERROR  db.c:238 Unable to prepare SQL statement
             'SELECT name FROM sqlite_master WHERE type='table' AND name=?'
  ...

I've noticed a huge difference in heap usage footprint compared to
generic malloc. At the same time, the recent versions (at least
3.24.0), work just fine.

Change-Id: Icfe67ed0f063b63e6794f9516da3003d01cf20a7
2019-07-30 17:15:17 +00:00
Vadim Yanitskiy dc30154fdf tests/db_test: close the database when test is finished
Change-Id: I96fedf9181e89e4d68815b04f494a9c2ecc0e057
2019-07-30 17:15:17 +00:00
Vadim Yanitskiy 37642177f9 build: fix mess with 'db_test_SOURCES' and 'db_test_LDADD'
Somehow both 'db_test_SOURCES' and 'db_test_LDADD' ended up in
'src/Makefile.am'. This causes automake / autoconf to complain.
Let's get rid of both useless declarations.

Furthermore, the actual 'db_test_LDADD' in 'tests/Makefile.am'
contained references to the source files from '$(top_srcdir)'.
Most likely, the original intention was to depend on the object
files in '$(top_builddir)'. Let's also fix this.

Change-Id: Ib2e436ed91d9b7551dc5b205329d468c2b0ced04
2019-07-30 17:15:17 +00:00
Oliver Smith 6401b90574 db_auc.c: verify hex key sizes read from DB
Replace commented out size check for Ki with a real check, and use it
consistently for Ki, K, OP and OPC. Add a test that sets all keys to the
wrong size and tries to read them.

Related: OS#2565
Change-Id: Ib8e8e9394fb65c6e7932ce9f8bebc321b99f7696
2019-07-25 14:52:20 +02:00
Vadim Yanitskiy c13599dc69 db_hlr.c: add db_subscr_exists_by_msisdn()
Check if a subscriber exists without generating an error log entry if
it does not. This is cheaper than db_subscr_get_by_msisdn(), as it
does not fetch the subscriber entry.

subscriber-create-on-demand will use this function to generate
a random unique MSISDN for new subscribers.

Related: OS#2542
Change-Id: Ibfbc408c966197682ba2b12d166ade4bfeb7abc2
2019-05-13 08:55:24 +02:00
Oliver Smith 6b73fd9678 db_hlr.c: add db_subscr_exists_by_imsi()
Check if a subscriber exists without generating an error log entry if
it does not. This is cheaper than db_subscr_get_by_imsi(), as it does
not fetch the subscriber entry. subscriber-create-on-demand will use
this function.

Related: OS#2542
Change-Id: I63818c0dd4fd22b41dadeeba2a07a651b5454c54
2019-05-13 08:55:24 +02:00
Oliver Smith cd2af5ead7 db_hlr.c: db_subscr_create(): add flags argument
Allow creating new subscribers without giving them access to CS or PS.
This will be used by the create-subscriber-on-demand feature.

Related: OS#2542
Change-Id: I1a6dd85387723dab5487c53b33d2d9ec6d05d006
2019-05-13 08:55:18 +02:00
Oliver Smith 3cf87fe22c tests: use -no-install libtool flag to avoid ./lt-* scripts
This ensures that the rpath of the generated binaries is set to use only
the just-compiled so-files and not any system-wide installed libraries
while avoiding the ugly shell script wrapper.

Change-Id: I927561289b17b313d52fb5c1da55e237fc1d33be
2019-03-19 13:04:49 +00:00
Oliver Smith 81db389fd4 Add IMEI column to subscriber table
Extend the database scheme, add imei to the hlr_subscriber struct and
create db_subscr_update_imei_by_imsi() and db_subscr_get_by_imei(). The
new functions are used in db_test, and in follow-up commits [1], [2].

Upgrade DB schema to version 2. SQLite can only insert new columns at
the end of the table, so this happens when upgrading the database. In
new databases, the column is placed after the IMEISV column (where it
makes more sense in my opinion). This should not have any effect, as
we never rely on the order of the columns in the tables.

Follow-up commit [1] will make use of this column to save the IMEI as
received from the MSC/VLR with the Check-IMEI Procedure. It was
decided to use Check-IMEI instead of the recent Automatic Device
Detection Procedure (which would send the IMEISV) in OS#3733, because
with Check-IMEI we don't need to rely on very recent releases of the
specification.

[1] change-id I09274ecbed64224f7ae305e09ede773931da2a57
    "Optionally store IMEI in subscriber table"
[2] change-id I1af7b573ca2a1cb22497052665012d9c1acf3b30
    "VTY: integrate IMEI"

Depends: Id2d2a3a93b033bafc74c62e15297034bf4aafe61 (libosmocore)
Related: OS#2541
Change-Id: If232c80bea35d5c6864b889ae92d477eeaa3f45d
2019-01-24 15:29:08 +00:00
Stefan Sperling 8f3a7cce80 add database schema versioning to the HLR database
Make use of pragma user_version to store our database schema version.
The present schema is now identitifed as 'version 0', which is also
the default value for databases on which we never ran the statement
'pragma user_version' before.

Only bootstrap the database if it hasn't been bootstrapped yet.
Previously, bootstrap SQL statements ran every time osmo-hlr
opened the database, and any errors were being ignored in SQL.
Instead, we now first run a query which checks whether tables
already exist, and only create them if necessary.
This change will allow future schema updates to work properly.

Prepare for future schema upgrades by adding a new command-line
option which enables upgrades. This option defaults to 'false'
in order to avoid accidental upgrades.

Change-Id: I8aeaa9a404b622657cbc7138106f38aa6ad8d01b
Related: OS#2838
2018-12-04 14:13:28 +01:00
Pau Espin 51530311a8 use osmo_init_logging2
Change-Id: If4449cce1af46be22cc370abd3a6da8e22a31fa5
2018-04-17 15:07:06 +02:00
Neels Hofmeyr ba1605afdc fix build: db_test: missing LIBOSMOABIS_CFLAGS and _LIBS
Change-Id: I2539f5dc7a512a57ad36c460a11195ccbd84d7d6
2018-01-16 13:35:46 +01:00
Neels Hofmeyr 2dee60ef44 db_test: also test db_get_auc() return values
Verify that it returns -ENOENT on non-existing IMSI and -ENOKEY for no auth
data.

Move the auc_compute_vectors() stub to the top near the db_get_auc() call, and
just return num_vec to get a successful return value when auth data is present.

Change-Id: Ic0158228afbd78b8ca21f62dffa9f868674682b9
2017-11-29 16:22:52 +00:00
Neels Hofmeyr bd1dca0859 db_get_auth_data / db_get_auc: clarify return values
Differentiate between "IMSI unknown" and "IMSI has no auth data": in case of
known IMSI lacking auth data, return -ENOKEY instead of -ENOENT.

Fix API doc comments to reflect what the functions actually return, on top of
adding the -ENOKEY detail.

Adjust db_test expectations from -ENOENT to -ENOKEY where appropriate.

Adjust VTY and CTRL command rc evaluation.

A subsequent patch will use these return values to log details on each of these
situations.

Change-Id: Icf6304d23585f2ed45e050fa27c787f2d66fd3f7
2017-11-29 16:22:29 +00:00
Neels Hofmeyr d3814b936b db_test: don't verify SQLite issued error messages, they might change
A user on openbsc@ complained that with SQLite 3.8.2, the db_test fails with

  --- expected
  +++ stderr
  -DDB (2067) abort at 18 in [INSERT INTO subscriber (imsi) VALUES ($imsi)]: UNIQUE constraint failed: subscriber.imsi
  +DDB (2067) abort at 35 in [INSERT INTO subscriber (imsi) VALUES ($imsi)]: UNIQUE constraint failed: subscriber.imsi

i.e. a trivial difference in the error message issued by SQLite.

For db_test, don't output any SQLite error messages: Add argument
enable_sqlite_logging, pass as true, except in db_test.c.
Remove the SQLite error messages from expected output.

(Note that there is a src/db_test.c program that's not of interest here, this
is about the tests/db/db_test.c)

Change-Id: I2513d71cc0072aef8d08f47d0a1959f311176229
2017-11-21 12:33:45 +01:00
Neels Hofmeyr 6eb231eccc fix default logging levels to NOTICE, not DEBUG
Tweak unit test binaries to still used DEBUG loglevels, so that their expected
outputs remain unchanged (and nicely verbose).

Adjust test_nodes.vty, now expecting the 'notice' log levels upon
'show running-config'.

Change-Id: Ic061e61c9625b49cef8bc2a2c0b936e262c22268
2017-10-28 16:49:46 +00:00
Neels Hofmeyr 7750d2cedc automatically create db tables on osmo-hlr invocation
If a database file is missing, osmo-hlr creates it, as is the default sqlite3
API behavior -- before this patch, that db file is created, but lacks useful
tables. Actually also create initial tables in it, as osmo-nitb did.

In effect, the 'vty-test' target in tests/Makefile.am no longer needs to create
a database manually. (The 'ctrl-test' still does, because it also wants to add
subscriber data on top of the bare tables.)

Note: it could be desirable to bail if the desired database file does not
exist. That is however a different semantic from this patch; this is not
changing the fact that a db file is created, this just creates a usable one.

Note: I am about to add osmo-hlr-db-tool to do database migration from
osmo-nitb. For that, it is desirable to bootstrap a usable database, which is
the core reason for this patch.

Don't plainly duplicate hlr.sql to .c, but create db_bootstrap.h as a
BUILT_SOURCE from reading in sql/hlr.sql and mangling via sed to a list of SQL
statement strings. On each db_open(), run this bootstrap sequence.

In sql/hlr.sql, these tweaks are necessary:
* Add 'IF NOT EXISTS' to 'CREATE TABLE', so that the bootstrap sequence can be
  run on an already bootstrapped db.
* Drop the final comment at the bottom, which ended up being an empty SQL
  statement and causing sqlite3 API errors, seemed to have no purpose anyway.

Note: by composing the statement strings as multiline and including the SQL
comments, sqlite3 actually retains the comments contained in table definitions
and prints them back during 'sqlite3 hlr.db .dump'.

Change-Id: If77dbbfe1af3e66aaec91cb6295b687f37678636
2017-10-28 16:49:33 +00:00
Neels Hofmeyr cd7fa4502c db_test: fix *FLAGS
The -I includes should be in CFLAGS, not CPPFLAGS.

I noticed problems with it when trying to add an -I$(builddir) in an upcoming
patch that adds a BUILT_SOURCE, If77dbbfe1af3e66aaec91cb6295b687f37678636.

Change-Id: Ie57a04b7efc7a1e16cf0e3625d8ad2f0ef0089b0
2017-10-28 16:49:33 +00:00
Neels Hofmeyr 00b1d43435 add hlr_subsrc_nam to put GSUP client notification in proper API
This code should not live in a CTRL interface function but be proper hlr_* API.

Change-Id: I4c9b8f9ad51d49517474e8b51afc3cc2e1c9299a
2017-10-17 02:28:43 +00:00
Neels Hofmeyr b6837e36a3 fix db_subscr_get_by_*(): clear output data; test in db_test.c
db_subscr_get_by_*() failed to clear the out-param struct, meaning that data
could remain in a struct even though it is not present in the database. Always
zero out the struct before writing to it.

Adjust the db_test to catch this error by writing "-invalid-data-" to each
struct before running db get functions.

Change-Id: I038bd437452c87841d709fcdd5ac30ab1356b2db
2017-10-15 05:52:39 +02:00
Neels Hofmeyr 1cbdb70b27 fix db_update_sqn(): reset stmt in all error cases
Use the common db_bind_int64() so that the stmt bindings are cleared for any
errors and to get error logging for free.

On error with sqlite3_step(), log the SQL error message, and make sure the stmt
is cleared of bindings and reset.

After sqlite3_step(), verify that exactly one row was modifed, log and return
errors otherwise.

After this patch, the DB interaction closely matches the other (refactored) DB
functions.

Change-Id: I0d870d405e2e0a830360d9ad19f0a3f9e09d8cf2
2017-10-11 22:32:19 +02:00
Neels Hofmeyr 57a8792f23 refactor db_get_auth_data return val
Adopt the error handling of the other db functions: return -ENOENT on unknown
subscriber and -EIO on SQL failure. Return 0 for no error, instead of the
number of rows modified.

Adjust the single caller: db_get_auc()
(and db_test.c).

Change-Id: I006f471962bdad95d00a3a4c41a28ebbc9740884
2017-10-11 22:32:19 +02:00
Neels Hofmeyr 1332a17a3d add db_subscr_update_aud_by_id(), complete db_subscr_delete_by_id()
Add ability to add and remove auc_2g and auc_3g table rows with
db_subscr_update_aud_by_id().

In db_subscr_delete_by_id(), make sure that when deleting a subscriber, also
all auth data associated with that user ID is removed as well. A newly created
subscriber must not obtain the same auth tokens just by getting the same id.

Depends: libosmocore Idf75946eb0a84e145adad13fc7c78bb7a267aa0a
Change-Id: Icb11b5e059fb920447a9aa414db1819a0c020529
2017-10-11 22:32:19 +02:00
Neels Hofmeyr e50121ec96 refactor db_subscr_purge
Use named parameters in the SQL statements.

Use db_bind_* functions to drop some code dup.

Adopt error handling (rc and logging) to match the other db functions: return
-ENOENT for unknown subscriber, -EIO for SQL failures.

Change-Id: Iad49d29b90a708c6cf55bfb3bcc02d9e29001a15
2017-10-11 22:32:19 +02:00
Neels Hofmeyr dd783056f7 refactor db_subscr_lu()
Use named parameters in the SQL statement.
Use db_bind_* functions to drop some code dup.
Use explicit subscriber id arg instead of subscriber struct.
Match return values and error logging to other db functions.

Change-Id: I35665e84ddbe54a6f218b24033df969ad2e669a0
2017-10-11 22:32:19 +02:00
Neels Hofmeyr e8ccd5013a refactor db_subscr_ps() to db_subscr_nam()
Allow to set nam_ps and nam_cs from this same function, by adding the is_ps
arg.

Combine both NAM_PS stmts to DB_STMT_UPD_NAM_PS_BY_IMSI, add another such stmt
for CS. Use named parameters instead of parameter indexes.

Improve error return values as well as error logging to clearly indicate
whether the operation could not find the requested IMSI, or other errors
occured.

Adjust the single caller.

This prepares for upcoming VTY and possibly CTRL commands, and the error
handling introduced here has been or will be adopted by other functions in
previous or subsequent patches.

Change-Id: I6e70e15228f5bb10bee6758ae5dc9687d65839bd
2017-10-11 22:32:19 +02:00
Neels Hofmeyr 9c2bbc840f add db_subscr_get_by_msisdn() and db_subscr_get_by_id()
Factor out the selected SQL columns as SEL_COLUMNS macro, so that each of the
new DB_STMTs will select identical columns: the old DB_STMT_SEL_BY_IMSI as well
as the new DB_STMT_SEL_BY_MSISDN and DB_STMT_SEL_BY_ID.

Add the new functions db_subscr_get_by_msisdn() and db_subscr_get_by_id() and
factor out common parts with db_subscr_get_by_imsi() to static db_sel().

Change-Id: I6d0ddd1b7e3f6b180b4b1b2663c5725d2a4a9428
2017-10-11 22:32:19 +02:00
Neels Hofmeyr d7d9697d85 less noise: simplify db_remove_reset()
db_remove_reset() needs to be called after each stmt run, whether it succeeded
or not.

In case sqlite3_clear_bindings() would fail to unbind a stmt, we would anyway
be beyond recovery. There seem to be no plausible situations where such failure
would occur, unless there have been no bindings in the first place.

In case there was an SQL stmt failure, sqlite3_reset() will re-barf the same
error message, we will always have logged it earlier already in the proper
context.

We are never evaluating the return value, nor would we know how to recover from
non-success.

The conclusions:
- db_remove_reset() does not need to log any errors.
- db_remove_reset() does not need to return success.

Change-Id: I21678463e59f607f5f5c5732963e274392f0fffd
2017-10-11 22:32:19 +02:00
Neels Hofmeyr 9850946013 add initial db_test: creating and deleting subscribers
Change-Id: I2a0d277f55162bf5ceb0fc7d50390f2994daed71
2017-10-11 22:32:19 +02:00