diff --git a/protocols/bgp/packet/decoder.go b/protocols/bgp/packet/decoder.go index 09dd330f..a9574bbb 100644 --- a/protocols/bgp/packet/decoder.go +++ b/protocols/bgp/packet/decoder.go @@ -10,6 +10,10 @@ import ( "github.com/pkg/errors" ) +const ( + addPathTupleSize = 4 +) + // Decode decodes a BGP message func Decode(buf *bytes.Buffer, opt *DecodeOptions) (*BGPMessage, error) { hdr, err := decodeHeader(buf) @@ -242,7 +246,7 @@ func decodeCapability(buf *bytes.Buffer) (Capability, error) { } cap.Value = mpCap case AddPathCapabilityCode: - addPathCap, err := decodeAddPathCapability(buf) + addPathCap, err := decodeAddPathCapability(buf, cap.Length) if err != nil { return cap, errors.Wrap(err, "Unable to decode add path capability") } @@ -280,20 +284,29 @@ func decodeMultiProtocolCapability(buf *bytes.Buffer) (MultiProtocolCapability, return mpCap, nil } -func decodeAddPathCapability(buf *bytes.Buffer) (AddPathCapability, error) { - addPathCap := AddPathCapability{} - fields := []interface{}{ - &addPathCap.AFI, - &addPathCap.SAFI, - &addPathCap.SendReceive, +func decodeAddPathCapability(buf *bytes.Buffer, capLength uint8) (AddPathCapability, error) { + addPathCaps := make(AddPathCapability, 0) + + if capLength%addPathTupleSize != 0 { + return nil, fmt.Errorf("Invalid caplength %d, must be multiple of %d", capLength, addPathTupleSize) } - err := decode.Decode(buf, fields) - if err != nil { - return addPathCap, err + for ; capLength >= addPathTupleSize; capLength -= addPathTupleSize { + addPathCap := AddPathCapabilityTuple{} + fields := []interface{}{ + &addPathCap.AFI, + &addPathCap.SAFI, + &addPathCap.SendReceive, + } + err := decode.Decode(buf, fields) + if err != nil { + return nil, err + } + + addPathCaps = append(addPathCaps, addPathCap) } - return addPathCap, nil + return addPathCaps, nil } func decodeASN4Capability(buf *bytes.Buffer) (ASN4Capability, error) { diff --git a/protocols/bgp/packet/decoder_test.go b/protocols/bgp/packet/decoder_test.go index 895597ee..10b8f352 100644 --- a/protocols/bgp/packet/decoder_test.go +++ b/protocols/bgp/packet/decoder_test.go @@ -1522,15 +1522,73 @@ func TestDecodeOptParams(t *testing.T) { Code: 69, Length: 4, Value: AddPathCapability{ - AFI: 1, - SAFI: 1, - SendReceive: 3, + AddPathCapabilityTuple{ + AFI: 1, + SAFI: 1, + SendReceive: 3, + }, + }, + }, + }, + }, + }, + }, + { + name: "Add path capability with multiple entries", + input: []byte{ + 2, // Type + 6, // Length + 69, // Code + 8, // Length + 0, 1, // AFI + 1, // SAFI + 3, // Send/Receive + + 0, 2, // AFI + 1, // SAFI + 3, // Send/Receive + }, + wantFail: false, + expected: []OptParam{ + { + Type: 2, + Length: 6, + Value: Capabilities{ + { + Code: 69, + Length: 8, + Value: AddPathCapability{ + AddPathCapabilityTuple{ + AFI: 1, + SAFI: 1, + SendReceive: 3, + }, + AddPathCapabilityTuple{ + AFI: 2, + SAFI: 1, + SendReceive: 3, + }, }, }, }, }, }, }, + { + name: "Add path capability with broken capability length", + input: []byte{ + 2, // Type + 6, // Length + 69, // Code + 5, // Length + 0, 1, // AFI + 1, // SAFI + 3, // Send/Receive + 1, // broken value + }, + wantFail: true, + expected: nil, + }, } for _, test := range tests { @@ -1568,9 +1626,11 @@ func TestDecodeCapability(t *testing.T) { Code: 69, Length: 4, Value: AddPathCapability{ - AFI: 1, - SAFI: 1, - SendReceive: 3, + AddPathCapabilityTuple{ + AFI: 1, + SAFI: 1, + SendReceive: 3, + }, }, }, wantFail: false, @@ -1627,9 +1687,11 @@ func TestDecodeAddPathCapability(t *testing.T) { input: []byte{0, 1, 1, 3}, wantFail: false, expected: AddPathCapability{ - AFI: 1, - SAFI: 1, - SendReceive: 3, + AddPathCapabilityTuple{ + AFI: 1, + SAFI: 1, + SendReceive: 3, + }, }, }, { @@ -1641,7 +1703,7 @@ func TestDecodeAddPathCapability(t *testing.T) { for _, test := range tests { buf := bytes.NewBuffer(test.input) - cap, err := decodeAddPathCapability(buf) + cap, err := decodeAddPathCapability(buf, uint8(len(test.input))) if err != nil { if test.wantFail { continue diff --git a/protocols/bgp/packet/encoder_test.go b/protocols/bgp/packet/encoder_test.go index 6358d01b..ba16adbc 100644 --- a/protocols/bgp/packet/encoder_test.go +++ b/protocols/bgp/packet/encoder_test.go @@ -116,9 +116,11 @@ func TestSerializeOptParams(t *testing.T) { Code: 69, Length: 4, Value: AddPathCapability{ - AFI: 1, - SAFI: 1, - SendReceive: 3, + AddPathCapabilityTuple{ + AFI: 1, + SAFI: 1, + SendReceive: 3, + }, }, }, }, diff --git a/protocols/bgp/packet/parameters.go b/protocols/bgp/packet/parameters.go index 42fb0c6c..584edb6e 100644 --- a/protocols/bgp/packet/parameters.go +++ b/protocols/bgp/packet/parameters.go @@ -43,18 +43,26 @@ func (c Capability) serialize(buf *bytes.Buffer) { buf.Write(payload) } -type AddPathCapability struct { +type AddPathCapabilityTuple struct { AFI uint16 SAFI uint8 SendReceive uint8 } -func (a AddPathCapability) serialize(buf *bytes.Buffer) { +func (a AddPathCapabilityTuple) serialize(buf *bytes.Buffer) { buf.Write(convert.Uint16Byte(a.AFI)) buf.WriteByte(a.SAFI) buf.WriteByte(a.SendReceive) } +type AddPathCapability []AddPathCapabilityTuple + +func (a AddPathCapability) serialize(buf *bytes.Buffer) { + for _, cap := range a { + cap.serialize(buf) + } +} + type ASN4Capability struct { ASN4 uint32 } diff --git a/protocols/bgp/server/bmp_router.go b/protocols/bgp/server/bmp_router.go index 4fb1c103..51a91eea 100644 --- a/protocols/bgp/server/bmp_router.go +++ b/protocols/bgp/server/bmp_router.go @@ -375,21 +375,23 @@ func (p *peer) configureBySentOpen(msg *packet.BGPOpen) { switch cap.Code { case packet.AddPathCapabilityCode: addPathCap := cap.Value.(packet.AddPathCapability) - peerFamily := p.addressFamily(addPathCap.AFI, addPathCap.SAFI) - if peerFamily == nil { - continue - } - switch addPathCap.SendReceive { - case packet.AddPathSend: - peerFamily.addPathSend = routingtable.ClientOptions{ - MaxPaths: 10, + for _, addPathCapTuple := range addPathCap { + peerFamily := p.addressFamily(addPathCapTuple.AFI, addPathCapTuple.SAFI) + if peerFamily == nil { + continue } - case packet.AddPathReceive: - peerFamily.addPathReceive = true - case packet.AddPathSendReceive: - peerFamily.addPathReceive = true - peerFamily.addPathSend = routingtable.ClientOptions{ - MaxPaths: 10, + switch addPathCapTuple.SendReceive { + case packet.AddPathSend: + peerFamily.addPathSend = routingtable.ClientOptions{ + MaxPaths: 10, + } + case packet.AddPathReceive: + peerFamily.addPathReceive = true + case packet.AddPathSendReceive: + peerFamily.addPathReceive = true + peerFamily.addPathSend = routingtable.ClientOptions{ + MaxPaths: 10, + } } } } diff --git a/protocols/bgp/server/fsm_open_sent.go b/protocols/bgp/server/fsm_open_sent.go index e7beb883..63b36897 100644 --- a/protocols/bgp/server/fsm_open_sent.go +++ b/protocols/bgp/server/fsm_open_sent.go @@ -203,33 +203,35 @@ func (s *openSentState) processMultiProtocolCapability(cap packet.MultiProtocolC } func (s *openSentState) processAddPathCapability(addPathCap packet.AddPathCapability) { - if addPathCap.SAFI != packet.UnicastSAFI { - return - } + for _, addPathCapTuple := range addPathCap { + if addPathCapTuple.SAFI != packet.UnicastSAFI { + continue + } - f := s.fsm.addressFamily(addPathCap.AFI, addPathCap.SAFI) - if f == nil { - return - } + f := s.fsm.addressFamily(addPathCapTuple.AFI, addPathCapTuple.SAFI) + if f == nil { + continue + } - peerAddressFamily := s.fsm.peer.addressFamily(addPathCap.AFI, addPathCap.SAFI) + peerAddressFamily := s.fsm.peer.addressFamily(addPathCapTuple.AFI, addPathCapTuple.SAFI) - switch addPathCap.SendReceive { - case packet.AddPathReceive: - if !peerAddressFamily.addPathSend.BestOnly { - f.addPathTX = peerAddressFamily.addPathSend - } - case packet.AddPathSend: - if peerAddressFamily.addPathReceive { - f.addPathRX = true - } - case packet.AddPathSendReceive: - if !peerAddressFamily.addPathSend.BestOnly { - f.addPathTX = peerAddressFamily.addPathSend - } + switch addPathCapTuple.SendReceive { + case packet.AddPathReceive: + if !peerAddressFamily.addPathSend.BestOnly { + f.addPathTX = peerAddressFamily.addPathSend + } + case packet.AddPathSend: + if peerAddressFamily.addPathReceive { + f.addPathRX = true + } + case packet.AddPathSendReceive: + if !peerAddressFamily.addPathSend.BestOnly { + f.addPathTX = peerAddressFamily.addPathSend + } - if peerAddressFamily.addPathReceive { - f.addPathRX = true + if peerAddressFamily.addPathReceive { + f.addPathRX = true + } } } } diff --git a/protocols/bgp/server/fsm_open_sent_test.go b/protocols/bgp/server/fsm_open_sent_test.go index 31b33dc7..128e7681 100644 --- a/protocols/bgp/server/fsm_open_sent_test.go +++ b/protocols/bgp/server/fsm_open_sent_test.go @@ -229,9 +229,11 @@ func TestProcessAddPathCapabilityTX(t *testing.T) { }, caps: []packet.AddPathCapability{ { - AFI: packet.IPv4AFI, - SAFI: packet.UnicastSAFI, - SendReceive: packet.AddPathReceive, + packet.AddPathCapabilityTuple{ + AFI: packet.IPv4AFI, + SAFI: packet.UnicastSAFI, + SendReceive: packet.AddPathReceive, + }, }, }, expected: routingtable.ClientOptions{MaxPaths: 3}, @@ -255,9 +257,11 @@ func TestProcessAddPathCapabilityTX(t *testing.T) { }, caps: []packet.AddPathCapability{ { - AFI: packet.IPv4AFI, - SAFI: packet.UnicastSAFI, - SendReceive: packet.AddPathReceive, + packet.AddPathCapabilityTuple{ + AFI: packet.IPv4AFI, + SAFI: packet.UnicastSAFI, + SendReceive: packet.AddPathReceive, + }, }, }, expected: routingtable.ClientOptions{BestOnly: true}, diff --git a/protocols/bgp/server/peer.go b/protocols/bgp/server/peer.go index 8b029fa3..6a298d7b 100644 --- a/protocols/bgp/server/peer.go +++ b/protocols/bgp/server/peer.go @@ -377,9 +377,11 @@ func addPathCapabilityForFamily(f *AddressFamilyConfig, afi uint16, safi uint8) return true, packet.Capability{ Code: packet.AddPathCapabilityCode, Value: packet.AddPathCapability{ - AFI: afi, - SAFI: safi, - SendReceive: addPath, + packet.AddPathCapabilityTuple{ + AFI: afi, + SAFI: safi, + SendReceive: addPath, + }, }, } }