Change in osmo-bsc[master]: rewire build_encr_info() to return errors

[neels]

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24676 )

Change subject: rewire build_encr_info() to return errors
......................................................................

rewire build_encr_info() to return errors

In build_encr_info(), validate the algorithm and key presence and return
negative if errors are encountered.

At all callers, handle the error case.

An upcoming patch will add handling of Kc128 for A5/4 encryption and
also wants to return error codes. This is a preparation for that patch:
I7c458c8a7350f34ff79531b3c891e1b367614469

Notice that osmo-bsc does send the key along even if A5/0 is chosen,
this patch keeps that behavior unchanged.

Related: SYS#5324
Change-Id: I125d8aabceddd5b34cb98978cee9b6d2fc8fd0f2
---
M src/osmo-bsc/abis_rsl.c
1 file changed, 35 insertions(+), 4 deletions(-)

Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, approved



diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 7525e31..c844f18 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -152,13 +152,36 @@
 		memset(out+len, 0x2b, GSM_MACBLOCK_LEN - len);
 }
 
-/* Chapter 9.3.7: Encryption Information */
+/* Chapter 9.3.7: Encryption Information
+ * Return negative on error, number of bytes written to 'out' on success.
+ * '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;
-	if (lchan->encr.key_len)
-		memcpy(out, lchan->encr.key, lchan->encr.key_len);
-	return lchan->encr.key_len + 1;
+	switch (lchan->encr.alg_id) {
+	case GSM0808_ALG_ID_A5_1:
+	case GSM0808_ALG_ID_A5_2:
+	case GSM0808_ALG_ID_A5_3:
+		if (!lchan->encr.key_len) {
+			LOG_LCHAN(lchan, LOGL_ERROR, "A5/%d encryption chosen, but missing Kc\n", lchan->encr.alg_id);
+			return -EINVAL;
+		}
+		/* fall through */
+	case GSM0808_ALG_ID_A5_0:
+		/* When A5/0 is chosen, no encryption is active, so technically, no key is needed. However, 3GPP TS
+		 * 48.058 9.3.7 Encryption Information stays quite silent about presence or absence of a key for A5/0.
+		 * The only thing specified is how to indicate the length of the key; the possibility that the key may
+		 * be zero length is not explicitly mentioned. So it seems that we should always send the key along,
+		 * even for A5/0. Currently our ttcn3 test suite does expect the key to be present also for A5/0, see
+		 * f_cipher_mode() in bsc/MSC_ConnectionHandler.ttcn. */
+		if (lchan->encr.key_len)
+			memcpy(out, lchan->encr.key, lchan->encr.key_len);
+		return 1 + lchan->encr.key_len;
+
+	default:
+		LOG_LCHAN(lchan, LOGL_ERROR, "A5/%d encryption not supported\n", lchan->encr.alg_id);
+		return -EINVAL;
+	}
 }
 
 /* If the TLV contain an RSL Cause IE, return pointer to the cause value. If there is no Cause IE, return
@@ -595,6 +618,10 @@
 		rc = build_encr_info(encr_info, lchan);
 		if (rc > 0)
 			msgb_tlv_put(msg, RSL_IE_ENCR_INFO, rc, encr_info);
+		if (rc < 0) {
+			msgb_free(msg);
+			return rc;
+		}
 	}
 
 	switch (act_type) {
@@ -688,6 +715,10 @@
 		rc = build_encr_info(encr_info, lchan);
 		if (rc > 0)
 			msgb_tlv_put(msg, RSL_IE_ENCR_INFO, rc, encr_info);
+		if (rc < 0) {
+			msgb_free(msg);
+			return rc;
+		}
 	}
 
 	if (gsm48_chan_mode_to_non_vamos(lchan->modify.ch_mode_rate.chan_mode) == GSM48_CMODE_SPEECH_AMR) {

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I125d8aabceddd5b34cb98978cee9b6d2fc8fd0f2
Gerrit-Change-Number: 24676
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210620/9e913237/attachment.htm>