diff options
author | Konrad Zemek <konrad.zemek@gmail.com> | 2021-08-12 16:21:13 +0000 |
---|---|---|
committer | Konrad Zemek <konrad.zemek@gmail.com> | 2021-08-12 16:21:13 +0000 |
commit | 696ff4de8ec2906d00de2a21df40e261af59803a (patch) | |
tree | 2b11bc4d207d2258de13f235c6dd58fce8ca4443 | |
parent | 8fbb5abc7ccef36ebeba6d3ae4f5b03fed19878f (diff) |
Fix extended-nexthop & add-path capability parsing.
The capabilities' DecodeFromBytes() was eating as much data as given,
while these functions would normally be called with all the data
remaining to parse - e.g. including following capabilities.
Fixes osrg/gobgp#2399
-rw-r--r-- | pkg/packet/bgp/bgp.go | 14 | ||||
-rw-r--r-- | pkg/packet/bgp/bgp_test.go | 61 |
2 files changed, 71 insertions, 4 deletions
diff --git a/pkg/packet/bgp/bgp.go b/pkg/packet/bgp/bgp.go index 418e8e63..ac7b77d2 100644 --- a/pkg/packet/bgp/bgp.go +++ b/pkg/packet/bgp/bgp.go @@ -507,11 +507,13 @@ type CapExtendedNexthop struct { func (c *CapExtendedNexthop) DecodeFromBytes(data []byte) error { c.DefaultParameterCapability.DecodeFromBytes(data) data = data[2:] - if len(data) < 6 { + capLen := int(c.CapLen) + if capLen%6 != 0 || capLen < 6 || len(data) < capLen { return NewMessageError(BGP_ERROR_OPEN_MESSAGE_ERROR, BGP_ERROR_SUB_UNSUPPORTED_CAPABILITY, nil, "Not all CapabilityExtendedNexthop bytes available") } + c.Tuples = []*CapExtendedNexthopTuple{} - for len(data) >= 6 { + for capLen >= 6 { t := &CapExtendedNexthopTuple{ binary.BigEndian.Uint16(data[0:2]), binary.BigEndian.Uint16(data[2:4]), @@ -519,6 +521,7 @@ func (c *CapExtendedNexthop) DecodeFromBytes(data []byte) error { } c.Tuples = append(c.Tuples, t) data = data[6:] + capLen -= 6 } return nil } @@ -756,17 +759,20 @@ type CapAddPath struct { func (c *CapAddPath) DecodeFromBytes(data []byte) error { c.DefaultParameterCapability.DecodeFromBytes(data) data = data[2:] - if len(data) < 4 { + capLen := int(c.CapLen) + if capLen%4 != 0 || capLen < 4 || len(data) < capLen { return NewMessageError(BGP_ERROR_OPEN_MESSAGE_ERROR, BGP_ERROR_SUB_UNSUPPORTED_CAPABILITY, nil, "Not all CapabilityAddPath bytes available") } + c.Tuples = []*CapAddPathTuple{} - for len(data) >= 4 { + for capLen >= 4 { t := &CapAddPathTuple{ RouteFamily: AfiSafiToRouteFamily(binary.BigEndian.Uint16(data[:2]), data[2]), Mode: BGPAddPathMode(data[3]), } c.Tuples = append(c.Tuples, t) data = data[4:] + capLen -= 4 } return nil } diff --git a/pkg/packet/bgp/bgp_test.go b/pkg/packet/bgp/bgp_test.go index 0e53293c..fd14aff7 100644 --- a/pkg/packet/bgp/bgp_test.go +++ b/pkg/packet/bgp/bgp_test.go @@ -3230,3 +3230,64 @@ func Test_PathAttributeLs(t *testing.T) { } } } + +func Test_BGPOpenDecodeCapabilities(t *testing.T) { + // BGP OPEN message with add-path and long-lived-graceful-restart capabilities, + // in that order. + openBytes := []byte{ + 0x04, // version: 4 + 0xfa, 0x7b, // my as: 64123 + 0x00, 0xf0, // hold time: 240 seconds + 0x7f, 0x00, 0x00, 0x02, // BGP identifier: 127.0.0.2 + 0x19, // optional parameters length: 25 + 0x02, // parameter type: capability + 0x17, // parameter length: 23 + + 0x05, // capability type: extended next hop + 0x06, // caability length: 6 + 0x00, 0x01, // AFI: IPv4 + 0x00, 0x01, // SAFI: unicast + 0x00, 0x02, // next hop AFI: IPv6 + + 0x45, // capability type: ADD-PATH + 0x04, // capability length: 4 + 0x00, 0x01, // AFI: IPv4 + 0x01, // SAFI: unicast + 0x02, // Send/Receive: Send + + 0x47, // capability type: Long-lived-graceful-restart + 0x07, // capability length: 7 + 0x00, 0x01, 0x01, 0x00, 0x00, 0x0e, 0x10, + } + + open := &BGPOpen{} + err := open.DecodeFromBytes(openBytes) + assert.NoError(t, err) + + capMap := make(map[BGPCapabilityCode][]ParameterCapabilityInterface) + for _, p := range open.OptParams { + if paramCap, y := p.(*OptionParameterCapability); y { + t.Logf("parameter capability: %+v", paramCap) + for _, c := range paramCap.Capability { + m, ok := capMap[c.Code()] + if !ok { + m = make([]ParameterCapabilityInterface, 0, 1) + } + capMap[c.Code()] = append(m, c) + } + } + } + + assert.Len(t, capMap[BGP_CAP_EXTENDED_NEXTHOP], 1) + nexthopTuples := capMap[BGP_CAP_EXTENDED_NEXTHOP][0].(*CapExtendedNexthop).Tuples + assert.Len(t, nexthopTuples, 1) + assert.Equal(t, nexthopTuples[0].NLRIAFI, uint16(AFI_IP)) + assert.Equal(t, nexthopTuples[0].NLRISAFI, uint16(SAFI_UNICAST)) + assert.Equal(t, nexthopTuples[0].NexthopAFI, uint16(AFI_IP6)) + + assert.Len(t, capMap[BGP_CAP_ADD_PATH], 1) + tuples := capMap[BGP_CAP_ADD_PATH][0].(*CapAddPath).Tuples + assert.Len(t, tuples, 1) + assert.Equal(t, tuples[0].RouteFamily, RF_IPv4_UC) + assert.Equal(t, tuples[0].Mode, BGP_ADD_PATH_SEND) +} |