From 37149e4fa8567ca78b4c6c6f84a1749d8d7963b5 Mon Sep 17 00:00:00 2001 From: claudio Date: Mon, 8 Jan 2024 16:39:17 +0000 Subject: [PATCH] Rework rtr_parse_header() and introduce rtr_check_session_id() to make the initial header parsing simpler. This also allows to simplify the version negotiation dance a bit. More is needed there. OK tb@ --- usr.sbin/bgpd/rtr_proto.c | 308 ++++++++++++++++++++++++-------------- 1 file changed, 198 insertions(+), 110 deletions(-) diff --git a/usr.sbin/bgpd/rtr_proto.c b/usr.sbin/bgpd/rtr_proto.c index 78df9e0b26d..eeef151880f 100644 --- a/usr.sbin/bgpd/rtr_proto.c +++ b/usr.sbin/bgpd/rtr_proto.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rtr_proto.c,v 1.23 2024/01/05 11:02:57 claudio Exp $ */ +/* $OpenBSD: rtr_proto.c,v 1.24 2024/01/08 16:39:17 claudio Exp $ */ /* * Copyright (c) 2020 Claudio Jeker @@ -33,7 +33,7 @@ struct rtr_header { uint8_t type; uint16_t session_id; /* or error code */ uint32_t length; -}; +} __packed; #define RTR_MAX_VERSION 2 #define RTR_MAX_LEN 2048 @@ -56,42 +56,71 @@ enum rtr_pdu_type { ASPA = 11, }; +struct rtr_notify { + struct rtr_header hdr; + uint32_t serial; +} __packed; + +struct rtr_query { + struct rtr_header hdr; + uint32_t serial; +} __packed; + +struct rtr_reset { + struct rtr_header hdr; +} __packed; + +struct rtr_response { + struct rtr_header hdr; +} __packed; + #define FLAG_ANNOUNCE 0x1 #define FLAG_MASK FLAG_ANNOUNCE struct rtr_ipv4 { - uint8_t flags; - uint8_t prefixlen; - uint8_t maxlen; - uint8_t zero; - uint32_t prefix; - uint32_t asnum; -}; + struct rtr_header hdr; + uint8_t flags; + uint8_t prefixlen; + uint8_t maxlen; + uint8_t zero; + uint32_t prefix; + uint32_t asnum; +} __packed; struct rtr_ipv6 { - uint8_t flags; - uint8_t prefixlen; - uint8_t maxlen; - uint8_t zero; - uint32_t prefix[4]; - uint32_t asnum; -}; + struct rtr_header hdr; + uint8_t flags; + uint8_t prefixlen; + uint8_t maxlen; + uint8_t zero; + uint32_t prefix[4]; + uint32_t asnum; +} __packed; + +struct rtr_routerkey { + struct rtr_header hdr; + uint8_t ski[20]; + uint32_t asnum; + /* followed by Subject Public Key Info */ +} __packed; #define FLAG_AFI_V6 0x1 #define FLAG_AFI_MASK FLAG_AFI_V6 struct rtr_aspa { - uint8_t flags; - uint8_t afi_flags; - uint16_t cnt; - uint32_t cas; + struct rtr_header hdr; + uint8_t flags; + uint8_t afi_flags; + uint16_t cnt; + uint32_t cas; /* array of spas with cnt elements follows */ -}; +} __packed; struct rtr_endofdata { - uint32_t serial; - uint32_t refresh; - uint32_t retry; - uint32_t expire; -}; + struct rtr_header hdr; + uint32_t serial; + uint32_t refresh; + uint32_t retry; + uint32_t expire; +} __packed; enum rtr_event { RTR_EVNT_START, @@ -109,6 +138,7 @@ enum rtr_event { RTR_EVNT_NO_DATA, RTR_EVNT_RESET_AND_CLOSE, RTR_EVNT_UNSUPP_PROTO_VERSION, + RTR_EVNT_NEGOTIATION_DONE, }; static const char *rtr_eventnames[] = { @@ -127,6 +157,7 @@ static const char *rtr_eventnames[] = { "no data", "connection closed with reset", "unsupported protocol version", + "negotiation done", }; enum rtr_state { @@ -344,7 +375,25 @@ rtr_send_serial_query(struct rtr_session *rs) } /* - * Validate the common rtr header (first 8 bytes) including the + * Check the session_id of the rtr_header to match the expected value. + * Returns -1 on failure and 0 on success. + */ +static int +rtr_check_session_id(struct rtr_session *rs, uint16_t session_id, + struct rtr_header *rh, struct ibuf *pdu) +{ + if (session_id != ntohs(rh->session_id)) { + log_warnx("rtr %s: received %s: bad session_id: %d != %d", + log_rtr(rs), log_rtr_type(rh->type), ntohs(rh->session_id), + session_id); + rtr_send_error(rs, CORRUPT_DATA, "bad session_id", pdu); + return -1; + } + return 0; +} + +/* + * Parse the common rtr header (first 8 bytes) including the * included length field. * Returns -1 on failure. On success msgtype and msglen are set * and the function return 0. @@ -354,88 +403,82 @@ rtr_parse_header(struct rtr_session *rs, struct ibuf *hdr, size_t *msglen, enum rtr_pdu_type *msgtype) { struct rtr_header rh; - uint32_t len = 16; /* default for ERROR_REPORT */ - int session_id; + size_t len; if (ibuf_get(hdr, &rh, sizeof(rh)) == -1) fatal("%s: ibuf_get", __func__); - if (rh.version != rs->version && rh.type != ERROR_REPORT) { - badversion: - log_warnx("rtr %s: received %s message: unexpected version %d", - log_rtr(rs), log_rtr_type(rh.type), rh.version); - rtr_send_error(rs, UNEXP_PROTOCOL_VERS, NULL, hdr); + len = ntohl(rh.length); + + if (len > RTR_MAX_LEN) { + log_warnx("rtr %s: received %s: msg too big: %zu byte", + log_rtr(rs), log_rtr_type(rh.type), len); + rtr_send_error(rs, CORRUPT_DATA, "too big", hdr); return -1; } - *msgtype = rh.type; - *msglen = ntohl(rh.length); + if (rs->state == RTR_STATE_NEGOTIATION) { + switch (rh.type) { + case CACHE_RESPONSE: + case CACHE_RESET: + case ERROR_REPORT: + if (rh.version < rs->version) + rs->version = rh.version; + rtr_fsm(rs, RTR_EVNT_NEGOTIATION_DONE); + break; + case SERIAL_NOTIFY: + /* ignore SERIAL_NOTIFY */ + break; + default: + log_warnx("rtr %s: received %s: out of context", + log_rtr(rs), log_rtr_type(rh.type)); + rtr_send_error(rs, CORRUPT_DATA, NULL, hdr); + return -1; + } + } else if (rh.version != rs->version && rh.type != ERROR_REPORT) { + goto badversion; + } switch (rh.type) { case SERIAL_NOTIFY: - session_id = rs->session_id; - len = 12; + if (len != sizeof(struct rtr_notify)) + goto badlen; break; case CACHE_RESPONSE: - /* set session_id if not yet happened */ - if (rs->session_id == -1) - rs->session_id = ntohs(rh.session_id); - session_id = rs->session_id; - len = 8; + if (len != sizeof(struct rtr_response)) + goto badlen; break; case IPV4_PREFIX: - session_id = 0; - len = 20; + if (len != sizeof(struct rtr_ipv4)) + goto badlen; break; case IPV6_PREFIX: - session_id = 0; - len = 32; + if (len != sizeof(struct rtr_ipv6)) + goto badlen; break; case END_OF_DATA: - session_id = rs->session_id; - len = 24; + if (len != sizeof(struct rtr_endofdata)) + goto badlen; break; case CACHE_RESET: - session_id = 0; - len = 8; + if (len != sizeof(struct rtr_reset)) + goto badlen; break; case ROUTER_KEY: + if (len < sizeof(struct rtr_routerkey)) + goto badlen; if (rs->version < 1) goto badversion; - len = 36; /* XXX probably too small, but we ignore it */ - /* FALLTHROUGH */ + break; case ERROR_REPORT: - if (*msglen > RTR_MAX_LEN) { - toobig: - log_warnx("rtr %s: received %s: msg too big: %zu byte", - log_rtr(rs), log_rtr_type(rh.type), *msglen); - rtr_send_error(rs, CORRUPT_DATA, "too big", hdr); - return -1; - } - if (*msglen < len) { - toosmall: - log_warnx("rtr %s: received %s: msg too small: " - "%zu byte", log_rtr(rs), log_rtr_type(rh.type), - *msglen); - rtr_send_error(rs, CORRUPT_DATA, "too small", hdr); - return -1; - } - /* - * session_id check omitted since ROUTER_KEY and ERROR_REPORT - * use the field for different things. - */ - return 0; + if (len < 16) + goto badlen; + break; case ASPA: + if (len < sizeof(struct rtr_aspa) || (len % 4) != 0) + goto badlen; if (rs->version < 2) goto badversion; - session_id = 0; - /* unlike all other messages ASPA is variable sized */ - if (*msglen > RTR_MAX_LEN) - goto toobig; - if (*msglen < sizeof(struct rtr_aspa)) - goto toosmall; - /* len must be a multiple of 4 */ - len = *msglen & ~0x3; break; default: log_warnx("rtr %s: received unknown message: type %s", @@ -444,33 +487,44 @@ rtr_parse_header(struct rtr_session *rs, struct ibuf *hdr, return -1; } - if (len != *msglen) { - log_warnx("rtr %s: received %s: illegal len: %zu byte not %u", - log_rtr(rs), log_rtr_type(rh.type), *msglen, len); - rtr_send_error(rs, CORRUPT_DATA, "bad length", hdr); - return -1; - } + *msglen = len; + *msgtype = rh.type; - if (session_id != ntohs(rh.session_id)) { - /* ignore SERIAL_NOTIFY during startup */ - if (rs->session_id == -1 && rh.type == SERIAL_NOTIFY) - return 0; + return 0; - log_warnx("rtr %s: received %s: bad session_id: %d != %d", - log_rtr(rs), log_rtr_type(rh.type), ntohs(rh.session_id), - session_id); - rtr_send_error(rs, CORRUPT_DATA, "bad session_id", hdr); - return -1; - } + badlen: + log_warnx("rtr %s: received %s: bad PDU length: %zu bytes", + log_rtr(rs), log_rtr_type(rh.type), len); + rtr_send_error(rs, CORRUPT_DATA, "bad length", hdr); + return -1; - return 0; + badversion: + log_warnx("rtr %s: received %s message: unexpected version %d", + log_rtr(rs), log_rtr_type(rh.type), rh.version); + rtr_send_error(rs, UNEXP_PROTOCOL_VERS, NULL, hdr); + return -1; } static int rtr_parse_notify(struct rtr_session *rs, struct ibuf *pdu) { - if (rs->state == RTR_STATE_EXCHANGE || - rs->state == RTR_STATE_NEGOTIATION) { + struct rtr_notify notify; + + /* ignore SERIAL_NOTIFY during startup */ + if (rs->state == RTR_STATE_NEGOTIATION) + return 0; + + if (ibuf_get(pdu, ¬ify, sizeof(notify)) == -1) { + log_warnx("rtr %s: received %s: bad pdu len", + log_rtr(rs), log_rtr_type(SERIAL_NOTIFY)); + rtr_send_error(rs, CORRUPT_DATA, "bad len", pdu); + return -1; + } + + if (rtr_check_session_id(rs, rs->session_id, ¬ify.hdr, pdu) == -1) + return -1; + + if (rs->state != RTR_STATE_EXCHANGE) { log_warnx("rtr %s: received %s: while in state %s (ignored)", log_rtr(rs), log_rtr_type(SERIAL_NOTIFY), rtr_statenames[rs->state]); @@ -484,8 +538,23 @@ rtr_parse_notify(struct rtr_session *rs, struct ibuf *pdu) static int rtr_parse_cache_response(struct rtr_session *rs, struct ibuf *pdu) { - if (rs->state != RTR_STATE_ESTABLISHED && - rs->state != RTR_STATE_NEGOTIATION) { + struct rtr_response resp; + + if (ibuf_get(pdu, &resp, sizeof(resp)) == -1) { + log_warnx("rtr %s: received %s: bad pdu len", + log_rtr(rs), log_rtr_type(CACHE_RESPONSE)); + rtr_send_error(rs, CORRUPT_DATA, "bad len", pdu); + return -1; + } + + /* set session_id if not yet happened */ + if (rs->session_id == -1) + rs->session_id = ntohs(resp.hdr.session_id); + + if (rtr_check_session_id(rs, rs->session_id, &resp.hdr, pdu) == -1) + return -1; + + if (rs->state != RTR_STATE_ESTABLISHED) { log_warnx("rtr %s: received %s: out of context", log_rtr(rs), log_rtr_type(CACHE_RESPONSE)); return -1; @@ -501,14 +570,16 @@ rtr_parse_ipv4_prefix(struct rtr_session *rs, struct ibuf *pdu) struct rtr_ipv4 ip4; struct roa *roa; - if (ibuf_skip(pdu, sizeof(struct rtr_header)) == -1 || - ibuf_get(pdu, &ip4, sizeof(ip4)) == -1) { + if (ibuf_get(pdu, &ip4, sizeof(ip4)) == -1) { log_warnx("rtr %s: received %s: bad pdu len", log_rtr(rs), log_rtr_type(IPV4_PREFIX)); rtr_send_error(rs, CORRUPT_DATA, "bad len", pdu); return -1; } + if (rtr_check_session_id(rs, 0, &ip4.hdr, pdu) == -1) + return -1; + if (rs->state != RTR_STATE_EXCHANGE) { log_warnx("rtr %s: received %s: out of context", log_rtr(rs), log_rtr_type(IPV4_PREFIX)); @@ -569,14 +640,16 @@ rtr_parse_ipv6_prefix(struct rtr_session *rs, struct ibuf *pdu) struct rtr_ipv6 ip6; struct roa *roa; - if (ibuf_skip(pdu, sizeof(struct rtr_header)) == -1 || - ibuf_get(pdu, &ip6, sizeof(ip6)) == -1) { + if (ibuf_get(pdu, &ip6, sizeof(ip6)) == -1) { log_warnx("rtr %s: received %s: bad pdu len", log_rtr(rs), log_rtr_type(IPV6_PREFIX)); rtr_send_error(rs, CORRUPT_DATA, "bad len", pdu); return -1; } + if (rtr_check_session_id(rs, 0, &ip6.hdr, pdu) == -1) + return -1; + if (rs->state != RTR_STATE_EXCHANGE) { log_warnx("rtr %s: received %s: out of context", log_rtr(rs), log_rtr_type(IPV6_PREFIX)); @@ -638,8 +711,7 @@ rtr_parse_aspa(struct rtr_session *rs, struct ibuf *pdu) struct aspa_set *aspa, *a; uint16_t cnt, i; - if (ibuf_skip(pdu, sizeof(struct rtr_header)) == -1 || - ibuf_get(pdu, &rtr_aspa, sizeof(rtr_aspa)) == -1) { + if (ibuf_get(pdu, &rtr_aspa, sizeof(rtr_aspa)) == -1) { log_warnx("rtr %s: received %s: bad pdu len", log_rtr(rs), log_rtr_type(ASPA)); rtr_send_error(rs, CORRUPT_DATA, "bad len", pdu); @@ -733,13 +805,15 @@ rtr_parse_end_of_data(struct rtr_session *rs, struct ibuf *pdu) struct rtr_endofdata eod; uint32_t t; - if (ibuf_skip(pdu, sizeof(struct rtr_header)) == -1 || - ibuf_get(pdu, &eod, sizeof(eod)) == -1) { + if (ibuf_get(pdu, &eod, sizeof(eod)) == -1) { log_warnx("rtr %s: received %s: bad pdu len", log_rtr(rs), log_rtr_type(END_OF_DATA)); return -1; } + if (rtr_check_session_id(rs, rs->session_id, &eod.hdr, pdu) == -1) + return -1; + if (rs->state != RTR_STATE_EXCHANGE) { log_warnx("rtr %s: received %s: out of context", log_rtr(rs), log_rtr_type(END_OF_DATA)); @@ -775,6 +849,17 @@ bad: static int rtr_parse_cache_reset(struct rtr_session *rs, struct ibuf *pdu) { + struct rtr_reset reset; + + if (ibuf_get(pdu, &reset, sizeof(reset)) == -1) { + log_warnx("rtr %s: received %s: bad pdu len", + log_rtr(rs), log_rtr_type(CACHE_RESET)); + return -1; + } + + if (rtr_check_session_id(rs, 0, &reset.hdr, pdu) == -1) + return -1; + if (rs->state != RTR_STATE_ESTABLISHED) { log_warnx("rtr %s: received %s: out of context", log_rtr(rs), log_rtr_type(CACHE_RESET)); @@ -1094,6 +1179,9 @@ rtr_fsm(struct rtr_session *rs, enum rtr_event event) /* flush receive buffer */ rs->r.wpos = 0; break; + case RTR_EVNT_NEGOTIATION_DONE: + rs->state = RTR_STATE_ESTABLISHED; + break; } log_debug("rtr %s: state change %s -> %s, reason: %s", -- 2.20.1