Re: [bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-28 Thread Job Snijders
On Fri, May 26, 2017 at 09:40:49PM +0200, Peter Hessler wrote:
> On 2017 May 26 (Fri) at 20:01:00 +0200 (+0200), Peter Hessler wrote:
> :Apropos of "I found it", I implemented support for RFC 7607.  It's a
> :super short RFC, but basically it forbids use of AS 0 anywhere.
> :
> :OK?
>
> Fixed some denglish in an error message, mention the RFC in the man
> page, and don't take down the session if we receive AS0 in the path.

Indeed, the better behaviour is to apply 'Treat-as-withdraw' to a BGP
UPDATE which contains AS 0 anywhere in the AS_PATH/AS4_PATH.

Kind regards,

Job



Re: [bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-26 Thread Jason McIntyre
On Fri, May 26, 2017 at 10:15:37PM +0200, Sebastian Benoit wrote:
> Peter Hessler(phess...@openbsd.org) on 2017.05.26 21:40:49 +0200:
> > On 2017 May 26 (Fri) at 20:01:00 +0200 (+0200), Peter Hessler wrote:
> > :Apropos of "I found it", I implemented support for RFC 7607.  It's a
> > :super short RFC, but basically it forbids use of AS 0 anywhere.
> > :
> > :OK?
> > :
> > :
> > 
> > Fixed some denglish in an error message, mention the RFC in the man
> > page, and don't take down the session if we receive AS0 in the path.
> > 
> > 
> > Index: bgpd.8
> > ===
> > RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.8,v
> > retrieving revision 1.52
> > diff -u -p -u -p -r1.52 bgpd.8
> > --- bgpd.8  19 Feb 2017 11:38:24 -  1.52
> > +++ bgpd.8  26 May 2017 18:29:49 -
> > @@ -357,6 +357,16 @@ control socket
> >  .Re
> >  .Pp
> >  .Rs
> > +.%A W. Kumari
> > +.%A R. Bush
> > +.%A H. Schiller
> > +.%A K. Patel
> > +.%D August 2015
> > +.%R RFC 7607
> > +.%T Codification of AS 0 Processing
> 
> diff is ok, but please consider this:
> 
> i think we should limit the list to the features we support so
> that users can check if a certain something should work or not.
> 
> this is not a feature, this is a protocol clarification/stability issue.
> 
> otherwise the list gets longer and useless.
> 
> if you leave this out, put a /* rfc 7607 */ comment next to the
> aspath_extract() below.
> 

longer, but not neccesarily useless. if you try and pick a subset of
rfcs to mention, no one will guess their relevance. what does it cost?
people uninterested skip the section anyway.

why leave people wondering whether such and such is implemented>

jmc



Re: [bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-26 Thread Peter Hessler
On 2017 May 26 (Fri) at 22:15:37 +0200 (+0200), Sebastian Benoit wrote:
:diff is ok, but please consider this:
:
:i think we should limit the list to the features we support so
:that users can check if a certain something should work or not.
:
:this is not a feature, this is a protocol clarification/stability issue.
:

Listing the RFC in the man page was requested by jmc@.

I don't have a strong feeling either way.

But, if we prefer not to list 7607, we should also not list 7606 as it
is clarification.


:otherwise the list gets longer and useless.
:
:if you leave this out, put a /* rfc 7607 */ comment next to the
:aspath_extract() below.
:

I added it, because I think it should have that even if the man page part
stays.


-- 
Money is the root of all evil, and man needs roots.



Re: [bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-26 Thread Sebastian Benoit
Peter Hessler(phess...@openbsd.org) on 2017.05.26 21:40:49 +0200:
> On 2017 May 26 (Fri) at 20:01:00 +0200 (+0200), Peter Hessler wrote:
> :Apropos of "I found it", I implemented support for RFC 7607.  It's a
> :super short RFC, but basically it forbids use of AS 0 anywhere.
> :
> :OK?
> :
> :
> 
> Fixed some denglish in an error message, mention the RFC in the man
> page, and don't take down the session if we receive AS0 in the path.
> 
> 
> Index: bgpd.8
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.8,v
> retrieving revision 1.52
> diff -u -p -u -p -r1.52 bgpd.8
> --- bgpd.819 Feb 2017 11:38:24 -  1.52
> +++ bgpd.826 May 2017 18:29:49 -
> @@ -357,6 +357,16 @@ control socket
>  .Re
>  .Pp
>  .Rs
> +.%A W. Kumari
> +.%A R. Bush
> +.%A H. Schiller
> +.%A K. Patel
> +.%D August 2015
> +.%R RFC 7607
> +.%T Codification of AS 0 Processing

diff is ok, but please consider this:

i think we should limit the list to the features we support so
that users can check if a certain something should work or not.

this is not a feature, this is a protocol clarification/stability issue.

otherwise the list gets longer and useless.

if you leave this out, put a /* rfc 7607 */ comment next to the
aspath_extract() below.

> +.Re
> +.Pp
> +.Rs
>  .%D August 2011
>  .%R draft-ietf-grow-mrt-17
>  .%T MRT routing information export format
> Index: parse.y
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.300
> diff -u -p -u -p -r1.300 parse.y
> --- parse.y   26 May 2017 14:08:51 -  1.300
> +++ parse.y   26 May 2017 18:15:33 -
> @@ -3661,6 +3661,11 @@ neighbor_consistent(struct peer *p)
>   return (-1);
>   }
>  
> + if (p->conf.remote_as == 0) {
> + yyerror("peer AS may not be zero");
> + return (-1);
> + }
> +
>   /* set default values if they where undefined */
>   p->conf.ebgp = (p->conf.remote_as != conf->as);
>   if (p->conf.announce_type == ANNOUNCE_UNDEF)
> Index: rde_attr.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde_attr.c,v
> retrieving revision 1.97
> diff -u -p -u -p -r1.97 rde_attr.c
> --- rde_attr.c24 Jan 2017 04:22:42 -  1.97
> +++ rde_attr.c26 May 2017 19:29:04 -
> @@ -460,6 +460,9 @@ aspath_verify(void *data, u_int16_t len,
>   if (seg_size == 0)
>   /* empty aspath segments are not allowed */
>   return (AS_ERR_BAD);
> +
> + if (aspath_extract(seg, 0) == 0)
> + return (AS_ERR_BAD);
>   }
>   return (error); /* aspath is valid but probably not loop free */
>  }
> Index: session.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.359
> diff -u -p -u -p -r1.359 session.c
> --- session.c 13 Feb 2017 14:48:44 -  1.359
> +++ session.c 5 May 2017 17:26:16 -
> @@ -2017,6 +2017,14 @@ parse_open(struct peer *peer)
>   memcpy(_as, p, sizeof(short_as));
>   p += sizeof(short_as);
>   as = peer->short_as = ntohs(short_as);
> + if (as == 0) {
> + log_peer_warnx(>conf,
> + "peer requests unacceptable AS %u", as);
> + session_notification(peer, ERR_OPEN, ERR_OPEN_AS,
> + NULL, 0);
> + change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
> + return (-1);
> + }
>  
>   memcpy(, p, sizeof(oholdtime));
>   p += sizeof(oholdtime);
> @@ -2477,6 +2485,14 @@ parse_capabilities(struct peer *peer, u_
>   }
>   memcpy(_as, capa_val, sizeof(remote_as));
>   *as = ntohl(remote_as);
> + if (*as == 0) {
> + log_peer_warnx(>conf,
> + "peer requests unacceptable AS %u", *as);
> + session_notification(peer, ERR_OPEN, 
> ERR_OPEN_AS,
> + NULL, 0);
> + change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
> + return (-1);
> + }
>   peer->capa.peer.as4byte = 1;
>   break;
>   default:
> 
> 
> 
> 
> -- 
> Madam, there's no such thing as a tough child -- if you parboil them
> first for seven hours, they always come out tender.
>   -- W. C. Fields
> 



Re: [bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-26 Thread Peter Hessler
On 2017 May 26 (Fri) at 20:01:00 +0200 (+0200), Peter Hessler wrote:
:Apropos of "I found it", I implemented support for RFC 7607.  It's a
:super short RFC, but basically it forbids use of AS 0 anywhere.
:
:OK?
:
:

Fixed some denglish in an error message, mention the RFC in the man
page, and don't take down the session if we receive AS0 in the path.


Index: bgpd.8
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/bgpd.8,v
retrieving revision 1.52
diff -u -p -u -p -r1.52 bgpd.8
--- bgpd.8  19 Feb 2017 11:38:24 -  1.52
+++ bgpd.8  26 May 2017 18:29:49 -
@@ -357,6 +357,16 @@ control socket
 .Re
 .Pp
 .Rs
+.%A W. Kumari
+.%A R. Bush
+.%A H. Schiller
+.%A K. Patel
+.%D August 2015
+.%R RFC 7607
+.%T Codification of AS 0 Processing
+.Re
+.Pp
+.Rs
 .%D August 2011
 .%R draft-ietf-grow-mrt-17
 .%T MRT routing information export format
Index: parse.y
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.300
diff -u -p -u -p -r1.300 parse.y
--- parse.y 26 May 2017 14:08:51 -  1.300
+++ parse.y 26 May 2017 18:15:33 -
@@ -3661,6 +3661,11 @@ neighbor_consistent(struct peer *p)
return (-1);
}
 
+   if (p->conf.remote_as == 0) {
+   yyerror("peer AS may not be zero");
+   return (-1);
+   }
+
/* set default values if they where undefined */
p->conf.ebgp = (p->conf.remote_as != conf->as);
if (p->conf.announce_type == ANNOUNCE_UNDEF)
Index: rde_attr.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde_attr.c,v
retrieving revision 1.97
diff -u -p -u -p -r1.97 rde_attr.c
--- rde_attr.c  24 Jan 2017 04:22:42 -  1.97
+++ rde_attr.c  26 May 2017 19:29:04 -
@@ -460,6 +460,9 @@ aspath_verify(void *data, u_int16_t len,
if (seg_size == 0)
/* empty aspath segments are not allowed */
return (AS_ERR_BAD);
+
+   if (aspath_extract(seg, 0) == 0)
+   return (AS_ERR_BAD);
}
return (error); /* aspath is valid but probably not loop free */
 }
Index: session.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.359
diff -u -p -u -p -r1.359 session.c
--- session.c   13 Feb 2017 14:48:44 -  1.359
+++ session.c   5 May 2017 17:26:16 -
@@ -2017,6 +2017,14 @@ parse_open(struct peer *peer)
memcpy(_as, p, sizeof(short_as));
p += sizeof(short_as);
as = peer->short_as = ntohs(short_as);
+   if (as == 0) {
+   log_peer_warnx(>conf,
+   "peer requests unacceptable AS %u", as);
+   session_notification(peer, ERR_OPEN, ERR_OPEN_AS,
+   NULL, 0);
+   change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
+   return (-1);
+   }
 
memcpy(, p, sizeof(oholdtime));
p += sizeof(oholdtime);
@@ -2477,6 +2485,14 @@ parse_capabilities(struct peer *peer, u_
}
memcpy(_as, capa_val, sizeof(remote_as));
*as = ntohl(remote_as);
+   if (*as == 0) {
+   log_peer_warnx(>conf,
+   "peer requests unacceptable AS %u", *as);
+   session_notification(peer, ERR_OPEN, 
ERR_OPEN_AS,
+   NULL, 0);
+   change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
+   return (-1);
+   }
peer->capa.peer.as4byte = 1;
break;
default:




-- 
Madam, there's no such thing as a tough child -- if you parboil them
first for seven hours, they always come out tender.
-- W. C. Fields



Re: [bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-26 Thread Raf Czlonka
On Fri, May 26, 2017 at 07:24:06PM BST, Claudio Jeker wrote:
> On Fri, May 26, 2017 at 08:01:00PM +0200, Peter Hessler wrote:
> > Apropos of "I found it", I implemented support for RFC 7607.  It's a
> > super short RFC, but basically it forbids use of AS 0 anywhere.
> > 
> > OK?
> > 
> > 
> > Index: parse.y
> > ===
> > RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
> > retrieving revision 1.300
> > diff -u -p -u -p -r1.300 parse.y
> > --- parse.y 26 May 2017 14:08:51 -  1.300
> > +++ parse.y 26 May 2017 17:55:11 -
> > @@ -3661,6 +3661,11 @@ neighbor_consistent(struct peer *p)
> > return (-1);
> > }
> >  
> > +   if (p->conf.remote_as == 0) {
> > +   yyerror("peer AS needs to be not zero");
> 
> Needs better wording.

My exact thought... not that I have any say in it :^)

To be more specific: "needs to be not" simply sounds odd.

Regards,

Raf

> 
> > +   return (-1);
> > +   }
> > +
> > /* set default values if they where undefined */
> > p->conf.ebgp = (p->conf.remote_as != conf->as);
> > if (p->conf.announce_type == ANNOUNCE_UNDEF)
> > Index: rde.c
> > ===
> > RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde.c,v
> > retrieving revision 1.361
> > diff -u -p -u -p -r1.361 rde.c
> > --- rde.c   25 Jan 2017 03:21:55 -  1.361
> > +++ rde.c   26 May 2017 17:43:30 -
> > @@ -1102,6 +1102,14 @@ rde_update_dispatch(struct imsg *imsg)
> > /* shift to NLRI information */
> > p += 2 + attrpath_len;
> >  
> > +   /* aspath must not contain AS 0 */
> > +   if (!aspath_loopfree(asp->aspath, 0)) {
> > +   log_peer_warnx(>conf, "bad AS 0 in UPDATE");
> > +   rde_update_err(peer, ERR_UPDATE, ERR_UPD_ASPATH,
> > +   NULL, 0);
> 
> This can't be right. We should not cause a NOTIFICATION and session reset
> because of this. Doing that is bad bad bad.
> 
> > +   goto done;
> > +   }
> > +
> > /* aspath needs to be loop free nota bene this is not a hard error */
> > if (peer->conf.ebgp && !aspath_loopfree(asp->aspath, conf->as))
> > asp->flags |= F_ATTR_LOOP;
> > Index: session.c
> > ===
> > RCS file: /cvs/openbsd/src/usr.sbin/bgpd/session.c,v
> > retrieving revision 1.359
> > diff -u -p -u -p -r1.359 session.c
> > --- session.c   13 Feb 2017 14:48:44 -  1.359
> > +++ session.c   5 May 2017 17:26:16 -
> > @@ -2017,6 +2017,14 @@ parse_open(struct peer *peer)
> > memcpy(_as, p, sizeof(short_as));
> > p += sizeof(short_as);
> > as = peer->short_as = ntohs(short_as);
> > +   if (as == 0) {
> > +   log_peer_warnx(>conf,
> > +   "peer requests unacceptable AS %u", as);
> 
> Why not use 0 here instaed of %u?
> 
> > +   session_notification(peer, ERR_OPEN, ERR_OPEN_AS,
> > +   NULL, 0);
> > +   change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
> > +   return (-1);
> > +   }
> >  
> > memcpy(, p, sizeof(oholdtime));
> > p += sizeof(oholdtime);
> > @@ -2477,6 +2485,14 @@ parse_capabilities(struct peer *peer, u_
> > }
> > memcpy(_as, capa_val, sizeof(remote_as));
> > *as = ntohl(remote_as);
> > +   if (*as == 0) {
> > +   log_peer_warnx(>conf,
> > +   "peer requests unacceptable AS %u", *as);
> 
> Same here.
> 
> > +   session_notification(peer, ERR_OPEN, 
> > ERR_OPEN_AS,
> > +   NULL, 0);
> > +   change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
> > +   return (-1);
> > +   }
> > peer->capa.peer.as4byte = 1;
> > break;
> > default:
> > 
> > 
> > -- 
> > Taxes, n.:
> > Of life's two certainties, the only one for which you can get
> > an extension.
> > 
> 
> -- 
> :wq Claudio
> 



Re: [bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-26 Thread Jason McIntyre
On Fri, May 26, 2017 at 08:01:00PM +0200, Peter Hessler wrote:
> Apropos of "I found it", I implemented support for RFC 7607.  It's a
> super short RFC, but basically it forbids use of AS 0 anywhere.
> 
> OK?
> 

hi. you probably want to add the rfc to the list in STANDARDS too,
after 7606.

jmc

> 
> Index: parse.y
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.300
> diff -u -p -u -p -r1.300 parse.y
> --- parse.y   26 May 2017 14:08:51 -  1.300
> +++ parse.y   26 May 2017 17:55:11 -
> @@ -3661,6 +3661,11 @@ neighbor_consistent(struct peer *p)
>   return (-1);
>   }
>  
> + if (p->conf.remote_as == 0) {
> + yyerror("peer AS needs to be not zero");
> + return (-1);
> + }
> +
>   /* set default values if they where undefined */
>   p->conf.ebgp = (p->conf.remote_as != conf->as);
>   if (p->conf.announce_type == ANNOUNCE_UNDEF)
> Index: rde.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.361
> diff -u -p -u -p -r1.361 rde.c
> --- rde.c 25 Jan 2017 03:21:55 -  1.361
> +++ rde.c 26 May 2017 17:43:30 -
> @@ -1102,6 +1102,14 @@ rde_update_dispatch(struct imsg *imsg)
>   /* shift to NLRI information */
>   p += 2 + attrpath_len;
>  
> + /* aspath must not contain AS 0 */
> + if (!aspath_loopfree(asp->aspath, 0)) {
> + log_peer_warnx(>conf, "bad AS 0 in UPDATE");
> + rde_update_err(peer, ERR_UPDATE, ERR_UPD_ASPATH,
> + NULL, 0);
> + goto done;
> + }
> +
>   /* aspath needs to be loop free nota bene this is not a hard error */
>   if (peer->conf.ebgp && !aspath_loopfree(asp->aspath, conf->as))
>   asp->flags |= F_ATTR_LOOP;
> Index: session.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.359
> diff -u -p -u -p -r1.359 session.c
> --- session.c 13 Feb 2017 14:48:44 -  1.359
> +++ session.c 5 May 2017 17:26:16 -
> @@ -2017,6 +2017,14 @@ parse_open(struct peer *peer)
>   memcpy(_as, p, sizeof(short_as));
>   p += sizeof(short_as);
>   as = peer->short_as = ntohs(short_as);
> + if (as == 0) {
> + log_peer_warnx(>conf,
> + "peer requests unacceptable AS %u", as);
> + session_notification(peer, ERR_OPEN, ERR_OPEN_AS,
> + NULL, 0);
> + change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
> + return (-1);
> + }
>  
>   memcpy(, p, sizeof(oholdtime));
>   p += sizeof(oholdtime);
> @@ -2477,6 +2485,14 @@ parse_capabilities(struct peer *peer, u_
>   }
>   memcpy(_as, capa_val, sizeof(remote_as));
>   *as = ntohl(remote_as);
> + if (*as == 0) {
> + log_peer_warnx(>conf,
> + "peer requests unacceptable AS %u", *as);
> + session_notification(peer, ERR_OPEN, 
> ERR_OPEN_AS,
> + NULL, 0);
> + change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
> + return (-1);
> + }
>   peer->capa.peer.as4byte = 1;
>   break;
>   default:
> 
> 
> -- 
> Taxes, n.:
>   Of life's two certainties, the only one for which you can get
>   an extension.
> 



Re: [bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-26 Thread Claudio Jeker
On Fri, May 26, 2017 at 08:01:00PM +0200, Peter Hessler wrote:
> Apropos of "I found it", I implemented support for RFC 7607.  It's a
> super short RFC, but basically it forbids use of AS 0 anywhere.
> 
> OK?
> 
> 
> Index: parse.y
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.300
> diff -u -p -u -p -r1.300 parse.y
> --- parse.y   26 May 2017 14:08:51 -  1.300
> +++ parse.y   26 May 2017 17:55:11 -
> @@ -3661,6 +3661,11 @@ neighbor_consistent(struct peer *p)
>   return (-1);
>   }
>  
> + if (p->conf.remote_as == 0) {
> + yyerror("peer AS needs to be not zero");

Needs better wording.

> + return (-1);
> + }
> +
>   /* set default values if they where undefined */
>   p->conf.ebgp = (p->conf.remote_as != conf->as);
>   if (p->conf.announce_type == ANNOUNCE_UNDEF)
> Index: rde.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.361
> diff -u -p -u -p -r1.361 rde.c
> --- rde.c 25 Jan 2017 03:21:55 -  1.361
> +++ rde.c 26 May 2017 17:43:30 -
> @@ -1102,6 +1102,14 @@ rde_update_dispatch(struct imsg *imsg)
>   /* shift to NLRI information */
>   p += 2 + attrpath_len;
>  
> + /* aspath must not contain AS 0 */
> + if (!aspath_loopfree(asp->aspath, 0)) {
> + log_peer_warnx(>conf, "bad AS 0 in UPDATE");
> + rde_update_err(peer, ERR_UPDATE, ERR_UPD_ASPATH,
> + NULL, 0);

This can't be right. We should not cause a NOTIFICATION and session reset
because of this. Doing that is bad bad bad.

> + goto done;
> + }
> +
>   /* aspath needs to be loop free nota bene this is not a hard error */
>   if (peer->conf.ebgp && !aspath_loopfree(asp->aspath, conf->as))
>   asp->flags |= F_ATTR_LOOP;
> Index: session.c
> ===
> RCS file: /cvs/openbsd/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.359
> diff -u -p -u -p -r1.359 session.c
> --- session.c 13 Feb 2017 14:48:44 -  1.359
> +++ session.c 5 May 2017 17:26:16 -
> @@ -2017,6 +2017,14 @@ parse_open(struct peer *peer)
>   memcpy(_as, p, sizeof(short_as));
>   p += sizeof(short_as);
>   as = peer->short_as = ntohs(short_as);
> + if (as == 0) {
> + log_peer_warnx(>conf,
> + "peer requests unacceptable AS %u", as);

Why not use 0 here instaed of %u?

> + session_notification(peer, ERR_OPEN, ERR_OPEN_AS,
> + NULL, 0);
> + change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
> + return (-1);
> + }
>  
>   memcpy(, p, sizeof(oholdtime));
>   p += sizeof(oholdtime);
> @@ -2477,6 +2485,14 @@ parse_capabilities(struct peer *peer, u_
>   }
>   memcpy(_as, capa_val, sizeof(remote_as));
>   *as = ntohl(remote_as);
> + if (*as == 0) {
> + log_peer_warnx(>conf,
> + "peer requests unacceptable AS %u", *as);

Same here.

> + session_notification(peer, ERR_OPEN, 
> ERR_OPEN_AS,
> + NULL, 0);
> + change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
> + return (-1);
> + }
>   peer->capa.peer.as4byte = 1;
>   break;
>   default:
> 
> 
> -- 
> Taxes, n.:
>   Of life's two certainties, the only one for which you can get
>   an extension.
> 

-- 
:wq Claudio



[bgpd] RFC 7607 Codification of AS 0 Processing

2017-05-26 Thread Peter Hessler
Apropos of "I found it", I implemented support for RFC 7607.  It's a
super short RFC, but basically it forbids use of AS 0 anywhere.

OK?


Index: parse.y
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.300
diff -u -p -u -p -r1.300 parse.y
--- parse.y 26 May 2017 14:08:51 -  1.300
+++ parse.y 26 May 2017 17:55:11 -
@@ -3661,6 +3661,11 @@ neighbor_consistent(struct peer *p)
return (-1);
}
 
+   if (p->conf.remote_as == 0) {
+   yyerror("peer AS needs to be not zero");
+   return (-1);
+   }
+
/* set default values if they where undefined */
p->conf.ebgp = (p->conf.remote_as != conf->as);
if (p->conf.announce_type == ANNOUNCE_UNDEF)
Index: rde.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.361
diff -u -p -u -p -r1.361 rde.c
--- rde.c   25 Jan 2017 03:21:55 -  1.361
+++ rde.c   26 May 2017 17:43:30 -
@@ -1102,6 +1102,14 @@ rde_update_dispatch(struct imsg *imsg)
/* shift to NLRI information */
p += 2 + attrpath_len;
 
+   /* aspath must not contain AS 0 */
+   if (!aspath_loopfree(asp->aspath, 0)) {
+   log_peer_warnx(>conf, "bad AS 0 in UPDATE");
+   rde_update_err(peer, ERR_UPDATE, ERR_UPD_ASPATH,
+   NULL, 0);
+   goto done;
+   }
+
/* aspath needs to be loop free nota bene this is not a hard error */
if (peer->conf.ebgp && !aspath_loopfree(asp->aspath, conf->as))
asp->flags |= F_ATTR_LOOP;
Index: session.c
===
RCS file: /cvs/openbsd/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.359
diff -u -p -u -p -r1.359 session.c
--- session.c   13 Feb 2017 14:48:44 -  1.359
+++ session.c   5 May 2017 17:26:16 -
@@ -2017,6 +2017,14 @@ parse_open(struct peer *peer)
memcpy(_as, p, sizeof(short_as));
p += sizeof(short_as);
as = peer->short_as = ntohs(short_as);
+   if (as == 0) {
+   log_peer_warnx(>conf,
+   "peer requests unacceptable AS %u", as);
+   session_notification(peer, ERR_OPEN, ERR_OPEN_AS,
+   NULL, 0);
+   change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
+   return (-1);
+   }
 
memcpy(, p, sizeof(oholdtime));
p += sizeof(oholdtime);
@@ -2477,6 +2485,14 @@ parse_capabilities(struct peer *peer, u_
}
memcpy(_as, capa_val, sizeof(remote_as));
*as = ntohl(remote_as);
+   if (*as == 0) {
+   log_peer_warnx(>conf,
+   "peer requests unacceptable AS %u", *as);
+   session_notification(peer, ERR_OPEN, 
ERR_OPEN_AS,
+   NULL, 0);
+   change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN);
+   return (-1);
+   }
peer->capa.peer.as4byte = 1;
break;
default:


-- 
Taxes, n.:
Of life's two certainties, the only one for which you can get
an extension.