misc: binary api fuzz test fixes

Add a hook to src/vlibapi/api_shared.c to fuzz (screw up) binary API
messages, e.g. by xoring random data into them before processing. We
specifically exempt client connection messages, and inband debug CLI
messages. We step over msg_id, client index, client context, and
sw_if_index. Otherwise, "make test" vectors fail too rapidly to learn
anything.

The goal is to reduce the number of crashes caused to zero. We're
fairly close with this patch.

Add vl_msg_api_max_length(void *mp), which returns the maximum
plausible length for a binary API message.

Use it to hardern vl_api_from_api_to_new_vec(...) which takes an
additional argument - message pointer - so it can verify that
astr->length is sane. If it's not sane, return a u8 *vector of the
form "insane astr->length nnnn\0".

Verify array lengths in vl_api_dhcp6_send_client_message_t_handler(...)
and vl_api_dhcp6_pd_send_client_message_t_handler(...).

Add a fairly effective binary API fuzz hook to the unittest plugin,
and modify the "make test" framework.py to pass "api-fuzz { on|off }"
to enable API fuzzing: "make API_FUZZ=on TEST=xxx test-debug" or similar

Type: improvement

Signed-off-by: Dave Barach <dave@barachs.net>
Change-Id: I0157267652a163c01553d5267620f719cc6c3bde

17 files changed, 250 insertions, 21 deletions

diff --git a/src/plugins/dhcp/dhcp_api.c b/src/plugins/dhcp/dhcp_api.c
index a5163ca2e95..d2e423572fb 100644
--- a/src/plugins/dhcp/dhcp_api.c
+++ b/src/plugins/dhcp/dhcp_api.c
@@ -549,6 +549,12 @@ void
   params.T1 = ntohl (mp->T1);
   params.T2 = ntohl (mp->T2);
   n_addresses = ntohl (mp->n_addresses);
+  /* Make sure that the number of addresses is sane */
+  if (n_addresses * sizeof (params.addresses) > vl_msg_api_max_length (mp))
+    {
+      rv = VNET_API_ERROR_INVALID_VALUE;
+      goto bad_sw_if_index;
+    }
   params.addresses = 0;
   if (n_addresses > 0)
     vec_validate (params.addresses, n_addresses - 1);
@@ -593,6 +599,14 @@ void
   params.T1 = ntohl (mp->T1);
   params.T2 = ntohl (mp->T2);
   n_prefixes = ntohl (mp->n_prefixes);
+
+  /* Minimal check to see that the number of prefixes is sane */
+  if (n_prefixes * sizeof (params.prefixes) > vl_msg_api_max_length (mp))
+    {
+      rv = VNET_API_ERROR_INVALID_VALUE;
+      goto bad_sw_if_index;
+    }
+
   params.prefixes = 0;
   if (n_prefixes > 0)
     vec_validate (params.prefixes, n_prefixes - 1);
diff --git a/src/plugins/ioam/lib-pot/pot_api.c b/src/plugins/ioam/lib-pot/pot_api.c
index 31ddf9da528..72d05d76590 100644
--- a/src/plugins/ioam/lib-pot/pot_api.c
+++ b/src/plugins/ioam/lib-pot/pot_api.c
@@ -14,7 +14,7 @@
  */
 /*
  *------------------------------------------------------------------
- * pot_api.c - Proof of Transit related APIs to create 
+ * pot_api.c - Proof of Transit related APIs to create
  *             and maintain profiles
  *------------------------------------------------------------------
  */
@@ -43,7 +43,7 @@ static void vl_api_pot_profile_add_t_handler
     pot_profile *profile = NULL;
     u8 *name = 0;
 
-    name = vl_api_from_api_to_new_vec(&mp->list_name);
+    name = vl_api_from_api_to_new_vec(mp, &mp->list_name);
 
     pot_profile_list_init(name);
     id = mp->id;
@@ -61,7 +61,7 @@ static void vl_api_pot_profile_add_t_handler
         (void)pot_profile_set_bit_mask(profile, mp->max_bits);
     } else {
         rv = -3;
-    }  
+    }
  ERROROUT:
     vec_free(name);
     REPLY_MACRO(VL_API_POT_PROFILE_ADD_REPLY);
@@ -121,14 +121,14 @@ static void vl_api_pot_profile_activate_t_handler
     u8 id;
     u8 *name = NULL;
 
-    name = vl_api_from_api_to_new_vec(&mp->list_name);
+    name = vl_api_from_api_to_new_vec(mp, &mp->list_name);
     if (!pot_profile_list_is_enabled(name)) {
         rv = -1;
     } else {
         id = mp->id;
 	rv = pot_profile_set_active(id);
     }
-     
+
     vec_free(name);
     REPLY_MACRO(VL_API_POT_PROFILE_ACTIVATE_REPLY);
 }
diff --git a/src/plugins/unittest/CMakeLists.txt b/src/plugins/unittest/CMakeLists.txt
index 7ab63dad6a4..3ea4752145b 100644
--- a/src/plugins/unittest/CMakeLists.txt
+++ b/src/plugins/unittest/CMakeLists.txt
@@ -13,6 +13,7 @@
 
 add_vpp_plugin(unittest
   SOURCES
+  api_fuzz_test.c
   bier_test.c
   bihash_test.c
   bitmap_test.c
diff --git a/src/plugins/unittest/api_fuzz_test.c b/src/plugins/unittest/api_fuzz_test.c
new file mode 100644
index 00000000000..113835300bb
--- /dev/null
+++ b/src/plugins/unittest/api_fuzz_test.c
@@ -0,0 +1,187 @@
+/*
+ *------------------------------------------------------------------
+ * api_fuzz_test.c - Binary API fuzz hook
+ *
+ * Copyright (c) 2020 Cisco and/or its affiliates.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *------------------------------------------------------------------
+ */
+#include <vppinfra/format.h>
+#include <vppinfra/byte_order.h>
+#include <vppinfra/error.h>
+#include <vlib/vlib.h>
+#include <vlib/unix/unix.h>
+#include <vlibapi/api.h>
+
+static u32 fuzz_seed = 0xdeaddabe;
+static u16 fuzz_first;
+static u16 fuzz_cli_first, fuzz_cli_last;
+
+extern void (*vl_msg_api_fuzz_hook) (u16, void *);
+
+static void
+fuzz_hook (u16 id, void *the_msg)
+{
+  /*
+   * Fuzz (aka screw up) this message? Leave connection establishment
+   * messages alone as well as CLI messages.
+   */
+  if ((id > fuzz_first) && !(id >= fuzz_cli_first && id < fuzz_cli_last))
+    {
+      msgbuf_t *mb;
+      u8 *limit, *start;
+
+      mb = (msgbuf_t *) (((u8 *) the_msg) - offsetof (msgbuf_t, data));
+
+      limit = (u8 *) (mb->data + ntohl (mb->data_len));
+
+      /*
+       * Leave the first 14 octets alone, aka msg_id, client_index,
+       * context, sw_if_index
+       */
+
+      start = ((u8 *) the_msg) + 14;
+
+      for (; start < limit; start++)
+	*start ^= (random_u32 (&fuzz_seed) & 0xFF);
+    }
+}
+
+static void
+default_fuzz_config (void)
+{
+  fuzz_first = vl_msg_api_get_msg_index
+    ((u8 *) "memclnt_keepalive_reply_e8d4e804");
+  fuzz_cli_first = vl_msg_api_get_msg_index ((u8 *) "cli_23bfbfff");
+  fuzz_cli_last = vl_msg_api_get_msg_index
+    ((u8 *) "cli_inband_reply_05879051");
+}
+
+static clib_error_t *
+test_api_fuzz_command_fn (vlib_main_t * vm,
+			  unformat_input_t * input, vlib_cli_command_t * cmd)
+{
+  u32 tmp;
+
+  default_fuzz_config ();
+
+  if (fuzz_first == 0xFFFF)
+    {
+      vlib_cli_output (vm, "Couldn't find 'memclnt_keepalive_reply' ID");
+      vlib_cli_output
+	(vm, "Manual setting required, use 'show api message table'");
+    }
+
+  if (fuzz_cli_first == 0xFFFF)
+    {
+      vlib_cli_output (vm, "Couldn't find 'cli' ID");
+      vlib_cli_output
+	(vm, "Manual setting required, use 'show api message table'");
+    }
+
+  if (fuzz_cli_last == 0xFFFF)
+    {
+      vlib_cli_output (vm, "Couldn't find 'cli_inband_reply' ID");
+      vlib_cli_output
+	(vm, "Manual setting required, use 'show api message table'");
+    }
+
+  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
+    {
+      if (unformat (input, "seed %d", &fuzz_seed))
+	;
+      else if (unformat (input, "disable") | unformat (input, "off"))
+	fuzz_first = ~0;
+      else if (unformat (input, "fuzz-first %d", &tmp))
+	fuzz_first = (u16) tmp;
+      else if (unformat (input, "fuzz-cli-first %d", &tmp))
+	fuzz_cli_first = (u16) tmp;
+      else if (unformat (input, "fuzz-cli-last %d", &tmp))
+	fuzz_cli_last = (u16) tmp;
+      else
+	break;
+    }
+
+  if (fuzz_first == 0xFFFF)
+    {
+      vl_msg_api_fuzz_hook = 0;
+      return clib_error_return (0, "fuzz_first is ~0, fuzzing disabled");
+    }
+  vl_msg_api_fuzz_hook = fuzz_hook;
+
+  vlib_cli_output (vm, "Fuzzing enabled: first %d, skip cli range %d - %d",
+		   (u32) fuzz_first, (u32) fuzz_cli_first,
+		   (u32) fuzz_cli_last);
+
+  return 0;
+}
+
+/* *INDENT-OFF* */
+VLIB_CLI_COMMAND (test_api_fuzz, static) = {
+   .path = "test api fuzz",
+   .short_help = "test api fuzz [disable][seed nnn]\n"
+   "           [fuzz-first nn][fuzz-cli-first nn][fuzz-cli-last nn]",
+   .function = test_api_fuzz_command_fn,
+  };
+/* *INDENT-ON* */
+
+static u8 main_loop_enter_enable_api_fuzz;
+
+static clib_error_t *
+api_fuzz_config (vlib_main_t * vm, unformat_input_t * input)
+{
+  while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
+    {
+      if (unformat (input, "off")
+	  || unformat (input, "disable") || unformat (input, "no"))
+	;			/* ok, no action */
+      else if (unformat (input, "on")
+	       || unformat (input, "enable") || unformat (input, "yes"))
+	main_loop_enter_enable_api_fuzz = 1;
+      else
+	return clib_error_return (0, "unknown input '%U'",
+				  format_unformat_error, input);
+    }
+  return 0;
+}
+
+VLIB_CONFIG_FUNCTION (api_fuzz_config, "api-fuzz");
+
+static clib_error_t *
+api_fuzz_api_init (vlib_main_t * vm)
+{
+  /* Are we supposed to fuzz API messages? */
+  if (main_loop_enter_enable_api_fuzz == 0)
+    return 0;
+
+  default_fuzz_config ();
+
+  if (fuzz_first == 0xFFFF)
+    {
+      return clib_error_return
+	(0, "Couldn't find 'memclnt_keepalive_reply' ID");
+    }
+  /* Turn on fuzzing */
+  vl_msg_api_fuzz_hook = fuzz_hook;
+  return 0;
+}
+
+VLIB_API_INIT_FUNCTION (api_fuzz_api_init);
+
+/*
+ * fd.io coding-style-patch-verification: ON
+ *
+ * Local Variables:
+ * eval: (c-set-style "gnu")
+ * End:
+ */
diff --git a/src/vat/api_format.c b/src/vat/api_format.c
index d443239a940..7d16441f03b 100644
--- a/src/vat/api_format.c
+++ b/src/vat/api_format.c
@@ -1165,7 +1165,7 @@ vl_api_cli_inband_reply_t_handler (vl_api_cli_inband_reply_t * mp)
 
   vam->retval = retval;
   if (retval == 0)
-    vam->cmd_reply = vl_api_from_api_to_new_vec (&mp->reply);
+    vam->cmd_reply = vl_api_from_api_to_new_vec (mp, &mp->reply);
   vam->result_ready = 1;
 }
 
@@ -1176,7 +1176,7 @@ vl_api_cli_inband_reply_t_handler_json (vl_api_cli_inband_reply_t * mp)
   vat_json_node_t node;
   u8 *reply = 0;		/* reply vector */
 
-  reply = vl_api_from_api_to_new_vec (&mp->reply);
+  reply = vl_api_from_api_to_new_vec (mp, &mp->reply);
   vec_reset_length (vam->cmd_reply);
 
   vat_json_init_object (&node);
diff --git a/src/vlibapi/api.h b/src/vlibapi/api.h
index 3d5c8698f73..431155c5e09 100644
--- a/src/vlibapi/api.h
+++ b/src/vlibapi/api.h
@@ -109,6 +109,7 @@ void vl_msg_api_handler_with_vm_node (api_main_t * am, svm_region_t * vlib_rp,
 				      void *the_msg, vlib_main_t * vm,
 				      vlib_node_runtime_t * node,
 				      u8 is_private);
+u32 vl_msg_api_max_length (void *mp);
 vl_api_trace_t *vl_msg_api_trace_get (api_main_t * am,
 				      vl_api_trace_which_t which);
 void vl_msg_api_add_msg_name_crc (api_main_t * am, const char *string,
diff --git a/src/vlibapi/api_shared.c b/src/vlibapi/api_shared.c
index 28e9f481650..fbaa9c9e013 100644
--- a/src/vlibapi/api_shared.c
+++ b/src/vlibapi/api_shared.c
@@ -533,6 +533,8 @@ msg_handler_internal (api_main_t * am,
     }
 }
 
+void (*vl_msg_api_fuzz_hook) (u16, void *);
+
 /* This is only to be called from a vlib/vnet app */
 void
 vl_msg_api_handler_with_vm_node (api_main_t * am, svm_region_t * vlib_rp,
@@ -600,6 +602,10 @@ vl_msg_api_handler_with_vm_node (api_main_t * am, svm_region_t * vlib_rp,
 	  am->vlib_rp = vlib_rp;
 	  am->shmem_hdr = (void *) vlib_rp->user_ctx;
 	}
+
+      if (PREDICT_FALSE (vl_msg_api_fuzz_hook != 0))
+	(*vl_msg_api_fuzz_hook) (id, the_msg);
+
       (*handler) (the_msg, vm, node);
       if (is_private)
 	{
@@ -870,6 +876,21 @@ vl_msg_api_queue_handler (svm_queue_t * q)
     vl_msg_api_handler ((void *) msg);
 }
 
+u32
+vl_msg_api_max_length (void *mp)
+{
+  msgbuf_t *mb;
+  u32 data_len = ~0;
+
+  /* Work out the maximum sane message length, and return it */
+  if (PREDICT_TRUE (mp != 0))
+    {
+      mb = (msgbuf_t *) (((u8 *) mp) - offsetof (msgbuf_t, data));
+      data_len = clib_net_to_host_u32 (mb->data_len);
+    }
+  return data_len;
+}
+
 vl_api_trace_t *
 vl_msg_api_trace_get (api_main_t * am, vl_api_trace_which_t which)
 {
@@ -1132,9 +1153,13 @@ vl_api_format_string (u8 * s, va_list * args)
  * NOT nul terminated.
  */
 u8 *
-vl_api_from_api_to_new_vec (vl_api_string_t * astr)
+vl_api_from_api_to_new_vec (void *mp, vl_api_string_t * astr)
 {
   u8 *v = 0;
+
+  if (vl_msg_api_max_length (mp) < clib_net_to_host_u32 (astr->length))
+    return format (0, "insane astr->length %u%c",
+		   clib_net_to_host_u32 (astr->length), 0);
   vec_add (v, astr->buf, clib_net_to_host_u32 (astr->length));
   return v;
 }
diff --git a/src/vlibapi/api_types.h b/src/vlibapi/api_types.h
index 21fcde53b0e..07c7aaba0c5 100644
--- a/src/vlibapi/api_types.h
+++ b/src/vlibapi/api_types.h
@@ -46,7 +46,7 @@ extern int vl_api_vec_to_api_string (const u8 *vec, vl_api_string_t * str);
 extern u32 vl_api_string_len (vl_api_string_t * astr);
 
 /* Returns new vector. NON nul terminated */
-extern u8 * vl_api_from_api_to_new_vec (vl_api_string_t *astr);
+extern u8 * vl_api_from_api_to_new_vec (void *mp, vl_api_string_t *astr);
 /* Returns new vector. Nul terminated */
 extern char * vl_api_from_api_to_new_c_string (vl_api_string_t *astr);
 
diff --git a/src/vnet/devices/tap/tapv2_api.c b/src/vnet/devices/tap/tapv2_api.c
index 05679377d2a..5aca93ec310 100644
--- a/src/vnet/devices/tap/tapv2_api.c
+++ b/src/vnet/devices/tap/tapv2_api.c
@@ -134,7 +134,7 @@ vl_api_tap_create_v2_t_handler (vl_api_tap_create_v2_t * mp)
   /* If a tag was supplied... */
   if (vl_api_string_len (&mp->tag))
     {
-      u8 *tag = vl_api_from_api_to_new_vec (&mp->tag);
+      u8 *tag = vl_api_from_api_to_new_vec (mp, &mp->tag);
       vnet_set_sw_interface_tag (vnm, tag, ap->sw_if_index);
     }
 
diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c
index 5b24a29efc9..846420d7378 100644
--- a/src/vnet/interface_api.c
+++ b/src/vnet/interface_api.c
@@ -361,7 +361,7 @@ vl_api_sw_interface_dump_t_handler (vl_api_sw_interface_dump_t * mp)
 
   if (mp->name_filter_valid)
     {
-      filter = vl_api_from_api_to_new_vec (&mp->name_filter);
+      filter = vl_api_from_api_to_new_vec (mp, &mp->name_filter);
       vec_add1 (filter, 0);	/* Ensure it's a C string for strcasecmp() */
     }
 
diff --git a/src/vnet/ip/ip_api.c b/src/vnet/ip/ip_api.c
index 3bc46fee59b..d5fa2d32f21 100644
--- a/src/vnet/ip/ip_api.c
+++ b/src/vnet/ip/ip_api.c
@@ -1544,13 +1544,14 @@ static void
 vl_api_ip_punt_redirect_dump_t_handler (vl_api_ip_punt_redirect_dump_t * mp)
 {
   vl_api_registration_t *reg;
-  fib_protocol_t fproto;
+  fib_protocol_t fproto = FIB_PROTOCOL_IP4;
 
   reg = vl_api_client_index_to_registration (mp->client_index);
   if (!reg)
     return;
 
-  fproto = mp->is_ipv6 ? FIB_PROTOCOL_IP6 : FIB_PROTOCOL_IP4;
+  if (mp->is_ipv6 == 1)
+    fproto = FIB_PROTOCOL_IP6;
 
   ip_punt_redirect_walk_ctx_t ctx = {
     .reg = reg,
diff --git a/src/vnet/ip/punt_api.c b/src/vnet/ip/punt_api.c
index 2acf8265dc1..3a964b4b7f5 100644
--- a/src/vnet/ip/punt_api.c
+++ b/src/vnet/ip/punt_api.c
@@ -372,7 +372,7 @@ vl_api_punt_reason_dump_t_handler (vl_api_punt_reason_dump_t * mp)
   punt_reason_dump_walk_ctx_t ctx = {
     .reg = reg,
     .context = mp->context,
-    .name = vl_api_from_api_to_new_vec (&mp->reason.name),
+    .name = vl_api_from_api_to_new_vec (mp, &mp->reason.name),
   };
 
   punt_reason_walk (punt_reason_dump_walk_cb, &ctx);
diff --git a/src/vnet/lldp/lldp_api.c b/src/vnet/lldp/lldp_api.c
index a6d7fb84bb7..ecb75bcac4b 100644
--- a/src/vnet/lldp/lldp_api.c
+++ b/src/vnet/lldp/lldp_api.c
@@ -57,7 +57,7 @@ vl_api_lldp_config_t_handler (vl_api_lldp_config_t * mp)
   int rv = 0;
   u8 *sys_name = 0;
 
-  sys_name = vl_api_from_api_to_new_vec (&mp->system_name);
+  sys_name = vl_api_from_api_to_new_vec (mp, &mp->system_name);
 
   if (lldp_cfg_set (&sys_name, ntohl (mp->tx_hold), ntohl (mp->tx_interval))
       != lldp_ok)
diff --git a/src/vnet/pg/pg_api.c b/src/vnet/pg/pg_api.c
index 18de1e9e8c4..bb58a4f0cec 100644
--- a/src/vnet/pg/pg_api.c
+++ b/src/vnet/pg/pg_api.c
@@ -122,7 +122,7 @@ vl_api_pg_enable_disable_t_handler (vl_api_pg_enable_disable_t * mp)
 
   if (vl_api_string_len (&mp->stream_name) > 0)
     {
-      u8 *stream_name = vl_api_from_api_to_new_vec (&mp->stream_name);
+      u8 *stream_name = vl_api_from_api_to_new_vec (mp, &mp->stream_name);
       uword *p = hash_get_mem (pg->stream_index_by_name, stream_name);
       if (p)
 	stream_index = *p;
diff --git a/src/vnet/session/session_api.c b/src/vnet/session/session_api.c
index cfeb7bcc349..593f2e18ba0 100644
--- a/src/vnet/session/session_api.c
+++ b/src/vnet/session/session_api.c
@@ -624,7 +624,7 @@ vl_api_app_attach_t_handler (vl_api_app_attach_t * mp)
   a->options = mp->options;
   a->session_cb_vft = &session_mq_cb_vft;
 
-  a->namespace_id = vl_api_from_api_to_new_vec (&mp->namespace_id);
+  a->namespace_id = vl_api_from_api_to_new_vec (mp, &mp->namespace_id);
 
   if ((rv = vnet_application_attach (a)))
     {
@@ -801,7 +801,7 @@ vl_api_app_namespace_add_del_t_handler (vl_api_app_namespace_add_del_t * mp)
       goto done;
     }
 
-  ns_id = vl_api_from_api_to_new_vec (&mp->namespace_id);
+  ns_id = vl_api_from_api_to_new_vec (mp, &mp->namespace_id);
 
   vnet_app_namespace_add_del_args_t args = {
     .ns_id = ns_id,
diff --git a/src/vpp/api/api.c b/src/vpp/api/api.c
index 01527a565f3..c3380be0b80 100644
--- a/src/vpp/api/api.c
+++ b/src/vpp/api/api.c
@@ -221,7 +221,7 @@ vl_api_cli_inband_t_handler (vl_api_cli_inband_t * mp)
       goto error;
     }
 
-  cmd_vec = vl_api_from_api_to_new_vec (&mp->cmd);
+  cmd_vec = vl_api_from_api_to_new_vec (mp, &mp->cmd);
 
   unformat_init_string (&input, (char *) cmd_vec,
 			vl_api_string_len (&mp->cmd));
diff --git a/src/vpp/api/custom_dump.c b/src/vpp/api/custom_dump.c
index e0ba9f93c3d..edbd3b65e9d 100644
--- a/src/vpp/api/custom_dump.c
+++ b/src/vpp/api/custom_dump.c
@@ -1799,7 +1799,7 @@ static void *vl_api_sw_interface_dump_t_print
 
   if (mp->name_filter_valid)
     {
-      u8 *v = vl_api_from_api_to_new_vec (&mp->name_filter);
+      u8 *v = vl_api_from_api_to_new_vec (mp, &mp->name_filter);
       s = format (s, "name_filter %v ", v);
       vec_free (v);
     }
@@ -1858,7 +1858,7 @@ static void *vl_api_cli_inband_t_print
   u8 *s;
   u8 *cmd = 0;
 
-  cmd = vl_api_from_api_to_new_vec (&mp->cmd);
+  cmd = vl_api_from_api_to_new_vec (mp, &mp->cmd);
 
   s = format (0, "SCRIPT: exec %v ", cmd);