Commit Graph

288 Commits

Author SHA1 Message Date
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 99a14c8ca1 tests/Makefile: use test db var instead of repeating the path
Change-Id: I9859b522b5ffa7f2c9ed33ab849199d4b4e6696c
2017-10-28 16:49:32 +00:00
Neels Hofmeyr c6a6d26f50 jenkins: use osmo-clean-workspace.sh before and after build
See osmo-ci change I2409b2928b4d7ebbd6c005097d4ad7337307dd93 for rationale.

Depends: I2409b2928b4d7ebbd6c005097d4ad7337307dd93
Change-Id: I9d35913f9cd60ff121d29f357919a0b0d62d6835
2017-10-28 04:37:41 +02:00
Neels Hofmeyr 3f697cdc71 test_subscriber.ctrl: test against octal/hex interpretation of id
Add a large enough subscriber id and add a test that ensures a leading zero is
not interpreted as octal, and that a leading 0x is invalid and not interpreted
as hexadecimal.

Change-Id: Ib468b7cb595cf52331ebb41e6de0e8f57f69e173
2017-10-27 02:37:20 +02:00
Neels Hofmeyr 446eb0f1bc ctrl: completely replace all CTRL commands
The previous commands are not conforming to how the CTRL interface is intended
to work:

  SET enable-ps <IMSI>
  SET disable-ps <IMSI>
  SET status-ps <IMSI>

'status-ps' is a write-only command even though it returns the status.
'enable-ps' / 'disable-ps' indicate the value instead of a variable name of an
entity. The entity <IMSI> takes the place of the variable value.

See also https://lists.osmocom.org/pipermail/openbsc/2017-September/011236.html

Instead, replace with

  SET subscriber.by-imsi-123456.ps-enabled {0,1}
  GET subscriber.by-imsi-123456.ps-enabled

and also provide further CTRL functions while at it:

  {SET,GET} subscriber.by-{imsi,msisdn,id}-123456.{cs,ps}-enabled {0,1}
  GET subscriber.by-{imsi,msisdn,id}-123456.{info,info-aud,info-all}

Provide CTRL tests in the form of transcripts.

Adjust tests/test_subscriber.sql to feature nonzero SQN, to see some values for
SQN in the CTRL transcript tests. (This does not affect the VTY tests, because
that creates its own subscribers, and there's no VTY command to set the SQN.)

This is the first time an application uses CTRL_NODE ids that are defined
outside of libosmocore, see 'Depends' below.

Implementation choice: the first idea was to have a '.' between the 'by-xxx'
and the value, like:

  subscriber.by-xxx.123456.function

but the difficulty with subscribers is that they are not in RAM, and I can't
just point node_data at a struct instance that is always there (like, say, a
global bts[0] struct in osmo-bsc). Instead, I want to store the selector and
later decide whether to read from the DB or whatever. With a '.' separating
things, the only way in a ctrl function to obtain both 'by-xxx' and '123456'
for picking a subscriber record would be to parse the entire variable path
string elements, including 'subscriber' and 'function', which would then also
clumsily fix at which node level we hook these commands; there could have been
separate CTRL_NODE_SUBSCR_BY_{IMSI,MSISDN,ID} parent nodes, but we cannot
introspect the current parent node dynamically within a ctrl function handler
(plus I'm not sure whether it's possible and a good idea to have the same
command under multiple parent nodes).

Rather than that, I store the 'by-foo-123' token in the node_data pointer to
have both bits of information pointed at by a single pointer; I use the
incoming command parsing to get this token pre-separated from surrounding node
names, and no need to re-allocate it, since the vector of tokens lives until
after command execution is complete. Each leaf command obtains this token from
cmd->node (aka node_data), and feeds this token to a common static function to
parse selector and value from it and to retrieve a subscriber record as needed.

(BTW, I have mentioned on the mailing list that this way might be necessary to
avoid numeric-only CTRL node names, but we don't need to, and that is not at
all related to this choice of structure.)

Depends: libosmocore I1bd62ae0d4eefde7e1517db15a2155640a1bab58
         libosmocore Ic9dba0e4a1eb5a7dc3cee2f181b9024ed4fc7005
Change-Id: I98ee6a06b3aa6a67adb868e0b63b0e04eb42eb50
2017-10-27 02:35:49 +02:00
Neels Hofmeyr 234f9cb701 cosmetic: tweak params of hlr_controlif_setup()
Cosmetically prepare for adding new CTRL commands in hlr_controlif_setup():
- drop unused 'gs' param.
- use ctrl_interface_setup_dynip2(), so far with default CTRL nodes; custom
  nodes will be added soon.

Prepares: I98ee6a06b3aa6a67adb868e0b63b0e04eb42eb50
Change-Id: I63004a7953b04988449697dbc5d55d7ed0c6d82d
2017-10-27 00:35:01 +00:00
Neels Hofmeyr 16140f70c5 db api: fix/add API docs
Change-Id: I854fafd8e56bd0b8394f8ed79d023c11c2fdbdca
2017-10-25 19:21:40 +02:00
Neels Hofmeyr 36bec87104 vty: fix output of empty IMSI
Check *subscr->imsi, not subscr->imsi, since it is a char[]; same as msisdn
below already does.

Was introduced in change I42b3b70a0439a8f2e4964d7cc31e593c1f0d7537 / commit
183e7009af.

Fixes: coverity CID 178166
Change-Id: I72e13efefbac0495b8dd1949a39fa44ebfd46b56
2017-10-23 18:47:58 +02: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 7ae8d878cf api doc: say that lu_op_tx_del_subscr_data() doesn't free
Change-Id: Ia341d8e5bfc6eb0dc59945281ce88eecfaab057e
2017-10-17 02:03:42 +02:00
Neels Hofmeyr 68f87915e4 fix mem leak in handle_cmd_ps(): free luop
Each GSUP client creates a luop, but since lu_op_tx_del_subscr_data() doesn't
free the luop, each allocated luop leaks memory.

Change-Id: If912dc992bc7f18c49d22ec0436d9679c1cd04f6
2017-10-17 02:03:01 +02:00
Neels Hofmeyr e86437cae4 luop: fix mem leak upon error in lu_op_alloc_conn()
Free allocated luop if osmo_gsup_conn_ccm_get() fails.

Change-Id: I3ebd5fb5e313be452de893248dd58b2bb73ba94a
2017-10-17 02:01:48 +02:00
Neels Hofmeyr 200f56e995 add lu_op_free(), use in luop.c
Add to luop.h, it will be used in db_hlr.c in an upcoming patch.

Change-Id: Ib44d9062edc957d2e0710b7e485604f97e4d5612
2017-10-17 02:01:08 +02:00
Neels Hofmeyr 50e4de7e49 replace ctrl_test_runner.py with transcript test_subscriber.ctrl
Use the new osmo_verify_transcript_ctrl.py from osmo-python-tests to completely
replace current ctrl_test_runner.py with a CTRL interaction transcript.

Add missing EXTRA_DIST entry of test_subscriber.sql.

Depends: osmo-python-tests Id47331009910e651372b9c9c76e12f2e8964cc2c
Change-Id: Iff93abe370b8f3ecf42082d1d0eaa1fbeca5b122
2017-10-17 01:07:32 +02:00
Neels Hofmeyr 86d09ec266 add test_nodes.vty
Automatically picked up by the vty-test target, by file name extension.

Change-Id: I8dba373cee1be954504f79c3305b0111071757e7
2017-10-17 00:59:00 +02:00
Neels Hofmeyr 183e7009af implement subscriber vty interface, tests
Implement VTY commands for subscriber manipulation:
- create / delete subscriber
- modify MSISDN
- add/edit/remove 2G and 3G authentication data
- show by IMSI, MSISDN or DB ID.
(enable/disable CS/PS and purge/unpurge to follow later.)

Implement VTY unit tests for the new commands using new
osmo_verify_transcript_vty.py from osmo-python-tests.

Depends: libosmocore I1e94f5b0717b947d2a7a7d36bacdf04a75cb3522
         osmo-python-tests Id47331009910e651372b9c9c76e12f2e8964cc2c
Change-Id: I42b3b70a0439a8f2e4964d7cc31e593c1f0d7537
2017-10-17 00:59:00 +02: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 2e86ab3a87 debian: 'make check' needs sqlite3, add to Build-Depends
At some point we should rather offer DB bootstrap as a DB API function instead
of an external .sql file, which would remove the dep on the sqlite3 binary.
For now, we need the binary to build debian packages for the 'make check' step.

Change-Id: I71938dff688675dcf1dbfbce2feb8b72b1de0910
2017-10-13 00:52:48 +02:00
Neels Hofmeyr c5122f2829 code undup: use db_bind_text() in db_get_auth_data()
To make the db_bind_text() error reporting mention "imsi", change the
DB_STMT_AUC_BY_IMSI to use a named parameter.

Change-Id: I49bd5eb78170cf4cdf8abb386c766d20d9f1cf73
2017-10-11 22:32:19 +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 76328e57d1 code undup: use db_remove_reset() in db_auc.c
Change-Id: I32d728e2b8a9771421c097647aa0e060e29a601f
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 32633e2b89 db: use int64_t as subscriber id
The SQLite db does not support uint64_t, and we are always binding the uint64_t
id actually as signed int64_t. Hence be consistent and actually handle it as
int64_t in the code as well.

This means that if we ever see a negative subscriber ID in the SQL database
(however unlikely), we will also see it negative in our log output.

The SQN handled in osmo_auth* is actually of unsigned type, and, unless we
store the SQN as 64bit hex string, we are forced to feed this unsigned value as
signed int64_t to the SQLite API. The upcoming db regression test for SQN in
change-id I0d870d405e2e0a830360d9ad19f0a3f9e09d8cf2 verifies that the SQN
uint64_t translates to signed int64_t and back as expected.

Change-Id: I83a47289a48ac37da0f712845d422e897a5e8171
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
Neels Hofmeyr f7c3e6e3a2 add db_subscr_create(), db_subscr_delete(), db_subscr_update_msisdn_by_imsi()
These will be needed by VTY commands to create, delete and modify subscribers.

Auth data editing will follow in another patch.

The FIXME "also remove authentication data from auc_2g and auc_3g" will get
fixed in change-id Icb11b5e059fb920447a9aa414db1819a0c020529.

Change-Id: I725273d36234331093e7fff7d5f12f6be6ab2623
2017-10-11 22:28:09 +02:00
Neels Hofmeyr 28da26ec19 add db_bind_int() and db_bind_int64()
Will be used in upcoming patches, e.g. change-IDs
- I6e70e15228f5bb10bee6758ae5dc9687d65839bd
- I83a47289a48ac37da0f712845d422e897a5e8171

Change-Id: I705a15eef242c98feb6e95a883916f6cf8173d70
2017-10-11 22:28:09 +02:00
Neels Hofmeyr cd83b8a44c cosmetic: don't log about missing SQLite log cb
SQLite3 seems to be commonly compiled without log callback support. It is then
misleading to see a seeming error message about this on each osmo-hlr startup.

Avoid the impression that we would miss out on important logging: query
sqlit3_compileoption_get() whether SQLITE_CONFIG_SQLLOG is enabled. Try to
register the callback only if present, if not, say so on DEBUG log.

See https://sqlite.org/compile.html "SQLITE_ENABLE_SQLLOG"

Change-Id: I78d75dc351eb587b0a022f82f147e9a31c0324c5
2017-10-11 22:28:08 +02:00
Neels Hofmeyr d3cd102505 gitignore: tests/package.m4
Change-Id: Ida4a61d4786d7db63dc59a641f44afb2ec2edd97
2017-10-11 20:25:29 +00:00
Neels Hofmeyr d4bb51ba1f ctrl_test_runner.py: use proper constant as test db path
Change-Id: I9533a9ff8c0f8d24c678583a9197143a187908f3
2017-10-11 20:25:29 +00:00
Neels Hofmeyr 1e31d18822 cosmetic: db_hlr: SL3_TXT: clarify indenting
Before, it looked like the nul term was within the if () body (despite no body
being present).

While at it, also remove one of the two tabs of indenting and put the opening
'do {' on its own line.

Change-Id: I8d03433b6fba90f4e46814bc54636bc3a444cc46
2017-10-11 20:25:29 +00:00
Neels Hofmeyr e9c0c5b272 cosmetic: log: "SQLite" with capital L
Change-Id: I43a6ea646f14cfea3a7cd4eb88237ada6d47f5f1
2017-10-11 20:25:29 +00:00
Alexander Couzens 3522819d8b debian/rules: show testsuite.log when tests are failing
Change-Id: If0b10c02f87ed81878593198e21da1fc9f8d4bbc
2017-10-11 07:10:57 +02:00
Neels Hofmeyr 40aa61ccf0 cosmetic: log IMSI='<imsi>', log "no such subscriber"
In LOGHLR and LOGAUC, log IMSI='<imsi>' instead of just <imsi>:
In the log, it is not always obvious to the reader that the printed number
refers to an IMSI (vs. an MSISDN or in the future an IMEI).

In db_get_auth_data(), log "No such subscriber" instead of just "Unknown", to
clarify what exactly is meant.

Change-Id: I2ec8ab5e67d4e95083f6e39232fc91ebaa080cb8
2017-10-10 02:39:09 +02:00
Neels Hofmeyr 0cac0a067e cosmetic: multi-line DB_STMT_AUC_BY_IMSI
In multiple lines, the statement becomes more readable.

I'd like to get this change out of the way before upcoming SQL statement edits
and additions.

Change-Id: Icf09f4bbb298a516aa52c81e3ca67d9d91d8c7c2
2017-10-10 02:38:56 +02:00
Neels Hofmeyr f31445915e cosmetic: refactor db_bind_imsi() as db_bind_text()
There are more uses for a generalized db_bind_text(), and in an upcoming patch
there will be similar functions like db_bind_int().

Also, add argument param_name, optionally indicating a named SQL parameter to
bind to, which will be used in subsequent patches. So far, all callers pass
NULL to yield previous db_bind_imsi() behavior of binding to the first param.

Change-Id: I87bc46a23a724677e8319d6a4b032976b7ba9394
2017-10-10 02:38:46 +02:00
Neels Hofmeyr 518335e688 cosmetic: rename db_subscr_get() to db_subscr_get_by_imsi()
There will be more additions, _by_msisdn() and _by_id(), to serve the upcoming
VTY commands, to allow flexibly selecting subscribers as in the old OsmoNITB.

Change-Id: I32fa676ccc5c10eba834c4390c8a42476b9c1961
2017-10-10 02:38:37 +02:00
Neels Hofmeyr 4bde949b34 cosmetic: prepend DB_STMT_ to enum stmt_idx entries
There are upcoming additions, and some seem too general without a proper common
prefix in the identifiers, like 'CREATE'.

Change-Id: I51b677db31a1ebbbc45dc7925074de7493fbde1f
2017-10-10 02:38:24 +02:00
Pau Espin 32c38f09e5 debian: remove unneeded dependency libdbd-sqlite3
Take the opportunity to remove duplicated pkg-config dependency.

Change-Id: I5bfe9c71740c1ced5bad0a41dfca568b9e00070c
2017-10-02 15:00:16 +00:00
Neels Hofmeyr f88c914efd add CTRL tests for enable-/disable-/status-ps
Change-Id: I014437db9c0f15d818e04810f6cb14bf475ee002
2017-09-28 18:52:57 +02:00
Neels Hofmeyr f95ce04cbd add basic CTRL interface tests
Prepare for adding tests of enable-/disable-/status-ps CTRL commands.

Change-Id: Ie195169c574716b514da7e04a3ce9727ef70a55e
2017-09-28 18:52:57 +02:00
Max 05c8b465ab Use value string check from osmo-ci
Change-Id: I56ea5be60d2a3cf8442f58e1121b13074e2e6a08
2017-08-26 06:10:32 +00:00
Max 43bf7bc5c5 Use release helper from libosmocore
Change-Id: I06b9ceff1e1ecfccc1b1a52ffe6b9d3f6dcaa34d
Related: OS#1861
2017-08-25 18:27:28 +02:00
Harald Welte 0b1b6b1f1e jenkins.sh: Proper error message if local environment isn't set up
Change-Id: I5251ba148f36014f70ce2838caff70062c1a3db1
2017-08-15 20:03:41 +02:00
Neels Hofmeyr 84201d3a4b use OSMO_GSUP_PORT == 4222 instead of hardcoded 2222
Depends: I4222e21686c823985be8ff1f16b1182be8ad6175 (libosmocore)
Change-Id: I9b372a4ac38677773bf813acba80cebcd88e2e20
2017-07-21 16:19:56 +02:00