Update DHCPv6 DUID code and fix coverity warnings

- Generate client DUID only when DHCPv6 PD feature is enabled
- Change client DUID type from DUID-LLT to DUID-LL
- Fix coverity warnings

Change-Id: I20e518fc9a1c5f3f7ea9add7e7e03a487c99e978
Signed-off-by: Juraj Sloboda <jsloboda@cisco.com>

diff --git a/src/vnet/dhcp/dhcp.api b/src/vnet/dhcp/dhcp.api
index 82c03fc..ab0f7b6 100644 (file)
--- a/src/vnet/dhcp/dhcp.api
+++ b/src/vnet/dhcp/dhcp.api
@@ -186,6 +186,18 @@ manual_endian manual_print define dhcp_proxy_details
   vl_api_dhcp_server_t servers[count];
 };
 
+/** \brief Set DHCPv6 DUID-LL
+    @param client_index - opaque cookie to identify the sender
+    @param context - sender context, to match reply w/ request
+    @param duid_ll - DUID-LL binary string
+*/
+autoreply define dhcp6_duid_ll_set
+{
+  u32 client_index;
+  u32 context;
+  u8 duid_ll[10];
+};
+
 /** \brief Enable/disable listening on DHCPv6 client port
     @param client_index - opaque cookie to identify the sender
     @param context - sender context, to match reply w/ request

diff --git a/src/vnet/dhcp/dhcp6_pd_client_cp.c b/src/vnet/dhcp/dhcp6_pd_client_cp.c
index ecb85e1..a2a0618 100644 (file)
--- a/src/vnet/dhcp/dhcp6_pd_client_cp.c
+++ b/src/vnet/dhcp/dhcp6_pd_client_cp.c
@@ -658,7 +658,7 @@ cp_ip6_construct_address (ip6_address_info_t * address_info, u32 prefix_index,
       clib_warning ("Prefix length is bigger that 64 bits");
       return 1;
     }
-  mask = (1 << (64 - prefix->prefix_length)) - 1;
+  mask = ((u64) 1 << (64 - prefix->prefix_length)) - 1;
   addr0 &= mask;
   pref = clib_host_to_net_u64 (prefix->prefix.as_u64[0]);
   pref &= ~mask;
@@ -871,10 +871,17 @@ cp_ip6_address_add_del (u32 sw_if_index, const u8 * prefix_group,
 {
 
   ip6_address_with_prefix_main_t *apm = &ip6_address_with_prefix_main;
+  vnet_main_t *vnm = vnet_get_main ();
   ip6_address_info_t *address_info;
   u32 prefix_group_index;
   u32 n;
 
+  if (!vnet_sw_interface_is_api_valid (vnm, sw_if_index))
+    {
+      clib_warning ("Invalid sw_if_index");
+      return VNET_API_ERROR_INVALID_VALUE;
+    }
+
   if (prefix_group != 0 && prefix_group[0] != '\0')
     {
       if (strnlen ((const char *) prefix_group, 64) == 64)
@@ -927,12 +934,9 @@ static void
   u8 prefix_length;
   int rv = 0;
 
+  VALIDATE_SW_IF_INDEX (mp);
+
   sw_if_index = ntohl (mp->sw_if_index);
-  if (!vnet_sw_if_index_is_api_valid (sw_if_index))
-    {
-      rv = VNET_API_ERROR_INVALID_SW_IF_INDEX;
-      goto bad_sw_if_index;
-    }
 
   memcpy (address.as_u8, mp->address, 16);
   prefix_length = mp->prefix_length;

diff --git a/src/vnet/dhcp/dhcp6_pd_client_dp.c b/src/vnet/dhcp/dhcp6_pd_client_dp.c
index c14a311..b2b8f88 100644 (file)
--- a/src/vnet/dhcp/dhcp6_pd_client_dp.c
+++ b/src/vnet/dhcp/dhcp6_pd_client_dp.c
@@ -112,13 +112,12 @@ typedef union
               {
               u16 duid_type;
               u16 hardware_type;
-              u32 time;
               u8 lla[6];
               });
-  char bin_string[14];
-} dhcpv6_duid_string_t;
+  char bin_string[10];
+} dhcpv6_duid_ll_string_t;
 
-static dhcpv6_duid_string_t client_duid;
+static dhcpv6_duid_ll_string_t client_duid;
 #define CLIENT_DUID_LENGTH sizeof (client_duid)
 #define DHCPV6_CLIENT_IAID 1
 
@@ -409,9 +408,20 @@ dhcpv6_pd_client_node_fn (vlib_main_t * vm, vlib_node_runtime_t * node,
                          discard = 1;
                        }
                      else
-                       report.server_index =
-                         server_index_get_or_create (option->data,
-                                                     ntohs (option->length));
+                       {
+                         u16 ol = ntohs (option->length);
+                         if (ol - 2 /* 2 byte DUID type code */  > 128)
+                           {
+                             clib_warning
+                               ("Server DUID (without type code) is longer than 128 octets");
+                             discard = 1;
+                           }
+                         else
+                           {
+                             report.server_index =
+                               server_index_get_or_create (option->data, ol);
+                           }
+                       }
                    }
                  else if (oo == DHCPV6_OPTION_PREFERENCE)
                    {
@@ -1049,37 +1059,29 @@ reply:
 }
 
 void
-dhcp6_clients_enable_disable (u8 enable)
+vl_api_dhcp6_duid_ll_set_t_handler (vl_api_dhcp6_duid_ll_set_t * mp)
 {
-  vlib_main_t *vm = vlib_get_main ();
-
-  if (enable)
-    udp_register_dst_port (vm, UDP_DST_PORT_dhcpv6_to_client,
-                          dhcpv6_pd_client_node.index, 0 /* is_ip6 */ );
-  else
-    udp_unregister_dst_port (vm, UDP_DST_PORT_dhcpv6_to_client,
-                            0 /* is_ip6 */ );
-}
-
-void
-  vl_api_dhcp6_clients_enable_disable_t_handler
-  (vl_api_dhcp6_clients_enable_disable_t * mp)
-{
-  vl_api_dhcp6_clients_enable_disable_reply_t *rmp;
+  vl_api_dhcp6_duid_ll_set_reply_t *rmp;
+  dhcpv6_duid_ll_string_t *duid;
   int rv = 0;
 
-  dhcp6_clients_enable_disable (mp->enable);
+  duid = (dhcpv6_duid_ll_string_t *) mp->duid_ll;
+  if (duid->duid_type != htonl (DHCPV6_DUID_LL))
+    {
+      rv = VNET_API_ERROR_INVALID_VALUE;
+      goto reply;
+    }
+  clib_memcpy (&client_duid, &duid, sizeof (client_duid));
 
-  REPLY_MACRO (VL_API_WANT_DHCP6_PD_REPLY_EVENTS_REPLY);
+reply:
+  REPLY_MACRO (VL_API_DHCP6_DUID_LL_SET_REPLY);
 }
 
 static void
-genereate_client_duid (void)
+generate_client_duid (void)
 {
-  client_duid.duid_type = htons (DHCPV6_DUID_LLT);
+  client_duid.duid_type = htons (DHCPV6_DUID_LL);
   client_duid.hardware_type = htons (1);
-  u32 time_since_2000 = (u32) time (0) - 946684800;
-  client_duid.time = htonl (time_since_2000);
 
   vnet_main_t *vnm = vnet_get_main ();
   vnet_interface_main_t *im = &vnm->interface_main;
@@ -1112,6 +1114,35 @@ genereate_client_duid (void)
     }
 }
 
+void
+dhcp6_clients_enable_disable (u8 enable)
+{
+  vlib_main_t *vm = vlib_get_main ();
+
+  if (enable)
+    {
+      if (client_duid.duid_type == 0)
+       generate_client_duid ();
+      udp_register_dst_port (vm, UDP_DST_PORT_dhcpv6_to_client,
+                            dhcpv6_pd_client_node.index, 0 /* is_ip6 */ );
+    }
+  else
+    udp_unregister_dst_port (vm, UDP_DST_PORT_dhcpv6_to_client,
+                            0 /* is_ip6 */ );
+}
+
+void
+  vl_api_dhcp6_clients_enable_disable_t_handler
+  (vl_api_dhcp6_clients_enable_disable_t * mp)
+{
+  vl_api_dhcp6_clients_enable_disable_reply_t *rmp;
+  int rv = 0;
+
+  dhcp6_clients_enable_disable (mp->enable);
+
+  REPLY_MACRO (VL_API_WANT_DHCP6_PD_REPLY_EVENTS_REPLY);
+}
+
 static clib_error_t *
 dhcp6_pd_client_init (vlib_main_t * vm)
 {
@@ -1124,9 +1155,6 @@ dhcp6_pd_client_init (vlib_main_t * vm)
 
   cm->seed = 0xdeaddabe;
 
-  // TODO: should be stored in non-volatile memory
-  genereate_client_duid ();
-
   return 0;
 }
 

diff --git a/src/vnet/dhcp/dhcp6_pd_client_dp.h b/src/vnet/dhcp/dhcp6_pd_client_dp.h
index eec8fe8..ef69f19 100644 (file)
--- a/src/vnet/dhcp/dhcp6_pd_client_dp.h
+++ b/src/vnet/dhcp/dhcp6_pd_client_dp.h
@@ -59,6 +59,7 @@ void
 void
   vl_api_dhcp6_clients_enable_disable_t_handler
   (vl_api_dhcp6_clients_enable_disable_t * mp);
+void vl_api_dhcp6_duid_ll_set_t_handler (vl_api_dhcp6_duid_ll_set_t * mp);
 
 extern vlib_node_registration_t dhcp6_pd_reply_process_node;
 

diff --git a/src/vnet/dhcp/dhcp_api.c b/src/vnet/dhcp/dhcp_api.c
index 5aed4c0..4e2ed86 100644 (file)
--- a/src/vnet/dhcp/dhcp_api.c
+++ b/src/vnet/dhcp/dhcp_api.c
@@ -53,7 +53,8 @@ _(DHCP_CLIENT_CONFIG, dhcp_client_config)         \
 _(DHCP_CLIENT_DUMP, dhcp_client_dump)             \
 _(WANT_DHCP6_PD_REPLY_EVENTS, want_dhcp6_pd_reply_events)               \
 _(DHCP6_PD_SEND_CLIENT_MESSAGE, dhcp6_pd_send_client_message)           \
-_(DHCP6_CLIENTS_ENABLE_DISABLE, dhcp6_clients_enable_disable)
+_(DHCP6_CLIENTS_ENABLE_DISABLE, dhcp6_clients_enable_disable)           \
+_(DHCP6_DUID_LL_SET, dhcp6_duid_ll_set)
 
 
 static void

diff --git a/test/test_dhcp6.py b/test/test_dhcp6.py
index 6acc344..3976f22 100644 (file)
--- a/test/test_dhcp6.py
+++ b/test/test_dhcp6.py
@@ -1,7 +1,7 @@
 from scapy.layers.dhcp6 import DHCP6_Advertise, DHCP6OptClientId, \
     DHCP6OptStatusCode, DHCP6OptPref, DHCP6OptIA_PD, DHCP6OptIAPrefix, \
     DHCP6OptServerId, DHCP6_Solicit, DHCP6_Reply, DHCP6_Request, DHCP6_Renew, \
-    DHCP6_Rebind, DUID_LLT, DHCP6_Release, DHCP6OptElapsedTime
+    DHCP6_Rebind, DUID_LL, DHCP6_Release, DHCP6OptElapsedTime
 from scapy.layers.inet6 import IPv6, Ether, UDP
 from scapy.utils6 import in6_mactoifaceid
 from scapy.utils import inet_ntop, inet_pton
@@ -36,9 +36,7 @@ class TestDHCPv6PD(VppTestCase):
             i.admin_up()
             i.config_ip6()
 
-        time_since_2000 = int(time()) - 946684800
-        self.server_duid = DUID_LLT(timeval=time_since_2000,
-                                    lladdr=self.pg0.remote_mac)
+        self.server_duid = DUID_LL(lladdr=self.pg0.remote_mac)
 
     def tearDown(self):
         for i in self.interfaces:
@@ -49,6 +47,8 @@ class TestDHCPv6PD(VppTestCase):
     def test_dhcp_send_solicit_receive_advertise(self):
         """ Verify DHCPv6 PD Solicit packet and received Advertise envent """
 
+        self.vapi.dhcp6_clients_enable_disable()
+
         self.pg_enable_capture(self.pg_interfaces)
         self.pg_start()
         prefix_bin = '\00\01\00\02\00\03' + '\00' * 10
@@ -85,7 +85,6 @@ class TestDHCPv6PD(VppTestCase):
         self.assert_equal(prefix.validlft, 120)
 
         self.vapi.want_dhcp6_pd_reply_events()
-        self.vapi.dhcp6_clients_enable_disable()
 
         ia_pd_opts = DHCP6OptIAPrefix(prefix='7:8::', plen=56, preflft=60,
                                       validlft=120)
@@ -103,7 +102,7 @@ class TestDHCPv6PD(VppTestCase):
         self.pg0.add_stream([p])
         self.pg_start()
 
-        ev = self.vapi.wait_for_event(10, "dhcp6_pd_reply_event")
+        ev = self.vapi.wait_for_event(1, "dhcp6_pd_reply_event")
 
         self.assert_equal(ev.preference, 7)
         self.assert_equal(ev.status_code, 1)
@@ -136,9 +135,7 @@ class TestDHCPv6PDControlPlane(VppTestCase):
         for i in self.interfaces:
             i.admin_up()
 
-        time_since_2000 = int(time()) - 946684800
-        self.server_duid = DUID_LLT(timeval=time_since_2000,
-                                    lladdr=self.pg0.remote_mac)
+        self.server_duid = DUID_LL(lladdr=self.pg0.remote_mac)
         self.client_duid = None
         self.T1 = 1
         self.T2 = 2
@@ -180,8 +177,8 @@ class TestDHCPv6PDControlPlane(VppTestCase):
         addresses = set(self.get_interface_addresses(fib, self.pg1))
         return addresses.difference(self.initial_addresses)
 
-    def validate_duid_llt(self, duid):
-        DUID_LLT(duid)
+    def validate_duid_ll(self, duid):
+        DUID_LL(duid)
 
     def validate_packet(self, packet, msg_type, is_resend=False):
         try:
@@ -189,7 +186,7 @@ class TestDHCPv6PDControlPlane(VppTestCase):
             client_duid = packet[DHCP6OptClientId].duid
             if self.client_duid is None:
                 self.client_duid = client_duid
-                self.validate_duid_llt(client_duid)
+                self.validate_duid_ll(client_duid)
             else:
                 self.assertEqual(self.client_duid, client_duid)
             if msg_type != DHCP6_Solicit and msg_type != DHCP6_Rebind: