libnm-core: fail verify() for NMSettingVlan for invalid vlan id

Point in case:

    # ip link add link dummy0 name dummy0.vlan type vlan id 4095
    RTNETLINK answers: Numerical result out of range

This potentially causes existing (invalid) connections to disappear
as they now fail verification.

Instead of adjusting the range of the GObject property
NM_SETTING_VLAN_ID, reject it during vlan. This is a bit more
forgiving to an older client that isn't aware of this new restriction,
so he can first set the value without raising a critical warning.

4 files changed, 24 insertions, 6 deletions

diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c
index 8428a36904..92a052ebc0 100644
--- a/libnm-core/nm-setting-vlan.c
+++ b/libnm-core/nm-setting-vlan.c
@@ -666,6 +666,15 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 		}
 	}
 
+	if (priv->id >= 4095) {
+		g_set_error (error,
+		             NM_CONNECTION_ERROR,
+		             NM_CONNECTION_ERROR_INVALID_PROPERTY,
+		             _("the vlan id must be in range 0-4094 but is %u"),
+		             priv->id);
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_VLAN_SETTING_NAME, NM_SETTING_VLAN_ID);
+	}
+
 	if (priv->flags & ~NM_VLAN_FLAGS_ALL) {
 		g_set_error_literal (error,
 		                     NM_CONNECTION_ERROR,
@@ -851,7 +860,7 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class)
 	 * NMSettingVlan:id:
 	 *
 	 * The VLAN identifier that the interface created by this connection should
-	 * be assigned.
+	 * be assigned. The valid range is from 0 to 4094, without the reserved id 4095.
 	 **/
 	/* ---ifcfg-rh---
 	 * property: id
diff --git a/libnm-util/nm-setting-vlan.c b/libnm-util/nm-setting-vlan.c
index 310a7da818..d101f45f29 100644
--- a/libnm-util/nm-setting-vlan.c
+++ b/libnm-util/nm-setting-vlan.c
@@ -593,6 +593,16 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
 		return FALSE;
 	}
 
+	if (priv->id >= 4095) {
+		g_set_error (error,
+		             NM_SETTING_VLAN_ERROR,
+		             NM_SETTING_VLAN_ERROR_INVALID_PROPERTY,
+		             _("the vlan id must be in range 0-4094 but is %u"),
+		             priv->id);
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_VLAN_SETTING_NAME, NM_SETTING_VLAN_ID);
+		return FALSE;
+	}
+
 	/* If interface_name is specified, it must be a valid interface name. We
 	 * don't check that it matches parent and/or id, because we allow
 	 * renaming vlans to arbitrary names.
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index dd7467c14d..49fb8cb047 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -2147,10 +2147,9 @@ nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id)
 
 	g_return_val_if_fail (parent_iface && *parent_iface, NULL);
 
-	if (vlan_id < 10) {
-		g_return_val_if_fail (vlan_id > 0, NULL);
+	if (vlan_id < 10)
 		id_len = 2;
-	} else if (vlan_id < 100)
+	else if (vlan_id < 100)
 		id_len = 3;
 	else if (vlan_id < 1000)
 		id_len = 4;
diff --git a/src/tests/test-general-with-expect.c b/src/tests/test-general-with-expect.c
index 31afd62d2a..91f3f33619 100644
--- a/src/tests/test-general-with-expect.c
+++ b/src/tests/test-general-with-expect.c
@@ -879,8 +879,8 @@ test_nm_utils_new_vlan_name (void)
 			gs_free char *vlan_id_s = NULL;
 			guint vlan_id;
 
-			/* Create a random VLAN id between 1 and 4094 */
-			vlan_id = 1 + nmtst_get_rand_int () % 4094;
+			/* Create a random VLAN id between 0 and 4094 */
+			vlan_id = nmtst_get_rand_int () % 4095;
 
 			vlan_id_s = g_strdup_printf (".%d", vlan_id);