[MERGED] osmo-ggsn[master]: ggsn: encaps_tun: Avoid forwarding packet if EUA is unassign...

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Fri Jan 26 18:20:24 UTC 2018


Harald Welte has submitted this change and it was merged.

Change subject: ggsn: encaps_tun: Avoid forwarding packet if EUA is unassigned, fix crash
......................................................................


ggsn: encaps_tun: Avoid forwarding packet if EUA is unassigned, fix crash

Check (before forwarding received GTP packets into the tun) if the pdp ctx
associated with the packet requested was assigned an EUA of the given IP version.
This way we avoid for instance forwarding an IPv6 packet (or sending
back a response to a Router Solicitation packet) in case the APN was
configured without IPv6 support or if the MS/SGSN didn't ask for an IPv6
while requesting an EUA.

As a side effect, this commit fixes an OSMO_ASSERT hit introduced in handle_router_mcast
in 2d6a69e69a4b4cb2b8cc63c4810dae44e5a4d8f6 due to a deffective MS
sending an icmpv6 Router Solicitation over IPv6 after having been
requesting and assigned an IPv4 EUA (so no IPv6 packets expected).
Before that commit, there was no crash but the message was being wrongly
answered and used an uninitialized .v6 addr field from the peer struct.

Fixes: OS#2843

Change-Id: Ib6d18a64c2b71f3bcf6cb7e3a978d2d3f9c7a79b
---
M ggsn/ggsn.c
M ggsn/icmpv6.c
M ggsn/icmpv6.h
3 files changed, 47 insertions(+), 13 deletions(-)

Approvals:
  Max: Looks good to me, but someone else must approve
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index 3e095f0..7954d5e 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -425,6 +425,30 @@
 	return false;
 }
 
+/*! Get the peer of pdp based on IP version used.
+ *  \param[in] pdp PDP context to select the peer from.
+ *  \param[in] v4v6 IP version to select. Valid values are 4 and 6.
+ *  \returns The selected peer matching the given IP version. NULL if not present.
+ */
+static struct ippoolm_t *pdp_get_peer_ipv(struct pdp_t *pdp, bool is_ipv6) {
+	uint8_t len1, len2, i;
+
+	if (is_ipv6) {
+		len1 = 8;
+		len2 = 16;
+	} else {
+		len1 = sizeof(struct in_addr);
+		len2 = len1;
+	}
+
+	for (i = 0; i < 2; i++) {
+		struct ippoolm_t * ippool = pdp->peer[i];
+		if (ippool && (ippool->addr.len == len1 || ippool->addr.len == len2))
+			return ippool;
+	}
+	return NULL;
+}
+
 /* determine if PDP context has IPv6 support */
 static bool pdp_has_v4(struct pdp_t *pdp)
 {
@@ -712,6 +736,7 @@
 	struct ip6_hdr *ip6h = (struct ip6_hdr *)pack;
 	struct tun_t *tun = (struct tun_t *)pdp->ipif;
 	struct apn_ctx *apn = tun->priv;
+	struct ippoolm_t *peer;
 
 	OSMO_ASSERT(tun);
 	OSMO_ASSERT(apn);
@@ -720,11 +745,25 @@
 
 	switch (iph->version) {
 	case 6:
+		peer = pdp_get_peer_ipv(pdp, true);
+		if (!peer) {
+			LOGPPDP(LOGL_ERROR, pdp, "Packet from MS IPv6 with unassigned EUA: %s\n",
+				osmo_hexdump(pack, len));
+			return -1;
+		}
+
 		/* daddr: all-routers multicast addr */
 		if (IN6_ARE_ADDR_EQUAL(&ip6h->ip6_dst, &all_router_mcast_addr))
-			return handle_router_mcast(pdp->gsn, pdp, &apn->v6_lladdr, pack, len);
+			return handle_router_mcast(pdp->gsn, pdp, &peer->addr.v6,
+						&apn->v6_lladdr, pack, len);
 		break;
 	case 4:
+		peer = pdp_get_peer_ipv(pdp, false);
+		if (!peer) {
+			LOGPPDP(LOGL_ERROR, pdp, "Packet from MS IPv4 with unassigned EUA: %s\n",
+				osmo_hexdump(pack, len));
+			return -1;
+		}
 		break;
 	default:
 		LOGPPDP(LOGL_ERROR, pdp, "Packet from MS is neither IPv4 nor IPv6: %s\n",
diff --git a/ggsn/icmpv6.c b/ggsn/icmpv6.c
index 6564a54..b7b97eb 100644
--- a/ggsn/icmpv6.c
+++ b/ggsn/icmpv6.c
@@ -180,21 +180,14 @@
 }
 
 /* handle incoming packets to the all-routers multicast address */
-int handle_router_mcast(struct gsn_t *gsn, struct pdp_t *pdp, const struct in6_addr *own_ll_addr,
+int handle_router_mcast(struct gsn_t *gsn, struct pdp_t *pdp,
+			const struct in6_addr *pdp_prefix,
+			const struct in6_addr *own_ll_addr,
 			const uint8_t *pack, unsigned len)
 {
-	struct ippoolm_t *member;
 	const struct ip6_hdr *ip6h = (struct ip6_hdr *)pack;
 	const struct icmpv6_hdr *ic6h = (struct icmpv6_hdr *) (pack + sizeof(*ip6h));
 	struct msgb *msg;
-
-	OSMO_ASSERT(pdp);
-
-	member = pdp->peer[0];
-	OSMO_ASSERT(member);
-	if (member->addr.len == sizeof(struct in_addr)) /* ipv4v6 context */
-		member = pdp->peer[1];
-	OSMO_ASSERT(member);
 
 	if (len < sizeof(*ip6h)) {
 		LOGP(DICMP6, LOGL_NOTICE, "Packet too short: %u bytes\n", len);
@@ -226,7 +219,7 @@
 		/* Send router advertisement from GGSN link-local
 		 * address to MS link-local address, including prefix
 		 * allocated to this PDP context */
-		msg = icmpv6_construct_ra(own_ll_addr, &ip6h->ip6_src, &member->addr.v6);
+		msg = icmpv6_construct_ra(own_ll_addr, &ip6h->ip6_src, pdp_prefix);
 		/* Send the constructed RA to the MS */
 		gtp_data_req(gsn, pdp, msgb_data(msg), msgb_length(msg));
 		msgb_free(msg);
diff --git a/ggsn/icmpv6.h b/ggsn/icmpv6.h
index b6eec63..bf91e27 100644
--- a/ggsn/icmpv6.h
+++ b/ggsn/icmpv6.h
@@ -3,5 +3,7 @@
 #include "../gtp/gtp.h"
 #include "../gtp/pdp.h"
 
-int handle_router_mcast(struct gsn_t *gsn, struct pdp_t *pdp, const struct in6_addr *own_ll_addr,
+int handle_router_mcast(struct gsn_t *gsn, struct pdp_t *pdp,
+			const struct in6_addr *pdp_prefix,
+			const struct in6_addr *own_ll_addr,
 			const uint8_t *pack, unsigned len);

-- 
To view, visit https://gerrit.osmocom.org/6098
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib6d18a64c2b71f3bcf6cb7e3a978d2d3f9c7a79b
Gerrit-PatchSet: 3
Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>



More information about the gerrit-log mailing list