libnm-core: use different ifname validation function for OVS bridges, ports and interfaces

OVS bridges and ports do not have the length limitation of 15 bytes, the
only requirements are that all chars must be alphanumeric and not be
forward or backward slashes.

For OVS interfaces only 'patch' types do not have the length limit, all
the other types do (according to whether they have a corresponding
kernel link or not).

Add related unit test.

https://bugzilla.redhat.com/show_bug.cgi?id=1788432

libnm-core/nm-connection-private.h

@@ -27,6 +27,7 @@ int _nm_setting_ovs_interface_verify_interface_type (NMSettingOvsInterface *self
                                                      NMConnection *connection,
                                                      gboolean normalize,
                                                      gboolean *out_modified,
+                                                     const char **normalized_type,
                                                      GError **error);
 
 #endif  /* __NM_CONNECTION_PRIVATE_H__ */

libnm-core/nm-connection.c

@@ -1239,6 +1239,7 @@ _normalize_ovs_interface_type (NMConnection *self)
 	                                                     self,
 	                                                     TRUE,
 	                                                     &modified,
+	                                                     NULL,
 	                                                     NULL);
 	if (v != TRUE)
 		g_return_val_if_reached (modified);

libnm-core/nm-setting-connection.c

@@ -973,20 +973,6 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		return FALSE;
 	}
 
-	if (priv->interface_name) {
-		GError *tmp_error = NULL;
-
-		if (!nm_utils_is_valid_iface_name (priv->interface_name, &tmp_error)) {
-			g_set_error (error,
-			             NM_CONNECTION_ERROR,
-			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
-			             "'%s': %s", priv->interface_name, tmp_error->message);
-			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME);
-			g_error_free (tmp_error);
-			return FALSE;
-		}
-	}
-
 	type = priv->type;
 	if (!type) {
 		if (   !connection
@@ -1044,6 +1030,34 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		}
 	}
 
+	if (priv->interface_name) {
+		GError *tmp_error = NULL;
+		gboolean valid_ifname = FALSE;
+
+		/* do not perform a interface name length check for OVS connection types
+		 * as they don't have a corresponding kernel link that enforces the 15 bytes limit.
+		 * Here we're whitelisting the OVS interface type as well, even if most OVS
+		 * iface types do have the limit, to let the OVS specific nm-setting verify whether the iface name
+		 * is good or not according to the internal type (internal, patch, ...) */
+		if (NM_IN_STRSET (type,
+		                  NM_SETTING_OVS_BRIDGE_SETTING_NAME,
+		                  NM_SETTING_OVS_PORT_SETTING_NAME,
+		                  NM_SETTING_OVS_INTERFACE_SETTING_NAME))
+			valid_ifname = nm_utils_ifname_valid (priv->interface_name, NMU_IFACE_OVS, &tmp_error);
+		else
+			valid_ifname = nm_utils_ifname_valid (priv->interface_name, NMU_IFACE_KERNEL, &tmp_error);
+
+		if (!valid_ifname) {
+			g_set_error (error,
+			             NM_CONNECTION_ERROR,
+			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
+			             "'%s': %s", priv->interface_name, tmp_error->message);
+			g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME);
+			g_error_free (tmp_error);
+			return FALSE;
+		}
+	}
+
 	is_slave = FALSE;
 	slave_setting_type = NULL;
 	slave_type = priv->slave_type;

libnm-core/nm-setting-ovs-interface.c

@@ -67,6 +67,7 @@ _nm_setting_ovs_interface_verify_interface_type (NMSettingOvsInterface *self,
                                                  NMConnection *connection,
                                                  gboolean normalize,
                                                  gboolean *out_modified,
+                                                 const char **normalized_type,
                                                  GError **error)
 {
 	const char *type;
@@ -82,6 +83,8 @@ _nm_setting_ovs_interface_verify_interface_type (NMSettingOvsInterface *self,
 	} else
 		g_return_val_if_fail (!connection || NM_IS_CONNECTION (connection), FALSE);
 
+	g_return_val_if_fail (!normalized_type || !(*normalized_type), FALSE);
+
 	NM_SET_OUT (out_modified, FALSE);
 
 	type = self->type;
@@ -212,6 +215,10 @@ _nm_setting_ovs_interface_verify_interface_type (NMSettingOvsInterface *self,
 		type = "internal";
 	else
 		type = "system";
+
+	if (normalized_type)
+		*normalized_type = type;
+
 normalize:
 	if (!normalize) {
 		if (!self) {
@@ -246,9 +253,11 @@ static int
 verify (NMSetting *setting, NMConnection *connection, GError **error)
 {
 	NMSettingOvsInterface *self = NM_SETTING_OVS_INTERFACE (setting);
+	NMSettingConnection *s_con = NULL;
+	const char *normalized_type = NULL;
+	int result = NM_SETTING_VERIFY_ERROR;
 
 	if (connection) {
-		NMSettingConnection *s_con;
 		const char *slave_type;
 
 		s_con = nm_connection_get_setting_connection (connection);
@@ -286,11 +295,39 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		}
 	}
 
-	return _nm_setting_ovs_interface_verify_interface_type (self,
+	result = _nm_setting_ovs_interface_verify_interface_type (self,
-	                                                        connection,
+	                                                          connection,
-	                                                        FALSE,
+	                                                          FALSE,
-	                                                        NULL,
+	                                                          NULL,
-	                                                        error);
+	                                                          &normalized_type,
+	                                                          error);
+
+	/* From 'man ovs-vswitchd.conf.db': OVS patch interfaces do not have
+	 * a limit on interface name length, all the other types do */
+	if (result != NM_SETTING_VERIFY_ERROR && s_con) {
+		gs_free_error GError *ifname_error = NULL;
+		const char *ifname = nm_setting_connection_get_interface_name (s_con);
+
+		normalized_type = self->type ? self->type : normalized_type;
+
+		if (   ifname
+		    && !nm_streq0 (normalized_type, "patch")
+		    && !nm_utils_ifname_valid (ifname,
+		                               NMU_IFACE_KERNEL,
+		                               &ifname_error)) {
+			g_clear_error (error);
+			g_set_error (error,
+			             NM_CONNECTION_ERROR,
+			             NM_CONNECTION_ERROR_INVALID_PROPERTY,
+			             "'%s': %s", ifname, ifname_error->message);
+			g_prefix_error (error, "%s.%s: ",
+			                NM_SETTING_CONNECTION_SETTING_NAME,
+			                NM_SETTING_CONNECTION_INTERFACE_NAME);
+			return NM_SETTING_VERIFY_ERROR;
+		}
+	}
+
+	return result;
 }
 
 /*****************************************************************************/

libnm-core/tests/test-general.c

@@ -8353,6 +8353,203 @@ test_nm_ip_addr_zero (void)
 	G_STATIC_ASSERT_EXPR (sizeof (a) == sizeof (a.array));
 }
 
+static void
+test_connection_ovs_ifname (gconstpointer test_data)
+{
+	const guint TEST_CASE = GPOINTER_TO_UINT (test_data);
+	gs_unref_object NMConnection *con = NULL;
+	NMSettingConnection *s_con = NULL;
+	NMSettingOvsBridge *s_ovs_bridge = NULL;
+	NMSettingOvsPort *s_ovs_port = NULL;
+	NMSettingOvsInterface *s_ovs_iface = NULL;
+	NMSettingOvsPatch *s_ovs_patch = NULL;
+	const char *ovs_iface_type = NULL;
+
+	switch (TEST_CASE) {
+	case 1:
+		con = nmtst_create_minimal_connection ("test_connection_ovs_ifname_bridge",
+		                                       NULL,
+		                                       NM_SETTING_OVS_BRIDGE_SETTING_NAME, &s_con);
+		s_ovs_bridge = nm_connection_get_setting_ovs_bridge (con);
+		g_assert (s_ovs_bridge);
+		break;
+	case 2:
+		con = nmtst_create_minimal_connection ("test_connection_ovs_ifname_port",
+		                                       NULL,
+		                                       NM_SETTING_OVS_PORT_SETTING_NAME, &s_con);
+
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_MASTER,
+		              "master0",
+		              NM_SETTING_CONNECTION_SLAVE_TYPE,
+		              NM_SETTING_OVS_BRIDGE_SETTING_NAME,
+		              NULL);
+
+		s_ovs_port = nm_connection_get_setting_ovs_port (con);
+		g_assert (s_ovs_port);
+		break;
+	case 3:
+		con = nmtst_create_minimal_connection ("test_connection_ovs_ifname_interface_patch",
+		                                       NULL,
+		                                       NM_SETTING_OVS_INTERFACE_SETTING_NAME, &s_con);
+		s_ovs_iface = nm_connection_get_setting_ovs_interface (con);
+		g_assert (s_ovs_iface);
+
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_MASTER,
+		              "master0",
+		              NM_SETTING_CONNECTION_SLAVE_TYPE,
+		              NM_SETTING_OVS_PORT_SETTING_NAME,
+		              NULL);
+
+		g_object_set (s_ovs_iface,
+		              NM_SETTING_OVS_INTERFACE_TYPE,
+		              "patch",
+		              NULL);
+
+		s_ovs_patch = NM_SETTING_OVS_PATCH (nm_setting_ovs_patch_new());
+		g_assert (s_ovs_patch);
+
+		g_object_set (s_ovs_patch,
+		              NM_SETTING_OVS_PATCH_PEER, "1.2.3.4",
+		              NULL);
+
+		nm_connection_add_setting (con, NM_SETTING (s_ovs_patch));
+		s_ovs_patch = nm_connection_get_setting_ovs_patch (con);
+		g_assert (s_ovs_patch);
+		ovs_iface_type = "patch";
+		break;
+	case 4:
+		con = nmtst_create_minimal_connection ("test_connection_ovs_ifname_interface_internal",
+		                                       NULL,
+		                                       NM_SETTING_OVS_INTERFACE_SETTING_NAME, &s_con);
+		s_ovs_iface = nm_connection_get_setting_ovs_interface (con);
+		g_assert (s_ovs_iface);
+
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_MASTER,
+		              "master0",
+		              NM_SETTING_CONNECTION_SLAVE_TYPE,
+		              NM_SETTING_OVS_PORT_SETTING_NAME,
+		              NULL);
+
+		g_object_set (s_ovs_iface,
+		              NM_SETTING_OVS_INTERFACE_TYPE,
+		              "internal",
+		              NULL);
+		ovs_iface_type = "internal";
+		break;
+	case 5:
+		con = nmtst_create_minimal_connection ("test_connection_ovs_ifname_interface_system",
+		                                       NULL,
+		                                       NM_SETTING_WIRED_SETTING_NAME, &s_con);
+
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_MASTER,
+		              "master0",
+		              NM_SETTING_CONNECTION_SLAVE_TYPE,
+		              NM_SETTING_OVS_PORT_SETTING_NAME,
+		              NULL);
+
+		s_ovs_iface = NM_SETTING_OVS_INTERFACE (nm_setting_ovs_interface_new());
+		g_assert (s_ovs_iface);
+
+		g_object_set (s_ovs_iface,
+		              NM_SETTING_OVS_INTERFACE_TYPE,
+		              "system",
+		              NULL);
+
+		nm_connection_add_setting (con, NM_SETTING (s_ovs_iface));
+		s_ovs_iface = nm_connection_get_setting_ovs_interface (con);
+		g_assert (s_ovs_iface);
+
+		ovs_iface_type = "system";
+		break;
+	case 6:
+		con = nmtst_create_minimal_connection ("test_connection_ovs_ifname_interface_dpdk",
+		                                       NULL,
+		                                       NM_SETTING_OVS_INTERFACE_SETTING_NAME, &s_con);
+		s_ovs_iface = nm_connection_get_setting_ovs_interface (con);
+		g_assert (s_ovs_iface);
+
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_MASTER,
+		              "master0",
+		              NM_SETTING_CONNECTION_SLAVE_TYPE,
+		              NM_SETTING_OVS_PORT_SETTING_NAME,
+		              NULL);
+
+		g_object_set (s_ovs_iface,
+		              NM_SETTING_OVS_INTERFACE_TYPE,
+		              "dpdk",
+		              NULL);
+		ovs_iface_type = "dpdk";
+		break;
+	}
+
+	if (!nm_streq0 (ovs_iface_type, "system")) {
+		/* wrong: contains backward slash */
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_INTERFACE_NAME, "ovs\\0",
+		              NULL);
+		nmtst_assert_connection_unnormalizable (con,
+		                                        NM_CONNECTION_ERROR,
+		                                        NM_CONNECTION_ERROR_INVALID_PROPERTY);
+
+		/* wrong: contains forward slash */
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_INTERFACE_NAME, "ovs/0",
+		              NULL);
+		nmtst_assert_connection_unnormalizable (con,
+		                                        NM_CONNECTION_ERROR,
+		                                        NM_CONNECTION_ERROR_INVALID_PROPERTY);
+
+		/* wrong: contains non-alphanumerical char */
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_INTERFACE_NAME, "ovs-0",
+		              NULL);
+		nmtst_assert_connection_unnormalizable (con,
+		                                        NM_CONNECTION_ERROR,
+		                                        NM_CONNECTION_ERROR_INVALID_PROPERTY);
+
+		g_object_set (s_con,
+		              NM_SETTING_CONNECTION_INTERFACE_NAME, "ovs@0",
+		              NULL);
+		nmtst_assert_connection_unnormalizable (con,
+		                                        NM_CONNECTION_ERROR,
+		                                        NM_CONNECTION_ERROR_INVALID_PROPERTY);
+	}
+
+	/* wrong: contains space */
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_INTERFACE_NAME, "ovs 0",
+	              NULL);
+	nmtst_assert_connection_unnormalizable (con,
+	                                        NM_CONNECTION_ERROR,
+	                                        NM_CONNECTION_ERROR_INVALID_PROPERTY);
+
+	/* good */
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_INTERFACE_NAME, "ovs0",
+	              NULL);
+	nmtst_assert_connection_verifies (con);
+
+	/* good if bridge, port, or patch interface */
+	g_object_set (s_con,
+	              NM_SETTING_CONNECTION_INTERFACE_NAME, "ovs123123123123130123123",
+	              NULL);
+
+	if (!ovs_iface_type || nm_streq (ovs_iface_type, "patch"))
+		nmtst_assert_connection_verifies (con);
+	else {
+		nmtst_assert_connection_unnormalizable (con,
+		                                        NM_CONNECTION_ERROR,
+		                                        NM_CONNECTION_ERROR_INVALID_PROPERTY);
+	}
+}
+
+
+
 /*****************************************************************************/
 
 NMTST_DEFINE ();
@@ -8445,6 +8642,13 @@ int main (int argc, char **argv)
 	g_test_add_data_func ("/core/general/test_connection_normalize_ovs_interface_type_ovs_interface/11", GUINT_TO_POINTER (11), test_connection_normalize_ovs_interface_type_ovs_interface);
 	g_test_add_data_func ("/core/general/test_connection_normalize_ovs_interface_type_ovs_interface/12", GUINT_TO_POINTER (12), test_connection_normalize_ovs_interface_type_ovs_interface);
 
+	g_test_add_data_func ("/core/general/test_connection_ovs_ifname/1", GUINT_TO_POINTER (1), test_connection_ovs_ifname);
+	g_test_add_data_func ("/core/general/test_connection_ovs_ifname/2", GUINT_TO_POINTER (2), test_connection_ovs_ifname);
+	g_test_add_data_func ("/core/general/test_connection_ovs_ifname/3", GUINT_TO_POINTER (3), test_connection_ovs_ifname);
+	g_test_add_data_func ("/core/general/test_connection_ovs_ifname/4", GUINT_TO_POINTER (4), test_connection_ovs_ifname);
+	g_test_add_data_func ("/core/general/test_connection_ovs_ifname/5", GUINT_TO_POINTER (5), test_connection_ovs_ifname);
+	g_test_add_data_func ("/core/general/test_connection_ovs_ifname/6", GUINT_TO_POINTER (6), test_connection_ovs_ifname);
+
 	g_test_add_func ("/core/general/test_setting_connection_permissions_helpers", test_setting_connection_permissions_helpers);
 	g_test_add_func ("/core/general/test_setting_connection_permissions_property", test_setting_connection_permissions_property);