Change in osmo-bsc[master]: rewire build_encr_info() to return errors
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
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-MessageType: merged
Change in osmo-bsc[master]: rewire build_encr_info() to return errors
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24676 ) Change subject: rewire build_encr_info() to return errors .. Patch Set 2: Code-Review+2 -- 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 Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Sat, 19 Jun 2021 09:54:04 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-bsc[master]: rewire build_encr_info() to return errors
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24676 ) Change subject: rewire build_encr_info() to return errors .. Patch Set 2: Code-Review+1 -- 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 Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Sat, 19 Jun 2021 09:36:50 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-bsc[master]: rewire build_encr_info() to return errors
Hello Jenkins Builder, laforge, pespin, dexter, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/24676 to look at the new patch set (#2). 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(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/76/24676/2 -- 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 Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-MessageType: newpatchset
Change in osmo-bsc[master]: rewire build_encr_info() to return errors
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24676 ) Change subject: rewire build_encr_info() to return errors .. Patch Set 1: Code-Review+1 -- 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: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 15 Jun 2021 19:12:54 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-bsc[master]: rewire build_encr_info() to return errors
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24676 ) Change subject: rewire build_encr_info() to return errors .. Patch Set 1: Code-Review+1 -- 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: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 15 Jun 2021 16:32:49 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-bsc[master]: rewire build_encr_info() to return errors
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/24676 ) Change subject: rewire build_encr_info() to return errors .. Patch Set 1: Code-Review+1 -- 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: 1 Gerrit-Owner: neels Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 15 Jun 2021 14:28:59 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in osmo-bsc[master]: rewire build_encr_info() to return errors
neels has uploaded this change for review. (
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
Related: SYS#5324
Change-Id: I125d8aabceddd5b34cb98978cee9b6d2fc8fd0f2
---
M src/osmo-bsc/abis_rsl.c
1 file changed, 28 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/76/24676/1
diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index ac8006d..f7072c9 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -152,13 +152,30 @@
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)
+ switch (lchan->encr.alg_id) {
+ case GSM0808_ALG_ID_A5_0:
+ return 1;
+
+ 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;
+ }
memcpy(out, lchan->encr.key, lchan->encr.key_len);
- return lchan->encr.key_len + 1;
+ 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 +612,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 +709,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: 1
Gerrit-Owner: neels
Gerrit-MessageType: newchange
