libnm-core: use different ifname validation function for OVS bridges, ports and interfacesac/ovs_ifname_length

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

5 files changed, 277 insertions, 20 deletions

diff --git a/libnm-core/nm-connection-private.h b/libnm-core/nm-connection-private.h
index ea7583e0e1..034c350f84 100644
--- a/libnm-core/nm-connection-private.h
+++ b/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__ */
diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c
index 36d2cd6dfe..072b568ed6 100644
--- a/libnm-core/nm-connection.c
+++ b/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);
diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c
index f59856317f..230c06fe2e 100644
--- a/libnm-core/nm-setting-connection.c
+++ b/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;
diff --git a/libnm-core/nm-setting-ovs-interface.c b/libnm-core/nm-setting-ovs-interface.c
index e8e73a9d17..9a7e1fb2ba 100644
--- a/libnm-core/nm-setting-ovs-interface.c
+++ b/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,
-	                                                        connection,
-	                                                        FALSE,
-	                                                        NULL,
-	                                                        error);
+	result = _nm_setting_ovs_interface_verify_interface_type (self,
+	                                                          connection,
+	                                                          FALSE,
+	                                                          NULL,
+	                                                          &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;
 }
 
 /*****************************************************************************/
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index e42ac737e0..d928d855bb 100644
--- a/libnm-core/tests/test-general.c
+++ b/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);