From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd, consistent RTR error handling
To: tech@openbsd.org
Date: Tue, 9 Jan 2024 15:02:13 +0100

On Tue, Jan 09, 2024 at 02:36:52PM +0100, Claudio Jeker wrote:
> Try to be more consistent with RTR parse error reporting.
> Mainly add the rtr_send_error() call into all rtr_parse_xyz functions
> and remove the ones in rtr_process_msg().
> 
> Try to add helpful error text to many rtr_send_error() calls unless the
> error pdu type and text would be close to identical.
> 
> In parse header also check the version first for router key and ASPA pdus.

Looks all good.

ok tb

two suggestions for your consideration.

> 
> -- 
> :wq Claudio
> 
> Index: rtr_proto.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rtr_proto.c,v
> diff -u -p -r1.24 rtr_proto.c
> --- rtr_proto.c	8 Jan 2024 16:39:17 -0000	1.24
> +++ rtr_proto.c	9 Jan 2024 13:25:17 -0000
> @@ -413,7 +413,7 @@ rtr_parse_header(struct rtr_session *rs,
>  	if (len > RTR_MAX_LEN) {
>  		log_warnx("rtr %s: received %s: msg too big: %zu byte",

Maybe s/msg/pdu or did you want to avoid the clash with 'aspa pdu'?

>  		    log_rtr(rs), log_rtr_type(rh.type), len);
> -		rtr_send_error(rs, CORRUPT_DATA, "too big", hdr);
> +		rtr_send_error(rs, CORRUPT_DATA, "pdu too big", hdr);
>  		return -1;
>  	}
>  


> @@ -1020,14 +1017,13 @@ rtr_process_msg(struct rtr_session *rs)
>  			}
>  			break;
>  		case ASPA:
> -			if (rtr_parse_aspa(rs, &msg) == -1) {
> +			if (rtr_parse_aspa(rs, &msg) == -1)
>  				return;
> -			}
>  			break;
>  		default:
>  			log_warnx("rtr %s: received %s: unexpected pdu type",

s/unexpected/unsupported ?

>  			    log_rtr(rs), log_rtr_type(msgtype));
> -			rtr_send_error(rs, INVALID_REQUEST, NULL, &msg);
> +			rtr_send_error(rs, UNSUPP_PDU_TYPE, NULL, &msg);
>  			return;
>  		}
>  	}
>