On Wed, Oct 12, 2016 at 05:39:17PM +0200, Rafael Zalamena wrote:
> This diff teaches switchd(8) how to validate flow_mod messages, more
> specifically the flow instructions and actions. The oxm validations
> were already implemented so we get them for free here.
I've updated the flow_mod diff to also include the following changes:
- Better loop detection like I did for tcpdump(8);
- A small fix in packet-in OXM parsing (see note below);
- Reuse actions validation for packet-out and implement missing
payload truncation check;
- Moved the new code away from packet-out to make diff looks less
confusing;
Note:
In packet-in we shouldn't use omlen with header size included, it only
works because the padding is zeroed out. To avoid one more loop and
errornous zero header reading we should remove the header size.
ok?
Index: sys/net/ofp.h
===================================================================
RCS file: /home/obsdcvs/src/sys/net/ofp.h,v
retrieving revision 1.2
diff -u -p -r1.2 ofp.h
--- sys/net/ofp.h 30 Sep 2016 12:40:00 -0000 1.2
+++ sys/net/ofp.h 12 Oct 2016 15:22:59 -0000
@@ -315,14 +315,14 @@ struct ofp_action_push {
uint16_t ap_type;
uint16_t ap_len;
uint16_t ap_ethertype;
- uint8_t pad[2];
+ uint8_t ap_pad[2];
} __packed;
struct ofp_action_pop_mpls {
uint16_t apm_type;
uint16_t apm_len;
uint16_t apm_ethertype;
- uint8_t pad[2];
+ uint8_t apm_pad[2];
} __packed;
struct ofp_action_group {
@@ -342,6 +342,12 @@ struct ofp_action_set_field {
uint16_t asf_type;
uint16_t asf_len;
uint8_t asf_field[4];
+} __packed;
+
+struct ofp_action_set_queue {
+ uint16_t asq_type;
+ uint16_t asq_len;
+ uint32_t asq_queue_id;
} __packed;
/* Packet-Out Message */
Index: usr.sbin/switchd/ofp13.c
===================================================================
RCS file: /home/obsdcvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.21
diff -u -p -r1.21 ofp13.c
--- usr.sbin/switchd/ofp13.c 13 Oct 2016 08:29:14 -0000 1.21
+++ usr.sbin/switchd/ofp13.c 24 Oct 2016 16:49:46 -0000
@@ -59,6 +59,12 @@ int ofp13_features_reply(struct switchd
int ofp13_validate_error(struct switchd *,
struct sockaddr_storage *, struct sockaddr_storage *,
struct ofp_header *, struct ibuf *);
+int ofp13_validate_action(struct switchd *, struct ofp_header *,
+ struct ibuf *, off_t *, struct ofp_action_header *);
+int ofp13_validate_instruction(struct switchd *, struct ofp_header *,
+ struct ibuf *, off_t *, struct ofp_instruction *);
+int ofp13_validate_flow_mod(struct switchd *, struct sockaddr_storage *,
+ struct sockaddr_storage *, struct ofp_header *, struct ibuf *);
int ofp13_validate_oxm_basic(struct ibuf *, off_t, int, uint8_t);
int ofp13_validate_oxm(struct switchd *, struct ofp_ox_match *,
struct ofp_header *, struct ibuf *, off_t);
@@ -129,7 +135,7 @@ struct ofp_callback ofp13_callbacks[] =
{ OFP_T_FLOW_REMOVED, ofp13_flow_removed, NULL },
{ OFP_T_PORT_STATUS, NULL, NULL },
{ OFP_T_PACKET_OUT, NULL, ofp13_validate_packet_out },
- { OFP_T_FLOW_MOD, NULL, NULL },
+ { OFP_T_FLOW_MOD, NULL, ofp13_validate_flow_mod },
{ OFP_T_GROUP_MOD, NULL, NULL },
{ OFP_T_PORT_MOD, NULL, NULL },
{ OFP_T_TABLE_MOD, NULL, NULL },
@@ -421,6 +427,7 @@ ofp13_validate_packet_in(struct switchd
log_debug("\tmatch type %s length %zu (padded to %zu)",
print_map(ntohs(om->om_type), ofp_match_map),
mlen, OFP_ALIGN(mlen) + ETHER_ALIGN);
+ mlen -= sizeof(*om);
/* current match offset, aligned offset after all matches */
moff = off + sizeof(*om);
@@ -468,10 +475,9 @@ ofp13_validate_packet_out(struct switchd
struct ofp_header *oh, struct ibuf *ibuf)
{
struct ofp_packet_out *pout;
- size_t len;
- off_t off;
+ size_t len, plen, diff;
+ off_t off, noff;
struct ofp_action_header *ah;
- struct ofp_action_output *ao;
off = 0;
if ((pout = ibuf_seek(ibuf, off, sizeof(*pout))) == NULL) {
@@ -480,36 +486,43 @@ ofp13_validate_packet_out(struct switchd
return (-1);
}
- log_debug("\tbuffer %d port %s "
- "actions length %u",
+ off += sizeof(*pout);
+ len = ntohs(pout->pout_actions_len);
+ log_debug("\tbuffer %d in_port %s actions_len %lu",
ntohl(pout->pout_buffer_id),
- print_map(ntohl(pout->pout_in_port), ofp_port_map),
- ntohs(pout->pout_actions_len));
- len = ntohl(pout->pout_actions_len);
+ print_map(ntohl(pout->pout_in_port), ofp_port_map), len);
- off += sizeof(*pout);
- while ((ah = ibuf_seek(ibuf, off, len)) != NULL &&
- ntohs(ah->ah_len) >= (uint16_t)sizeof(*ah)) {
- switch (ntohs(ah->ah_type)) {
- case OFP_ACTION_OUTPUT:
- ao = (struct ofp_action_output *)ah;
- log_debug("\t\taction type %s length %d "
- "port %s max length %d",
- print_map(ntohs(ao->ao_type), ofp_action_map),
- ntohs(ao->ao_len),
- print_map(ntohs(ao->ao_port), ofp_port_map),
- ntohs(ao->ao_max_len));
- break;
- default:
- log_debug("\t\taction type %s length %d",
- print_map(ntohs(ah->ah_type), ofp_action_map),
- ntohs(ah->ah_len));
- break;
- }
- if (pout->pout_buffer_id == (uint32_t)-1)
- break;
- off += ntohs(ah->ah_len);
+parse_next_action:
+ if ((ah = ibuf_seek(ibuf, off, sizeof(*ah))) == NULL)
+ return (-1);
+
+ noff = off;
+ ofp13_validate_action(sc, oh, ibuf, &off, ah);
+
+ diff = off - noff;
+ /* Loop prevention. */
+ if (off < noff || diff == 0)
+ return (-1);
+
+ len -= diff;
+ if (len)
+ goto parse_next_action;
+
+ /* Check for encapsulated packet truncation. */
+ len = ntohs(oh->oh_length) - off;
+ plen = ibuf_length(ibuf) - off;
+
+ if (plen < len) {
+ log_debug("\ttruncated packet %lu < %lu", plen, len);
+
+ /* Buffered packets can be truncated */
+ if (pout->pout_buffer_id != OFP_PKTOUT_NO_BUFFER)
+ len = plen;
+ else
+ return (-1);
}
+ if (ibuf_seek(ibuf, off, len) == NULL)
+ return (-1);
return (0);
}
@@ -646,6 +659,298 @@ ofp13_features_reply(struct switchd *sc,
#endif
ofp13_setconfig(sc, con, OFP_CONFIG_FRAG_NORMAL,
OFP_CONTROLLER_MAXLEN_NO_BUFFER);
+
+ return (0);
+}
+
+int
+ofp13_validate_action(struct switchd *sc, struct ofp_header *oh,
+ struct ibuf *ibuf, off_t *off, struct ofp_action_header *ah)
+{
+ struct ofp_action_output *ao;
+ struct ofp_action_mpls_ttl *amt;
+ struct ofp_action_push *ap;
+ struct ofp_action_pop_mpls *apm;
+ struct ofp_action_group *ag;
+ struct ofp_action_nw_ttl *ant;
+ struct ofp_action_set_field *asf;
+ struct ofp_action_set_queue *asq;
+ struct ofp_ox_match *oxm;
+ size_t len;
+ int type;
+ off_t moff;
+
+ type = ntohs(ah->ah_type);
+ len = ntohs(ah->ah_len);
+
+ switch (type) {
+ case OFP_ACTION_OUTPUT:
+ if (len != sizeof(*ao))
+ return (-1);
+ if ((ao = ibuf_seek(ibuf, *off, sizeof(*ao))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\t\taction %s len %lu port %s max_len %d",
+ print_map(type, ofp_action_map), len,
+ print_map(ntohl(ao->ao_port), ofp_port_map),
+ ntohs(ao->ao_max_len));
+ break;
+ case OFP_ACTION_SET_MPLS_TTL:
+ if (len != sizeof(*amt))
+ return (-1);
+ if ((amt = ibuf_seek(ibuf, *off, sizeof(*amt))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\t\taction %s len %lu ttl %d",
+ print_map(type, ofp_action_map), len, amt->amt_ttl);
+ break;
+ case OFP_ACTION_PUSH_VLAN:
+ case OFP_ACTION_PUSH_MPLS:
+ case OFP_ACTION_PUSH_PBB:
+ if (len != sizeof(*ap))
+ return (-1);
+ if ((ap = ibuf_seek(ibuf, *off, sizeof(*ap))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\t\taction %s len %lu ethertype %#04x",
+ print_map(type, ofp_action_map), len,
+ ntohs(ap->ap_ethertype));
+ break;
+ case OFP_ACTION_POP_MPLS:
+ if (len != sizeof(*apm))
+ return (-1);
+ if ((apm = ibuf_seek(ibuf, *off, sizeof(*apm))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\t\taction %s len %lu ethertype %#04x",
+ print_map(type, ofp_action_map), len,
+ ntohs(apm->apm_ethertype));
+ break;
+ case OFP_ACTION_SET_QUEUE:
+ if (len != sizeof(*asq))
+ return (-1);
+ if ((asq = ibuf_seek(ibuf, *off, sizeof(*asq))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\t\taction %s len %lu queue_id %u",
+ print_map(type, ofp_action_map), len,
+ ntohl(asq->asq_queue_id));
+ break;
+ case OFP_ACTION_GROUP:
+ if (len != sizeof(*ag))
+ return (-1);
+ if ((ag = ibuf_seek(ibuf, *off, sizeof(*ag))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\t\taction %s len %lu group_id %u",
+ print_map(type, ofp_action_map), len,
+ ntohl(ag->ag_group_id));
+ break;
+ case OFP_ACTION_SET_NW_TTL:
+ if (len != sizeof(*ant))
+ return (-1);
+ if ((ant = ibuf_seek(ibuf, *off, sizeof(*ant))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\t\taction %s len %lu ttl %d",
+ print_map(type, ofp_action_map), len, ant->ant_ttl);
+ break;
+ case OFP_ACTION_SET_FIELD:
+ if (len < sizeof(*asf))
+ return (-1);
+ if ((asf = ibuf_seek(ibuf, *off, sizeof(*asf))) == NULL)
+ return (-1);
+
+ moff = *off + sizeof(*asf) - sizeof(asf->asf_field);
+ *off += len;
+ log_debug("\t\taction %s len %lu",
+ print_map(type, ofp_action_map), len);
+
+ len -= sizeof(*asf) - sizeof(asf->asf_field);
+ while (len) {
+ 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;
+ }
+ break;
+
+ default:
+ if (len < sizeof(*ah))
+ return (-1);
+
+ /* Generic header without information. */
+ *off += len;
+ log_debug("\t\taction %s len %lu",
+ print_map(type, ofp_action_map), len);
+ break;
+ }
+
+ return (0);
+}
+
+int
+ofp13_validate_instruction(struct switchd *sc, struct ofp_header *oh,
+ struct ibuf *ibuf, off_t *off, struct ofp_instruction *i)
+{
+ struct ofp_instruction_actions *ia;
+ struct ofp_instruction_goto_table *igt;
+ struct ofp_instruction_write_metadata *iwm;
+ struct ofp_instruction_meter *im;
+ struct ofp_action_header *ah;
+ int type;
+ size_t len;
+ off_t oldoff, diff;
+
+ type = ntohs(i->i_type);
+ len = ntohs(i->i_len);
+
+ switch (type) {
+ case OFP_INSTRUCTION_T_GOTO_TABLE:
+ if (len != sizeof(*igt))
+ return (-1);
+ if ((igt = ibuf_seek(ibuf, *off, sizeof(*igt))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\tinstruction %s length %lu table_id %d",
+ print_map(type, ofp_instruction_t_map), len,
+ igt->igt_table_id);
+ break;
+ case OFP_INSTRUCTION_T_WRITE_META:
+ if (len != sizeof(*iwm))
+ return (-1);
+ if ((iwm = ibuf_seek(ibuf, *off, sizeof(*iwm))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\tinstruction %s length %lu "
+ "metadata %llu mask %llu",
+ print_map(type, ofp_instruction_t_map), len,
+ be64toh(iwm->iwm_metadata),
+ be64toh(iwm->iwm_metadata_mask));
+ break;
+ case OFP_INSTRUCTION_T_METER:
+ if (len != sizeof(*im))
+ return (-1);
+ if ((im = ibuf_seek(ibuf, *off, sizeof(*im))) == NULL)
+ return (-1);
+
+ *off += len;
+ log_debug("\tinstruction %s length %lu meter_id %d",
+ print_map(type, ofp_instruction_t_map), len,
+ im->im_meter_id);
+ break;
+ case OFP_INSTRUCTION_T_WRITE_ACTIONS:
+ case OFP_INSTRUCTION_T_CLEAR_ACTIONS:
+ case OFP_INSTRUCTION_T_APPLY_ACTIONS:
+ if (len < sizeof(*ia))
+ return (-1);
+ if ((ia = ibuf_seek(ibuf, *off, sizeof(*ia))) == NULL)
+ return (-1);
+
+ log_debug("\tinstruction %s length %lu",
+ print_map(type, ofp_instruction_t_map), len);
+
+ *off += sizeof(*ia);
+ len -= sizeof(*ia);
+ while (len) {
+ oldoff = *off;
+ if ((ah = ibuf_seek(ibuf, *off, sizeof(*ah))) == NULL ||
+ ofp13_validate_action(sc, oh, ibuf, off, ah) == -1)
+ return (-1);
+
+ diff = *off - oldoff;
+ /* Loop prevention. */
+ if (*off < oldoff || diff == 0)
+ break;
+
+ len -= diff;
+ }
+ break;
+ default:
+ if (len < sizeof(*i))
+ return (-1);
+
+ log_debug("\tinstruction %s length %lu",
+ print_map(type, ofp_instruction_t_map), len);
+ *off += len;
+ break;
+ }
+
+ return (0);
+}
+
+int
+ofp13_validate_flow_mod(struct switchd *sc,
+ struct sockaddr_storage *src, struct sockaddr_storage *dst,
+ struct ofp_header *oh, struct ibuf *ibuf)
+{
+ struct ofp_flow_mod *fm;
+ struct ofp_match *om;
+ struct ofp_instruction *i;
+ struct ofp_ox_match *oxm;
+ off_t off, moff, offdiff;
+ int matchlen, matchtype, left;
+
+ off = 0;
+ if ((fm = ibuf_seek(ibuf, off, sizeof(*fm))) == NULL)
+ return (-1);
+
+ log_debug("\tcommand %s table %d timeout (idle %d hard %d) "
+ "priority %d buffer_id %u out_port %u out_group %u "
+ "flags %#04x cookie %llu mask %llu",
+ print_map(fm->fm_command, ofp_flowcmd_map),
+ fm->fm_table_id, ntohs(fm->fm_idle_timeout),
+ ntohs(fm->fm_hard_timeout), ntohs(fm->fm_priority),
+ ntohl(fm->fm_buffer_id), ntohl(fm->fm_out_port),
+ ntohl(fm->fm_out_group), ntohs(fm->fm_flags),
+ be64toh(fm->fm_cookie), be64toh(fm->fm_cookie_mask));
+
+ off += offsetof(struct ofp_flow_mod, fm_match);
+
+ om = &fm->fm_match;
+ matchtype = ntohs(om->om_type);
+ matchlen = ntohs(om->om_length);
+
+ moff = off + sizeof(*om);
+ off += OFP_ALIGN(matchlen);
+
+ matchlen -= sizeof(*om);
+ while (matchlen) {
+ if ((oxm = ibuf_seek(ibuf, moff, sizeof(*oxm))) == NULL ||
+ ofp13_validate_oxm(sc, oxm, oh, ibuf, moff) == -1)
+ return (-1);
+ moff += sizeof(*oxm) + oxm->oxm_length;
+ matchlen -= sizeof(*oxm) + oxm->oxm_length;
+ }
+
+ left = ntohs(oh->oh_length) - off;
+ moff = off;
+ while (left) {
+ if ((i = ibuf_seek(ibuf, moff, sizeof(*i))) == NULL ||
+ ofp13_validate_instruction(sc, oh, ibuf, &moff, i) == -1)
+ return (-1);
+
+ offdiff = moff - off;
+ /* Loop prevention. */
+ if (moff < off || offdiff == 0)
+ break;
+
+ left -= offdiff;
+ off = moff;
+ }
return (0);
}
