dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27290 )
Change subject: rename RSL_ENC_ALG_A5 to ALG_A5_NR_TO_RSL, clarify ......................................................................
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 --- M include/osmocom/bsc/gsm_data.h M src/osmo-bsc/abis_rsl.c M src/osmo-bsc/gsm_04_08_rr.c M src/osmo-bsc/osmo_bsc_bssap.c 4 files changed, 22 insertions(+), 5 deletions(-)
Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve dexter: Looks good to me, approved
diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h index 238b2f7..7657a5f 100644 --- a/include/osmocom/bsc/gsm_data.h +++ b/include/osmocom/bsc/gsm_data.h @@ -506,7 +506,24 @@ #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 402ea27..a49cc65 100644 --- a/src/osmo-bsc/abis_rsl.c +++ b/src/osmo-bsc/abis_rsl.c @@ -660,7 +660,7 @@ 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 @@ 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 6c0615e..03b1e98 100644 --- a/src/osmo-bsc/gsm_04_08_rr.c +++ b/src/osmo-bsc/gsm_04_08_rr.c @@ -374,7 +374,7 @@
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 904da62..cf8254c 100644 --- a/src/osmo-bsc/osmo_bsc_bssap.c +++ b/src/osmo-bsc/osmo_bsc_bssap.c @@ -534,7 +534,7 @@ 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);