gsup: Convert PDP-Type IE to PDP-Address IE

The previous PDP-Type IE should have been a PDP-Address from the
start, since having only PDP-Type with no address is only a specific
case (dynamic addressing).
This becomes clear by looking at other similar protocols like:
* MAP: APN-Configuration IE has servedPartyIP-IP{v4,v6}-Address IEs
* Diameter S6b, 3GPP TS 29.272 7.3.35 APN-Configuration contains
  Served-Party-IP-Address AVPs
* Diameter SWx, 3GPP TS 29.273 APN-Configuration.
* GTPv1C Ts 29.060 7.7.29 PDP Context containing PDP Address.

Since PDP-Type on its own really makes no sense, being it a special case
of PDP-Address, let's keep the IE by renaming it (keeping old name too
for API backward compat) and extend it to support lengths > 2 bytes.

Old implementation of libosmogsm gsup actually ignored lengths > 2
bytes, so we are safe acting against older implementations here, both
on the sending and receiving side on the wire.

The big drawback of this commit is that it breaks ABI compatibility due
to adding "struct osmo_sockaddr pdp_address[2];" to struct
osmo_gsup_pdp_info, which in turn affects shift of fields in struct
osmo_gsup_message. Unfortunately, there's not much that can be done to
improve the situation when adding the missing field, due to existing API
having the same struct for all messages. Ideally we'd have 1 union with
structs per message type inside, this way the ABI break would be far
less pronounced.

The GSUP test output change is becaue we now accept some of the len>2
cases for PDP-Type/Address IE which were being rejected since a couple
commits ago.

libosmogsm gsup code is now disabled in EMBEDDED mode, since it nows
depends on core/socket.h (struct osmo_sockaddr) which is not available
in EMBEDDED, and hence fails during build:
"""
In file included from /build/include/osmocom/gsm/gsup.h:45,
                 from /build/src/gsm/gsup_sms.c:28:
/build/include/osmocom/core/socket.h:15:10: fatal error: arpa/inet.h: No such file or directory
   15 | #include <arpa/inet.h>
      |          ^~~~~~~~~~~~~
"""

Related: OS#6091
Change-Id: I775ff9c3be165d9f30d6ab55d03f99b6104eadd6

6 files changed, 110 insertions, 15 deletions

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 3a0c80c3..a8e812bf 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -17,3 +17,4 @@ isdn      ADD				initial implementation of the V.110 Terminal Adapter
 gsm		ABI change		add T200 timer states to lapdm_datalink
 gsm		ABI change		add UI queue to struct lapdm_datalink
 gsm		ADD			gsup.h: struct osmo_gsup_pdp_info fields pdp_type_nr, pdp_type_org, deprecate pdp_type.
+gsm		ABI change			gsup.h: Add field pdp_address in struct osmo_gsup_pdp_info shifts the struct, and in turn fields in struct osmo_gsup_message.
diff --git a/include/osmocom/gsm/gsup.h b/include/osmocom/gsm/gsup.h
index 49d12648..a17a729c 100644
--- a/include/osmocom/gsm/gsup.h
+++ b/include/osmocom/gsm/gsup.h
@@ -37,11 +37,13 @@
  *
  */
 #pragma once
+#if (!EMBEDDED)
 
 #include <stdint.h>
 #include <osmocom/core/msgb.h>
 #include <osmocom/core/defs.h>
 #include <osmocom/core/endian.h>
+#include <osmocom/core/socket.h>
 #include <osmocom/gsm/gsup_sms.h>
 #include <osmocom/gsm/protocol/gsm_23_003.h>
 #include <osmocom/gsm/protocol/gsm_03_40.h>
@@ -60,8 +62,6 @@
 #define OSMO_GSUP_MAX_MSISDN_LEN		9
 #define OSMO_GSUP_MAX_CALLED_PARTY_BCD_LEN	43 /* TS 24.008 10.5.4.7 */
 
-#define OSMO_GSUP_PDP_TYPE_SIZE			2
-
 /*! Information Element Identifiers for GSUP IEs */
 enum osmo_gsup_iei {
 	OSMO_GSUP_IMSI_IE			= 0x01,
@@ -75,7 +75,8 @@ enum osmo_gsup_iei {
 	OSMO_GSUP_HLR_NUMBER_IE			= 0x09,
 	OSMO_GSUP_MESSAGE_CLASS_IE		= 0x0a,
 	OSMO_GSUP_PDP_CONTEXT_ID_IE		= 0x10,
-	OSMO_GSUP_PDP_TYPE_IE			= 0x11,
+	OSMO_GSUP_PDP_ADDRESS_IE		= 0x11,
+#define OSMO_GSUP_PDP_TYPE_IE			OSMO_GSUP_PDP_ADDRESS_IE /* Backward compat */
 	OSMO_GSUP_ACCESS_POINT_NAME_IE		= 0x12,
 	OSMO_GSUP_PDP_QOS_IE			= 0x13,
 	OSMO_GSUP_CHARG_CHAR_IE			= 0x14,
@@ -275,6 +276,7 @@ struct osmo_gsup_pdp_info {
 #endif
 		};
 	};
+	struct osmo_sockaddr		pdp_address[2];
 	/*! APN information, still in encoded form. Can be NULL if no
 	 * APN information included */
 	const uint8_t			*apn_enc;
@@ -412,4 +414,5 @@ int osmo_gsup_encode(struct msgb *msg, const struct osmo_gsup_message *gsup_msg)
 int osmo_gsup_get_err_msg_type(enum osmo_gsup_message_type type_in)
 	OSMO_DEPRECATED("Use OSMO_GSUP_TO_MSGT_ERROR() instead");
 
+#endif /* (!EMBEDDED) */
 /*! @} */
diff --git a/src/gsm/Makefile.am b/src/gsm/Makefile.am
index 67ecf9d8..e6b687d2 100644
--- a/src/gsm/Makefile.am
+++ b/src/gsm/Makefile.am
@@ -33,10 +33,13 @@ libgsmint_la_SOURCES =  a5.c rxlev_stat.c tlv_parser.c comp128.c comp128v23.c \
 			milenage/aes-internal.c milenage/aes-internal-enc.c \
 			milenage/milenage.c gan.c ipa.c gsm0341.c apn.c \
 			tuak/KeccakP-1600-3gpp.c tuak/tuak.c auth_tuak.c \
-			gsup.c gsup_sms.c gprs_gea.c gsm0503_conv.c oap.c gsm0808_utils.c \
+			gprs_gea.c gsm0503_conv.c oap.c gsm0808_utils.c \
 			gsm23003.c gsm23236.c mncc.c bts_features.c oap_client.c \
 			gsm29118.c gsm48_rest_octets.c cbsp.c gsm48049.c \
 			gad.c bsslap.c bssmap_le.c kdf.c iuup.c gsm44021.c gsm44068.c rlp.c
+if !EMBEDDED
+libgsmint_la_SOURCES += gsup.c gsup_sms.c
+endif # !EMBEDDED
 
 libgsmint_la_LDFLAGS = -no-undefined
 libgsmint_la_LIBADD = $(top_builddir)/src/core/libosmocore.la $(top_builddir)/src/isdn/libosmoisdn.la
diff --git a/src/gsm/gsup.c b/src/gsm/gsup.c
index 00a9a60a..3daf75c8 100644
--- a/src/gsm/gsup.c
+++ b/src/gsm/gsup.c
@@ -126,6 +126,62 @@ int osmo_gsup_get_err_msg_type(enum osmo_gsup_message_type type_in)
 	return OSMO_GSUP_TO_MSGT_ERROR(type_in);
 }
 
+static int decode_pdp_address(const uint8_t *data, size_t data_len, struct osmo_gsup_pdp_info *pdp_info)
+{
+	const struct gsm48_pdp_address *pdp_addr = (const struct gsm48_pdp_address *)data;
+	/* Explicitly pre-nitialize them to AF_UNSPEC to signal they are empty: */
+	pdp_info->pdp_address[0].u.sa.sa_family = AF_UNSPEC;
+	pdp_info->pdp_address[1].u.sa.sa_family = AF_UNSPEC;
+
+	if (data_len < 2)
+		return -GMM_CAUSE_PROTO_ERR_UNSPEC;
+
+	pdp_info->pdp_type_org = pdp_addr->organization;
+	pdp_info->pdp_type_nr = pdp_addr->type;
+
+	if (pdp_info->pdp_type_org != PDP_TYPE_ORG_IETF)
+		return -GMM_CAUSE_PROTO_ERR_UNSPEC;
+
+	/* Skip type-org + type-nr for easy calculations below: */
+	data_len -= 2;
+
+	switch (pdp_info->pdp_type_nr) {
+	case PDP_TYPE_N_IETF_IPv4:
+		if (data_len == 0)
+			return 0; /* empty, marked as AF_UNSET. */
+		if (data_len != sizeof(pdp_addr->ietf.v4))
+			return -GMM_CAUSE_PROTO_ERR_UNSPEC;
+		pdp_info->pdp_address[0].u.sa.sa_family = AF_INET;
+		pdp_info->pdp_address[0].u.sin.sin_addr.s_addr = pdp_addr->ietf.v4;
+		return 0;
+	case PDP_TYPE_N_IETF_IPv6:
+		if (data_len == 0)
+			return 0; /* empty, marked as AF_UNSET. */
+		if (data_len != sizeof(pdp_addr->ietf.v6))
+			return -GMM_CAUSE_PROTO_ERR_UNSPEC;
+		pdp_info->pdp_address[0].u.sa.sa_family = AF_INET6;
+		memcpy(&pdp_info->pdp_address[0].u.sin6.sin6_addr,
+			pdp_addr->ietf.v6,
+			sizeof(pdp_addr->ietf.v6));
+		return 0;
+	case PDP_TYPE_N_IETF_IPv4v6:
+		if (data_len == 0)
+			return 0; /* empty, marked as AF_UNSET. */
+		if (data_len != sizeof(pdp_addr->ietf.v4v6))
+			return -GMM_CAUSE_PROTO_ERR_UNSPEC;
+		pdp_info->pdp_address[0].u.sa.sa_family = AF_INET;
+		pdp_info->pdp_address[0].u.sin.sin_addr.s_addr = pdp_addr->ietf.v4v6.v4;
+		pdp_info->pdp_address[1].u.sa.sa_family = AF_INET6;
+		memcpy(&pdp_info->pdp_address[1].u.sin6.sin6_addr,
+			pdp_addr->ietf.v4v6.v6,
+			sizeof(pdp_addr->ietf.v4v6.v6));
+		return 0;
+	default:
+		/* reserved, both pdp_info->pdp_address are preinitialied to AF_UNSET. */
+		return 0;
+	}
+}
+
 static int decode_pdp_info(uint8_t *data, size_t data_len,
 			  struct osmo_gsup_pdp_info *pdp_info)
 {
@@ -149,11 +205,9 @@ static int decode_pdp_info(uint8_t *data, size_t data_len,
 			pdp_info->context_id = osmo_decode_big_endian(value, value_len);
 			break;
 
-		case OSMO_GSUP_PDP_TYPE_IE:
-			if (value_len < 2)
-				return -GMM_CAUSE_PROTO_ERR_UNSPEC;
-			pdp_info->pdp_type_org = value[0] & 0x0f;
-			pdp_info->pdp_type_nr = value[1];
+		case OSMO_GSUP_PDP_ADDRESS_IE:
+			if ((rc = decode_pdp_address(value, value_len, pdp_info)) < 0)
+				return rc;
 			break;
 
 		case OSMO_GSUP_ACCESS_POINT_NAME_IE:
@@ -359,7 +413,7 @@ int osmo_gsup_decode(const uint8_t *const_data, size_t data_len,
 
 		switch (iei) {
 		case OSMO_GSUP_IMSI_IE:
-		case OSMO_GSUP_PDP_TYPE_IE:
+		case OSMO_GSUP_PDP_ADDRESS_IE:
 		case OSMO_GSUP_ACCESS_POINT_NAME_IE:
 		case OSMO_GSUP_SRES_IE:
 		case OSMO_GSUP_KC_IE:
@@ -605,12 +659,42 @@ static void encode_pdp_info(struct msgb *msg, enum osmo_gsup_iei iei,
 
 	if (pdp_info->pdp_type_org == PDP_TYPE_ORG_IETF) {
 		struct gsm48_pdp_address pdp_addr;
+		uint8_t pdp_addr_len = 2;
 		pdp_addr.spare = 0x0f;
 		pdp_addr.organization = pdp_info->pdp_type_org;
 		pdp_addr.type = pdp_info->pdp_type_nr;
 
-		msgb_tlv_put(msg, OSMO_GSUP_PDP_TYPE_IE,
-			     OSMO_GSUP_PDP_TYPE_SIZE,
+		switch (pdp_info->pdp_type_nr) {
+		case PDP_TYPE_N_IETF_IPv4:
+			if (pdp_info->pdp_address[0].u.sa.sa_family == AF_INET) {
+				pdp_addr.ietf.v4 = pdp_info->pdp_address[0].u.sin.sin_addr.s_addr;
+				pdp_addr_len += sizeof(pdp_addr.ietf.v4);
+			}
+			break;
+		case PDP_TYPE_N_IETF_IPv6:
+			if (pdp_info->pdp_address[0].u.sa.sa_family == AF_INET6) {
+				memcpy(pdp_addr.ietf.v6,
+				       &pdp_info->pdp_address[0].u.sin6.sin6_addr,
+				       sizeof(pdp_addr.ietf.v6));
+				pdp_addr_len += sizeof(pdp_addr.ietf.v6);
+			}
+			break;
+		case PDP_TYPE_N_IETF_IPv4v6:
+			if (pdp_info->pdp_address[0].u.sa.sa_family == AF_INET) {
+				pdp_addr.ietf.v4v6.v4 = pdp_info->pdp_address[0].u.sin.sin_addr.s_addr;
+				pdp_addr_len += sizeof(pdp_addr.ietf.v4v6.v4);
+			}
+			if (pdp_info->pdp_address[0].u.sa.sa_family == AF_INET6) {
+				memcpy(pdp_addr.ietf.v4v6.v6,
+				       &pdp_info->pdp_address[1].u.sin6.sin6_addr,
+				       sizeof(pdp_addr.ietf.v4v6.v6));
+				pdp_addr_len += sizeof(pdp_addr.ietf.v4v6.v6);
+			}
+			break;
+		}
+
+		msgb_tlv_put(msg, OSMO_GSUP_PDP_ADDRESS_IE,
+			     pdp_addr_len,
 			     (const uint8_t *)&pdp_addr);
 	}
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 153f2e8b..5042b3c4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -20,7 +20,7 @@ check_PROGRAMS = timer/timer_test sms/sms_test ussd/ussd_test		\
 		 comp128/comp128_test                         		\
 		 bitvec/bitvec_test msgb/msgb_test bits/bitcomp_test	\
 		 bits/bitfield_test					\
-		 tlv/tlv_test gsup/gsup_test oap/oap_test		\
+		 tlv/tlv_test oap/oap_test				\
 		 write_queue/wqueue_test socket/socket_test		\
 		 coding/coding_test conv/conv_gsm0503_test		\
 		 abis/abis_test endian/endian_test sercomm/sercomm_test	\
@@ -87,9 +87,11 @@ endif
 
 if !EMBEDDED
 check_PROGRAMS += \
+	gsup/gsup_test \
 	stats/stats_test \
 	stats/stats_vty_test \
-	exec/exec_test
+	exec/exec_test \
+	$(NULL)
 endif
 
 if ENABLE_GB
@@ -647,9 +649,11 @@ endif
 		>$(srcdir)/timer/clk_override_test.ok
 	tlv/tlv_test \
 		>$(srcdir)/tlv/tlv_test.ok
+if !EMBEDDED
 	gsup/gsup_test \
 		>$(srcdir)/gsup/gsup_test.ok \
 		2>$(srcdir)/gsup/gsup_test.err
+endif
 if ENABLE_CTRL
 	fsm/fsm_test \
 		>$(srcdir)/fsm/fsm_test.ok \
diff --git a/tests/gsup/gsup_test.err b/tests/gsup/gsup_test.err
index fc3c86e2..73a0776b 100644
--- a/tests/gsup/gsup_test.err
+++ b/tests/gsup/gsup_test.err
@@ -164,7 +164,7 @@ DLGSUP Stopping DLGSUP logging
   message 2: tested 21248 modifications, 2577 parse failures
   message 3: tested 2816 modifications, 510 parse failures
   message 4: tested 3584 modifications, 770 parse failures
-  message 5: tested 20736 modifications, 4028 parse failures
+  message 5: tested 20736 modifications, 4517 parse failures
   message 6: tested 3584 modifications, 771 parse failures
   message 7: tested 3584 modifications, 770 parse failures
   message 8: tested 2816 modifications, 510 parse failures