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.

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

paths: Add check for hopfield count <= 64 when deserializing a scion path #4483

Merged
merged 6 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions pkg/slayers/path/scion/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ go_test(
],
deps = [
":go_default_library",
"//pkg/private/serrors:go_default_library",
"//pkg/slayers/path:go_default_library",
"@com_github_stretchr_testify//assert:go_default_library",
"@com_github_stretchr_testify//require:go_default_library",
Expand Down
25 changes: 21 additions & 4 deletions pkg/slayers/path/scion/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,17 @@ import (
"github.com/scionproto/scion/pkg/slayers/path"
)

// MetaLen is the length of the PathMetaHeader.
const MetaLen = 4
const (
// MaxINFs is the maximum number of info fields in a SCION path.
MaxINFs = 3
// MaxHops is the maximum number of hop fields in a SCION path.
MaxHops = 64

const PathType path.Type = 1
// MetaLen is the length of the PathMetaHeader.
MetaLen = 4

PathType path.Type = 1
)

func RegisterPath() {
path.RegisterPath(path.Metadata{
Expand Down Expand Up @@ -66,6 +73,14 @@ func (s *Base) DecodeFromBytes(data []byte) error {
}
s.NumHops += int(s.PathMeta.SegLen[i])
}

// We must check the validity of NumHops. It is possible to fit more than 64 hops in
// the length of a scion header. Yet a path of more than 64 hops cannot be followed to
// the end because CurrHF is only 6 bits long.
if s.NumHops > MaxHops {
return serrors.New("NumHops too large", "NumHops", s.NumHops, "Maximum", MaxHops)
}

return nil
}

Expand Down Expand Up @@ -113,7 +128,9 @@ func (s *Base) infIndexForHF(hf uint8) uint8 {
}
}

// Len returns the length of the path in bytes.
// Len returns the length of the path in bytes. That is, the number of byte required to
// store it, based on the metadata. The actual number of bytes available to contain it
// can be inferred from the common header field HdrLen. It may or may not be consistent.
func (s *Base) Len() int {
return MetaLen + s.NumINF*path.InfoLen + s.NumHops*path.HopLen
}
Expand Down
7 changes: 0 additions & 7 deletions pkg/slayers/path/scion/decoded.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ import (
"github.com/scionproto/scion/pkg/slayers/path"
)

const (
// MaxINFs is the maximum number of info fields in a SCION path.
MaxINFs = 3
// MaxHops is the maximum number of hop fields in a SCION path.
MaxHops = 64
)

// Decoded implements the SCION (data-plane) path type. Decoded is intended to be used in
// non-performance critical code paths, where the convenience of having a fully parsed path trumps
// the loss of performance.
Expand Down
25 changes: 24 additions & 1 deletion pkg/slayers/path/scion/raw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/scionproto/scion/pkg/private/serrors"
"github.com/scionproto/scion/pkg/slayers/path"
"github.com/scionproto/scion/pkg/slayers/path/scion"
)
Expand Down Expand Up @@ -51,6 +52,19 @@ var emptyRawTestPath = &scion.Raw{
Raw: make([]byte, scion.MetaLen),
}

var overlongPath = &scion.Raw{
Base: scion.Base{
PathMeta: scion.MetaHdr{
CurrINF: 0,
CurrHF: 0,
SegLen: [3]uint8{24, 24, 17},
},
NumINF: 3,
NumHops: 65,
},
Raw: rawPath,
}

func TestRawSerialize(t *testing.T) {
b := make([]byte, rawTestPath.Len())
assert.NoError(t, rawTestPath.SerializeTo(b))
Expand All @@ -63,14 +77,23 @@ func TestRawDecodeFromBytes(t *testing.T) {
assert.Equal(t, rawTestPath, s)
}

func TestRawSerliazeDecode(t *testing.T) {
func TestRawSerializeDecode(t *testing.T) {
b := make([]byte, rawTestPath.Len())
assert.NoError(t, rawTestPath.SerializeTo(b))
s := &scion.Raw{}
assert.NoError(t, s.DecodeFromBytes(b))
assert.Equal(t, rawTestPath, s)
}

func TestOverlongSerializeDecode(t *testing.T) {
b := make([]byte, overlongPath.Len())
assert.NoError(t, overlongPath.SerializeTo(b)) // permitted, if only to enable this test.
s := &scion.Raw{}
expected := serrors.New("NumHops too large", "NumHops", 65, "Maximum", scion.MaxHops)
err := s.DecodeFromBytes(b)
assert.Equal(t, expected.Error(), err.Error())
}

func TestRawReverse(t *testing.T) {
for name, tc := range pathReverseCases {
name, tc := name, tc
Expand Down
50 changes: 50 additions & 0 deletions router/dataplane_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (

"github.com/scionproto/scion/pkg/addr"
libepic "github.com/scionproto/scion/pkg/experimental/epic"
"github.com/scionproto/scion/pkg/private/serrors"
"github.com/scionproto/scion/pkg/private/util"
"github.com/scionproto/scion/pkg/private/xtest"
"github.com/scionproto/scion/pkg/scrypto"
Expand Down Expand Up @@ -620,6 +621,55 @@ func TestProcessPkt(t *testing.T) {
egressInterface: 0,
assertFunc: assert.NoError,
},
"inbound_longpath": {
prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
return router.NewDP(fakeExternalInterfaces,
nil, mock_router.NewMockBatchConn(ctrl),
fakeInternalNextHops, nil,
xtest.MustParseIA("1-ff00:0:110"), nil, key)
},
mockMsg: func(afterProcessing bool) *ipv4.Message {
spkt, dpath := prepBaseMsg(now)
spkt.DstIA = xtest.MustParseIA("1-ff00:0:110")
dst := addr.MustParseHost("10.0.100.100")
_ = spkt.SetDstAddr(dst)
dpath.HopFields = []path.HopField{
{ConsIngress: 41, ConsEgress: 40},
{ConsIngress: 31, ConsEgress: 30},
{ConsIngress: 1, ConsEgress: 0},
}

// Everything is the same a in the inbound test, except that we tossed in
// 64 extra hops and two extra segments.
dpath.Base.PathMeta.CurrHF = 2
dpath.Base.PathMeta.SegLen = [3]uint8{24, 24, 17}
dpath.InfoFields = append(
dpath.InfoFields,
path.InfoField{SegID: 0x112, ConsDir: true, Timestamp: util.TimeToSecs(now)},
path.InfoField{SegID: 0x113, ConsDir: true, Timestamp: util.TimeToSecs(now)},
)
dpath.Base.NumINF = 3
dpath.Base.NumHops = 65

dpath.HopFields[2].Mac = computeMAC(t, key, dpath.InfoFields[0], dpath.HopFields[2])
ret := toMsg(t, spkt, dpath)
if afterProcessing {
ret.Addr = &net.UDPAddr{IP: dst.IP().AsSlice(), Port: topology.EndhostPort}
ret.Flags, ret.NN, ret.N, ret.OOB = 0, 0, 0, nil
}
return ret
},
srcInterface: 1,
egressInterface: 0,
assertFunc: func(t assert.TestingT, err error, _ ...interface{}) bool {
if !assert.Error(t, err) {
return false
}
expected := serrors.New("NumHops too large",
"NumHops", 65, "Maximum", scion.MaxHops)
return assert.Equal(t, expected.Error(), err.Error())
},
},
"outbound": {
prepareDP: func(ctrl *gomock.Controller) *router.DataPlane {
return router.NewDP(
Expand Down
Loading