ifcfg: log messages about invalid an unrecognized lines in ifcfg files

Problems of this patch:

- the code does not differentiate between an ifcfg file and an alias
  file. Different shell variables are honored however depending on the
  context and the warning should reflect that.

- there are no warnings about /etc/sysconfig/network. The main problem
  is that we read this file for every ifcfg file we parse, and we would
  need to ratelimit the number of warnings. Another problem is that
  the file likely contains keys that we intentionally don't support.
  We would need a new way to omit warnings about those lines.

Example:

    TYPE=Ethernet
    PROXY_METHOD=none
    BROWSER_ONLY=no
    BOOTPROTO=dhcp
    DEFROUTE=yes
    STABLE_ID=$'xxx\xF4yy'
    IPV4_FAILURE_FATAL=no
    IPV6INIT=yes
    XX=foo
    XX1=foo'
    '
    IPV6_AUTOCONF=yes xxxx
    IPV6_DEFROUTE=yesx
    IPV6_DEFROUTE=yes
    IPV6_FAILURE_FATAL=no
    IPV6_ADDR_GEN_MODE=stable-privacy
    NAME=xxx
    UUID=9d8ed7ff-3cdd-4336-9e26-3e978dc87102
    ONBOOT=no

  <warn>  [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:6: key STABLE_ID does not contain valid UTF-8 and is treated as ""
  <debug> [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:9: key XX is unknown and ignored
  <warn>  [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:10: key XX1 is badly quoted and is treated as ""
  <warn>  [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:11: invalid line ignored
  <warn>  [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:12: key IPV6_AUTOCONF is badly quoted and is treated as ""
  <warn>  [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:13: key IPV6_DEFROUTE is duplicated and the early occurrence ignored

https://bugzilla.redhat.com/show_bug.cgi?id=1959656
This commit is contained in:
Thomas Haller 2021-07-09 09:38:41 +02:00
parent 7c9b0d68e4
commit 41be0c8fde
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
6 changed files with 111 additions and 4 deletions

View file

@ -2154,6 +2154,8 @@ read_aliases(NMSettingIPConfig *s_ip4, gboolean read_defroute, const char *filen
continue;
}
svWarnInvalid(parsed, "alias", _NMLOG_DOMAIN);
device = svGetValueStr(parsed, "DEVICE", &device_value);
if (!device) {
PARSE_WARNING("alias file '%s' has no DEVICE", full_path);
@ -6302,6 +6304,7 @@ connection_from_file_full(const char *filename,
NMSetting * s_ip4;
NMSetting * s_ip6;
const char * ifcfg_name = NULL;
gs_free char * s_tmp = NULL;
gboolean has_ip4_defroute = FALSE;
gboolean has_complex_routes_v4;
gboolean has_complex_routes_v6;
@ -6329,8 +6332,6 @@ connection_from_file_full(const char *filename,
if (!main_ifcfg)
return NULL;
network_ifcfg = svOpenFile(network_file, NULL);
if (!svGetValueBoolean(main_ifcfg, "NM_CONTROLLED", TRUE)) {
connection = create_unhandled_connection(filename, main_ifcfg, "unmanaged", out_unhandled);
if (!connection) {
@ -6344,6 +6345,16 @@ connection_from_file_full(const char *filename,
return g_steal_pointer(&connection);
}
if (NM_IN_STRSET(svGetValueStr(main_ifcfg, "DEVICE", &s_tmp), "lo")) {
/* "lo" is not handled by NetworkManager and we ignore it. */
} else
svWarnInvalid(main_ifcfg, "ifcfg", _NMLOG_DOMAIN);
nm_clear_g_free(&s_tmp);
network_ifcfg = svOpenFile(network_file, NULL);
/* we don't call svWarnInvalid(network_ifcfg), because we will load this file for
* every profile. So we would get a large number of duplicate warnings. */
/* iBFT is handled by nm-initrd-generator during boot. */
bootproto = svGetValueStr_cp(main_ifcfg, "BOOTPROTO");
if (bootproto && !g_ascii_strcasecmp(bootproto, "ibft")) {

View file

@ -1523,6 +1523,91 @@ svUnsetValue(shvarFile *s, const char *key)
/*****************************************************************************/
void
svWarnInvalid(shvarFile *s, const char *file_type, NMLogDomain log_domain)
{
shvarLine *line;
gsize n;
if (!nm_logging_enabled(LOGL_WARN, log_domain))
return;
n = 0;
c_list_for_each_entry (line, &s->lst_head, lst) {
gs_free char *s_tmp = NULL;
n++;
if (!line->key) {
const char *str;
nm_assert(line->line);
str = nm_str_skip_leading_spaces(line->line);
if (!NM_IN_SET(str[0], '\0', '#')) {
nm_log_warn(log_domain,
"ifcfg-rh: %s,%s:%zu: invalid line ignored",
file_type,
s->fileName,
n);
}
continue;
}
if (g_hash_table_lookup(s->lst_idx, line) != line) {
nm_log_warn(
log_domain,
"ifcfg-rh: %s,%s:%zu: key %s is duplicated and the early occurrence ignored",
file_type,
s->fileName,
n,
line->key);
continue;
}
if (!line->line) {
/* the line is deleted via svUnsetValue(). Ignore. */
continue;
}
if (!svUnescape(line->line, &s_tmp)) {
if (!svUnescape_full(line->line, &s_tmp, FALSE)) {
nm_log_warn(log_domain,
"ifcfg-rh: %s,%s:%zu: key %s is badly quoted and is treated as \"\"",
file_type,
s->fileName,
n,
line->key);
} else {
nm_log_warn(log_domain,
"ifcfg-rh: %s,%s:%zu: key %s does not contain valid UTF-8 and is "
"treated as \"\"",
file_type,
s->fileName,
n,
line->key);
}
continue;
}
/* TODO: we read different shell scripts, and whether a key is recognized
* depends on the type. For example, alias files only accept a subset of
* known keys.
*
* Basically, depending on the @file_type, different keys are valid. */
if (!nms_ifcfg_rh_utils_is_well_known_key(line->key)) {
nm_log_dbg(log_domain,
"ifcfg-rh: %s,%s:%zu: key %s is unknown and ignored",
file_type,
s->fileName,
n,
line->key);
continue;
}
}
}
/*****************************************************************************/
/* Write the current contents iff modified. Returns FALSE on error
* and TRUE on success. Do not write if no values have been modified.
* The mode argument is only used if creating the file, not if

View file

@ -121,4 +121,6 @@ _nm_auto_shvar_file_close(shvarFile **p_s)
}
#define nm_auto_shvar_file_close nm_auto(_nm_auto_shvar_file_close)
void svWarnInvalid(shvarFile *s, const char *file_type, NMLogDomain log_domain);
#endif /* _SHVAR_H */

View file

@ -3,7 +3,6 @@ DEVICE=eth2
HWADDR=00:16:41:11:22:33
BOOTPROTO=dhcp
ONBOOT=yes
ONBOOT=yes
USERCTL=yes
IPV6INIT=no
NM_CONTROLLED=yes

View file

@ -4,7 +4,6 @@ DEVICE=eth2
HWADDR=00:16:41:11:22:33
BOOTPROTO=dhcp
ONBOOT=yes
ONBOOT=yes
USERCTL=yes
IPV6INIT=no
NM_CONTROLLED=yes

View file

@ -738,10 +738,13 @@ test_read_variables_corner_cases(void)
const char * mac;
char expected_mac_address[ETH_ALEN] = {0x00, 0x16, 0x41, 0x11, 0x22, 0x33};
NMTST_EXPECT_NM_WARN("*key NAME is badly quoted and is treated as \"\"*");
NMTST_EXPECT_NM_WARN("*key ZONE is badly quoted and is treated as \"\"*");
connection = _connection_from_file(TEST_IFCFG_DIR "/ifcfg-test-variables-corner-cases-1",
NULL,
TYPE_ETHERNET,
NULL);
g_test_assert_expected_messages();
/* ===== CONNECTION SETTING ===== */
s_con = nm_connection_get_setting_connection(connection);
@ -830,10 +833,12 @@ test_read_unrecognized(void)
gs_free char * unhandled_spec = NULL;
guint64 expected_timestamp = 0;
NMTST_EXPECT_NM_WARN("*key NAME is badly quoted and is treated as \"\"*");
connection = _connection_from_file(TEST_IFCFG_DIR "/ifcfg-test-unrecognized",
NULL,
NULL,
&unhandled_spec);
g_test_assert_expected_messages();
g_assert_cmpstr(unhandled_spec, ==, "unrecognized:mac:00:11:22:33");
/* ===== CONNECTION SETTING ===== */
@ -1004,10 +1009,12 @@ test_read_wired_dhcp(void)
char expected_mac_address[ETH_ALEN] = {0x00, 0x11, 0x22, 0x33, 0x44, 0xee};
const char * mac;
NMTST_EXPECT_NM_WARN("*key IPV6INIT is duplicated and the early occurrence ignored*");
connection = _connection_from_file(TEST_IFCFG_DIR "/ifcfg-test-wired-dhcp",
NULL,
TYPE_ETHERNET,
&unmanaged);
g_test_assert_expected_messages();
g_assert(unmanaged == NULL);
/* ===== CONNECTION SETTING ===== */
@ -3583,10 +3590,12 @@ test_read_wifi_wpa_eap_tls(void)
char * unmanaged = NULL;
const char * expected_privkey_password = "test1";
NMTST_EXPECT_NM_WARN("*key ONBOOT is duplicated and the early occurrence ignored*");
connection = _connection_from_file(TEST_IFCFG_DIR "/ifcfg-test-wifi-wpa-eap-tls",
NULL,
TYPE_ETHERNET,
&unmanaged);
g_test_assert_expected_messages();
g_assert(!unmanaged);
/* ===== WIRELESS SETTING ===== */
@ -3791,10 +3800,12 @@ test_read_wifi_wep_eap_ttls_chap(void)
NMSetting8021x * s_8021x;
char * unmanaged = NULL;
NMTST_EXPECT_NM_WARN("*key ONBOOT is duplicated and the early occurrence ignored*");
connection = _connection_from_file(TEST_IFCFG_DIR "/ifcfg-test-wifi-wep-eap-ttls-chap",
NULL,
TYPE_WIRELESS,
&unmanaged);
g_test_assert_expected_messages();
g_assert(!unmanaged);
/* ===== WIRELESS SETTING ===== */