From eaa8cda558068e6aa964cf918ddc02dbfe609b4b Mon Sep 17 00:00:00 2001 From: Sergey Matov Date: Thu, 23 Mar 2023 16:23:55 +0400 Subject: [PATCH] [wip] Don't allow UE-UE access --- test/e2e/upg_e2e.go | 31 +++++++++++++++++++++++++++++++ upf/upf.h | 24 +----------------------- upf/upf_session_dpo.c | 31 +++++++++++++++++++++++++++++-- 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/test/e2e/upg_e2e.go b/test/e2e/upg_e2e.go index 65c9436..29a12b8 100644 --- a/test/e2e/upg_e2e.go +++ b/test/e2e/upg_e2e.go @@ -845,6 +845,37 @@ var _ = ginkgo.Describe("UPG Binary API", func() { }) }) +var _ = ginkgo.Describe("Accessing SGi interface", func() { + ginkgo.Context("from UE", func() { + f := framework.NewDefaultFramework(framework.UPGModeTDF, framework.UPGIPModeV4) + ginkgo.It("ping should be ignored", func() { + sessionCfg := &framework.SessionConfig{ + IdBase: 1, + UEIP: f.UEIP(), + Mode: f.Mode, + } + anotherSessionCfg := &framework.SessionConfig{ + IdBase: 2, + UEIP: net.ParseIP("10.0.1.2"), + Mode: f.Mode, + } + _, err := f.PFCP.EstablishSession(f.Context, 0, sessionCfg.SessionIEs()...) + framework.ExpectNoError(err) + _, err = f.PFCP.EstablishSession(f.Context, 0, anotherSessionCfg.SessionIEs()...) + framework.ExpectNoError(err) + tg, clientNS, serverNS := newTrafficGen(f, &traffic.ICMPPingConfig{ + //ServerIP: sgiAddress, + ServerIP: net.ParseIP("10.0.1.2"), + PacketCount: 10, // 10s + Retry: true, + Delay: 100 * time.Millisecond, + }, &traffic.SimpleTrafficRec{}) + tg.Start(f.Context, clientNS, serverNS) + // Expect no crash here + }) + }) +}) + var _ = ginkgo.Describe("Clearing message queue", func() { ginkgo.Context("during session deletion", func() { f := framework.NewDefaultFramework(framework.UPGModeTDF, framework.UPGIPModeV4) diff --git a/upf/upf.h b/upf/upf.h index 31f9104..9051060 100644 --- a/upf/upf.h +++ b/upf/upf.h @@ -78,38 +78,16 @@ STATIC_ASSERT (sizeof (upf_buffer_opaque_t) <= ((upf_buffer_opaque_t *)((u8 *)((b)->opaque2) + \ STRUCT_OFFSET_OF (vnet_buffer_opaque_t, unused))) -#if CLIB_DEBUG > 0 - -/* - * For debug builds, we add a flag to each buffer when we initialize - * GTPU metadata when the buffer is processed by one of the UPF - * entry nodes (upf-gtpu[46]-input, upf-ip[46]-session-dpo, - * upf-ip[46]-proxy-server-output) - */ #define UPF_BUFFER_F_GTPU_INITIALIZED VNET_BUFFER_F_AVAIL1 #define UPF_ENTER_SUBGRAPH(b, sidx, is_ip4) \ do { \ - ASSERT (!((b)->flags & UPF_BUFFER_F_GTPU_INITIALIZED)); \ clib_memset(upf_buffer_opaque (b), 0, sizeof(upf_buffer_opaque_t)); \ b->flags |= UPF_BUFFER_F_GTPU_INITIALIZED; \ upf_buffer_opaque (b)->gtpu.session_index = sidx; \ upf_buffer_opaque (b)->gtpu.flags = \ is_ip4 ? BUFFER_GTP_UDP_IP4 : BUFFER_GTP_UDP_IP6; \ } while (0) -#define UPF_CHECK_INNER_NODE(b) ASSERT (b->flags & UPF_BUFFER_F_GTPU_INITIALIZED) - -#else - -#define UPF_ENTER_SUBGRAPH(b, sidx, is_ip4) \ - do { \ - clib_memset(upf_buffer_opaque (b), 0, sizeof(upf_buffer_opaque_t)); \ - upf_buffer_opaque (b)->gtpu.session_index = sidx; \ - upf_buffer_opaque (b)->gtpu.flags = \ - is_ip4 ? BUFFER_GTP_UDP_IP4 : BUFFER_GTP_UDP_IP6; \ - } while (0) -#define UPF_CHECK_INNER_NODE(b) - -#endif +#define UPF_CHECK_INNER_NODE(b) (b->flags & UPF_BUFFER_F_GTPU_INITIALIZED) #define BUFFER_FAR_ONLY (1<<3) /* don't include in QER/URR processing */ #define BUFFER_HAS_GTP_HDR (1<<4) diff --git a/upf/upf_session_dpo.c b/upf/upf_session_dpo.c index 2481c72..da31067 100644 --- a/upf/upf_session_dpo.c +++ b/upf/upf_session_dpo.c @@ -259,8 +259,9 @@ VLIB_INIT_FUNCTION (upf_session_dpo_module_init); #endif /* CLIB_MARCH_VARIANT */ /* Statistics (not all errors) */ -#define foreach_upf_session_dpo_error \ - _(SESSION_DPO, "good packets session_dpo") \ +#define foreach_upf_session_dpo_error \ + _(SESSION_DPO, "good packets session_dpo") \ + _(SUBGRAPH_ENTRY, "packet re-entered subgraph") \ _(PROXY_LOOP, "proxy output loop detected") static char *upf_session_dpo_error_strings[] = { @@ -423,6 +424,19 @@ VLIB_NODE_FN (upf_ip4_session_dpo_node) (vlib_main_t * vm, goto trace; } + /* Traffic hits UPF Subgraph for second time. This might happen if + * packet was originated from UE to another UE or local SGi interface + * and hit DPO of SGi table. By security reasons this is not allowed. + */ + if (UPF_CHECK_INNER_NODE (b)) + { + upf_debug ("UPF Subgraph re-entered: %U", format_ip4_header, + ip0, b->current_length); + error0 = UPF_SESSION_DPO_ERROR_SUBGRAPH_ENTRY; + next = UPF_SESSION_DPO_NEXT_DROP; + goto trace; + } + UPF_ENTER_SUBGRAPH (b, sidx, 1); error0 = IP4_ERROR_NONE; next = UPF_SESSION_DPO_NEXT_FLOW_PROCESS; @@ -556,6 +570,19 @@ VLIB_NODE_FN (upf_ip6_session_dpo_node) (vlib_main_t * vm, goto trace; } + /* Traffic hits UPF Subgraph for second time. This might happen if + * packet was originated from UE to another UE or local SGi interface + * and hit DPO of SGi table. By security reasons this is not allowed. + */ + if (UPF_CHECK_INNER_NODE (b)) + { + upf_debug ("UPF Subgraph re-entered: %U", format_ip4_header, + ip0, b->current_length); + error0 = UPF_SESSION_DPO_ERROR_SUBGRAPH_ENTRY; + next = UPF_SESSION_DPO_NEXT_DROP; + goto trace; + } + UPF_ENTER_SUBGRAPH (b, sidx, 0); error0 = IP6_ERROR_NONE; next = UPF_SESSION_DPO_NEXT_FLOW_PROCESS;