Encode an Insert Subscr Data with is_ps == true to trigger the encoding bug
described in OS#3231, i.e. show that it is fixed.
Move osmo_gsup_addr_send() to a separate .c file, so that it can be overridden
in the regression test to just dump the msgb instead.
I used this test to reproduce issue OS#3231, and now that it's here we might as
well keep it, and possibly expand on it in the future.
Related: OS#3231
Change-Id: Id1453351758f3e1a9ff03bd99fefaf51886e77da
In osmo_gsup_configure_wildcard_apn(), do not compose APN into a local buffer
that becomes invalid as soon as the function exits. Instead, use a caller
provided buf.
Fixes OS#3231 crash:
==20030==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7fffffffd9c0 at pc 0x7ffff6e9b6c2 bp 0x7fffffffd900 sp 0x7fffffffd0b0
READ of size 2 at 0x7fffffffd9c0 thread T0
#0 0x7ffff6e9b6c1 (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x766c1)
#1 0x7ffff6314419 in tlv_put ../../../../src/libosmocore/include/osmocom/gsm/tlv.h:107
#2 0x7ffff6314419 in msgb_tlv_put ../../../../src/libosmocore/include/osmocom/gsm/tlv.h:299
#3 0x7ffff6314419 in encode_pdp_info ../../../../src/libosmocore/src/gsm/gsup.c:419
#4 0x7ffff6314419 in osmo_gsup_encode ../../../../src/libosmocore/src/gsm/gsup.c:535
#5 0x555555580016 in _luop_tx_gsup ../../../src/osmo-hlr/src/luop.c:54
#6 0x5555555809d8 in lu_op_tx_insert_subscr_data ../../../src/osmo-hlr/src/luop.c:264
#7 0x55555558b356 in rx_upd_loc_req ../../../src/osmo-hlr/src/hlr.c:306
#8 0x55555558b356 in read_cb ../../../src/osmo-hlr/src/hlr.c:365
#9 0x555555586671 in osmo_gsup_server_read_cb ../../../src/osmo-hlr/src/gsup_server.c:105
#10 0x7ffff5b35911 in ipa_server_conn_read ../../../src/libosmo-abis/src/input/ipa.c:356
#11 0x7ffff5b35911 in ipa_server_conn_cb ../../../src/libosmo-abis/src/input/ipa.c:387
#12 0x7ffff5e5541f in osmo_fd_disp_fds ../../../src/libosmocore/src/select.c:216
#13 0x7ffff5e5541f in osmo_select_main ../../../src/libosmocore/src/select.c:256
#14 0x5555555791b6 in main ../../../src/osmo-hlr/src/hlr.c:600
#15 0x7ffff4707a86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21a86)
#16 0x555555579679 in _start (/usr/local/bin/osmo-hlr+0x25679)
Address 0x7fffffffd9c0 is located in stack of thread T0 at offset 16 in frame
#0 0x7ffff63131ff in osmo_gsup_encode ../../../../src/libosmocore/src/gsm/gsup.c:481
This frame has 1 object(s):
[32, 64) 'bcd_buf' <== Memory access at offset 16 underflows this variable
Related: OS#3231
Change-Id: I83d9ef2868bbb01e3f1ddb7920fe735aca172b15
In rx_upd_loc_req() we set the connection's supports_ps field but
forgot to also set the equivalent field in the corresponding luop.
Change-Id: Ie175a067ac1324cdd39d7f756a40fab923421793
Related: OS#2785
This function relied on implementation details of the luop code.
Port what is necessary for an independent Insert Subscriber Data
Tx operation from the luop code into this function.
A next possible step would be to try to merge both of these
into a common implementation. This will be addressed in a
follow-up change as soon as this change is merged.
The TTCN3 test TC_vty_msisdn_isd is still passing (it currently
triggers the "circuit switched domain" case because it does not
advertise itself as an SGSN in the IPA unit name).
Change-Id: I06c43ece2b48dc63d599000eb6d6d51e08963067
Related: OS#2785
Add a function which triggers subscriber update notifications to
all connected GSUP clients, and invoke it when the MSISDN of a
subscriber is changed via VTY.
This makes the TTCN3 HLR test TC_vty_msisdn_isd pass.
Note that the new function currently relies on implementation
details of the Location Update Operation (luop) code.
Because of this we currently log a slightly misleading message
when the updated Insert Subscriber Data message is sent:
"luop.c:161 LU OP state change: LU RECEIVED -> ISD SENT"
This message is misleading because, in fact, no location update
message was received from a GSUP client at that moment.
So while this change fixes the externally visible behaviour, we may
want to follow this up with some refactoring to avoid relying on
luop internals. It seems acceptable to do that in a separate step
since such a change will be more involved and harder to review.
We may want to trigger such notifications in other situations as well.
This is left for future work, too. There are no TTCN3 test cases for
other situations yet, as far as I can see.
Related: OS#2785
Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4
Use osmo_timer_setup() to set up the luop timer, instead of
settting the timer up manually.
Delete the timer before the luop is freed to prevent a potential
crash in case the timer is already armed and the function call
chain leading up to lu_op_free() does not cancel the timer.
Found while studying code to prepare work on issue OS#2785.
This change has been tested with 'make check' and TTCN3 HLR tests.
Related: OS#2785
Change-Id: I1a7596675b2d94217895f0f3d3f67b86ef123c2e
Provide a sane means of adding the -Werror compiler flag.
Currently, some of our jenkins.sh add -Werror by passing 'CFLAGS="-Werror"',
but that actually *overwrites* all the other CFLAGS we might want to have set.
Maintain these exceptions from -Werror:
a) deprecation (allow upstream to mark deprecation without breaking builds);
b) "#warning" pragmas (allow to remind ourselves of errors without breaking
builds)
As a last configure step before generating the output files, print the complete
CFLAGS and CPPFLAGS by means of AC_MSG_RESULT.
Change-Id: Id5c0740a37067cbe8986d52d63c6134769c71c47
When performing PURGE MS, OsmoHLR before this patch used toreturn
an error even in the successful case. The reasone for this is some
wrong assumptions about the return values of db_subscr_purge().
Change-Id: Ie3005e2eeb424715fd77f202e9fe18464ba211b7
It's a bit confusing to the user if he wants to set AUD=none
and it's already none. Avoid printing error messages in that case.
Change-Id: I5f32dd5d6e4939c738faf442c7e86671d18777f8
Ever since libosmocore Change-Id I1c931bff1f1723aa82bead9dfe548e4cc5b685e0
was merged, the VTY tests were broken. Let's fix that by adjusting
the expected test output.
Change-Id: I929ca7f366220b5d1238d43eddc96fa9dde916b6
After libosmocore I96a9b6b6a3a5e0b80513aa9eaa727ae8c9c7d7a1 the CTRL interface
returns stricter errors. Adjust the expectations of
test_subscriber_errors.ctrl to fix the external tests on master.
Change-Id: I9337b6b4f3fa8822c91760deb01f18a77a073d19
Don't log "deriving 2G from 3G" when we're actually calculating separately; log
it when we're actually deriving from 3G.
Add log "calculating 2G separately" in the right place.
The test output changes show that each test said "separate 2G" at the top while
logging "deriving 2G from 3G" further down, which was obviously wrong.
Change-Id: I6679d7ef8fdcae39a0c2aff7ac638e63dddb10dc
libosmocore Ie35a02555b76913bb12734a76fc40fde7ffb244d
"ctrl: on parse errors, return a detailed message to sender"
the test_subscriber_errors.ctrl test fails.
Adjust the expected error message.
Change-Id: I3aee1507721cd073f72369150d0fb3cff0fdf66f
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
For unknown IMSI and no auth data for a known IMSI, log respective messages on
NOTICE level.
For database error, log on ERROR level.
Change-Id: I3838fa78567e7e92d797d90b8b90865d9ebba90a
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
If we have a subscriber entry that lacks auth data, we currently return
GMM_CAUSE_NET_FAIL, which in the MSC log looks like the HLR is not connected or
something grave. Instead, return GMM_CAUSE_IMSI_UNKNOWN.
This changes the OsmoMSC log in this way:
Before:
DVLR <001e> VLR_Authenticate(901700000014701)[0x5555558dabb0]{VLR_SUB_AS_NEEDS_AUTH_WAIT_AI}: GSUP: rx Auth Info Error cause: 17: Network failure
After:
DVLR <001e> VLR_Authenticate(901700000014701)[0x5555558dabb0]{VLR_SUB_AS_NEEDS_AUTH_WAIT_AI}: GSUP: rx Auth Info Error cause: 2: IMSI unknown in HLR
A better cause value would be something that says "IMSI known, but we have no
auth data", but since such cause value is not defined, the plain "IMSI unknown"
seems to be the best match.
Change-Id: I90df7b255317df1e5d968e7ce3b9d2c404b98db8
Prepare for tweaking error handling in a subsequent patch: use switch() instead
of if().
Prepares-for: I90df7b255317df1e5d968e7ce3b9d2c404b98db8
Change-Id: I1f628aa9d62b778951726bebec8cf338444fc897
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
Each arg parsing should increment optind, so if there are any surplus args in
the end, that's an error.
Change-Id: I9fc0a87d11db8c35061568e3f8b5a5547931a961
Coverity wants us to evaluate sqlite3_reset, but it is of no use to do so.
Related: coverity CID#178653
Change-Id: I64ac8c148f48be60f9c0d346df0c5152bb527494
It appears that hlr_subscriber.imsi is 16 buffers in size:
15 chars for IMSI + 1 byte NUL. However, osmo_gsup_message.imsi
is 17 bytes (for whatever reason), so we cannot simply do a strpy()
as this might overflow the hlr_subscriber.imsi field!
TODO: check if weactually ever receive a too-long IMSI in GSUP and
reject that at an earlier time in the code flow.
Fixes: Coverity CID#164746
Change-Id: I9ff94e6bb0ad2ad2a7c010d3ea7dad9af0f3c048
The db API returns negative errno values, need to flip the sign before feeding
to strerror.
Fixes: coverity CID#178658
Change-Id: Iaab46f565a1112d8a7def8ea90a5cd440c0a3b41
vty_install_default() and install_default() will soon be deprecated.
Depends: I5021c64a787b63314e0f2f1cba0b8fc7bff4f09b
Change-Id: I09762f110c7bcaf85c0ef2f472eb43ac543c74e9
Move macro copy_sqlite3_text_to_buf() to db.h, so it can be used in
hlr_db_tool.c.
Add _dbd_decode_binary() from libdbi to avoid depending on the entire libdbi
just for KI BLOB decoding. Add it in a separate file, copying its own license,
the lGPL.
Offer commandline option "import-nitb-db" to read in an old osmo-nitb database
and copy subscriber IMSIs and 2G auth data to OsmoHLR db format.
Anticipate future command line options like "import-csv", so keep the code
generalized.
Change-Id: I0dfa6ec033dd93161c1adc2ce1637195fe5b7a63
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
Rename SL3_TXT to more accurate copy_sqlite3_text_to_buf(), and use
osmo_strlcpy() instead of essentially dup'ing it.
The macro will also be used by hlr_db_tool.c in upcoming patch. This patch
prepares for a move to db.h.
Change-Id: I1dadeddddcfe0109195c09c0e706201b0df009cc
By moving the comments inside the table row definitions, they are dumped back
during 'sqlite3 hlr.db .dump'. When they are between SQL statements like before
this patch, the comments are lost.
Tweak wording.
Change-Id: I280c2e2d3e9b7f1dc632722724d9e1c54d041820
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
