ofp-parse: Harden checking with group selection_method.

Only allow fields when "selection_method=hash".  Only allow
selection_method_param when a non-nil selection_method is given.

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 92c469356fa4dd2a1044671e075a4e976bd43199..2980a1df3cc2705b9a560fb5e2bc6aaf7b5d3b80 100644 (file)
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1668,6 +1668,18 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, int command,
         goto out;
     }
 
+    /* Exclude fields for non "hash" selection method. */
+    if (strcmp(gm->props.selection_method, "hash") &&
+        gm->props.fields.values_size) {
+        error = xstrdup("fields may only be specified with \"selection_method=hash\"");
+        goto out;
+    }
+    /* Exclude selection_method_param if no selection_method is given. */
+    if (gm->props.selection_method[0] == 0
+        && gm->props.selection_method_param != 0) {
+        error = xstrdup("selection_method_param is only allowed with \"selection_method\"");
+        goto out;
+    }
     if (fields & F_COMMAND_BUCKET_ID) {
         if (!(fields & F_COMMAND_BUCKET_ID_ALL || had_command_bucket_id)) {
             error = xstrdup("must specify a command bucket id");

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 2978cc5c670b35bea909af3ed5d190eb7f053889..cc388584b09fc86b4d1981a78a3f3134f67313ba 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -449,6 +449,16 @@ AT_CHECK([sort results | uniq | sed 's/1[[01]]/1?/'], [0],
   [Datapath actions: 1?
 ])
 
+# Check that fields are rejected without "selection_method=hash".
+AT_CHECK([ovs-ofctl -O OpenFlow15 add-group br0 'group_id=1235,type=select,fields(eth_dst,ip_dst,tcp_dst),bucket=output:10,bucket=output:11'], 1, [], [dnl
+ovs-ofctl: fields may only be specified with "selection_method=hash"
+])
+
+# Check that selection_method_param without selection_method is rejected.
+AT_CHECK([ovs-ofctl -O OpenFlow15 add-group br0 'group_id=1235,type=select,selection_method_param=1,bucket=output:10,bucket=output:11'], 1, [], [dnl
+ovs-ofctl: selection_method_param is only allowed with "selection_method"
+])
+
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 

diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index b648ab847f64636e4d641a152a46e9d014b398a6..675c3084d551fa555d6ef907b834973b5cd60830 100644 (file)
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -2887,14 +2887,14 @@ when using Open vSwitch 2.4 and later with OpenFlow 1.5 and later.
 .IP \fBfields\fR=\fIfield\fR
 .IQ \fBfields(\fIfield\fR[\fB=\fImask\fR]\fR...\fB)\fR
 The field parameters to selection method selected by the
-\fBselection_method\fR field.  The syntax is described in \fBFlow Syntax\fR
-with the additional restrictions that if a value is provided it is
-treated as a wildcard mask and wildcard masks following a slash are
-prohibited. The pre-requisites of fields must be provided by any flows that
-output to the group. The use of the fields is defined by the lower-layer
-that implements the \fBselection_method\fR.  They are optional if the
-\fBselection_method\fR field is specified as a non-empty string.
-Prohibited otherwise. The default is no fields.
+\fBselection_method\fR field.  The syntax is described in \fBFlow
+Syntax\fR with the additional restrictions that if a value is provided
+it is treated as a wildcard mask and wildcard masks following a slash
+are prohibited. The pre-requisites of fields must be provided by any
+flows that output to the group.  The use of the fields is defined by
+the lower-layer that implements the \fBselection_method\fR.  They are
+optional if the \fBselection_method\fR field is specified as ``hash',
+prohibited otherwise.  The default is no fields.
 .IP
 This option will use a Netronome OpenFlow extension which is only supported
 when using Open vSwitch 2.4 and later with OpenFlow 1.5 and later.