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@

diff --git a/usr.sbin/bgpd/config.c b/usr.sbin/bgpd/config.c
index 057dc86..7894319 100644 (file)
--- 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 <henning@openbsd.org>
@@ -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 c6ce7ff..4e328d1 100644 (file)
--- 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 <claudio@openbsd.org>
@@ -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 6919e22..fa8047d 100644 (file)
--- 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 <henning@openbsd.org>
@@ -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 cdac0d8..13a7320 100644 (file)
--- 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 <henning@openbsd.org>
@@ -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 73dce0a..558d851 100644 (file)
--- 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 <henning@openbsd.org>
@@ -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 8849ec3..d0bfb5d 100644 (file)
--- 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 <claudio@openbsd.org> 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 00d12f9..9ddd692 100644 (file)
--- 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 <claudio@openbsd.org>
@@ -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 43c61ad..2658e44 100644 (file)
--- 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 <henning@openbsd.org>
@@ -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);