laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/43237?usp=email )

Change subject: sgs_iface: clean up code in function decode_mme_name
......................................................................

sgs_iface: clean up code in function decode_mme_name

The function decode_mme_name is a bit hard to read and also has
some minor problems we can optimize.

- Do not call TLVP_LEN each time we need the length of the
  TLV IE. Call it once and keep the value in a variable.
- mme_name_enc holds the value part of the TLV IE, we can
  use this variable instead of calling TLVP_VAL all all
  the time.
- When we have copied the value part of the TLV IE using
  memset, let's ensure that the string is terminated.
- Add/fix spec references and improve comments so that it
  is more clear how the decoding actually works.

Related: OS#7058
Change-Id: I9aec8300f15264b68ac8e7805e93e621b12cafb2
---
M src/libmsc/sgs_iface.c
1 file changed, 17 insertions(+), 7 deletions(-)

Approvals:
  laforge: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve
  Jenkins Builder: Verified




diff --git a/src/libmsc/sgs_iface.c b/src/libmsc/sgs_iface.c
index 4425a93..db9246c 100644
--- a/src/libmsc/sgs_iface.c
+++ b/src/libmsc/sgs_iface.c
@@ -162,27 +162,37 @@
 /* Decode and verify MME name */
 static int decode_mme_name(char *mme_name, size_t mme_name_len, const struct 
tlv_parsed *tp)
 {
+       /* The MME name is a fixed length FQDN (e.g. 
mmec00.mmegi0000.mme.epc.mncMNC.mccMCC.3gppnetwork.org,
+        * see also 3GPP TS 23.003, section 19.4.2.1 and 3GPP TS 29.118, 
section 9.4.13 */
        const uint8_t *mme_name_enc = TLVP_VAL_MINLEN(tp, SGSAP_IE_MME_NAME, 
SGS_MME_NAME_LEN);
+       size_t mme_name_enc_len = TLVP_LEN(tp, SGSAP_IE_MME_NAME);
        struct osmo_gummei gummei;

        if (!mme_name_enc)
                return -EINVAL;

-       /* do not accept over-long SGSAP_IE_MME_NAME IEs which would exceed the 
length
+       /* Do not accept over-long SGSAP_IE_MME_NAME IEs which would exceed the 
length
         * of the output buffer. */
-       if (TLVP_LEN(tp, SGSAP_IE_MME_NAME) >= mme_name_len)
+       if (mme_name_enc_len >= mme_name_len)
                return -EINVAL;

-       /* some implementations use FDQN format violating TS 29.118 9.3.14 */
+       /* We attempt to parse the MME name into a GUMMEI to make sure it has 
the form
+        * specified by 3GPP TS 29.118, section 9.4.13. In that case, we may 
copy the
+        * MME name to the output buffer and exit. */
        if (!osmo_parse_mme_domain(&gummei, (const char *) mme_name_enc)) {
-               memcpy(mme_name, mme_name_enc, TLVP_LEN(tp, SGSAP_IE_MME_NAME));
+               memcpy(mme_name, mme_name_enc, mme_name_enc_len);
+               mme_name[mme_name_enc_len] = '\0';
                return 0;
        }

-       /* decode the MME name from DNS labels to string */
-       osmo_apn_to_str(mme_name, TLVP_VAL(tp, SGSAP_IE_MME_NAME), TLVP_LEN(tp, 
SGSAP_IE_MME_NAME));
+       /* In some cases the MME name may be supplied in the domain name format 
specified
+        * by RFC-1035, section 3.1, which is a spec violation we want to 
tolerate. We
+        * may convert the domain name from the LV representtaion into the 
string string
+        * representation required by 3GPP TS 29.118, section 9.4.13 */
+       osmo_apn_to_str(mme_name, mme_name_enc, mme_name_enc_len);

-       /* try to parse the MME name into a GUMMEI as a test for the format */
+       /* To make sure the format conversion has produced a valid MME name, we 
attempt to
+        * parse the output buffer again. */
        if (osmo_parse_mme_domain(&gummei, mme_name) < 0)
                return -EINVAL;


--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/43237?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I9aec8300f15264b68ac8e7805e93e621b12cafb2
Gerrit-Change-Number: 43237
Gerrit-PatchSet: 7
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>

Reply via email to