From e561e74bfec8e6d928755766491bf6906c1aa9b9 Mon Sep 17 00:00:00 2001
From: Neels Hofmeyr <nhofmeyr@sysmocom.de>
Date: Thu, 17 Feb 2022 23:10:08 +0100
Subject: rename RSL_ENC_ALG_A5 to ALG_A5_NR_TO_RSL, clarify

The naming confused me so that I wrote buggy code again. Hopefully this
clarifies which representations the code paths are using.

In the macro code, highlight the error case of n <= -1 explicitly.

Also add ALG_A5_NR_TO_PERM_ALG_BITS. I need the 1<<n case in an
upcoming patch.

Related: SYS#5839
Change-Id: I7557ae97764bba09c906748a18e9031dfb362611
---
 include/osmocom/bsc/gsm_data.h | 19 ++++++++++++++++++-
 src/osmo-bsc/abis_rsl.c        |  4 ++--
 src/osmo-bsc/gsm_04_08_rr.c    |  2 +-
 src/osmo-bsc/osmo_bsc_bssap.c  |  2 +-
 4 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 238b2f78a..7657a5f6a 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -506,7 +506,24 @@ struct om2k_mo {
 #define A38_XOR_MIN_KEY_LEN	12
 #define A38_XOR_MAX_KEY_LEN	16
 #define A38_COMP128_KEY_LEN	16
-#define RSL_ENC_ALG_A5(x)	(x+1)
+
+/* There are these representations of A5/n:
+ *
+ * - (uint8_t)(1<<n), either as a single bit, or combined as a list of
+ *   permitted algorithms.
+ *   A5/0 == 0x01, A5/3 == 0x08, none = 0
+ *
+ * - n+1, used on the RSL wire.
+ *   A5/0 == 1, A5/3 == 4, none = 0
+ *
+ * - n, used for human interaction and returned by select_best_cipher().
+ *   A5/0 == 0, A5/3 == 3, none <= -1
+ *
+ * These macros convert from n to the other representations:
+ */
+#define ALG_A5_NR_TO_RSL(A5_N) ((A5_N) >= 0 ? (A5_N)+1 : 0)
+#define ALG_A5_NR_TO_PERM_ALG_BITS(A5_N) ((A5_N) >= 0 ? 1<<(A5_N) : 0)
+
 /* Up to 16 SI2quater are multiplexed; each fits 3 EARFCNS, so the practical maximum is 3*16.
  * The real maximum that fits in a total of 16 SI2quater rest octets also depends on the bits left by other SI2quater
  * rest octets elements, so to really fit 48 EARFCNs most other SI2quater elements need to be omitted. */
diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 402ea27bc..a49cc6529 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -660,7 +660,7 @@ int rsl_tx_chan_activ(struct gsm_lchan *lchan, uint8_t act_type, uint8_t ho_ref)
 	msg->l3h = len + 1;
 	*len = msgb_l3len(msg);
 
-	if (lchan->encr.alg_id > RSL_ENC_ALG_A5(0)) {
+	if (lchan->encr.alg_id > ALG_A5_NR_TO_RSL(0)) {
 		uint8_t encr_info[MAX_A5_KEY_LEN+2];
 		rc = build_encr_info(encr_info, lchan);
 		if (rc > 0)
@@ -764,7 +764,7 @@ int rsl_chan_mode_modify_req(struct gsm_lchan *lchan)
 	msgb_tlv_put(msg, RSL_IE_CHAN_MODE, sizeof(cm),
 		     (uint8_t *) &cm);
 
-	if (lchan->encr.alg_id > RSL_ENC_ALG_A5(0)) {
+	if (lchan->encr.alg_id > ALG_A5_NR_TO_RSL(0)) {
 		uint8_t encr_info[MAX_A5_KEY_LEN+2];
 		rc = build_encr_info(encr_info, lchan);
 		if (rc > 0)
diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c
index 6c0615e80..03b1e98b3 100644
--- a/src/osmo-bsc/gsm_04_08_rr.c
+++ b/src/osmo-bsc/gsm_04_08_rr.c
@@ -374,7 +374,7 @@ int gsm48_send_rr_ciph_mode(struct gsm_lchan *lchan, int want_imeisv)
 
 	DEBUGP(DRR, "TX CIPHERING MODE CMD\n");
 
-	if (lchan->encr.alg_id <= RSL_ENC_ALG_A5(0))
+	if (lchan->encr.alg_id <= ALG_A5_NR_TO_RSL(0))
 		ciph_mod_set = 0;
 	else
 		ciph_mod_set = (lchan->encr.alg_id-2)<<1 | 1;
diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 904da626c..cf8254c01 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -534,7 +534,7 @@ static int bssmap_handle_cipher_mode(struct gsm_subscriber_connection *conn,
 		goto reject;
 	}
 
-	conn->lchan->encr.alg_id = RSL_ENC_ALG_A5(chosen_cipher);
+	conn->lchan->encr.alg_id = ALG_A5_NR_TO_RSL(chosen_cipher);
 	if (enc_key_len) {
 		conn->lchan->encr.key_len = enc_key_len;
 		memcpy(conn->lchan->encr.key, enc_key, enc_key_len);
-- 
cgit v1.2.3