Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

router: strengthen the checks performed by parsePath #4556

Merged
merged 11 commits into from
Jul 11, 2024
Merged

router: strengthen the checks performed by parsePath #4556

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a0393b7
strengthen the checks performed in parsePath
jcp19 Jun 28, 2024
e834b50
Merge branch 'master' into strengthen-parse-path
jcp19 Jun 29, 2024
0ad340e
Merge branch 'master' into strengthen-parse-path
jcp19 Jul 1, 2024
3a12dbc
Merge branch 'master' into strengthen-parse-path
jcp19 Jul 2, 2024
2e390ef
check that all segments have at least size 1
jcp19 Jul 8, 2024
29597c7
fix test as suggested by matzf
jcp19 Jul 8, 2024
f37eb47
Merge branch 'master' into strengthen-parse-path
jcp19 Jul 10, 2024
51424e1
fix typo
jcp19 Jul 10, 2024
e3891ab
Merge branch 'master' into strengthen-parse-path
jcp19 Jul 11, 2024
bd65b95
fix test
jcp19 Jul 11, 2024
fffaaeb
Merge branch 'master' into strengthen-parse-path
matzf Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/slayers/path/scion/raw.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
11 changes: 11 additions & 0 deletions router/dataplane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1359,6 +1359,17 @@ func (p *scionPacketProcessor) parsePath() disposition {
// TODO(lukedirtwalker) parameter problem invalid path?
return errorDiscard("error", err)
}
// 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 errorDiscard("error", malformedPath)
}
if !p.path.CurrINFMatchesCurrHF() {
return errorDiscard("error", malformedPath)
}
return pForward
}

Expand Down
19 changes: 11 additions & 8 deletions router/dataplane_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1185,8 +1185,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,
Expand All @@ -1198,24 +1199,26 @@ 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])

var dstAddr *net.UDPAddr
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[2].Mac)
dpath.InfoFields[0].UpdateSegID(dpath.HopFields[1].Mac)
}

return router.NewPacket(toBytes(t, spkt, dpath), nil, dstAddr, ingress, egress)
},
assertFunc: notDiscarded,
Expand Down
Loading