diff --git a/sys/net/switchofp.c b/sys/net/switchofp.c index f2a34be7add..587a9c99d38 100644 --- a/sys/net/switchofp.c +++ b/sys/net/switchofp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: switchofp.c,v 1.75 2019/11/21 17:24:15 akoshibe Exp $ */ +/* $OpenBSD: switchofp.c,v 1.76 2019/11/27 17:37:32 akoshibe Exp $ */ /* * Copyright (c) 2016 Kazuya GODA @@ -2174,7 +2174,8 @@ swofp_validate_action(struct switch_softc *sc, struct ofp_action_header *ah, } break; case OFP_ACTION_SET_FIELD: - if (ahlen < sizeof(struct ofp_action_set_field)) { + if (ahlen < sizeof(struct ofp_action_set_field) || + ahlen != OFP_ALIGN(ahlen)) { *err = OFP_ERRACTION_LEN; return (-1); } @@ -2189,22 +2190,37 @@ swofp_validate_action(struct switch_softc *sc, struct ofp_action_header *ah, dptr = (uint8_t *)ah; dptr += sizeof(struct ofp_action_set_field) - offsetof(struct ofp_action_set_field, asf_field); - while (oxmlen > 0) { - oxm = (struct ofp_ox_match *)dptr; - if (swofp_validate_oxm(oxm, err)) { - if (*err == OFP_ERRMATCH_BAD_LEN) - *err = OFP_ERRACTION_SET_LEN; - else - *err = OFP_ERRACTION_SET_TYPE; + oxm = (struct ofp_ox_match *)dptr; + oxmlen -= sizeof(struct ofp_ox_match); + if (oxmlen < oxm->oxm_length) { + *err = OFP_ERRACTION_SET_LEN; + return (-1); + } + /* Remainder is padding. */ + oxmlen -= oxm->oxm_length; + if (oxmlen >= OFP_ALIGNMENT) { + *err = OFP_ERRACTION_SET_LEN; + return (-1); + } + if (swofp_validate_oxm(oxm, err)) { + if (*err == OFP_ERRMATCH_BAD_LEN) + *err = OFP_ERRACTION_SET_LEN; + else + *err = OFP_ERRACTION_SET_TYPE; + return (-1); + } + + dptr += sizeof(struct ofp_ox_match) + oxm->oxm_length; + while (oxmlen > 0) { + if (*dptr != 0) { + *err = OFP_ERRACTION_SET_ARGUMENT; return (-1); } - - dptr += sizeof(*oxm) + oxm->oxm_length; - oxmlen -= sizeof(*oxm) + oxm->oxm_length; + oxmlen--; + dptr++; } break; - default: /* Unknown/unsupported action. */ *err = OFP_ERRACTION_TYPE; diff --git a/usr.sbin/switchd/ofp13.c b/usr.sbin/switchd/ofp13.c index f02ad1f8840..721302451a7 100644 --- a/usr.sbin/switchd/ofp13.c +++ b/usr.sbin/switchd/ofp13.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ofp13.c,v 1.46 2019/11/21 06:22:57 akoshibe Exp $ */ +/* $OpenBSD: ofp13.c,v 1.47 2019/11/27 17:37:32 akoshibe Exp $ */ /* * Copyright (c) 2013-2016 Reyk Floeter @@ -780,7 +780,7 @@ ofp13_validate_action(struct switchd *sc, struct ofp_header *oh, print_map(type, ofp_action_map), len, ant->ant_ttl); break; case OFP_ACTION_SET_FIELD: - if (len < sizeof(*asf)) + if (len < sizeof(*asf) || len != OFP_ALIGN(len)) return (-1); if ((asf = ibuf_seek(ibuf, *off, sizeof(*asf))) == NULL) return (-1); @@ -791,16 +791,14 @@ ofp13_validate_action(struct switchd *sc, struct ofp_header *oh, print_map(type, ofp_action_map), len); len -= sizeof(*asf) - sizeof(asf->asf_field); - while (len > 0) { - if ((oxm = ibuf_seek(ibuf, moff, sizeof(*oxm))) - == NULL) - return (-1); - if (ofp13_validate_oxm(sc, oxm, oh, ibuf, moff) == -1) - return (-1); + if ((oxm = ibuf_seek(ibuf, moff, sizeof(*oxm))) == NULL) + return (-1); + if (ofp13_validate_oxm(sc, oxm, oh, ibuf, moff) == -1) + return (-1); - len -= sizeof(*oxm) + oxm->oxm_length; - moff += sizeof(*oxm) + oxm->oxm_length; - } + len -= sizeof(*oxm) + oxm->oxm_length; + if (len >= OFP_ALIGNMENT) + return (-1); break; default: diff --git a/usr.sbin/tcpdump/print-ofp.c b/usr.sbin/tcpdump/print-ofp.c index 8dd70d4830c..c9037183512 100644 --- a/usr.sbin/tcpdump/print-ofp.c +++ b/usr.sbin/tcpdump/print-ofp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: print-ofp.c,v 1.11 2016/11/28 17:47:15 jca Exp $ */ +/* $OpenBSD: print-ofp.c,v 1.12 2019/11/27 17:37:32 akoshibe Exp $ */ /* * Copyright (c) 2016 Rafael Zalamena @@ -231,11 +231,40 @@ ofp_print_setconfig(const u_char *bp, u_int length) ofp_controller_maxlen_map)); } +void +ofp_print_oxm_field(const u_char *bp, u_int length, int omlen, int once) +{ + struct ofp_ox_match *oxm; + + do { + if (length < sizeof(*oxm)) { + printf(" [|OpenFlow]"); + return; + } + + oxm = (struct ofp_ox_match *)bp; + bp += sizeof(*oxm); + length -= sizeof(*oxm); + if (length < oxm->oxm_length) { + printf(" [|OpenFlow]"); + return; + } + + ofp_print_oxm(oxm, bp, length); + bp += oxm->oxm_length; + length -= oxm->oxm_length; + + if (once) + return; + + omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen); + } while (omlen > 0); +} + void ofp_print_packetin(const u_char *bp, u_int length) { struct ofp_packet_in *pin; - struct ofp_ox_match *oxm; int omtype, omlen; int haspacket = 0; const u_char *pktptr; @@ -276,27 +305,7 @@ ofp_print_packetin(const u_char *bp, u_int length) if (omlen == 0) goto print_packet; - parse_next_oxm: - if (length < sizeof(*oxm)) { - printf(" [|OpenFlow]"); - return; - } - - oxm = (struct ofp_ox_match *)bp; - bp += sizeof(*oxm); - length -= sizeof(*oxm); - if (length < oxm->oxm_length) { - printf(" [|OpenFlow]"); - return; - } - - ofp_print_oxm(oxm, bp, length); - - bp += oxm->oxm_length; - length -= oxm->oxm_length; - omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen); - if (omlen) - goto parse_next_oxm; + ofp_print_oxm_field(bp, length, omlen, 0); print_packet: if (haspacket == 0) @@ -322,7 +331,6 @@ void ofp_print_flowremoved(const u_char *bp, u_int length) { struct ofp_flow_removed *fr; - struct ofp_ox_match *oxm; int omtype, omlen; if (length < sizeof(*fr)) { @@ -355,27 +363,7 @@ ofp_print_flowremoved(const u_char *bp, u_int length) bp += sizeof(*fr); length -= sizeof(*fr); - parse_next_oxm: - if (length < sizeof(*oxm)) { - printf(" [|OpenFlow]"); - return; - } - - oxm = (struct ofp_ox_match *)bp; - bp += sizeof(*oxm); - length -= sizeof(*oxm); - if (length < oxm->oxm_length) { - printf(" [|OpenFlow]"); - return; - } - - ofp_print_oxm(oxm, bp, length); - - bp += oxm->oxm_length; - length -= oxm->oxm_length; - omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen); - if (omlen) - goto parse_next_oxm; + ofp_print_oxm_field(bp, length, omlen, 0); } void @@ -453,7 +441,6 @@ void ofp_print_flowmod(const u_char *bp, u_int length) { struct ofp_flow_mod *fm; - struct ofp_ox_match *oxm; struct ofp_instruction *i; int omtype, omlen, ilen; int instructionslen, padsize; @@ -498,27 +485,10 @@ ofp_print_flowmod(const u_char *bp, u_int length) goto parse_next_instruction; } - parse_next_oxm: - if (length < sizeof(*oxm)) { - printf(" [|OpenFlow]"); - return; - } + ofp_print_oxm_field(bp, length, omlen, 0); - oxm = (struct ofp_ox_match *)bp; - bp += sizeof(*oxm); - length -= sizeof(*oxm); - if (length < oxm->oxm_length) { - printf(" [|OpenFlow]"); - return; - } - - ofp_print_oxm(oxm, bp, length); - - bp += oxm->oxm_length; - length -= oxm->oxm_length; - omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen); - if (omlen) - goto parse_next_oxm; + bp += omlen; + length -= omlen; /* Skip padding if any. */ if (padsize) { @@ -1017,7 +987,6 @@ void action_print_setfield(const u_char *bp, u_int length) { struct ofp_action_set_field *asf; - struct ofp_ox_match *oxm; int omlen; if (length < (sizeof(*asf) - AH_UNPADDED)) { @@ -1030,27 +999,7 @@ action_print_setfield(const u_char *bp, u_int length) if (omlen == 0) return; - parse_next_oxm: - if (length < sizeof(*oxm)) { - printf(" [|OpenFlow]"); - return; - } - - oxm = (struct ofp_ox_match *)bp; - bp += sizeof(*oxm); - length -= sizeof(*oxm); - if (length < oxm->oxm_length) { - printf(" [|OpenFlow]"); - return; - } - - ofp_print_oxm(oxm, bp, length); - - bp += oxm->oxm_length; - length -= oxm->oxm_length; - omlen -= min(sizeof(*oxm) + oxm->oxm_length, omlen); - if (omlen) - goto parse_next_oxm; + ofp_print_oxm_field(bp, length, omlen, 1); } void @@ -1094,6 +1043,7 @@ ofp_print_action(struct ofp_action_header *ah, const u_char *bp, u_int length) break; case OFP_ACTION_SET_FIELD: + action_print_setfield(bp, length); break; case OFP_ACTION_COPY_TTL_OUT: