ensure to not create evil-twin IMSIs in the VLR

Change-Id: Ia26998c6764250989622e8e10055515c20be14f7

3 files changed, 44 insertions, 6 deletions

diff --git a/include/osmocom/msc/vlr.h b/include/osmocom/msc/vlr.h
index 56314b747..8abe5c01d 100644
--- a/include/osmocom/msc/vlr.h
+++ b/include/osmocom/msc/vlr.h
@@ -311,7 +311,7 @@ void vlr_loc_update_cancel(struct osmo_fsm_inst *fi,
 /* tell the VLR that the RAN connection is gone */
 int vlr_subscr_disconnected(struct vlr_subscr *vsub);
 bool vlr_subscr_expire(struct vlr_subscr *vsub);
-int vlr_subscr_rx_id_resp(struct vlr_subscr *vsub, const uint8_t *mi, size_t mi_len);
+int vlr_subscr_rx_id_resp(struct vlr_subscr *vsub, struct vlr_subscr **changed_vsub, const uint8_t *mi, size_t mi_len);
 int vlr_subscr_rx_auth_resp(struct vlr_subscr *vsub, bool is_r99, bool is_utran,
 			    const uint8_t *res, uint8_t res_len);
 int vlr_subscr_rx_auth_fail(struct vlr_subscr *vsub, const uint8_t *auts);
diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 0bdc4fbe8..8bd4414b9 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -183,6 +183,8 @@ static int mm_rx_id_resp(struct msc_a *msc_a, struct msgb *msg)
 	uint8_t *mi = gh->data+1;
 	uint8_t mi_len = gh->data[0];
 	struct vlr_subscr *vsub = msc_a_vsub(msc_a);
+	struct vlr_subscr *changed_vsub;
+	int rc;
 
 	if (!vsub) {
 		LOGP(DMM, LOGL_ERROR,
@@ -194,7 +196,15 @@ static int mm_rx_id_resp(struct msc_a *msc_a, struct msgb *msg)
 
 	osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, gh->data);
 
-	return vlr_subscr_rx_id_resp(vsub, mi, mi_len);
+	rc = vlr_subscr_rx_id_resp(vsub, &changed_vsub, mi, mi_len);
+
+	if (changed_vsub) {
+		/* VLR instructs us to use this other vsub record instead */
+		msub_set_vsub(msc_a->c.msub, NULL);
+		msub_set_vsub(msc_a->c.msub, changed_vsub);
+	}
+
+	return rc;
 }
 
 /* 9.2.5 CM service accept */
@@ -1174,10 +1184,12 @@ static int gsm48_rx_rr_pag_resp(struct msc_a *msc_a, struct msgb *msg)
 static int gsm48_rx_rr_ciphering_mode_complete(struct msc_a *msc_a, struct msgb *msg)
 {
 	struct vlr_subscr *vsub = msc_a_vsub(msc_a);
+	struct vlr_subscr *changed_vsub;
 	struct gsm48_hdr *gh = msgb_l3(msg);
 	unsigned int payload_len = msgb_l3len(msg) - sizeof(*gh);
 	struct tlv_parsed tp;
 	struct tlv_p_entry *mi;
+	int rc;
 
 	tlv_parse(&tp, &gsm48_att_tlvdef, gh->data, payload_len, 0, 0);
 	mi = TLVP_GET(&tp, GSM48_IE_MOBILE_ID);
@@ -1191,7 +1203,15 @@ static int gsm48_rx_rr_ciphering_mode_complete(struct msc_a *msc_a, struct msgb
 	if (!vsub)
 		return 0;
 
-	return vlr_subscr_rx_id_resp(vsub, mi->val, mi->len);
+	rc = vlr_subscr_rx_id_resp(vsub, &changed_vsub, mi->val, mi->len);
+
+	if (changed_vsub) {
+		/* VLR instructs us to use this other vsub record instead */
+		msub_set_vsub(msc_a->c.msub, NULL);
+		msub_set_vsub(msc_a->c.msub, changed_vsub);
+	}
+
+	return rc;
 }
 
 static int gsm48_rx_rr_app_info(struct msc_a *msc_a, struct msgb *msg)
diff --git a/src/libvlr/vlr.c b/src/libvlr/vlr.c
index 80710b2e1..62a68581a 100644
--- a/src/libvlr/vlr.c
+++ b/src/libvlr/vlr.c
@@ -1121,11 +1121,14 @@ int vlr_gsup_rx(struct gsup_client_mux *gcm, void *data, const struct osmo_gsup_
 }
 
 /* MSC->VLR: Subscriber has provided IDENTITY RESPONSE */
-int vlr_subscr_rx_id_resp(struct vlr_subscr *vsub,
+int vlr_subscr_rx_id_resp(struct vlr_subscr *vsub, struct vlr_subscr **changed_vsub,
 			  const uint8_t *mi, size_t mi_len)
 {
 	char mi_string[GSM48_MI_SIZE];
 	uint8_t mi_type = mi[0] & GSM_MI_TYPE_MASK;
+	struct vlr_subscr *twin;
+
+	*changed_vsub = NULL;
 
 	gsm48_mi_to_string(mi_string, sizeof(mi_string), mi, mi_len);
 
@@ -1141,8 +1144,23 @@ int vlr_subscr_rx_id_resp(struct vlr_subscr *vsub,
 			LOGVSUBP(LOGL_ERROR, vsub, "IMSI in ID RESP differs:"
 				 " %s\n", mi_string);
 			/* XXX Should we return an error, e.g. -EINVAL ? */
-		} else
-			vlr_subscr_set_imsi(vsub, mi_string);
+			break;
+		}
+
+		/* Guard against creating evil twin vlr_subscr (duplicate entries for an IMSI) */
+		twin = vlr_subscr_find_by_imsi(vsub->vlr, mi_string, __func__);
+		if (twin) {
+			/* We already have a different vlr_subscr for this IMSI. Instruct the caller to use that one
+			 * instead. */
+			LOGVSUBP(LOGL_INFO, twin,
+				 "On ID Response (IMSI), found an already existing VLR entry for this IMSI."
+				 " Switching to the already existing VLR entry.\n");
+			*changed_vsub = twin;
+			/* The other vsub should be released by use count release of the caller. */
+			return 0;
+		}
+
+		vlr_subscr_set_imsi(vsub, mi_string);
 		break;
 	case GSM_MI_TYPE_IMEI:
 		vlr_subscr_set_imei(vsub, mi_string);