From a0393b7d0674cd318fd355562745d5f72251b03a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= Date: Fri, 28 Jun 2024 13:55:45 +0200 Subject: [PATCH 1/5] strengthen the checks performed in parsePath --- pkg/slayers/path/scion/raw.go | 6 ++++++ router/dataplane.go | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/pkg/slayers/path/scion/raw.go b/pkg/slayers/path/scion/raw.go index 8591093774..c8e9796298 100644 --- a/pkg/slayers/path/scion/raw.go +++ b/pkg/slayers/path/scion/raw.go @@ -172,3 +172,9 @@ func (s *Raw) IsPenultimateHop() bool { func (s *Raw) IsLastHop() bool { return int(s.PathMeta.CurrHF) == (s.NumHops - 1) } + +// CurrINFMatchesCurrHF returns whether the the path's current hopfield +// is in the path's current segment. +func (s *Raw) CurrINFMatchesCurrHF() bool { + return s.PathMeta.CurrINF == s.infIndexForHF(s.PathMeta.CurrHF) +} diff --git a/router/dataplane.go b/router/dataplane.go index 4a307e1b1c..d90484bb1e 100644 --- a/router/dataplane.go +++ b/router/dataplane.go @@ -1294,6 +1294,14 @@ func (p *scionPacketProcessor) parsePath() (processResult, error) { // TODO(lukedirtwalker) parameter problem invalid path? return processResult{}, err } + // All segments without the Peering flag need to consist of at least two HFs. + // (https://github.com/scionproto/scion/issues/4524) + if !p.infoField.Peer && p.path.PathMeta.SegLen[p.path.PathMeta.CurrINF] < 2 { + return processResult{}, malformedPath + } + if !p.path.CurrINFMatchesCurrHF() { + return processResult{}, malformedPath + } return processResult{}, nil } From 2e390eff326b73166844963e4b8bef2724c6988f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= Date: Mon, 8 Jul 2024 23:45:43 +0200 Subject: [PATCH 2/5] check that all segments have at least size 1 --- router/dataplane.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/router/dataplane.go b/router/dataplane.go index af4af5a350..430fd3681e 100644 --- a/router/dataplane.go +++ b/router/dataplane.go @@ -1311,9 +1311,12 @@ func (p *scionPacketProcessor) parsePath() (processResult, error) { // TODO(lukedirtwalker) parameter problem invalid path? return processResult{}, err } - // All segments without the Peering flag need to consist of at least two HFs. - // (https://github.com/scionproto/scion/issues/4524) - if !p.infoField.Peer && p.path.PathMeta.SegLen[p.path.PathMeta.CurrINF] < 2 { + // Segments without the Peering flag must consist of at least two HFs: + // https://github.com/scionproto/scion/issues/4524 + hasSingletonSegment := p.path.PathMeta.SegLen[0] == 1 || + p.path.PathMeta.SegLen[1] == 1 || + p.path.PathMeta.SegLen[2] == 1 + if !p.infoField.Peer && hasSingletonSegment { return processResult{}, malformedPath } if !p.path.CurrINFMatchesCurrHF() { From 29597c795c833010ece8a19feb958db5a3b2b3dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= Date: Mon, 8 Jul 2024 23:59:03 +0200 Subject: [PATCH 3/5] fix test as suggested by matzf --- router/dataplane_test.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/router/dataplane_test.go b/router/dataplane_test.go index 7750bb1861..c315c75a0a 100644 --- a/router/dataplane_test.go +++ b/router/dataplane_test.go @@ -1206,8 +1206,9 @@ func TestProcessPkt(t *testing.T) { dpath := &scion.Decoded{ Base: scion.Base{ PathMeta: scion.MetaHdr{ - CurrHF: 2, - SegLen: [3]uint8{2, 2, 0}, + CurrINF: 0, + CurrHF: 1, + SegLen: [3]uint8{2, 2, 0}, }, NumINF: 2, NumHops: 4, @@ -1219,20 +1220,23 @@ func TestProcessPkt(t *testing.T) { {SegID: 0x222, ConsDir: false, Timestamp: util.TimeToSecs(now)}, }, HopFields: []path.HopField{ - {ConsIngress: 0, ConsEgress: 1}, // IA 110 {ConsIngress: 31, ConsEgress: 0}, // Src - {ConsIngress: 0, ConsEgress: 51}, // Dst + {ConsIngress: 0, ConsEgress: 51}, // IA 110 {ConsIngress: 3, ConsEgress: 0}, // IA 110 + {ConsIngress: 0, ConsEgress: 1}, // Dst }, } - dpath.HopFields[2].Mac = computeMAC(t, key, dpath.InfoFields[0], dpath.HopFields[2]) - dpath.HopFields[3].Mac = computeMAC(t, key, dpath.InfoFields[1], dpath.HopFields[3]) + dpath.HopFields[1].Mac = computeMAC(t, key, dpath.InfoFields[0], dpath.HopFields[1]) + dpath.HopFields[2].Mac = computeMAC(t, key, dpath.InfoFields[1], dpath.HopFields[2]) if !afterProcessing { - dpath.InfoFields[0].UpdateSegID(dpath.HopFields[2].Mac) + dpath.InfoFields[0].UpdateSegID(dpath.HopFields[1].Mac) return toMsg(t, spkt, dpath) } - require.NoError(t, dpath.IncPath()) + + dpath.PathMeta.CurrHF++ + dpath.PathMeta.CurrINF++ + ret := toMsg(t, spkt, dpath) ret.Addr = &net.UDPAddr{IP: net.ParseIP("10.0.200.200").To4(), Port: 30043} ret.Flags, ret.NN, ret.N, ret.OOB = 0, 0, 0, nil From 51424e19c09bddca0f7177ce66aaf13e89b60ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= Date: Wed, 10 Jul 2024 14:12:53 +0200 Subject: [PATCH 4/5] fix typo --- router/dataplane_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/dataplane_test.go b/router/dataplane_test.go index 7cd8616691..b452df8f39 100644 --- a/router/dataplane_test.go +++ b/router/dataplane_test.go @@ -1218,7 +1218,7 @@ func TestProcessPkt(t *testing.T) { dpath.InfoFields[0].UpdateSegID(dpath.HopFields[1].Mac) } - dpath.PathMeta.CurrHF++ + dpath.PathMeta.CurrHF++ dpath.PathMeta.CurrINF++ return router.NewPacket(toBytes(t, spkt, dpath), nil, dstAddr, ingress, egress) From bd65b95ac5680b98a803f93552ed99f74064af77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= Date: Thu, 11 Jul 2024 10:45:56 +0200 Subject: [PATCH 5/5] fix test --- router/dataplane_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/router/dataplane_test.go b/router/dataplane_test.go index b452df8f39..2ea5e06454 100644 --- a/router/dataplane_test.go +++ b/router/dataplane_test.go @@ -1212,15 +1212,13 @@ func TestProcessPkt(t *testing.T) { ingress := uint16(51) // == consEgress, bc non-consdir egress := uint16(0) // Cross-over. The egress happens in the next segment. if afterProcessing { - require.NoError(t, dpath.IncPath()) + dpath.PathMeta.CurrHF++ + dpath.PathMeta.CurrINF++ dstAddr = &net.UDPAddr{IP: net.ParseIP("10.0.200.200").To4(), Port: 30043} } else { dpath.InfoFields[0].UpdateSegID(dpath.HopFields[1].Mac) } - dpath.PathMeta.CurrHF++ - dpath.PathMeta.CurrINF++ - return router.NewPacket(toBytes(t, spkt, dpath), nil, dstAddr, ingress, egress) }, assertFunc: notDiscarded,