Merge pull request #5755 from slankdev/bgpd-fix-prefix-sid-parse-error

bgpd: fix Prefix-SID parse error
This commit is contained in:
Quentin Young 2020-03-19 12:28:36 -04:00 committed by GitHub
commit cbbd3b30ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 453 additions and 25 deletions

View File

@ -2382,8 +2382,7 @@ static int bgp_attr_encap(uint8_t type, struct peer *peer, /* IN */
* Returns 0 if there was an error that needs to be passed up the stack * Returns 0 if there was an error that needs to be passed up the stack
*/ */
static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length, static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length,
struct bgp_attr_parser_args *args, struct bgp_attr_parser_args *args)
struct bgp_nlri *mp_update)
{ {
struct peer *const peer = args->peer; struct peer *const peer = args->peer;
struct attr *const attr = args->attr; struct attr *const attr = args->attr;
@ -2421,15 +2420,6 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length,
/* Store label index; subsequently, we'll check on /* Store label index; subsequently, we'll check on
* address-family */ * address-family */
attr->label_index = label_index; attr->label_index = label_index;
/*
* Ignore the Label index attribute unless received for
* labeled-unicast
* SAFI.
*/
if (!mp_update->length
|| mp_update->safi != SAFI_LABELED_UNICAST)
attr->label_index = BGP_INVALID_LABEL_INDEX;
} }
/* Placeholder code for the IPv6 SID type */ /* Placeholder code for the IPv6 SID type */
@ -2628,8 +2618,7 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length,
/* Prefix SID attribute /* Prefix SID attribute
* draft-ietf-idr-bgp-prefix-sid-05 * draft-ietf-idr-bgp-prefix-sid-05
*/ */
bgp_attr_parse_ret_t bgp_attr_prefix_sid(struct bgp_attr_parser_args *args, bgp_attr_parse_ret_t bgp_attr_prefix_sid(struct bgp_attr_parser_args *args)
struct bgp_nlri *mp_update)
{ {
struct peer *const peer = args->peer; struct peer *const peer = args->peer;
struct attr *const attr = args->attr; struct attr *const attr = args->attr;
@ -2640,8 +2629,10 @@ bgp_attr_parse_ret_t bgp_attr_prefix_sid(struct bgp_attr_parser_args *args,
uint8_t type; uint8_t type;
uint16_t length; uint16_t length;
size_t headersz = sizeof(type) + sizeof(length); size_t headersz = sizeof(type) + sizeof(length);
size_t psid_parsed_length = 0;
while (STREAM_READABLE(peer->curr) > 0) { while (STREAM_READABLE(peer->curr) > 0
&& psid_parsed_length < args->length) {
if (STREAM_READABLE(peer->curr) < headersz) { if (STREAM_READABLE(peer->curr) < headersz) {
flog_err( flog_err(
@ -2667,10 +2658,23 @@ bgp_attr_parse_ret_t bgp_attr_prefix_sid(struct bgp_attr_parser_args *args,
args->total); args->total);
} }
ret = bgp_attr_psid_sub(type, length, args, mp_update); ret = bgp_attr_psid_sub(type, length, args);
if (ret != BGP_ATTR_PARSE_PROCEED) if (ret != BGP_ATTR_PARSE_PROCEED)
return ret; return ret;
psid_parsed_length += length + headersz;
if (psid_parsed_length > args->length) {
flog_err(
EC_BGP_ATTR_LEN,
"Malformed Prefix SID attribute - TLV overflow by attribute (need %zu"
" for TLV length, have %zu overflowed in UPDATE)",
length + headersz, psid_parsed_length - (length + headersz));
return bgp_attr_malformed(
args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
} }
return BGP_ATTR_PARSE_PROCEED; return BGP_ATTR_PARSE_PROCEED;
@ -3066,7 +3070,7 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
startp); startp);
break; break;
case BGP_ATTR_PREFIX_SID: case BGP_ATTR_PREFIX_SID:
ret = bgp_attr_prefix_sid(&attr_args, mp_update); ret = bgp_attr_prefix_sid(&attr_args);
break; break;
case BGP_ATTR_PMSI_TUNNEL: case BGP_ATTR_PMSI_TUNNEL:
ret = bgp_attr_pmsi_tunnel(&attr_args); ret = bgp_attr_pmsi_tunnel(&attr_args);
@ -3113,6 +3117,17 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
} }
} }
/*
* draft-ietf-idr-bgp-prefix-sid-27#section-3:
* About Prefix-SID path attribute,
* Label-Index TLV(type1) and The Originator SRGB TLV(type-3)
* may only appear in a BGP Prefix-SID attribute attached to
* IPv4/IPv6 Labeled Unicast prefixes ([RFC8277]).
* It MUST be ignored when received for other BGP AFI/SAFI combinations.
*/
if (!attr->mp_nexthop_len || mp_update->safi != SAFI_LABELED_UNICAST)
attr->label_index = BGP_INVALID_LABEL_INDEX;
/* Check final read pointer is same as end pointer. */ /* Check final read pointer is same as end pointer. */
if (BGP_INPUT_PNT(peer) != endp) { if (BGP_INPUT_PNT(peer) != endp) {
flog_warn(EC_BGP_ATTRIBUTES_MISMATCH, flog_warn(EC_BGP_ATTRIBUTES_MISMATCH,

View File

@ -351,8 +351,7 @@ extern int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
extern int bgp_mp_unreach_parse(struct bgp_attr_parser_args *args, extern int bgp_mp_unreach_parse(struct bgp_attr_parser_args *args,
struct bgp_nlri *); struct bgp_nlri *);
extern bgp_attr_parse_ret_t extern bgp_attr_parse_ret_t
bgp_attr_prefix_sid(struct bgp_attr_parser_args *args, bgp_attr_prefix_sid(struct bgp_attr_parser_args *args);
struct bgp_nlri *mp_update);
extern struct bgp_attr_encap_subtlv * extern struct bgp_attr_encap_subtlv *
encap_tlv_dup(struct bgp_attr_encap_subtlv *orig); encap_tlv_dup(struct bgp_attr_encap_subtlv *orig);

View File

@ -951,12 +951,19 @@ static struct test_segment mp_prefix_sid[] = {
"PREFIX-SID", "PREFIX-SID",
"PREFIX-SID Test 1", "PREFIX-SID Test 1",
{ {
0x01, 0x00, 0x07, /* TLV[0] Latel-Index TLV */
0x00, 0x00, 0x00, 0x00, 0x01, /* Type 0x01:Label-Index */
0x00, 0x00, 0x02, 0x00, 0x07, /* Length */
0x03, 0x00, 0x08, 0x00, 0x00, /* RESERVED */
0x00, 0x0a, 0x1b, 0xfe, 0x00, 0x00, /* Flags */
0x00, 0x00, 0x0a 0x00, 0x00, 0x00, 0x02, /* Label Index */
/* TLV[1] SRGB TLV */
0x03, /* Type 0x03:SRGB */
0x00, 0x08, /* Length */
0x00, 0x00, /* Flags */
0x0a, 0x1b, 0xfe, /* SRGB[0] first label */
0x00, 0x00, 0x0a /* SRBG[0] nb-labels in range */
}, },
.len = 21, .len = 21,
.parses = SHOULD_PARSE, .parses = SHOULD_PARSE,
@ -1027,7 +1034,7 @@ static void parse_test(struct peer *peer, struct test_segment *t, int type)
parse_ret = bgp_mp_unreach_parse(&attr_args, &nlri); parse_ret = bgp_mp_unreach_parse(&attr_args, &nlri);
break; break;
case BGP_ATTR_PREFIX_SID: case BGP_ATTR_PREFIX_SID:
parse_ret = bgp_attr_prefix_sid(&attr_args, &nlri); parse_ret = bgp_attr_prefix_sid(&attr_args);
break; break;
default: default:
printf("unknown type"); printf("unknown type");

View File

@ -0,0 +1,53 @@
[exabgp.api]
encoder = text
highres = false
respawn = false
socket = ''
[exabgp.bgp]
openwait = 60
[exabgp.cache]
attributes = true
nexthops = true
[exabgp.daemon]
daemonize = true
pid = '/var/run/exabgp/exabgp.pid'
user = 'exabgp'
[exabgp.log]
all = false
configuration = true
daemon = true
destination = '/var/log/exabgp.log'
enable = true
level = INFO
message = false
network = true
packets = false
parser = false
processes = true
reactor = true
rib = false
routes = false
short = false
timers = false
[exabgp.pdb]
enable = false
[exabgp.profile]
enable = false
file = ''
[exabgp.reactor]
speed = 1.0
[exabgp.tcp]
acl = false
bind = ''
delay = 0
once = false
port = 179

View File

@ -0,0 +1,103 @@
group controller {
neighbor 10.0.0.1 {
router-id 10.0.0.101;
local-address 10.0.0.101;
local-as 2;
peer-as 1;
family {
ipv4 nlri-mpls;
}
static {
# ref: draft-ietf-idr-bgp-prefix-sid-27
#
# IANA temporarily assigned the following:
# attribute code type (suggested value: 40) to
# the BGP Prefix-SID attribute
#
# 0 1 2 3
# 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | Type | Length | RESERVED |
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | Flags | Label Index |
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | Label Index |
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# Figure. Label-Index TLV (Prefix-SID type-1)
#
# 0 1 2 3
# 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | Type | Length | Flags |
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | Flags |
# +-+-+-+-+-+-+-+-+
#
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | SRGB 1 (6 octets) |
# | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | |
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
#
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | SRGB n (6 octets) |
# | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
# | |
# +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+<Paste>
# Figure. Originator SRGB TLV (Prefix-SID type-3)
# ExaBGP generic-attribute binary pattern:
# Attribute-type: 0x28 (40:BGP_PREFIX_SID)
# Attribute-flag: 0xc0 (Option, Transitive)
# Attribute-body: Label-Index TLV and Originator SRGB TLV
# Label-Index TLV: 0x01000700000000000001
# Type (08bit): 0x01
# Length (16bit): 0x0007
# RESERVED (08bit): 0x00
# Flags (16bit): 0x0000
# Label Index (32bit): 0x00000001
# Originator SRGB TLV: 0x03000800000c350000000a
# Type (08bit): 0x03
# Length (16bit): 0x0008 (nb-SRGB is 1)
# Flags (16bit): 0x0000
# SRGB1 (48bit): 0x0c3500:0x00000a (800000-800010 is SRGB1)
route 3.0.0.1/32 next-hop 10.0.0.101 label [800001] attribute [0x28 0xc0 0x0100070000000000000103000800000c350000000a];
# ExaBGP generic-attribute binary pattern:
# Attribute-type: 0x28 (40:BGP_PREFIX_SID)
# Attribute-flag: 0xc0 (Option, Transitive)
# Attribute-body: Label-Index TLV and Originator SRGB TLV
# Label-Index TLV: 0x01000700000000000001
# Type (08bit): 0x01
# Length (16bit): 0x0007
# RESERVED (08bit): 0x00
# Flags (16bit): 0x0000
# Label Index (32bit): 0x00000002
# Originator SRGB TLV: 0x03000800000c350000000a
# Type (08bit): 0x03
# Length (16bit): 0x0008 (nb-SRGB is 1)
# Flags (16bit): 0x0000
# SRGB1 (48bit): 0x0c3500:0x00000a (800000-800010 is SRGB1)
route 3.0.0.2/32 next-hop 10.0.0.101 label [800002] attribute [0x28 0xc0 0x0100070000000000000203000800000c350000000a];
# ExaBGP generic-attribute binary pattern:
# Attribute-type: 0x28 (40:BGP_PREFIX_SID)
# Attribute-flag: 0xc0 (Option, Transitive)
# Attribute-body: Label-Index TLV and Originator SRGB TLV
# Label-Index TLV: 0x01000700000000000001
# Type (08bit): 0x01
# Length (16bit): 0x0007
# RESERVED (08bit): 0x00
# Flags (16bit): 0x0000
# Label Index (32bit): 0x00000003
# Originator SRGB TLV: 0x03000800000c350000000a
# Type (08bit): 0x03
# Length (16bit): 0x0008 (nb-SRGB is 1)
# Flags (16bit): 0x0000
# SRGB1 (48bit): 0x0c3500:0x00000a (800000-800010 is SRGB1)
route 3.0.0.3/32 next-hop 10.0.0.101 label [800003] attribute [0x28 0xc0 0x0100070000000000000303000800000c350000000a];
}
}
}

View File

@ -0,0 +1,37 @@
#!/usr/bin/env python
"""
exa-receive.py: Save received routes form ExaBGP into file
"""
from sys import stdin,argv
from datetime import datetime
# 1st arg is peer number
peer = int(argv[1])
# When the parent dies we are seeing continual newlines, so we only access so many before stopping
counter = 0
routesavefile = open('/tmp/peer%s-received.log' % peer, 'w')
while True:
try:
line = stdin.readline()
routesavefile.write(line)
routesavefile.flush()
if line == "":
counter += 1
if counter > 100:
break
continue
counter = 0
except KeyboardInterrupt:
pass
except IOError:
# most likely a signal during readline
pass
routesavefile.close()

View File

@ -0,0 +1,19 @@
group controller {
process receive-routes {
run "/etc/exabgp/exa-receive.py 2";
receive-routes;
encoder json;
}
neighbor 10.0.0.1 {
router-id 10.0.0.102;
local-address 10.0.0.102;
local-as 3;
peer-as 1;
family {
ipv4 nlri-mpls;
}
}
}

View File

@ -0,0 +1,15 @@
log stdout notifications
log monitor notifications
log commands
!
router bgp 1
bgp router-id 10.0.0.1
no bgp default ipv4-unicast
neighbor 10.0.0.101 remote-as 2
neighbor 10.0.0.102 remote-as 3
!
address-family ipv4 labeled-unicast
neighbor 10.0.0.101 activate
neighbor 10.0.0.102 activate
exit-address-family
!

View File

@ -0,0 +1,7 @@
hostname r1
!
interface r1-eth0
ip address 10.0.0.1/24
no shutdown
!
line vty

View File

@ -0,0 +1,173 @@
#!/usr/bin/env python
#
# test_bgp_prefix_sid.py
# Part of NetDEF Topology Tests
#
# Copyright (c) 2020 by LINE Corporation
# Copyright (c) 2020 by Hiroki Shirokura <slank.dev@gmail.com>
#
# Permission to use, copy, modify, and/or distribute this software
# for any purpose with or without fee is hereby granted, provided
# that the above copyright notice and this permission notice appear
# in all copies.
#
# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES
# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR
# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
# OF THIS SOFTWARE.
#
"""
test_bgp_prefix_sid.py: Test BGP topology with EBGP on prefix-sid
"""
import json
import os
import sys
import functools
import pytest
CWD = os.path.dirname(os.path.realpath(__file__))
sys.path.append(os.path.join(CWD, '../'))
# pylint: disable=C0413
from lib import topotest
from lib.topogen import Topogen, TopoRouter, get_topogen
from lib.topolog import logger
from mininet.topo import Topo
class TemplateTopo(Topo):
def build(self, **_opts):
tgen = get_topogen(self)
router = tgen.add_router('r1')
switch = tgen.add_switch('s1')
switch.add_link(router)
switch = tgen.gears['s1']
peer1 = tgen.add_exabgp_peer('peer1', ip='10.0.0.101', defaultRoute='via 10.0.0.1')
peer2 = tgen.add_exabgp_peer('peer2', ip='10.0.0.102', defaultRoute='via 10.0.0.1')
switch.add_link(peer1)
switch.add_link(peer2)
def setup_module(module):
tgen = Topogen(TemplateTopo, module.__name__)
tgen.start_topology()
router = tgen.gears['r1']
router.load_config(TopoRouter.RD_ZEBRA, os.path.join(CWD, '{}/zebra.conf'.format('r1')))
router.load_config(TopoRouter.RD_BGP, os.path.join(CWD, '{}/bgpd.conf'.format('r1')))
router.start()
logger.info('starting exaBGP on peer1')
peer_list = tgen.exabgp_peers()
for pname, peer in peer_list.iteritems():
peer_dir = os.path.join(CWD, pname)
env_file = os.path.join(CWD, 'exabgp.env')
logger.info('Running ExaBGP peer')
peer.start(peer_dir, env_file)
logger.info(pname)
def teardown_module(module):
tgen = get_topogen()
tgen.stop_topology()
def test_r1_receive_and_advertise_prefix_sid_type1():
tgen = get_topogen()
router = tgen.gears['r1']
def _check_type1_r1(router, prefix, remoteLabel, labelIndex):
output = router.vtysh_cmd('show bgp ipv4 labeled-unicast {} json'.format(prefix))
output = json.loads(output)
expected = {
'prefix': prefix,
'advertisedTo': { '10.0.0.101':{}, '10.0.0.102':{} },
'paths': [{
'valid':True,
'remoteLabel': remoteLabel,
'labelIndex': labelIndex,
}]
}
return topotest.json_cmp(output, expected)
test_func = functools.partial(_check_type1_r1, router, '3.0.0.1/32', 800001, 1)
success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
assert result is None, 'Failed _check_type1_r1 in "{}"'.format(router)
test_func = functools.partial(_check_type1_r1, router, '3.0.0.2/32', 800002, 2)
success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
assert result is None, 'Failed _check_type1_r1 in "{}"'.format(router)
def exabgp_get_update_prefix(filename, afi, nexthop, prefix):
with open('/tmp/peer2-received.log') as f:
for line in f.readlines():
output = json.loads(line)
ret = output.get('neighbor')
if ret is None:
continue
ret = ret.get('message')
if ret is None:
continue
ret = ret.get('update')
if ret is None:
continue
ret = ret.get('announce')
if ret is None:
continue
ret = ret.get(afi)
if ret is None:
continue
ret = ret.get(nexthop)
if ret is None:
continue
ret = ret.get(prefix)
if ret is None:
continue
return output
return "Not found"
def test_peer2_receive_prefix_sid_type1():
tgen = get_topogen()
peer2 = tgen.gears['peer2']
def _check_type1_peer2(prefix, labelindex):
output = exabgp_get_update_prefix('/tmp/peer2-received.log', 'ipv4 nlri-mpls', '10.0.0.101', prefix)
expected = {
'type': 'update',
'neighbor': {
'ip': '10.0.0.1',
'message': {
'update': {
'attribute': {
'attribute-0x28-0xE0': '0x010007000000{:08x}'.format(labelindex)
},
'announce': { 'ipv4 nlri-mpls': { '10.0.0.101': {} } }
}
}
}
}
return topotest.json_cmp(output, expected)
test_func = functools.partial(_check_type1_peer2, '3.0.0.1/32', labelindex=1)
success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
assert result is None, 'Failed _check_type1_peer2 in "{}"'.format('peer2')
test_func = functools.partial(_check_type1_peer2, '3.0.0.2/32', labelindex=2)
success, result = topotest.run_and_expect(test_func, None, count=10, wait=0.5)
assert result is None, 'Failed _check_type1_peer2 in "{}"'.format('peer2')
if __name__ == '__main__':
args = ["-s"] + sys.argv[1:]
ret = pytest.main(args)
sys.exit(ret)