use new osmo_mobile_identity API everywhere

Depends: If4f7be606e54cfa1c59084cf169785b1cbda5cf5 (libosmocore)
Change-Id: I4cacb10bac419633ca0c14f244f9903f7f517b49

7 files changed, 137 insertions, 120 deletions

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 3096c4b54..e5e3b39c6 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -1,2 +1,4 @@
 #component	what		description / commit summary line
 manual				needs common chapter cs7-config.adoc from osmo-gsm-manuals > 0.3.0
+libosmocore	osmo_mobile_identity	Depends on libosmocore > 1.3.0
+osmo-bsc	Mobile Identity Coding	OsmoBSC is stricter in rejecting invalid coding of Mobile Identity IEs
diff --git a/src/gbproxy/gb_proxy_patch.c b/src/gbproxy/gb_proxy_patch.c
index 6235b04f4..2bc3b4b74 100644
--- a/src/gbproxy/gb_proxy_patch.c
+++ b/src/gbproxy/gb_proxy_patch.c
@@ -436,28 +436,26 @@ int gbproxy_set_patch_filter(struct gbproxy_match *match, const char *filter,
 int gbproxy_check_imsi(struct gbproxy_match *match,
 		       const uint8_t *imsi, size_t imsi_len)
 {
-	char mi_buf[200];
 	int rc;
+	struct osmo_mobile_identity mi;
 
 	if (!match->enable)
 		return 1;
 
-	rc = gprs_is_mi_imsi(imsi, imsi_len);
-	if (rc > 0)
-		rc = gsm48_mi_to_string(mi_buf, sizeof(mi_buf), imsi, imsi_len);
-	if (rc <= 0) {
+	rc = osmo_mobile_identity_decode(&mi, imsi, imsi_len, false);
+	if (rc || mi.type != GSM_MI_TYPE_IMSI) {
 		LOGP(DGPRS, LOGL_NOTICE, "Invalid IMSI %s\n",
 		     osmo_hexdump(imsi, imsi_len));
 		return -1;
 	}
 
-	LOGP(DGPRS, LOGL_DEBUG, "Checking IMSI '%s' (%d)\n", mi_buf, rc);
+	LOGP(DGPRS, LOGL_DEBUG, "Checking IMSI '%s' (%d)\n", mi.imsi, rc);
 
-	rc = regexec(&match->re_comp, mi_buf, 0, NULL, 0);
+	rc = regexec(&match->re_comp, mi.imsi, 0, NULL, 0);
 	if (rc == REG_NOMATCH) {
 		LOGP(DGPRS, LOGL_INFO,
 		       "IMSI '%s' doesn't match pattern '%s'\n",
-		       mi_buf, match->re_str);
+		       mi.imsi, match->re_str);
 		return 0;
 	}
 
diff --git a/src/gbproxy/gb_proxy_tlli.c b/src/gbproxy/gb_proxy_tlli.c
index 4e21ede86..e9271c22c 100644
--- a/src/gbproxy/gb_proxy_tlli.c
+++ b/src/gbproxy/gb_proxy_tlli.c
@@ -401,14 +401,16 @@ static void gbproxy_assign_imsi(struct gbproxy_peer *peer,
 		peer, parse_ctx->imsi, parse_ctx->imsi_len);
 
 	if (other_link_info && other_link_info != link_info) {
-		char mi_buf[200];
-		mi_buf[0] = '\0';
-		gsm48_mi_to_string(mi_buf, sizeof(mi_buf),
-				   parse_ctx->imsi, parse_ctx->imsi_len);
-		LOGP(DGPRS, LOGL_INFO,
-		     "Removing TLLI %08x from list (IMSI %s re-used)\n",
-		     other_link_info->tlli.current, mi_buf);
-		gbproxy_delete_link_info(peer, other_link_info);
+		struct osmo_mobile_identity mi;
+		if (osmo_mobile_identity_decode(&mi, parse_ctx->imsi, parse_ctx->imsi_len, false)
+		    || mi.type != GSM_MI_TYPE_IMSI) {
+			LOGP(DGPRS, LOGL_ERROR, "Failed to decode Mobile Identity\n");
+		} else {
+			LOGP(DGPRS, LOGL_INFO,
+			     "Removing TLLI %08x from list (IMSI %s re-used)\n",
+			     other_link_info->tlli.current, mi.imsi);
+			gbproxy_delete_link_info(peer, other_link_info);
+		}
 	}
 
 	/* Update the IMSI field */
diff --git a/src/gbproxy/gb_proxy_vty.c b/src/gbproxy/gb_proxy_vty.c
index 5c4f45420..355b23fb1 100644
--- a/src/gbproxy/gb_proxy_vty.c
+++ b/src/gbproxy/gb_proxy_vty.c
@@ -554,7 +554,6 @@ DEFUN(show_gbproxy_links, show_gbproxy_links_cmd, "show gbproxy links",
        SHOW_STR "Display information about the Gb proxy\n" "Show logical links\n")
 {
 	struct gbproxy_peer *peer;
-	char mi_buf[200];
 	time_t now;
 	struct timespec ts = {0,};
 
@@ -569,17 +568,20 @@ DEFUN(show_gbproxy_links, show_gbproxy_links_cmd, "show gbproxy links",
 
 		llist_for_each_entry(link_info, &state->logical_links, list) {
 			time_t age = now - link_info->timestamp;
+			struct osmo_mobile_identity mi;
+			const char *imsi_str;
 
 			if (link_info->imsi > 0) {
-				snprintf(mi_buf, sizeof(mi_buf), "(invalid)");
-				gsm48_mi_to_string(mi_buf, sizeof(mi_buf),
-						   link_info->imsi,
-						   link_info->imsi_len);
+				if (osmo_mobile_identity_decode(&mi, link_info->imsi, link_info->imsi_len, false)
+				    || mi.type != GSM_MI_TYPE_IMSI)
+					imsi_str = "(invalid)";
+				else
+					imsi_str = mi.imsi;
 			} else {
-				snprintf(mi_buf, sizeof(mi_buf), "(none)");
+				imsi_str = "(none)";
 			}
 			vty_out(vty, "  TLLI %08x, IMSI %s, AGE %d",
-				link_info->tlli.current, mi_buf, (int)age);
+				link_info->tlli.current, imsi_str, (int)age);
 
 			if (link_info->stored_msgs_len)
 				vty_out(vty, ", STORED %"PRIu32"/%"PRIu32,
@@ -708,7 +710,6 @@ DEFUN(delete_gb_link_by_id, delete_gb_link_by_id_cmd,
 	struct gbproxy_peer *peer = 0;
 	struct gbproxy_link_info *link_info, *nxt;
 	struct gbproxy_patch_state *state;
-	char mi_buf[200];
 	int found = 0;
 
 	match = argv[1][0];
@@ -729,6 +730,8 @@ DEFUN(delete_gb_link_by_id, delete_gb_link_by_id_cmd,
 	state = &peer->patch_state;
 
 	llist_for_each_entry_safe(link_info, nxt, &state->logical_links, list) {
+		struct osmo_mobile_identity mi;
+
 		switch (match) {
 		case MATCH_TLLI:
 			if (link_info->tlli.current != ident)
@@ -741,12 +744,10 @@ DEFUN(delete_gb_link_by_id, delete_gb_link_by_id_cmd,
 		case MATCH_IMSI:
 			if (!link_info->imsi)
 				continue;
-			mi_buf[0] = '\0';
-			gsm48_mi_to_string(mi_buf, sizeof(mi_buf),
-					   link_info->imsi,
-					   link_info->imsi_len);
-
-			if (strcmp(mi_buf, imsi) != 0)
+			if (osmo_mobile_identity_decode(&mi, link_info->imsi, link_info->imsi_len, false)
+			    || mi.type != GSM_MI_TYPE_IMSI)
+				continue;
+			if (strcmp(mi.imsi, imsi) != 0)
 				continue;
 			break;
 		}
diff --git a/src/gprs/gprs_gb_parse.c b/src/gprs/gprs_gb_parse.c
index 379674a91..e5de4d4c7 100644
--- a/src/gprs/gprs_gb_parse.c
+++ b/src/gprs/gprs_gb_parse.c
@@ -604,13 +604,12 @@ void gprs_gb_log_parse_context(int log_level,
 	}
 
 	if (parse_ctx->imsi) {
-		char mi_buf[200];
-		mi_buf[0] = '\0';
-		gsm48_mi_to_string(mi_buf, sizeof(mi_buf),
-				   parse_ctx->imsi, parse_ctx->imsi_len);
-		LOGPC(DGPRS, log_level, "%s IMSI %s",
-		     sep, mi_buf);
-		sep = ",";
+		struct osmo_mobile_identity mi;
+		if (osmo_mobile_identity_decode(&mi, parse_ctx->imsi, parse_ctx->imsi_len, false) == 0
+		    && mi.type == GSM_MI_TYPE_IMSI) {
+			LOGPC(DGPRS, log_level, "%s IMSI %s", sep, mi.imsi);
+			sep = ",";
+		}
 	}
 	if (parse_ctx->invalidate_tlli) {
 		LOGPC(DGPRS, log_level, "%s invalidate", sep);
diff --git a/src/sgsn/gprs_gmm.c b/src/sgsn/gprs_gmm.c
index 0391229eb..182276cfa 100644
--- a/src/sgsn/gprs_gmm.c
+++ b/src/sgsn/gprs_gmm.c
@@ -280,8 +280,10 @@ int gsm48_tx_gmm_att_ack(struct sgsn_mm_ctx *mm)
 	struct msgb *msg = gsm48_msgb_alloc_name("GSM 04.08 ATT ACK");
 	struct gsm48_hdr *gh;
 	struct gsm48_attach_ack *aa;
-	uint8_t *mid;
 	unsigned long t;
+	struct osmo_mobile_identity mi;
+	uint8_t *l;
+	int rc;
 #if 0
 	uint8_t *ptsig;
 #endif
@@ -321,9 +323,18 @@ int gsm48_tx_gmm_att_ack(struct sgsn_mm_ctx *mm)
 
 #ifdef PTMSI_ALLOC
 	/* Optional: Allocated P-TMSI */
-	mid = msgb_put(msg, GSM48_MID_TMSI_LEN);
-	gsm48_generate_mid_from_tmsi(mid, mm->p_tmsi);
-	mid[0] = GSM48_IE_GMM_ALLOC_PTMSI;
+	mi = (struct osmo_mobile_identity){
+		.type = GSM_MI_TYPE_TMSI,
+		.tmsi = mm->p_tmsi,
+	};
+	l = msgb_tl_put(msg, GSM48_IE_GMM_ALLOC_PTMSI);
+	rc = osmo_mobile_identity_encode_msgb(msg, &mi, false);
+	if (rc < 0) {
+		LOGMMCTXP(LOGL_ERROR, mm, "Cannot encode Mobile Identity\n");
+		msgb_free(msg);
+		return -EINVAL;
+	}
+	*l = rc;
 #endif
 
 	/* Optional: MS-identity (combined attach) */
@@ -1026,31 +1037,35 @@ void gsm0408_gprs_access_cancelled(struct sgsn_mm_ctx *ctx, int gmm_cause)
 static int gsm48_rx_gmm_id_resp(struct sgsn_mm_ctx *ctx, struct msgb *msg)
 {
 	struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(msg);
-	uint8_t mi_type = gh->data[1] & GSM_MI_TYPE_MASK;
-	long mi_typel = mi_type;
-	char mi_string[GSM48_MI_SIZE];
+	long mi_typel;
+	char mi_log_string[32];
+	struct osmo_mobile_identity mi;
 
-	gsm48_mi_to_string(mi_string, sizeof(mi_string), &gh->data[1], gh->data[0]);
 	if (!ctx) {
 		DEBUGP(DMM, "from unknown TLLI 0x%08x?!? This should not happen\n", msgb_tlli(msg));
 		return -EINVAL;
 	}
 
-	LOGMMCTXP(LOGL_DEBUG, ctx, "-> GMM IDENTITY RESPONSE: MI(%s)=%s\n",
-		gsm48_mi_type_name(mi_type), mi_string);
+	if (osmo_mobile_identity_decode(&mi, &gh->data[1], gh->data[0], false)) {
+		LOGMMCTXP(LOGL_ERROR, ctx, "-> GMM IDENTITY RESPONSE: cannot decode Mobile Identity\n");
+		return -EINVAL;
+	}
+	osmo_mobile_identity_to_str_buf(mi_log_string, sizeof(mi_log_string), &mi);
+
+	LOGMMCTXP(LOGL_DEBUG, ctx, "-> GMM IDENTITY RESPONSE: MI=%s\n", mi_log_string);
 
 	if (ctx->t3370_id_type == GSM_MI_TYPE_NONE) {
 		LOGMMCTXP(LOGL_NOTICE, ctx,
-			  "Got unexpected IDENTITY RESPONSE: MI(%s)=%s, "
+			  "Got unexpected IDENTITY RESPONSE: MI=%s, "
 			  "ignoring message\n",
-			  gsm48_mi_type_name(mi_type), mi_string);
+			  mi_log_string);
 		return -EINVAL;
 	}
 
-	if (mi_type == ctx->t3370_id_type)
+	if (mi.type == ctx->t3370_id_type)
 		mmctx_timer_stop(ctx, 3370);
 
-	switch (mi_type) {
+	switch (mi.type) {
 	case GSM_MI_TYPE_IMSI:
 		/* we already have a mm context with current TLLI, but no
 		 * P-TMSI / IMSI yet.  What we now need to do is to fill
@@ -1058,7 +1073,7 @@ static int gsm48_rx_gmm_id_resp(struct sgsn_mm_ctx *ctx, struct msgb *msg)
 		if (strlen(ctx->imsi) == 0) {
 			/* Check if we already have a MM context for this IMSI */
 			struct sgsn_mm_ctx *ictx;
-			ictx = sgsn_mm_ctx_by_imsi(mi_string);
+			ictx = sgsn_mm_ctx_by_imsi(mi.imsi);
 			if (ictx) {
 				/* Handle it like in gsm48_rx_gmm_det_req,
 				 * except that no messages are sent to the BSS */
@@ -1070,16 +1085,17 @@ static int gsm48_rx_gmm_id_resp(struct sgsn_mm_ctx *ctx, struct msgb *msg)
 				mm_ctx_cleanup_free(ictx, "GPRS IMSI re-use");
 			}
 		}
-		osmo_strlcpy(ctx->imsi, mi_string, sizeof(ctx->imsi));
+		OSMO_STRLCPY_ARRAY(ctx->imsi, mi.imsi);
 		break;
 	case GSM_MI_TYPE_IMEI:
-		osmo_strlcpy(ctx->imei, mi_string, sizeof(ctx->imei));
+		OSMO_STRLCPY_ARRAY(ctx->imei, mi.imei);
 		break;
 	case GSM_MI_TYPE_IMEISV:
 		break;
 	}
 
 	/* Check if we can let the mobile station enter */
+	mi_typel = mi.type;
 	return osmo_fsm_inst_dispatch(ctx->gmm_att_req.fsm, E_IDEN_RESP_RECV, (void *)mi_typel);
 }
 
@@ -1131,14 +1147,14 @@ static int gsm48_rx_gmm_att_req(struct sgsn_mm_ctx *ctx, struct msgb *msg,
 				struct gprs_llc_llme *llme)
 {
 	struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(msg);
-	uint8_t *cur = gh->data, *msnc, *mi, *ms_ra_acc_cap;
-	uint8_t msnc_len, att_type, mi_len, mi_type, ms_ra_acc_cap_len;
+	uint8_t *cur = gh->data, *msnc, *mi_data, *ms_ra_acc_cap;
+	uint8_t msnc_len, att_type, mi_len, ms_ra_acc_cap_len;
 	uint16_t drx_par;
-	uint32_t tmsi;
-	char mi_string[GSM48_MI_SIZE];
+	char mi_log_string[32];
 	struct gprs_ra_id ra_id;
 	uint16_t cid = 0;
 	enum gsm48_gmm_cause reject_cause;
+	struct osmo_mobile_identity mi;
 	int rc;
 
 	LOGMMCTXP(LOGL_INFO, ctx, "-> GMM ATTACH REQUEST ");
@@ -1182,15 +1198,15 @@ static int gsm48_rx_gmm_att_req(struct sgsn_mm_ctx *ctx, struct msgb *msg,
 
 	/* Mobile Identity (P-TMSI or IMSI) 10.5.1.4 */
 	mi_len = *cur++;
-	mi = cur;
-	if (mi_len > 8)
-		goto err_inval;
-	mi_type = *mi & GSM_MI_TYPE_MASK;
+	mi_data = cur;
 	cur += mi_len;
 
-	gsm48_mi_to_string(mi_string, sizeof(mi_string), mi, mi_len);
+	rc = osmo_mobile_identity_decode(&mi, mi_data, mi_len, false);
+	if (rc)
+		goto err_inval;
+	osmo_mobile_identity_to_str_buf(mi_log_string, sizeof(mi_log_string), &mi);
 
-	DEBUGPC(DMM, "MI(%s) type=\"%s\" ", mi_string,
+	DEBUGPC(DMM, "MI(%s) type=\"%s\" ", mi_log_string,
 		get_value_string(gprs_att_t_strs, att_type));
 
 	/* Old routing area identification 10.5.5.15. Skip it */
@@ -1207,11 +1223,11 @@ static int gsm48_rx_gmm_att_req(struct sgsn_mm_ctx *ctx, struct msgb *msg,
 
 	/* Optional: Old P-TMSI Signature, Requested READY timer, TMSI Status */
 
-	switch (mi_type) {
+	switch (mi.type) {
 	case GSM_MI_TYPE_IMSI:
 		/* Try to find MM context based on IMSI */
 		if (!ctx)
-			ctx = sgsn_mm_ctx_by_imsi(mi_string);
+			ctx = sgsn_mm_ctx_by_imsi(mi.imsi);
 		if (!ctx) {
 			if (MSG_IU_UE_CTX(msg))
 				ctx = sgsn_mm_ctx_alloc_iu(MSG_IU_UE_CTX(msg));
@@ -1221,15 +1237,13 @@ static int gsm48_rx_gmm_att_req(struct sgsn_mm_ctx *ctx, struct msgb *msg,
 				reject_cause = GMM_CAUSE_NET_FAIL;
 				goto rejected;
 			}
-			osmo_strlcpy(ctx->imsi, mi_string, sizeof(ctx->imsi));
+			OSMO_STRLCPY_ARRAY(ctx->imsi, mi.imsi);
 		}
 		break;
 	case GSM_MI_TYPE_TMSI:
-		memcpy(&tmsi, mi+1, 4);
-		tmsi = ntohl(tmsi);
 		/* Try to find MM context based on P-TMSI */
 		if (!ctx)
-			ctx = sgsn_mm_ctx_by_ptmsi(tmsi);
+			ctx = sgsn_mm_ctx_by_ptmsi(mi.tmsi);
 		if (!ctx) {
 			/* Allocate a context as most of our code expects one.
 			 * Context will not have an IMSI ultil ID RESP is received */
@@ -1241,12 +1255,12 @@ static int gsm48_rx_gmm_att_req(struct sgsn_mm_ctx *ctx, struct msgb *msg,
 				reject_cause = GMM_CAUSE_NET_FAIL;
 				goto rejected;
 			}
-			ctx->p_tmsi = tmsi;
+			ctx->p_tmsi = mi.tmsi;
 		}
 		break;
 	default:
 		LOGMMCTXP(LOGL_NOTICE, ctx, "Rejecting ATTACH REQUEST with "
-			"MI type %s\n", gsm48_mi_type_name(mi_type));
+			"MI %s\n", mi_log_string);
 		reject_cause = GMM_CAUSE_MS_ID_NOT_DERIVED;
 		goto rejected;
 	}
@@ -1275,8 +1289,8 @@ static int gsm48_rx_gmm_att_req(struct sgsn_mm_ctx *ctx, struct msgb *msg,
 					   ctx->ciph_algo)) {
 		reject_cause = GMM_CAUSE_PROTO_ERR_UNSPEC;
 		LOGMMCTXP(LOGL_NOTICE, ctx, "Rejecting ATTACH REQUEST with MI "
-			  "type %s because MS do not support required %s "
-			  "encryption\n", gsm48_mi_type_name(mi_type),
+			  "%s because MS do not support required %s "
+			  "encryption\n", mi_log_string,
 			  get_value_string(gprs_cipher_names,ctx->ciph_algo));
 		goto rejected;
 	}
@@ -1423,8 +1437,10 @@ static int gsm48_tx_gmm_ra_upd_ack(struct sgsn_mm_ctx *mm)
 	struct msgb *msg = gsm48_msgb_alloc_name("GSM 04.08 UPD ACK");
 	struct gsm48_hdr *gh;
 	struct gsm48_ra_upd_ack *rua;
-	uint8_t *mid;
 	unsigned long t;
+	uint8_t *l;
+	int rc;
+	struct osmo_mobile_identity mi;
 
 	rate_ctr_inc(&sgsn->rate_ctrs->ctr[CTR_GPRS_ROUTING_AREA_ACKED]);
 	LOGMMCTXP(LOGL_INFO, mm, "<- ROUTING AREA UPDATE ACCEPT\n");
@@ -1454,9 +1470,17 @@ static int gsm48_tx_gmm_ra_upd_ack(struct sgsn_mm_ctx *mm)
 
 #ifdef PTMSI_ALLOC
 	/* Optional: Allocated P-TMSI */
-	mid = msgb_put(msg, GSM48_MID_TMSI_LEN);
-	gsm48_generate_mid_from_tmsi(mid, mm->p_tmsi);
-	mid[0] = GSM48_IE_GMM_ALLOC_PTMSI;
+	mi = (struct osmo_mobile_identity){
+		.type = GSM_MI_TYPE_TMSI,
+		.tmsi = mm->p_tmsi,
+	};
+	l = msgb_tl_put(msg, GSM48_IE_GMM_ALLOC_PTMSI);
+	rc = osmo_mobile_identity_encode_msgb(msg, &mi, false);
+	if (rc < 0) {
+		msgb_free(msg);
+		return -EINVAL;
+	}
+	*l = rc;
 #endif
 
 	/* Optional: Negotiated READY timer value */
@@ -1606,19 +1630,14 @@ static int gsm48_rx_gmm_ra_upd_req(struct sgsn_mm_ctx *mmctx, struct msgb *msg,
 		} else if (TLVP_PRESENT(&tp, GSM48_IE_GMM_ALLOC_PTMSI)) {
 #ifdef BUILD_IU
 			/* In Iu mode search only for ptmsi */
-			char mi_string[GSM48_MI_SIZE];
-			uint8_t mi_len = TLVP_LEN(&tp, GSM48_IE_GMM_ALLOC_PTMSI);
-			const uint8_t *mi = TLVP_VAL(&tp, GSM48_IE_GMM_ALLOC_PTMSI);
-			uint8_t mi_type = *mi & GSM_MI_TYPE_MASK;
-			uint32_t tmsi;
-
-			gsm48_mi_to_string(mi_string, sizeof(mi_string), mi, mi_len);
-
-			if (mi_type == GSM_MI_TYPE_TMSI) {
-				memcpy(&tmsi, mi+1, 4);
-				tmsi = ntohl(tmsi);
-				mmctx = sgsn_mm_ctx_by_ptmsi(tmsi);
+			struct osmo_mobile_identity mi;
+			if (osmo_mobile_identity_decode(&mi, TLVP_VAL(&tp, GSM48_IE_GMM_ALLOC_PTMSI),
+							TLVP_LEN(&tp, GSM48_IE_GMM_ALLOC_PTMSI), false)
+			    || mi.type != GSM_MI_TYPE_TMSI) {
+				LOGIUP(MSG_IU_UE_CTX(msg), LOGL_ERROR, "Cannot decode P-TMSI\n");
+				goto rejected;
 			}
+			mmctx = sgsn_mm_ctx_by_ptmsi(mi.tmsi);
 #else
 			LOGIUP(MSG_IU_UE_CTX(msg), LOGL_ERROR,
 			       "Rejecting GMM RA Update Request: No Iu support\n");
@@ -1802,11 +1821,11 @@ static int gsm48_rx_gmm_ptmsi_reall_compl(struct sgsn_mm_ctx *mmctx)
 static int gsm48_rx_gmm_service_req(struct sgsn_mm_ctx *ctx, struct msgb *msg)
 {
 	struct gsm48_hdr *gh = (struct gsm48_hdr *) msgb_gmmh(msg);
-	uint8_t *cur = gh->data, *mi;
-	uint8_t service_type, mi_len, mi_type;
-	uint32_t tmsi;
+	uint8_t *cur = gh->data, *mi_data;
+	uint8_t service_type, mi_len;
 	struct tlv_parsed tp;
-	char mi_string[GSM48_MI_SIZE];
+	struct osmo_mobile_identity mi;
+	char mi_log_string[32];
 	enum gsm48_gmm_cause reject_cause;
 	int rc;
 
@@ -1826,15 +1845,14 @@ static int gsm48_rx_gmm_service_req(struct sgsn_mm_ctx *ctx, struct msgb *msg)
 
 	/* Mobile Identity (P-TMSI or IMSI) 10.5.1.4 */
 	mi_len = *cur++;
-	mi = cur;
-	if (mi_len > 8)
-		goto err_inval;
-	mi_type = *mi & GSM_MI_TYPE_MASK;
+	mi_data = cur;
 	cur += mi_len;
+	rc = osmo_mobile_identity_decode(&mi, mi_data, mi_len, false);
+	if (rc)
+		goto err_inval;
+	osmo_mobile_identity_to_str_buf(mi_log_string, sizeof(mi_log_string), &mi);
 
-	gsm48_mi_to_string(mi_string, sizeof(mi_string), mi, mi_len);
-
-	DEBUGPC(DMM, "MI(%s) type=\"%s\" ", mi_string,
+	DEBUGPC(DMM, "MI(%s) type=\"%s\" ", mi_log_string,
 		get_value_string(gprs_service_t_strs, service_type));
 
 	LOGPC(DMM, LOGL_INFO, "\n");
@@ -1842,11 +1860,11 @@ static int gsm48_rx_gmm_service_req(struct sgsn_mm_ctx *ctx, struct msgb *msg)
 	/* Optional: PDP context status, MBMS context status, Uplink data status, Device properties */
 	tlv_parse(&tp, &gsm48_gmm_att_tlvdef, cur, (msg->data + msg->len) - cur, 0, 0);
 
-	switch (mi_type) {
+	switch (mi.type) {
 	case GSM_MI_TYPE_IMSI:
 		/* Try to find MM context based on IMSI */
 		if (!ctx)
-			ctx = sgsn_mm_ctx_by_imsi(mi_string);
+			ctx = sgsn_mm_ctx_by_imsi(mi.imsi);
 		if (!ctx) {
 			/* FIXME: We need to have a context for service request? */
 			reject_cause = GMM_CAUSE_IMPL_DETACHED;
@@ -1855,11 +1873,9 @@ static int gsm48_rx_gmm_service_req(struct sgsn_mm_ctx *ctx, struct msgb *msg)
 		msgid2mmctx(ctx, msg);
 		break;
 	case GSM_MI_TYPE_TMSI:
-		memcpy(&tmsi, mi+1, 4);
-		tmsi = ntohl(tmsi);
 		/* Try to find MM context based on P-TMSI */
 		if (!ctx)
-			ctx = sgsn_mm_ctx_by_ptmsi(tmsi);
+			ctx = sgsn_mm_ctx_by_ptmsi(mi.tmsi);
 		if (!ctx) {
 			/* FIXME: We need to have a context for service request? */
 			reject_cause = GMM_CAUSE_IMPL_DETACHED;
@@ -1869,7 +1885,7 @@ static int gsm48_rx_gmm_service_req(struct sgsn_mm_ctx *ctx, struct msgb *msg)
 		break;
 	default:
 		LOGMMCTXP(LOGL_NOTICE, ctx, "Rejecting SERVICE REQUEST with "
-			"MI type %s\n", gsm48_mi_type_name(mi_type));
+			"MI %s\n", mi_log_string);
 		reject_cause = GMM_CAUSE_MS_ID_NOT_DERIVED;
 		goto rejected;
 	}
diff --git a/tests/gbproxy/gbproxy_test.c b/tests/gbproxy/gbproxy_test.c
index 6dfe2c566..6433eb606 100644
--- a/tests/gbproxy/gbproxy_test.c
+++ b/tests/gbproxy/gbproxy_test.c
@@ -158,7 +158,8 @@ static int dump_peers(FILE *stream, int indent, time_t now,
 		fprintf(stream, "%*s    TLLI-Cache: %d\n",
 			indent, "", state->logical_link_count);
 		llist_for_each_entry(link_info, &state->logical_links, list) {
-			char mi_buf[200];
+			struct osmo_mobile_identity mi;
+			const char *imsi_str;
 			time_t age = now ? now - link_info->timestamp : 0;
 			int stored_msgs = 0;
 			struct llist_head *iter;
@@ -166,13 +167,14 @@ static int dump_peers(FILE *stream, int indent, time_t now,
 			llist_for_each(iter, &link_info->stored_msgs)
 				stored_msgs++;
 
-			if (link_info->imsi_len > 0) {
-				snprintf(mi_buf, sizeof(mi_buf), "(invalid)");
-				gsm48_mi_to_string(mi_buf, sizeof(mi_buf),
-						   link_info->imsi,
-						   link_info->imsi_len);
+			if (link_info->imsi > 0) {
+				if (osmo_mobile_identity_decode(&mi, link_info->imsi, link_info->imsi_len, false)
+				    || mi.type != GSM_MI_TYPE_IMSI)
+					imsi_str = "(invalid)";
+				else
+					imsi_str = mi.imsi;
 			} else {
-				snprintf(mi_buf, sizeof(mi_buf), "(none)");
+				imsi_str = "(none)";
 			}
 			fprintf(stream, "%*s      TLLI %08x",
 				     indent, "", link_info->tlli.current);
@@ -186,7 +188,7 @@ static int dump_peers(FILE *stream, int indent, time_t now,
 						link_info->sgsn_tlli.assigned);
 			}
 			fprintf(stream, ", IMSI %s, AGE %d",
-				mi_buf, (int)age);
+				imsi_str, (int)age);
 
 			if (stored_msgs)
 				fprintf(stream, ", STORED %d", stored_msgs);
@@ -4809,10 +4811,7 @@ static void test_gbproxy_imsi_matching(void)
 
 	OSMO_ASSERT(gbproxy_check_imsi(&match, imsi1, ARRAY_SIZE(imsi1)) == 1);
 	OSMO_ASSERT(gbproxy_check_imsi(&match, imsi2, ARRAY_SIZE(imsi2)) == 1);
-	/* imsi3_bad contains 0xE and 0xF digits, but the conversion function
-	 * doesn't complain, so gbproxy_check_imsi() doesn't return -1 in this
-	 * case. */
-	OSMO_ASSERT(gbproxy_check_imsi(&match, imsi3_bad, ARRAY_SIZE(imsi3_bad)) == 0);
+	OSMO_ASSERT(gbproxy_check_imsi(&match, imsi3_bad, ARRAY_SIZE(imsi3_bad)) == -1);
 	OSMO_ASSERT(gbproxy_check_imsi(&match, tmsi1, ARRAY_SIZE(tmsi1)) == -1);
 	OSMO_ASSERT(gbproxy_check_imsi(&match, tmsi2_bad, ARRAY_SIZE(tmsi2_bad)) == -1);
 	OSMO_ASSERT(gbproxy_check_imsi(&match, imei1, ARRAY_SIZE(imei1)) == -1);
@@ -4823,7 +4822,7 @@ static void test_gbproxy_imsi_matching(void)
 
 	OSMO_ASSERT(gbproxy_check_imsi(&match, imsi1, ARRAY_SIZE(imsi1)) == 0);
 	OSMO_ASSERT(gbproxy_check_imsi(&match, imsi2, ARRAY_SIZE(imsi2)) == 0);
-	OSMO_ASSERT(gbproxy_check_imsi(&match, imsi3_bad, ARRAY_SIZE(imsi3_bad)) == 0);
+	OSMO_ASSERT(gbproxy_check_imsi(&match, imsi3_bad, ARRAY_SIZE(imsi3_bad)) == -1);
 	OSMO_ASSERT(gbproxy_check_imsi(&match, tmsi1, ARRAY_SIZE(tmsi1)) == -1);
 	OSMO_ASSERT(gbproxy_check_imsi(&match, tmsi2_bad, ARRAY_SIZE(tmsi2_bad)) == -1);
 	OSMO_ASSERT(gbproxy_check_imsi(&match, imei1, ARRAY_SIZE(imei1)) == -1);