libnm: verify ethernet.s390-options.bridge_role value

I don't want to fix this for all "ethernet.s390-options" options,
but at least strictly validate the newly introduced option.

4 files changed, 43 insertions, 20 deletions

diff --git a/src/libnm-core-impl/nm-setting-wired.c b/src/libnm-core-impl/nm-setting-wired.c
index ca310fee7c..0d79f0a5c2 100644
--- a/src/libnm-core-impl/nm-setting-wired.c
+++ b/src/libnm-core-impl/nm-setting-wired.c
@@ -135,6 +135,31 @@ _nm_setting_wired_is_valid_s390_option(const char *option)
                >= 0);
 }
 
+gboolean
+_nm_setting_wired_is_valid_s390_option_value(const char *name, const char *option)
+{
+    nm_assert(name);
+
+    if (!option)
+        return FALSE;
+
+    /* For historic reasons, the s390-options values were not validated beyond
+     * simple length check (below).
+     *
+     * Here, for certain (recently added) options we add strict validation.
+     * As this is only done for a few hand picked options, do it right here.
+     *
+     * Maybe we should find a backward compatible way to validate all options.
+     * In that case, the validation should become more elaborate, like we do
+     * for bond options. */
+
+    if (nm_streq(name, "bridge_role")) {
+        return NM_IN_STRSET(option, "primary", "secondary", "none");
+    }
+
+    return option[0] != '\0' && strlen(option) <= NM_SETTING_WIRED_S390_OPTION_MAX_LEN;
+}
+
 /**
  * nm_setting_wired_get_port:
  * @setting: the #NMSettingWired
@@ -812,7 +837,7 @@ verify(NMSetting *setting, NMConnection *connection, GError **error)
                            NM_SETTING_WIRED_S390_OPTIONS);
             return FALSE;
         }
-        if (!_nm_setting_wired_is_valid_s390_option_value(v->value_str)) {
+        if (!_nm_setting_wired_is_valid_s390_option_value(v->name, v->value_str)) {
             g_set_error(error,
                         NM_CONNECTION_ERROR,
                         NM_CONNECTION_ERROR_INVALID_PROPERTY,
diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c
index ea68a02683..0f80826bcb 100644
--- a/src/libnm-core-impl/tests/test-setting.c
+++ b/src/libnm-core-impl/tests/test-setting.c
@@ -2904,17 +2904,20 @@ _rndt_wired_add_s390_options(NMSettingWired *s_wired, char **out_keyfile_entries
     opt_vals  = g_new0(char *, n_opts + 1);
     opt_found = g_new0(bool, n_opts + 1);
     for (i = 0; i < n_opts; i++) {
-        guint p = nmtst_get_rand_uint32() % 1000;
-
-        if (p < 200)
-            opt_vals[i] = nm_strdup_int(i);
+        if (nm_streq(opt_keys[i], "bridge_role"))
+            opt_vals[i] = g_strdup(nmtst_rand_select_str("primary", "secondary", "none"));
         else {
-            opt_vals[i] = g_strdup_printf("%s%s%s%s-%zu",
-                                          ((p % 5) % 2) ? "\n" : "",
-                                          ((p % 7) % 2) ? "\t" : "",
-                                          ((p % 11) % 2) ? "x" : "",
-                                          ((p % 13) % 2) ? "=" : "",
-                                          i);
+            guint p = nmtst_get_rand_uint32() % 1000;
+            if (p < 200)
+                opt_vals[i] = nm_strdup_int(i);
+            else {
+                opt_vals[i] = g_strdup_printf("%s%s%s%s-%zu",
+                                              ((p % 5) % 2) ? "\n" : "",
+                                              ((p % 7) % 2) ? "\t" : "",
+                                              ((p % 11) % 2) ? "x" : "",
+                                              ((p % 13) % 2) ? "=" : "",
+                                              i);
+            }
         }
     }
 
diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h
index d8e69ad831..d026520d79 100644
--- a/src/libnm-core-intern/nm-core-internal.h
+++ b/src/libnm-core-intern/nm-core-internal.h
@@ -309,16 +309,11 @@ typedef gpointer (*NMUtilsCopyFunc)(gpointer);
 const char **
 _nm_ip_address_get_attribute_names(const NMIPAddress *addr, gboolean sorted, guint *out_length);
 
-void     _nm_setting_wired_clear_s390_options(NMSettingWired *setting);
-gboolean _nm_setting_wired_is_valid_s390_option(const char *option);
-
 #define NM_SETTING_WIRED_S390_OPTION_MAX_LEN 200u
 
-static inline gboolean
-_nm_setting_wired_is_valid_s390_option_value(const char *option)
-{
-    return option && option[0] != '\0' && strlen(option) <= NM_SETTING_WIRED_S390_OPTION_MAX_LEN;
-}
+void     _nm_setting_wired_clear_s390_options(NMSettingWired *setting);
+gboolean _nm_setting_wired_is_valid_s390_option(const char *option);
+gboolean _nm_setting_wired_is_valid_s390_option_value(const char *name, const char *option);
 
 gboolean _nm_ip_route_attribute_validate_all(const NMIPRoute *route, GError **error);
 const char **
diff --git a/src/nm-initrd-generator/nmi-cmdline-reader.c b/src/nm-initrd-generator/nmi-cmdline-reader.c
index a461890cda..2f50426d8d 100644
--- a/src/nm-initrd-generator/nmi-cmdline-reader.c
+++ b/src/nm-initrd-generator/nmi-cmdline-reader.c
@@ -979,7 +979,7 @@ reader_parse_rd_znet(Reader *reader, char *argument, gboolean net_ifnames)
             val[0] = '\0';
             val++;
             if (!_nm_setting_wired_is_valid_s390_option(key)
-                || !_nm_setting_wired_is_valid_s390_option_value(val)) {
+                || !_nm_setting_wired_is_valid_s390_option_value(key, val)) {
                 /* Invalid setting. Silently ignore, but also ensure we
                  * didn't already set it. */
                 nm_setting_wired_remove_s390_option(s_wired, key);