fixeria has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/27509 )


Change subject: struct gsm_encr: store alg_id in human-readable format
......................................................................

struct gsm_encr: store alg_id in human-readable format

I find it weird that we store the A5 algorithm ID in a format that
is used on the wire: N + 1 (valid for both A-bis and A interfaces).
What confused me even more is that in some functions we print it
as if it was in a normal, human-readable format.  And this is
also why one can see weird constructions like:

  if (lchan->encr.alg_id > ALG_A5_NR_TO_RSL(0))
      ciph_mod_set = (lchan->encr.alg_id-2)<<1 | 1;

Let's ensure that our internal structures use the A5/N format:

  alg_id=0: A5/0   (0x01 on the A-bis/A interface)
  alg_id=1: A5/1   (0x02 on the A-bis/A interface)
  alg_id=2: A5/2   (0x03 on the A-bis/A interface)
  ...
  alg_id=7: A5/7   (0x08 on the A-bis/A interface)

so that we can print and compare the value alg_id without using
additional arithmetics.  This is how the above snippet changes:

  if (lchan->encr.alg_id > 0)
      ciph_mod_set = (lchan->encr.alg_id << 1) | 1;

Change-Id: Ieb50c9a352cfa5481aebac2379e0a461663543ec
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/assignment_fsm.c
M src/osmo-bsc/gsm_04_08_rr.c
M src/osmo-bsc/gsm_08_08.c
M src/osmo-bsc/handover_fsm.c
M src/osmo-bsc/lchan_fsm.c
M src/osmo-bsc/osmo_bsc_bssap.c
8 files changed, 19 insertions(+), 18 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/09/27509/1

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 4fe97a2..434af5f 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -546,6 +546,7 @@
  * 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_BSSAP(A5_N) ALG_A5_NR_TO_RSL(A5_N)
 #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.
@@ -608,6 +609,7 @@
 #define MAX_A5_KEY_LEN (128/8)

 struct gsm_encr {
+       /* Human readable: 0 (A5/0), 1 (A5/1), ... 7 (A5/7) */
        uint8_t alg_id;
        uint8_t key_len;
        uint8_t key[MAX_A5_KEY_LEN];
diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index a49cc65..b367149 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -159,8 +159,8 @@
  * 'out' must provide room for 17 bytes. */
 static int build_encr_info(uint8_t *out, struct gsm_lchan *lchan)
 {
-       *out++ = lchan->encr.alg_id & 0xff;
-       switch (lchan->encr.alg_id) {
+       *out = ALG_A5_NR_TO_RSL(lchan->encr.alg_id);
+       switch (*out++) {
        case GSM0808_ALG_ID_A5_1:
        case GSM0808_ALG_ID_A5_2:
        case GSM0808_ALG_ID_A5_3:
@@ -660,7 +660,7 @@
        msg->l3h = len + 1;
        *len = msgb_l3len(msg);

-       if (lchan->encr.alg_id > ALG_A5_NR_TO_RSL(0)) {
+       if (lchan->encr.alg_id > 0) {
                uint8_t encr_info[MAX_A5_KEY_LEN+2];
                rc = build_encr_info(encr_info, lchan);
                if (rc > 0)
@@ -764,7 +764,7 @@
        msgb_tlv_put(msg, RSL_IE_CHAN_MODE, sizeof(cm),
                     (uint8_t *) &cm);

-       if (lchan->encr.alg_id > ALG_A5_NR_TO_RSL(0)) {
+       if (lchan->encr.alg_id > 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/assignment_fsm.c b/src/osmo-bsc/assignment_fsm.c
index 0243058..2b8b8b0 100644
--- a/src/osmo-bsc/assignment_fsm.c
+++ b/src/osmo-bsc/assignment_fsm.c
@@ -226,7 +226,7 @@

        resp = gsm0808_create_ass_compl2(lchan->abis_ip.ass_compl.rr_cause,
                                         chosen_channel,
-                                        lchan->encr.alg_id, perm_spch,
+                                        
ALG_A5_NR_TO_BSSAP(lchan->encr.alg_id), perm_spch,
                                         addr_local_p, sc_ptr, NULL, 
lcls_get_status(conn));

        if (!resp) {
diff --git a/src/osmo-bsc/gsm_04_08_rr.c b/src/osmo-bsc/gsm_04_08_rr.c
index 560eb16..2232762 100644
--- a/src/osmo-bsc/gsm_04_08_rr.c
+++ b/src/osmo-bsc/gsm_04_08_rr.c
@@ -374,10 +374,10 @@

        DEBUGP(DRR, "TX CIPHERING MODE CMD\n");

-       if (lchan->encr.alg_id <= ALG_A5_NR_TO_RSL(0))
-               ciph_mod_set = 0;
+       if (lchan->encr.alg_id > 0)
+               ciph_mod_set = (lchan->encr.alg_id << 1) | 0x01;
        else
-               ciph_mod_set = (lchan->encr.alg_id-2)<<1 | 1;
+               ciph_mod_set = 0x00;

        gh = (struct gsm48_hdr *) msgb_put(msg, sizeof(*gh) + 1);
        gh->proto_discr = GSM48_PDISC_RR;
@@ -592,9 +592,8 @@
         * Shall be included in the case of inter-RAT handover. */
        if (ho_scope & HO_INTER_BSC_IN) {
                uint8_t cms = 0x00;
-               /* This formula copied from gsm48_send_rr_ciph_mode() */
-               if (new_lchan->encr.alg_id > ALG_A5_NR_TO_RSL(0))
-                       cms = (new_lchan->encr.alg_id - 2) << 1 | 1;
+               if (new_lchan->encr.alg_id > 0)
+                       cms = (new_lchan->encr.alg_id << 1) | 1;
                /* T (4 bit) + V (4 bit), see 3GPP TS 44.018, 10.5.2.9 */
                msgb_v_put(msg, GSM48_IE_CIP_MODE_SET | (cms & 0x0f));
        }
diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c
index a99db43..ad64a9c 100644
--- a/src/osmo-bsc/gsm_08_08.c
+++ b/src/osmo-bsc/gsm_08_08.c
@@ -94,7 +94,7 @@
                return;

        LOGP(DMSC, LOGL_DEBUG, "CIPHER MODE COMPLETE from MS, forwarding to 
MSC\n");
-       resp = gsm0808_create_cipher_complete(msg, chosen_encr);
+       resp = gsm0808_create_cipher_complete(msg, 
ALG_A5_NR_TO_BSSAP(chosen_encr));
        rate_ctr_inc(rate_ctr_group_get_ctr(conn->sccp.msc->msc_ctrs, 
MSC_CTR_BSSMAP_TX_DT1_CIPHER_COMPLETE));
        rc = osmo_fsm_inst_dispatch(conn->fi, GSCON_EV_TX_SCCP, resp);
        if (rc != 0)
diff --git a/src/osmo-bsc/handover_fsm.c b/src/osmo-bsc/handover_fsm.c
index 5ae3787..6a12d04 100644
--- a/src/osmo-bsc/handover_fsm.c
+++ b/src/osmo-bsc/handover_fsm.c
@@ -756,7 +756,7 @@
        }

        /* Put encryption info in the chan activation info */
-       info.encr.alg_id = ALG_A5_NR_TO_RSL(chosen_a5_n);
+       info.encr.alg_id = chosen_a5_n;
        if (chosen_a5_n > 0) {
                if (req->ei.key_len > sizeof(info.encr.key)) {
                        ho_fail(HO_RESULT_ERROR, "Encryption Information IE key 
length is too large: %u",
@@ -933,7 +933,7 @@
        ho_perf_params.chosen_channel_present = true;

        /* Chosen Encryption Algorithm 3.2.2.44 */
-       ho_perf_params.chosen_encr_alg = lchan->encr.alg_id;
+       ho_perf_params.chosen_encr_alg = ALG_A5_NR_TO_BSSAP(lchan->encr.alg_id);
        ho_perf_params.chosen_encr_alg_present = true;

        if (ho->new_lchan->activate.info.requires_voice_stream) {
diff --git a/src/osmo-bsc/lchan_fsm.c b/src/osmo-bsc/lchan_fsm.c
index 84f8dc5..3a87d42 100644
--- a/src/osmo-bsc/lchan_fsm.c
+++ b/src/osmo-bsc/lchan_fsm.c
@@ -750,7 +750,7 @@
                        : "none",
                  gsm_lchant_name(lchan->type),
                  gsm48_chan_mode_name(lchan->activate.ch_mode_rate.chan_mode),
-                 (lchan->activate.info.encr.alg_id ? : 1)-1,
+                 lchan->activate.info.encr.alg_id,
                  lchan->activate.info.encr.key_len ? 
osmo_hexdump_nospc(lchan->activate.info.encr.key,
                                                                         
lchan->activate.info.encr.key_len) : "none");

diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index df22a09..ed0b7b8 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -532,7 +532,7 @@
                goto reject;
        }

-       conn->lchan->encr.alg_id = ALG_A5_NR_TO_RSL(chosen_cipher);
+       conn->lchan->encr.alg_id = chosen_cipher;
        if (enc_key_len) {
                conn->lchan->encr.key_len = enc_key_len;
                memcpy(conn->lchan->encr.key, enc_key, enc_key_len);
@@ -1436,7 +1436,7 @@
                .l3_info_len = rr_ho_command->len,
                .chosen_channel_present = true,
                .chosen_channel = gsm0808_chosen_channel(new_lchan->type, 
new_lchan->current_ch_mode_rate.chan_mode),
-               .chosen_encr_alg = new_lchan->encr.alg_id,
+               .chosen_encr_alg = ALG_A5_NR_TO_BSSAP(new_lchan->encr.alg_id),
                .chosen_speech_version = 
gsm0808_permitted_speech(new_lchan->type,
                                                                  
new_lchan->current_ch_mode_rate.chan_mode),
        };
@@ -1506,7 +1506,7 @@

        struct gsm0808_handover_complete params = {
                .chosen_encr_alg_present = true,
-               .chosen_encr_alg = lchan->encr.alg_id,
+               .chosen_encr_alg = ALG_A5_NR_TO_BSSAP(lchan->encr.alg_id),

                .chosen_channel_present = true,
                .chosen_channel = gsm0808_chosen_channel(lchan->type, 
lchan->current_ch_mode_rate.chan_mode),

--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27509
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ieb50c9a352cfa5481aebac2379e0a461663543ec
Gerrit-Change-Number: 27509
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
Gerrit-MessageType: newchange

Reply via email to