Commit Graph

741 Commits

Author SHA1 Message Date
Neels Hofmeyr 3cffc2055d cosmetic: gsm_04_08.c: drop unused struct gsm_lai
Change-Id: I3236841b1701dc902754d4a388c7806b0685a91e
2018-03-21 18:07:20 +00:00
Philipp Maier a2f633184b cosmetic: remove unused variable
Change-Id: Ic19d0401557d506d104b82f88b4dcf72055d62e1
2018-03-21 16:23:58 +01:00
Harald Welte 8a991edd36 a_iface: If L3/DTAP returns error in COMPL L3, close SCCP connection
It's pretty amazing that we print error messages anrd return error
codes, but nobody ever looks at the error code and/or closes the
connection.  Let's change that.

Change-Id: Iec693d8012a7816d1ded8206c2d979ac0546fb6e
2018-03-18 23:00:27 +01:00
Harald Welte 4de011561f a_iface_bssap: Treat inbound RESET as implicit RESET ACK
If both sides are sending RESET at the same time, they are not aware
of each other.  This leads to synchronization problems in wich
the remote side is transmitting e.g. a COMPL L3 INFO after receiving
a RESET ACK, but before even receiving or processing the RESET in
the inverse direction.  So let's treat receiving a RESET as an implicit
RESET ACK to any RESET we may have sent.

Change-Id: I0ae34fbb3735592bb7cffa5aaf421b14a8acc90e
2018-03-18 21:47:15 +00:00
Harald Welte 5060f563c6 BSSAP: Return error code if COMPL L3 with no or too short L3 payload
Change-Id: Ie3bf1351ed11a9eb261737c2da0361e632e7b6e5
2018-03-18 21:55:37 +01:00
Pau Espin 9fac985972 msc_mgcp.c: Fix several wrong ptr printf fmt
Fixes several of these type of warnings:
include/osmocom/core/fsm.h:123:38: warning: format ‘%x’ expects argument of type ‘unsigned int’, but argument 9 has type ‘char *’ [-Wformat=]
   LOGP((fi)->fsm->log_subsys, level, "%s{%s}: " fmt, \
                                      ^
src/libmsc/msc_mgcp.c:277:71: note: format string is defined here
    "CRCX/RAN: creating connection for the RAN side on MGW endpoint:0x%x...\n", mgcp_ctx->rtp_endpoint);
                                                                      ~^

Change-Id: I17b7bed8fc39612286ba66f250b6b26da01d38c0
2018-03-17 01:54:34 +01:00
Philipp Maier 04d6ddb299 msc_mgcp: to not access higher layers after release
The higher layers (gsm_04_08.c) are informed errors occur. But it
is not checked if the call was already released. If an error occurs
after the call control stack calls msc_mgcp_call_release() then
the higher layers might already have cleaned up and the code
accesses memory that is already freed (trans)

- fix use after free by guarding the call to mncc_tx_to_cc()

Change-Id: I78f1b6a9149488a4ad3f120c1e190a83c07d4b89
Related OS#2881
Related OS#2882
2018-03-16 18:51:27 +00:00
Neels Hofmeyr e9e2f5cde6 cosmetic: rename conn_fsm "bump" event to "release_when_unused"
The naming of "bump" was short and made sense to me at the time of writing, but
it is keeping pretty much everyone else at a distance, no-one intuitively gets
what it is supposed to mean.

Clarify by renaming to "release_when_unused".

Adjust test expectations.

Change-Id: I4dcc55f536f63b13a3da29fff1df5fe16751f83a
2018-03-15 14:24:57 +00:00
Neels Hofmeyr fe718bc760 cosmetic: vlr_auth: log decision to send UMTS or GSM AKA challenge
Also indicate in msc_vlr_test_gsm_authen.c that we're indeed sending no
capability to do R99 in the Classmark 1 during LU request.

Change-Id: Id79a77ca1f218d55dad21d9dd3de92445fb5d6bf
2018-03-15 14:24:57 +00:00
Philipp Maier c813355a73 cosmetic: remove disabled (debug) code gsm_04_08.c
gsm_04_08.c seems to contain some lines of old debug code that
is commented out. Presumably the commented lines are a leftover
from a debug session.

- remove those commented code lines

Change-Id: Ifb84e4b0696fef1326c3f9ebc8427581057db44f
2018-03-15 11:16:17 +00:00
Stefan Sperling be7e069810 use libosmocom to parse cell identifier in bssmap_rx_l3_compl()
Use the new gsm0808_dec_cell_id_list2() API to decode the cell
identifier in the bssap COMPLETE LAYER 3 information message.

Also, actually compare the MCC-MNC in WHOLE_GLOBAL and LAI_AND_LAC
cell identifiers to the network configuration, and drop messages
with mismatching MCC-MNC (addresses OS#2980).

Related: OS#2847
Related: OS#2980
Change-Id: I855477507e4d65fb9890da0ceea26dd2c4dfaf82
2018-03-15 10:42:07 +01:00
Philipp Maier a2353c69cc mcgp: let the MGW allocate the MGCP endpoint
osmo-msc still uses endpoints that are allocated locally by the
MGCP-Client. Since osmo-mgw now supports the more comfortable,
dynamic variant we should make use of it.

- Replace the endpoint numer allocation by the client with a
  wildcarded CRCX. Use the endpoint that is assigned by the
  MGW.

Related: OS#2710
Change-Id: Iee3e446b6689626516f01c521abe3d4603cd3e13
2018-03-14 21:08:30 +00:00
Philipp Maier 3a77652cd7 msc_mgcp: use more conceise error msg on truncation
When a truncation check (osmo_strlcpy()) fails handle_error()
is called with MGCP_ERR_NOMEM as cause code. Which finally
results in an "out of memory" message. MGCP_ERR_NOMEM is only
used for failed truncation checks, so it makes sense to choose
a message that describes the cause of the problem better.

- rename MGCP_ERR_NOMEM to MGCP_ERR_TOOLONG
- replace error message string

Change-Id: Ifedb85c1933a30b2aa491b495119919f45782e2c
2018-03-14 21:04:50 +00:00
Neels Hofmeyr 5c56bc5761 trans_free: safeguard against a still running CC timer on free
Make sure to deactivate trans.cc.timer when freeing a CC transaction.
Log an error if should be necessary.

This prevents a segfault when we receive a BSSMAP Clear Request from BSC during
an ongoing CC operation. The BSSMAP Clear Request currently triggers immediate
freeing of the conn, while we should still do a graceful release first. While
this patch does not fix the underlying error, it does prevent the MSC from
crashing due to a stale timer, whatever the cause might be.

Related: OS#3062
Change-Id: I86b666f23402a6d94af2d903e514770d1fd5157f
2018-03-14 14:17:53 +01:00
Philipp Maier addf63b523 mgcp: be sure that pending mgcp transactions are canceled before free
When the FSM reaches ST_HALT it frees itsself and all context
information but it is not ensured that there are still pending
MGW transactions that might hit late and eventually cause a use after
free situation.

- if an MGW transaction is still pending, cancel it.

Change-Id: I8ff55e48a95cc4c556a97ad2593bad1cc1aa69bd
2018-03-14 13:55:04 +01:00
Philipp Maier 4eef20bdbc msc_mgcp: fix mgw timeout handling
When the MGW does not respond to an MGCP message then the mgcp FSM
terminates, but the CC handler (gsm_04_08.c) is not informed. This
lets the CC handler think that the MGCP connection would be successful,
so it also does not take any action to release the non functional
connection.

- make sure the CC handler is always informed on any kind of
  error, especially on MGW timeouts

Change-Id: I3fcd0d71fad53274e82f6d87c80042d06283bc5d
Related OS#2881
Related OS#2882
2018-03-14 10:56:06 +01:00
Stefan Sperling 3a74128ccc restore sending of optional MM info messages
Since commit 2483f1b050 the function
gsm48_tx_mm_info() was not called anymore. No MM info messages were
transmitted to phones even if MM info messages were enabled via VTY.

With this commit, we call gsm48_tx_mm_info() after successfully
processing an IMSI ATTACH location update.

Change-Id: Ice5963d84253eb8c803cd2dfa8b25a4db5382827
Related: OS#2850
2018-03-13 21:13:27 +01:00
Stefan Sperling 621c729d8d Track libosmocore API change in osmo-msc.
struct gsm0808_cell_id_list in libosmocore is deprecated by
https://gerrit.osmocom.org/#/c/6509/

This updates the only API user I am aware of.

Change-Id: I67377270cf3b081ac5dc9cd7b4dc28f74143753a
Depends: Ib7e754f538df0c83298a3c958b4e15a32fcb8abb
2018-03-13 15:50:18 +01:00
Neels Hofmeyr fa10eda6d7 vlr_ciph_result: fix use after free of imeisv
Define the struct vlr_ciph_result member .imeisv not as a char* but a char[] of
appropriate length, to avoid the need to point to external memory.

Thus fix a use-after-free in msc_cipher_mode_compl(), which defined the
imeisv[] buffer in a sub-scope within that function, so that the .imeisv
pointer was already invalid when fed to vlr_subscr_rx_ciph_res().

Did you notice that the commit summary rhymes?

Closes: OS#3053
Change-Id: I90cfb952a7dec6d104200872164ebadb25d0260d
2018-03-13 01:29:04 +01:00
Neels Hofmeyr a9099bc99a cosmetic: vlr_auth_fsm: log RAN and size along with SRES/RES
Change-Id: Ib0f9f573ffac2302fbd3ee28f48ccd8fce5fe286
2018-03-10 22:23:13 +01:00
Neels Hofmeyr 11d2ce3e34 cosmetic: vlr_auth_fsm: clarify decision on UMTS AKA or GSM AKA
The code deciding on whether UMTS AKA is used was cascaded and convoluted. By
flattening the decisions, they become easier to read and possibly catch more
weird corner cases / log information more clearly.

- First decide what AKA the RES length reflects.
- Then decide whether all prerequisites for UMTS AKA are satisfied.
- Finally, on UTRAN, turn down the auth if we don't have UMTS AKA, and neatly
  log all of the potential causes.

One corner case that should never occur is that the UMTS AKA RES length is
actually the same length as the GSM AKA SRES. If this nevertheless occurs, log
this as an error, though not turning down authentication because of it. (The
effect is that we would favor UMTS AKA when it has a res_len == sizeof(sres)
and would not succeed to GSM AKA. At least the log will tell us why, now.)

Adjust an expected test output, trivial logging difference.

Change-Id: I43f7f301ea85e518bac91f707391a53182e54fab
2018-03-10 22:23:13 +01:00
Neels Hofmeyr d97821f8e5 cosmetic: gsm48_rx_mm_auth_resp(): log 'UMTS AUTH', not 'R99 AUTH'
Change-Id: Iba43c685cbe238d96175267e9cc954b2f2f3e7fc
2018-03-10 22:22:56 +01:00
Neels Hofmeyr 25f69d5615 gsm48_rx_mm_auth_resp(): pass is_r99 from classmark, not response size
Do not interpret the SRES/RES length returned in the auth response as the R99
capability bit, instead determine it from the actual Classmark information
associated with the conn.

This fixes the is_r99 flag passed in to vlr_subscr_rx_auth_resp(), which ends
up in the struct vlr_auth_resp_par dispatched to the auth_fi and influences the
authentication acceptance.

Though the effect of a wrongly-set-to-false R99 flag is not harmful in this
code path, let's not get this confused.

Change-Id: Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
2018-03-10 22:14:01 +01:00
Neels Hofmeyr 8e0af0ba69 vlr auth: gracefully reject malformed auth response
Instead of just closing down the conn hard, actually feed invalid auth response
data to vlr_subscr_rc_auth_resp() in order to trigger all the actions we want
to see with a failed authentication:
- a GSUP signal that the auth failed,
- a LU reject.
Verify this in new test_wrong_sres_length() in msc_vlr_test_gsm_authen.c.

Note that in gsm48_rx_mm_auth_resp(), the is_r99 flag is falsely derived from
the RES length, which upcoming commit Ib7f7d89a8b9455d2c022d53d74328fa7488577f4
will fix.

Change-Id: I4179a290069ac61d0662de4ec7ca3edb76988899
2018-03-10 22:12:13 +01:00
Neels Hofmeyr 7795a19ced vlr: fix GSM AKA in a UMTS AKA capable environment
Switch by vsub->sec_ctx to use the proper Kc for ciphering.

Even on an R99 capable MS with a UMTS AKA capable USIM, the MS may still choose
to only perform GSM AKA, as long as the bearer is GERAN. The VLR already stores
whether the MS replied with a GSM AKA SRES or a UMTS AKA RES in vsub->sec_ctx.
So far, though, we were always using the UMTS AKA Kc just because the USIM and
core net are capable of it, ignoring the choice the MS might have made in the
Authentication Response.

In msc_vlr_test_gsm_ciph, fix the test expectations to the correct GSM AKA Kc
keys, showing that all of LU, CM Service Request and Paging Response now
support MS choosing GSM AKA in a UMTS capable environment.

Related: OS#2793
Change-Id: I42ce51ae979f42d173a45ae69273071c426bf97c
2018-03-10 20:58:24 +00:00
Neels Hofmeyr d656dff235 silent call: clarify rc and error messages logged on vty
In gsm_silent_call_{start,stop}(), return meaningful error codes and interpret
them on the VTY to clearly indicate the result.

Change-Id: Id5abb8f2ba901689e03040af8e51483b6c618e7f
2018-03-10 05:17:38 +01:00
Neels Hofmeyr e9495388d9 msc_main: do not say 'osmo-nitb' in the usage
Change-Id: I2090097dc0d7e0251c116b95b802076df3419455
2018-03-10 05:17:38 +01:00
Neels Hofmeyr 596ef65a6b vty: add 'subscriber ... paging' cmd
Allow issuing an empty paging from the VTY, for debugging purposes.

Change-Id: I403904cb789ece699f14b4cbd52c336eb02d45e4
2018-03-10 05:17:38 +01:00
Neels Hofmeyr 11010b1b85 vty: add 'msisdn' as alias for 'extension'
Rationale: in the HLR, it is called 'msisdn' after the database column, so a
user going back and forth between osmo-hlr and osmo-msc would appreciate being
able to type 'msisdn' in the MSC's vty as well.

Change-Id: I7b46f9736421e8edd8a95ae89e025ebe486fde4c
2018-03-10 05:17:38 +01:00
Neels Hofmeyr 72a41dd198 vty: drop unused commands
Change-Id: I76d6777ebe9105b8abf37993b86c3749a7e18008
2018-03-10 05:17:38 +01:00
Neels Hofmeyr 1db394f22b fix: clear vlr_subscr->msc_conn_ref when the conn is discarded
Before this, it was for example possible to crash the MSC by the vty 'show
subscriber' command, which would dereference a potentially stale
vsub->msc_conn_ref pointer.

Related: OS#3050
Change-Id: Ia4105d9f135ba3216ad3c86157be7658b1d568fb
2018-03-10 05:17:38 +01:00
Philipp Maier ce1298b7dc a_iface: fix BSSMAP reset handling
When osmo-msc restarts it looses all information about the BSC. The
BSC will not be aware of the reboot and on the next communication
attemt it will notice that something is wrong and start the reset
procedure on his side. osmo-msc will receive the reset messages
and send a reset.

The reset is received. Osmo-msc detects that no context information
is created yet. The context is created. Then it is checked if the
UNITTDATA message that came in is a reset. If it is one. Nothing
happens. The UNITTDATA is passed on and triggers the RESET-ACK
some layers above. Unfortunately by the current code this also
means that no reset FSM is created and therefore a_reset_conn_ready()
can never be true. Which means it will also drop any legitimate
reset from the BSC in the future.

- Ensure that the reset FSM is always created when a new BSC
  context is created

- Make sure that reset related traffic always passes so that
  the higher layers can handle the procedure properly

Change-Id: I3fdcec5dbeaa0e21fd6a92568a623faa368239be
2018-03-07 18:10:23 +01:00
Max 48d4ec06e1 VLR: fix potential NULL dereference
The vlr_subscr_get() can return NULL if its argument is NULL
(which isn't checked for) so before dereferencing it's result
we should check for it.

Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
2018-03-05 14:29:55 +00:00
Neels Hofmeyr da21a52c92 msc_vlr_tests: improve cipher mode coverage
Actually call msc_vlr_set_ciph_mode() and wrap away a_iface_tx_cipher_mode()
and ranap_iu_tx_sec_mode_cmd(). Hence we'll see decisions and errors in
msc_vlr_set_ciph_mode() as well.

Change-Id: Id23bc245d4b5707edcd27c44db272fbb211bf9bd
2018-03-02 17:00:37 +01:00
Neels Hofmeyr c5e0ace751 vlr_lu_fsm: guard against using the wrong fi
Various functions in vlr_lu_fsm.c belong to one of the four FSMs defined in
that file. After the recent error was uncovered where the lu_fsm called
lu_compl_fsm()'s termination function, I want to make sure it's correct.

Introduce distinct inline functions to dereference the respective fi->priv
pointers, each asserting that the fi indeed belongs to the proper FSM. Use
those *everywhere* to dereference fi->priv.

From this patch on, we are sure beyond doubt that we are not inadvertently
passing an fi pointer to the wrong FSM's handling functions, though we will
only catch this at runtime -- but then will immediately know the reason.

vlr_lu_fsm.c is the only file defining more than one FSM, so the other FSM
definitions are already reasonably safe.

Change-Id: I7419a780ff2d8b02efc4195bb1702818e4df181c
2018-03-02 03:21:14 +01:00
Neels Hofmeyr d2278ec899 vlr: fix post-auth LU failure handling
From the vlr_loc_update() FSM, don't call the vlr_lu_compl_fsm_failure()
function. These are two distinct FSMs with distinct priv pointers, but they are
defined in the same .c file.

In vlr_loc_upd_post_auth(), change two erratic calls of
vlr_lu_compl_fsm_failure() to lu_fsm_failure(), so that the proper fi and priv
struct are used.

Fixes: OS#2947
Change-Id: I7fd2c6fa23254fffd0d526e53541f4068153929f
2018-03-02 03:20:21 +01:00
Neels Hofmeyr 379d5799f0 implement support for 3-digit MNC with leading zeros
Add 3-digit flags and use the new RAI and LAI API from libosmocore throughout
the code base to be able to handle an MNC < 100 that has three digits (leading
zeros).

Depends: Id2240f7f518494c9df6c8bda52c0d5092f90f221 (libosmocore),
         Ib7176b1d65a03b76f41f94bc9d3293a8a07d24c6 (libosmocore)
Change-Id: I82f0016d9512ee8722a3489a3cb4b6c704a271fc
2018-02-28 19:30:14 +01:00
Neels Hofmeyr 7f48420923 cosmetic: gsm_network_init(): imply default 001-01 PLMN
All callers pass mcc=1, mnc=1, so just have it as default.
(Prepare for net->country_code etc to be replaced by net->plmn)

Change-Id: Ibcd1cc38f170895305ae176a5574384c74a33939
2018-02-27 13:01:42 +01:00
Philipp Maier e4f9172f44 msc_mgcp: Add FSM event names
The FSM (fsm_msc_mgcp) lacks a proper definition of the FSM event
names. This causes problems when inspecting the FSM using the VTY.

- Add proper FSM Event names

Closes: OS#2924

Change-Id: I6823756a63b08a71e5518130e49751aa073dbcd2
2018-02-26 15:50:17 +00:00
Philipp Maier 58616788ba a_reset: Add FSM event names
The FSM lacks a proper definition of the FSM event names. This causes
problems when inspecting the FSM using the VTY.

- Add proper FSM Event names

Change-Id: I76d7d9e0accffd433a3f3b5e5f8ab17ecd4a348c
Related: OS#2924
2018-02-26 16:03:05 +01:00
Stefan Sperling 617ac806c7 enable osmo_fsm vty commands in osmo-msc vty
Call osmo_fsm_vty_add_cmds() to make osmo_fsm VTY commands available
in osmo-msc's VTY interface.

Change-Id: Iaf970f6039c3f668f275dd8c21fb9071774a5d9e
Related: OS#2967
2018-02-22 17:58:20 +01:00
Neels Hofmeyr 2568f01779 drop unused libmsc/meas_feed.h
Change I0d57ac214e574e267fa9752daf76566197b9aa64 forgot to remove this
file along with meas_feed.c.

Note also the weirdness: that patch removes the proper
include/osmocom/msc/meas_feed.h, but there's also this other one.

This libmsc/meas_feed.h always existed from the start as an unused
orphan, see: 

https://git.osmocom.org/osmo-bsc/diff/openbsc/src/libmsc/Makefile.am?id=b4771a6871efb3cf12b371aedc575912984ca528

No need to drop from Makefile.am, since it is already gone from there.

(meas_feed from the old osmo-nitb (openbsc.git) has / should have moved to
osmo-bsc. There are no measurement reports in the MSC. Refer to osmo-bsc.git
instead from now on.)

Change-Id: Ib2566013dd30b21ce2774cd4cc7dcba2408f938f
2018-02-19 17:01:11 +00:00
Daniel Willmann 58d9dd8b3f libmsc: Pretend MNCC requested release in handle_error()
Send a release request to the MS so the connection does not stay open
indefinitely.

Change-Id: I7669d29cf5be3e4a60a1d121edbfcf9056f6d82b
2018-02-19 08:29:17 +00:00
Pau Espin 02a79d8545 libmsc: bssap: Remove fixme and properly update msgb tail
Change-Id: I28073efd5cff58cd212341bceee784caf08d5ad8
2018-02-16 13:29:57 +01:00
Daniel Willmann 4e825b6a68 libmsc: Add a function to return a unique ID of the subscriber conn
The ID will include the type of connection (GERAN_A, UTRAN_IU) followed
by the SCCP conn_id.
This can be used for the fsm instance ID before we know the IMSI.

Change-Id: I4b875772e3994ad3458ee60dbf880604486d9afd
2018-02-16 08:09:33 +00:00
Daniel Willmann 6fbd3bf732 iucs: Add a function to return the connection ID for the IU SCCP conn
Change-Id: Ica4a7f463fcf1bd67fe082296cecea8d10f2fc5b
2018-02-16 08:09:32 +00:00
Pau Espin 75559284d0 libmsc: bssap: Catch TLV parse failures
Change-Id: I1d1951f4a5daf200e85c76fea14a35e952491d27
2018-02-14 14:31:45 +01:00
Pau Espin 31776ff58f libmsc: bssap: Refactor rx paths to to avoid parse_tlv code duplication
Change-Id: I6aef9a94fa5b2e0b62a9c1744b8e18e5985f788f
2018-02-14 14:31:45 +01:00
Pau Espin 9f055f5983 libmsc: bssap: Fix typo in log message
Change-Id: I941cd7f4e9eec3e4f2786100a0e64770d5aee4a7
2018-02-14 14:13:43 +01:00
Harald Welte c0e425dbd0 remove dead code (gsm_parse_reg)
Change-Id: I47b0260561a9c2bad65c50c8164873e4c23d61b7
2018-02-14 09:04:56 +01:00