From a0454784538ee003751adf31dd18316818a28545 Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Tue, 10 Sep 2019 18:30:22 +0200 Subject: sgsn: Don't send Attach Accept if initial LU is rejected This should fix TTCN3, SGSN_Tests.TC_attach_gsup_lu_reject, wher eit can be seen that before this patch SGSN sent Attach Accept before sending/receing the first LU (so if LU was rejected by HLR, then attach was already accepted). Similary, it also fixes SGSN_Tests.TC_attach_gsup_lu_reject. Change-Id: Ie3970a790cf2b674da4d9e23a83c4a65d27d5de4 --- include/osmocom/sgsn/gprs_gmm_attach.h | 2 ++ include/osmocom/sgsn/gprs_sgsn.h | 1 + src/sgsn/gprs_gmm.c | 4 +-- src/sgsn/gprs_gmm_attach.c | 54 +++++++++++++++++++++++++--------- src/sgsn/gprs_subscriber.c | 1 + src/sgsn/sgsn_auth.c | 11 +++++-- 6 files changed, 54 insertions(+), 19 deletions(-) diff --git a/include/osmocom/sgsn/gprs_gmm_attach.h b/include/osmocom/sgsn/gprs_gmm_attach.h index 0aa2123a8..10d62f3d6 100644 --- a/include/osmocom/sgsn/gprs_gmm_attach.h +++ b/include/osmocom/sgsn/gprs_gmm_attach.h @@ -11,6 +11,7 @@ enum gmm_attach_req_fsm_states { ST_RETRIEVE_AUTH, ST_AUTH, ST_ASK_VLR, + ST_WAIT_UPDATE_LOCATION, ST_IU_SECURITY_CMD, ST_ACCEPT, ST_REJECT @@ -25,6 +26,7 @@ enum gmm_attach_req_fsm_events { E_ATTACH_ACCEPTED, E_ATTACH_ACCEPT_SENT, E_ATTACH_COMPLETE_RECV, + E_UPDATE_LOCATION_RESP_RECV_SUCCESS, E_REJECT, E_VLR_ANSWERED, }; diff --git a/include/osmocom/sgsn/gprs_sgsn.h b/include/osmocom/sgsn/gprs_sgsn.h index 0a52a7df5..0795bea09 100644 --- a/include/osmocom/sgsn/gprs_sgsn.h +++ b/include/osmocom/sgsn/gprs_sgsn.h @@ -480,6 +480,7 @@ int sgsn_acl_del(const char *imsi, struct sgsn_config *cfg); /* Request authorization */ int sgsn_auth_request(struct sgsn_mm_ctx *mm); enum sgsn_auth_state sgsn_auth_state(struct sgsn_mm_ctx *mm); +bool sgsn_auth_needs_update_location(struct sgsn_mm_ctx *mm); void sgsn_auth_update(struct sgsn_mm_ctx *mm); struct gsm_auth_tuple *sgsn_auth_get_tuple(struct sgsn_mm_ctx *mmctx, unsigned key_seq); diff --git a/src/sgsn/gprs_gmm.c b/src/sgsn/gprs_gmm.c index afae369a2..49f5a332a 100644 --- a/src/sgsn/gprs_gmm.c +++ b/src/sgsn/gprs_gmm.c @@ -961,8 +961,8 @@ void gsm0408_gprs_access_granted(struct sgsn_mm_ctx *ctx) "Authorized, continuing procedure, IMSI=%s\n", ctx->imsi); /* Continue with the authorization */ - if (ctx->gmm_att_req.fsm->state != ST_INIT) - osmo_fsm_inst_dispatch(ctx->gmm_att_req.fsm, E_VLR_ANSWERED, (void *) 0); + osmo_fsm_inst_dispatch(ctx->gmm_att_req.fsm, E_UPDATE_LOCATION_RESP_RECV_SUCCESS, (void *) 0); + break; default: LOGMMCTXP(LOGL_INFO, ctx, diff --git a/src/sgsn/gprs_gmm_attach.c b/src/sgsn/gprs_gmm_attach.c index 130f8d1c0..72252f802 100644 --- a/src/sgsn/gprs_gmm_attach.c +++ b/src/sgsn/gprs_gmm_attach.c @@ -170,13 +170,13 @@ static void st_auth(struct osmo_fsm_inst *fi, uint32_t event, void *data) switch (event) { case E_AUTH_RESP_RECV_SUCCESS: - sgsn_auth_request(ctx); -#ifdef BUILD_IU - if (ctx->ran_type == MM_CTX_T_UTRAN_Iu && !ctx->iu.ue_ctx->integrity_active) - gmm_attach_fsm_state_chg(fi, ST_IU_SECURITY_CMD); - else -#endif /* BUILD_IU */ + if (sgsn_auth_needs_update_location(ctx)) { + LOGMMCTXP(LOGL_INFO, ctx, + "Missing information, requesting subscriber data\n"); + gmm_attach_fsm_state_chg(fi, ST_WAIT_UPDATE_LOCATION); + } else { gmm_attach_fsm_state_chg(fi, ST_ACCEPT); + } break; case E_AUTH_RESP_RECV_RESYNC: if (ctx->gmm_att_req.auth_reattempt <= 1) @@ -187,6 +187,30 @@ static void st_auth(struct osmo_fsm_inst *fi, uint32_t event, void *data) } } +static void st_wait_lu_resp_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state) +{ + struct sgsn_mm_ctx *ctx = fi->priv; + int rc = gprs_subscr_request_update_location(ctx); + if (rc < 0) + LOGMMCTXP(LOGL_INFO, ctx, "Failed requesting Update Location\n"); +} + +static void st_wait_lu_resp(struct osmo_fsm_inst *fi, uint32_t event, void *data) +{ + struct sgsn_mm_ctx *ctx = fi->priv; + + switch (event) { + case E_UPDATE_LOCATION_RESP_RECV_SUCCESS: +#ifdef BUILD_IU + if (ctx->ran_type == MM_CTX_T_UTRAN_Iu && !ctx->iu.ue_ctx->integrity_active) + gmm_attach_fsm_state_chg(fi, ST_IU_SECURITY_CMD); + else +#endif /* BUILD_IU */ + gmm_attach_fsm_state_chg(fi, ST_ACCEPT); + break; + } +} + static void st_accept_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state) { struct sgsn_mm_ctx *ctx = fi->priv; @@ -209,12 +233,6 @@ static void st_accept(struct osmo_fsm_inst *fi, uint32_t event, void *data) extract_subscr_hlr(ctx); osmo_fsm_inst_state_chg(fi, ST_INIT, 0, 0); break; - case E_VLR_ANSWERED: - extract_subscr_msisdn(ctx); - extract_subscr_hlr(ctx); - LOGMMCTXP(LOGL_NOTICE, ctx, - "Unusual event: if MS got no data connection, check that it has APN configured.\n"); - break; } } @@ -302,11 +320,18 @@ static struct osmo_fsm_state gmm_attach_req_fsm_states[] = { }, [ST_AUTH] = { .in_event_mask = X(E_AUTH_RESP_RECV_SUCCESS) | X(E_AUTH_RESP_RECV_RESYNC), - .out_state_mask = X(ST_INIT) | X(ST_AUTH) | X(ST_IU_SECURITY_CMD) | X(ST_ACCEPT) | X(ST_ASK_VLR) | X(ST_REJECT), + .out_state_mask = X(ST_INIT) | X(ST_AUTH) | X(ST_IU_SECURITY_CMD) | X(ST_ACCEPT) | X(ST_ASK_VLR) | X(ST_WAIT_UPDATE_LOCATION) | X(ST_REJECT), .name = "Authenticate", .onenter = st_auth_on_enter, .action = st_auth, }, + [ST_WAIT_UPDATE_LOCATION] = { + .in_event_mask = X(E_UPDATE_LOCATION_RESP_RECV_SUCCESS), + .out_state_mask = X(ST_INIT) | X(ST_ACCEPT) | X(ST_IU_SECURITY_CMD) | X(ST_REJECT), + .name = "WaitLocationUpdateResp", + .onenter = st_wait_lu_resp_on_enter, + .action = st_wait_lu_resp, + }, [ST_IU_SECURITY_CMD] = { .in_event_mask = X(E_IU_SECURITY_CMD_COMPLETE), .out_state_mask = X(ST_INIT) | X(ST_AUTH) | X(ST_ACCEPT) | X(ST_REJECT), @@ -315,7 +340,7 @@ static struct osmo_fsm_state gmm_attach_req_fsm_states[] = { .action = st_iu_security_cmd, }, [ST_ACCEPT] = { - .in_event_mask = X(E_ATTACH_COMPLETE_RECV) | X(E_VLR_ANSWERED), + .in_event_mask = X(E_ATTACH_COMPLETE_RECV), .out_state_mask = X(ST_INIT) | X(ST_REJECT), .name = "WaitAttachComplete", .onenter = st_accept_on_enter, @@ -338,6 +363,7 @@ const struct value_string gmm_attach_req_fsm_event_names[] = { OSMO_VALUE_STRING(E_ATTACH_ACCEPT_SENT), OSMO_VALUE_STRING(E_ATTACH_COMPLETE_RECV), OSMO_VALUE_STRING(E_IU_SECURITY_CMD_COMPLETE), + OSMO_VALUE_STRING(E_UPDATE_LOCATION_RESP_RECV_SUCCESS), OSMO_VALUE_STRING(E_REJECT), OSMO_VALUE_STRING(E_VLR_ANSWERED), { 0, NULL } diff --git a/src/sgsn/gprs_subscriber.c b/src/sgsn/gprs_subscriber.c index 484c7ef4e..9626cbfd8 100644 --- a/src/sgsn/gprs_subscriber.c +++ b/src/sgsn/gprs_subscriber.c @@ -33,6 +33,7 @@ #include #include #include +#include #include diff --git a/src/sgsn/sgsn_auth.c b/src/sgsn/sgsn_auth.c index b8d803590..56dc2830e 100644 --- a/src/sgsn/sgsn_auth.c +++ b/src/sgsn/sgsn_auth.c @@ -145,6 +145,13 @@ enum sgsn_auth_state sgsn_auth_state(struct sgsn_mm_ctx *mmctx) return SGSN_AUTH_REJECTED; } +bool sgsn_auth_needs_update_location(struct sgsn_mm_ctx *mmctx) +{ + return sgsn->cfg.require_update_location && + (mmctx->subscr == NULL || + mmctx->pending_req == GSM48_MT_GMM_ATTACH_REQ); +} + /* * This function is directly called by e.g. the GMM layer. It returns either * after calling sgsn_auth_update directly or after triggering an asynchronous @@ -164,9 +171,7 @@ int sgsn_auth_request(struct sgsn_mm_ctx *mmctx) return 0; } - need_update_location = sgsn->cfg.require_update_location && - (mmctx->subscr == NULL || - mmctx->pending_req == GSM48_MT_GMM_ATTACH_REQ); + need_update_location = sgsn_auth_needs_update_location(mmctx); /* This has the side effect of registering the subscr with the mmctx */ subscr = gprs_subscr_get_or_create_by_mmctx(mmctx); -- cgit v1.2.3