ciphering: Better state tracking and HACK around L1 race condition

We now check if the received message is an LAPDm I frame in order to
determine if we have received the first valid encrypted message on the
radio link.  This relates to the fact that we often see 'old' UI frames
coming up from L1, even after it has confirmed decryption has been
enabled.

3 files changed, 39 insertions, 14 deletions

diff --git a/src/common/rsl.c b/src/common/rsl.c
index d0d38e86..2fc76af4 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -630,7 +630,8 @@ static int rsl_rx_chan_activ(struct msgb *msg)
 
 		if (encr_info2lchan(lchan, val, len) < 0)
 			 return rsl_tx_error_report(msg->trx, RSL_ERR_IE_CONTENT);
-	}
+	} else
+		memset(&lchan->encr, 0, sizeof(lchan->encr));
 
 	/* 9.3.9 Handover Reference */
 
diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c
index 02d930e9..353ff59d 100644
--- a/src/osmo-bts-sysmo/l1_if.c
+++ b/src/osmo-bts-sysmo/l1_if.c
@@ -301,14 +301,6 @@ get_lapdm_chan_by_hl2(struct gsm_bts_trx *trx, uint32_t hLayer2)
 	return &lchan->lapdm_ch;
 }
 
-enum lchan_ciph_state {
-	LCHAN_CIPH_NONE,
-	LCHAN_CIPH_RX_REQ,
-	LCHAN_CIPH_RX_CONF,
-	LCHAN_CIPH_TXRX_REQ,
-	LCHAN_CIPH_TXRX_CONF,
-};
-
 /* check if the message is a GSM48_MT_RR_CIPH_M_CMD, and if yes, enable
  * uni-directional de-cryption on the uplink. We need this ugly layering
  * violation as we have no way of passing down L3 metadata (RSL CIPHERING CMD)
@@ -316,6 +308,16 @@ enum lchan_ciph_state {
 static int check_for_ciph_cmd(struct femtol1_hdl *fl1h,
 			      struct msgb *msg, struct gsm_lchan *lchan)
 {
+
+	/* only do this if we are in the right state */
+	switch (lchan->ciph_state) {
+	case LCHAN_CIPH_NONE:
+	case LCHAN_CIPH_RX_REQ:
+		break;
+	default:
+		return 0;
+	}
+
 	/* First byte (Address Field) of LAPDm header) */
 	if (msg->data[0] != 0x03)
 		return 0;
@@ -621,10 +623,15 @@ static int handle_ph_data_ind(struct femtol1_hdl *fl1, GsmL1_PhDataInd_t *data_i
 	case GsmL1_Sapi_FacchH:
 		/* if this is the first valid message after enabling Rx
 		 * decryption, we have to enable Tx encryption */
-		if (lchan->ciph_state == LCHAN_CIPH_RX_REQ ||
-		    lchan->ciph_state == LCHAN_CIPH_RX_CONF) {
-			l1if_enable_ciphering(fl1, lchan, 1);
-			lchan->ciph_state = LCHAN_CIPH_TXRX_REQ;
+		if (lchan->ciph_state == LCHAN_CIPH_RX_CONF) {
+			/* HACK: check if it's an I frame, in order to
+			 * ignore some still buffered/queued UI frames received
+			 * before decryption was enabled */
+			if (data_ind->msgUnitParam.u8Buffer[0] == 0x01 &&
+			    (data_ind->msgUnitParam.u8Buffer[1] & 0x01) == 0) {
+				l1if_enable_ciphering(fl1, lchan, 1);
+				lchan->ciph_state = LCHAN_CIPH_TXRX_REQ;
+			}
 		}
 
 		/* SDCCH, SACCH and FACCH all go to LAPDm */
diff --git a/src/osmo-bts-sysmo/oml.c b/src/osmo-bts-sysmo/oml.c
index 00c088cd..bb5b9a9a 100644
--- a/src/osmo-bts-sysmo/oml.c
+++ b/src/osmo-bts-sysmo/oml.c
@@ -660,6 +660,8 @@ int lchan_activate(struct gsm_lchan *lchan)
 		/* send the primitive for all GsmL1_Sapi_* that match the LCHAN */
 		l1if_req_compl(fl1h, msg, 0, lchan_act_compl_cb, lchan);
 
+		/* FIXME: check if encryption parameters are present, and issue
+		 * MPH-CONFIG.req */
 	}
 	lchan->state = LCHAN_S_ACT_REQ;
 
@@ -728,6 +730,20 @@ static int chmod_modif_compl_cb(struct msgb *l1_msg, void *data)
 	case GsmL1_ConfigParamId_SetNbTsc:
 	case GsmL1_ConfigParamId_SetTxPowerLevel:
 	case GsmL1_ConfigParamId_SetCipheringParams:
+		switch (lchan->ciph_state) {
+		case LCHAN_CIPH_RX_REQ:
+			LOGPC(DL1C, LOGL_INFO, "RX_REQ -> RX_CONF\n");
+			lchan->ciph_state = LCHAN_CIPH_RX_CONF;
+			break;
+		case LCHAN_CIPH_TXRX_REQ:
+			LOGPC(DL1C, LOGL_INFO, "TX_REQ -> TX_CONF\n");
+			lchan->ciph_state = LCHAN_CIPH_TXRX_CONF;
+			break;
+		default:
+			LOGPC(DL1C, LOGL_INFO, "unhandled state %u\n", lchan->ciph_state);
+			break;
+		}
+		break;
 	default:
 		LOGPC(DL1C, LOGL_INFO, "\n");
 		break;
@@ -791,7 +807,7 @@ int l1if_enable_ciphering(struct femtol1_hdl *fl1h,
 	struct msgb *msg = l1p_msgb_alloc();
 	struct GsmL1_MphConfigReq_t *cfgr;
 
-	LOGP(DL1C, LOGL_DEBUG, "%s enable_ciphering(dir_downlink=%u)\n",
+	LOGP(DL1C, LOGL_NOTICE, "%s enable_ciphering(dir_downlink=%u)\n",
 		gsm_lchan_name(lchan), dir_downlink);
 
 	cfgr = prim_init(msgb_l1prim(msg), GsmL1_PrimId_MphConfigReq, fl1h);
@@ -893,6 +909,7 @@ int lchan_deactivate(struct gsm_lchan *lchan)
 
 	}
 	lchan->state = LCHAN_S_ACT_REQ;
+	lchan->ciph_state = 0; /* FIXME: do this in common/\*.c */
 
 	return 0;
 }