From bcd6516ba4adf10771c6e4097bc676f8cd1788b9 Mon Sep 17 00:00:00 2001 From: claudio Date: Wed, 22 May 2024 08:41:14 +0000 Subject: [PATCH] Convert bgpid, remote_bgpid and clusterid to host byte order. Before the RDE used host byte order for remote_bgpid but all the other code used network byte order. The reason for that was that bgpid was initially an IPv4 address but since RFC 6286 in 2011 this is much more relaxed and so it makes more sense to just treat them as numbers and so host byte order. OK tb@ --- usr.sbin/bgpd/config.c | 7 ++++--- usr.sbin/bgpd/mrt.c | 4 ++-- usr.sbin/bgpd/parse.y | 8 ++++---- usr.sbin/bgpd/printconf.c | 6 +++--- usr.sbin/bgpd/rde.c | 29 +++++++++++++++-------------- usr.sbin/bgpd/rde.h | 4 ++-- usr.sbin/bgpd/rde_peer.c | 4 ++-- usr.sbin/bgpd/session.c | 13 +++++++------ 8 files changed, 39 insertions(+), 36 deletions(-) diff --git a/usr.sbin/bgpd/config.c b/usr.sbin/bgpd/config.c index 057dc863457..789431986b6 100644 --- a/usr.sbin/bgpd/config.c +++ b/usr.sbin/bgpd/config.c @@ -1,4 +1,4 @@ -/* $OpenBSD: config.c,v 1.108 2023/08/16 08:26:35 claudio Exp $ */ +/* $OpenBSD: config.c,v 1.109 2024/05/22 08:41:14 claudio Exp $ */ /* * Copyright (c) 2003, 2004, 2005 Henning Brauer @@ -466,7 +466,7 @@ get_bgpid(void) struct ifaddrs *ifap, *ifa; uint32_t ip = 0, cur, localnet; - localnet = htonl(INADDR_LOOPBACK & IN_CLASSA_NET); + localnet = INADDR_LOOPBACK & IN_CLASSA_NET; if (getifaddrs(&ifap) == -1) fatal("getifaddrs"); @@ -476,9 +476,10 @@ get_bgpid(void) ifa->ifa_addr->sa_family != AF_INET) continue; cur = ((struct sockaddr_in *)ifa->ifa_addr)->sin_addr.s_addr; + cur = ntohl(cur); if ((cur & localnet) == localnet) /* skip 127/8 */ continue; - if (ntohl(cur) > ntohl(ip)) + if (cur > ip) ip = cur; } freeifaddrs(ifap); diff --git a/usr.sbin/bgpd/mrt.c b/usr.sbin/bgpd/mrt.c index c6ce7ff5028..4e328d1a8dd 100644 --- a/usr.sbin/bgpd/mrt.c +++ b/usr.sbin/bgpd/mrt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mrt.c,v 1.116 2023/07/14 10:30:53 claudio Exp $ */ +/* $OpenBSD: mrt.c,v 1.117 2024/05/22 08:41:14 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Claudio Jeker @@ -824,7 +824,7 @@ mrt_dump_v2_hdr(struct mrt *mrt, struct bgpd_config *conf) return (-1); } - if (ibuf_add(buf, &conf->bgpid, sizeof(conf->bgpid)) == -1) + if (ibuf_add_n32(buf, conf->bgpid) == -1) goto fail; nlen = strlen(mrt->rib); if (nlen > 0) diff --git a/usr.sbin/bgpd/parse.y b/usr.sbin/bgpd/parse.y index 6919e2278ef..fa8047d5bbf 100644 --- a/usr.sbin/bgpd/parse.y +++ b/usr.sbin/bgpd/parse.y @@ -1,4 +1,4 @@ -/* $OpenBSD: parse.y,v 1.462 2024/04/24 10:41:34 claudio Exp $ */ +/* $OpenBSD: parse.y,v 1.463 2024/05/22 08:41:14 claudio Exp $ */ /* * Copyright (c) 2002, 2003, 2004 Henning Brauer @@ -742,7 +742,7 @@ conf_main : AS as4number { yyerror("router-id must be an IPv4 address"); YYERROR; } - conf->bgpid = $2.v4.s_addr; + conf->bgpid = ntohl($2.v4.s_addr); } | HOLDTIME NUMBER { if ($2 < MIN_HOLDTIME || $2 > USHRT_MAX) { @@ -2236,14 +2236,14 @@ peeropts : REMOTEAS as4number { YYERROR; } if ((conf->flags & BGPD_FLAG_REFLECTOR) && - conf->clusterid != $2.v4.s_addr) { + conf->clusterid != ntohl($2.v4.s_addr)) { yyerror("only one route reflector " "cluster allowed"); YYERROR; } conf->flags |= BGPD_FLAG_REFLECTOR; curpeer->conf.reflector_client = 1; - conf->clusterid = $2.v4.s_addr; + conf->clusterid = ntohl($2.v4.s_addr); } | DEPEND ON STRING { if (strlcpy(curpeer->conf.if_depend, $3, diff --git a/usr.sbin/bgpd/printconf.c b/usr.sbin/bgpd/printconf.c index cdac0d8d82c..13a73208cb4 100644 --- a/usr.sbin/bgpd/printconf.c +++ b/usr.sbin/bgpd/printconf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: printconf.c,v 1.172 2024/04/24 10:41:34 claudio Exp $ */ +/* $OpenBSD: printconf.c,v 1.173 2024/05/22 08:41:14 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -383,7 +383,7 @@ print_mainconf(struct bgpd_config *conf) printf("AS %s", log_as(conf->as)); if (conf->as > USHRT_MAX && conf->short_as != AS_TRANS) printf(" %u", conf->short_as); - ina.s_addr = conf->bgpid; + ina.s_addr = htonl(conf->bgpid); printf("\nrouter-id %s\n", inet_ntoa(ina)); printf("socket \"%s\"\n", conf->csock); @@ -800,7 +800,7 @@ print_peer(struct peer_config *p, struct bgpd_config *conf, const char *c) if (conf->clusterid == 0) printf("%s\troute-reflector\n", c); else { - ina.s_addr = conf->clusterid; + ina.s_addr = htonl(conf->clusterid); printf("%s\troute-reflector %s\n", c, inet_ntoa(ina)); } diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c index 73dce0a5bb3..558d851f581 100644 --- a/usr.sbin/bgpd/rde.c +++ b/usr.sbin/bgpd/rde.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.c,v 1.624 2024/03/20 09:35:46 claudio Exp $ */ +/* $OpenBSD: rde.c,v 1.625 2024/05/22 08:41:14 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -2647,48 +2647,49 @@ rde_reflector(struct rde_peer *peer, struct rde_aspath *asp) /* check for originator id if eq router_id drop */ if ((a = attr_optget(asp, ATTR_ORIGINATOR_ID)) != NULL) { - if (memcmp(&conf->bgpid, a->data, sizeof(conf->bgpid)) == 0) { + id = htonl(conf->bgpid); + if (memcmp(&id, a->data, sizeof(id)) == 0) { /* this is coming from myself */ asp->flags |= F_ATTR_LOOP; return; } } else if (conf->flags & BGPD_FLAG_REFLECTOR) { if (peer->conf.ebgp) - id = conf->bgpid; + id = htonl(conf->bgpid); else id = htonl(peer->remote_bgpid); if (attr_optadd(asp, ATTR_OPTIONAL, ATTR_ORIGINATOR_ID, - &id, sizeof(uint32_t)) == -1) + &id, sizeof(id)) == -1) fatalx("attr_optadd failed but impossible"); } /* check for own id in the cluster list */ if (conf->flags & BGPD_FLAG_REFLECTOR) { + id = htonl(conf->clusterid); if ((a = attr_optget(asp, ATTR_CLUSTER_LIST)) != NULL) { - for (len = 0; len < a->len; - len += sizeof(conf->clusterid)) + for (len = 0; len < a->len; len += sizeof(id)) /* check if coming from my cluster */ - if (memcmp(&conf->clusterid, a->data + len, - sizeof(conf->clusterid)) == 0) { + if (memcmp(&id, a->data + len, + sizeof(id)) == 0) { asp->flags |= F_ATTR_LOOP; return; } /* prepend own clusterid by replacing attribute */ - len = a->len + sizeof(conf->clusterid); + len = a->len + sizeof(id); if (len < a->len) fatalx("rde_reflector: cluster-list overflow"); if ((p = malloc(len)) == NULL) fatal("rde_reflector"); - memcpy(p, &conf->clusterid, sizeof(conf->clusterid)); - memcpy(p + sizeof(conf->clusterid), a->data, a->len); + memcpy(p, &id, sizeof(id)); + memcpy(p + sizeof(id), a->data, a->len); attr_free(asp, a); if (attr_optadd(asp, ATTR_OPTIONAL, ATTR_CLUSTER_LIST, p, len) == -1) fatalx("attr_optadd failed but impossible"); free(p); } else if (attr_optadd(asp, ATTR_OPTIONAL, ATTR_CLUSTER_LIST, - &conf->clusterid, sizeof(conf->clusterid)) == -1) + &id, sizeof(id)) == -1) fatalx("attr_optadd failed but impossible"); } } @@ -3584,11 +3585,11 @@ rde_reload_done(void) nconf = NULL; /* sync peerself with conf */ - peerself->remote_bgpid = ntohl(conf->bgpid); + peerself->remote_bgpid = conf->bgpid; peerself->conf.local_as = conf->as; peerself->conf.remote_as = conf->as; peerself->conf.remote_addr.aid = AID_INET; - peerself->conf.remote_addr.v4.s_addr = conf->bgpid; + peerself->conf.remote_addr.v4.s_addr = htonl(conf->bgpid); peerself->conf.remote_masklen = 32; peerself->short_as = conf->short_as; diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h index 8849ec3e060..d0bfb5d2db2 100644 --- a/usr.sbin/bgpd/rde.h +++ b/usr.sbin/bgpd/rde.h @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.h,v 1.301 2024/05/19 03:31:05 jsg Exp $ */ +/* $OpenBSD: rde.h,v 1.302 2024/05/22 08:41:14 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Claudio Jeker and @@ -90,7 +90,7 @@ struct rde_peer { struct prefix_tree withdraws[AID_MAX]; struct filter_head *out_rules; time_t staletime[AID_MAX]; - uint32_t remote_bgpid; /* host byte order! */ + uint32_t remote_bgpid; uint32_t path_id_tx; unsigned int local_if_scope; enum peer_state state; diff --git a/usr.sbin/bgpd/rde_peer.c b/usr.sbin/bgpd/rde_peer.c index 00d12f9afec..9ddd692266c 100644 --- a/usr.sbin/bgpd/rde_peer.c +++ b/usr.sbin/bgpd/rde_peer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde_peer.c,v 1.36 2024/03/20 09:35:46 claudio Exp $ */ +/* $OpenBSD: rde_peer.c,v 1.37 2024/05/22 08:41:14 claudio Exp $ */ /* * Copyright (c) 2019 Claudio Jeker @@ -422,7 +422,7 @@ peer_up(struct rde_peer *peer, struct session_up *sup) peer->stats.prefix_out_cnt = 0; peer->state = PEER_DOWN; } - peer->remote_bgpid = ntohl(sup->remote_bgpid); + peer->remote_bgpid = sup->remote_bgpid; peer->short_as = sup->short_as; peer->remote_addr = sup->remote_addr; peer->local_v4_addr = sup->local_v4_addr; diff --git a/usr.sbin/bgpd/session.c b/usr.sbin/bgpd/session.c index 43c61adb087..2658e44f488 100644 --- a/usr.sbin/bgpd/session.c +++ b/usr.sbin/bgpd/session.c @@ -1,4 +1,4 @@ -/* $OpenBSD: session.c,v 1.477 2024/05/20 10:01:52 claudio Exp $ */ +/* $OpenBSD: session.c,v 1.478 2024/05/22 08:41:14 claudio Exp $ */ /* * Copyright (c) 2003, 2004, 2005 Henning Brauer @@ -1602,7 +1602,7 @@ session_open(struct peer *p) errs += ibuf_add_n16(buf->buf, p->conf.local_short_as); errs += ibuf_add_n16(buf->buf, holdtime); /* is already in network byte order */ - errs += ibuf_add(buf->buf, &conf->bgpid, sizeof(conf->bgpid)); + errs += ibuf_add_n32(buf->buf, conf->bgpid); errs += ibuf_add_n8(buf->buf, optparamlen); if (extlen) { @@ -2228,8 +2228,7 @@ parse_open(struct peer *peer) change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN); return (-1); } - /* strore remote_bgpid in network byte order */ - peer->remote_bgpid = htonl(bgpid); + peer->remote_bgpid = bgpid; if (optparamlen != 0) { struct ibuf oparams, op; @@ -2332,8 +2331,10 @@ parse_open(struct peer *peer) /* on iBGP sessions check for bgpid collision */ if (!peer->conf.ebgp && peer->remote_bgpid == conf->bgpid) { - log_peer_warnx(&peer->conf, "peer BGPID %u conflicts with ours", - ntohl(bgpid)); + struct in_addr ina; + ina.s_addr = htonl(bgpid); + log_peer_warnx(&peer->conf, "peer BGPID %s conflicts with ours", + inet_ntoa(ina)); session_notification(peer, ERR_OPEN, ERR_OPEN_BGPID, NULL); change_state(peer, STATE_IDLE, EVNT_RCVD_OPEN); return (-1); -- 2.20.1