Commit graph

28301 commits

Author SHA1 Message Date
Thomas Haller
9e3b7b2e99
contrib/rpm: let NetworkManager-libnm package conflict with exact NetworkManager-glib version
This "Conflicts" is since commit b85b8ed6fa ('contrib/rpm: let
NetworkManager-libnm and NetworkManager-glib of differing version
conflict'). This was probably fine back then, but NetworkManager-glib is
long gone.

Also, not hard coding the version number leads to rpmdiff messages like:

  NEEDS INSPECTION  NetworkManager-libnm  changed from Conflicts: NetworkManager-glib < 1:1.32.0-0.2.el8 to Conflicts: NetworkManager-glib < 1:1.32.0-0.3.el8 on all architectures

As NetworkManager-glib is long gone, hard code the version with which
we conflict.
2021-05-12 13:35:28 +02:00
Thomas Haller
d784fdb9d9
glib-aux: use nm_str_buf_append_c() in nm_str_buf_append_c_hex() 2021-05-12 10:06:21 +02:00
Thomas Haller
77fb782060
glib-aux: avoid potential undefined behavior for nm_str_buf_append_printf()
The string buffer may be empty and _priv_str still %NULL. Doing
pointer arithmetic with a %NULL pointer is undefined behavior.
Avoid that.

It's probably not an issue, because it results in computing &(((char *) NULL)[0],
and then g_vsnprintf() would not even inspect the pointer (so it doesn't
matter whether the computed pointer is bogus). But still, there is
undefined behavior involved.
2021-05-12 09:38:15 +02:00
Thomas Haller
4bc9c59c07
wifi: refactor parsing secrutiy flags of supplicant in security_from_vardict()
I think if-else is easier to read. It also makes it clear that the
options are really exclusive, and for each option only one flag is set.
That was not easy to see previously.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/844
2021-05-11 14:03:37 +02:00
Thomas Haller
17f45689bf
all: merge branch 'th/coverity-fix-some-leaks'
https://bugzilla.redhat.com/show_bug.cgi?id=1938672

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/843
2021-05-11 14:00:17 +02:00
Thomas Haller
e5f37477c0
cli: fix leak in print_wifi_connection()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772): [#def297] [important]
  NetworkManager-1.31.3/src/nmcli/devices.c:4610: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
  NetworkManager-1.31.3/src/nmcli/devices.c:4610: var_assign: Assigning: "ssid" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid_bytes, NULL), g_bytes_get_size(ssid_bytes))".
  NetworkManager-1.31.3/src/nmcli/devices.c:4612: noescape: Resource "ssid" is not freed or pointed-to in "g_print".
  NetworkManager-1.31.3/src/nmcli/devices.c:4642: noescape: Resource "ssid" is not freed or pointed-to in "string_append_mecard".
  NetworkManager-1.31.3/src/nmcli/devices.c:4654: leaked_storage: Variable "ssid" going out of scope leaks the storage it points to.
  # 4652|
  # 4653|       g_print("\n");
  # 4654|-> }
  # 4655|
  # 4656|   static gboolean

Fixes: 7061341a41 ('cli: add "nmcli d wifi show"')
2021-05-11 13:56:50 +02:00
Thomas Haller
61029d4064
libnmt-newt: use cleanup macro in nmt_newt_button_build_component() 2021-05-11 13:56:50 +02:00
Thomas Haller
853f411567
libnmt-newt: fix leak in nmt_newt_button_build_component()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772): [#def274] [important]
  NetworkManager-1.31.3/src/libnmt-newt/nmt-newt-button.c:118: alloc_fn: Storage is returned from allocation function "g_strdup_printf".
  NetworkManager-1.31.3/src/libnmt-newt/nmt-newt-button.c:118: var_assign: Assigning: "label" = storage returned from "g_strdup_printf(" <%s>", priv->label)".
  NetworkManager-1.31.3/src/libnmt-newt/nmt-newt-button.c:119: noescape: Resource "label" is not freed or pointed-to in "nmt_newt_locale_from_utf8".
  NetworkManager-1.31.3/src/libnmt-newt/nmt-newt-button.c:125: leaked_storage: Variable "label" going out of scope leaks the storage it points to.
  #  123|       }
  #  124|
  #  125|->     return co;
  #  126|   }
  #  127|

Fixes: 3bda3fb60c ('nmtui: initial import of nmtui')
2021-05-11 13:56:49 +02:00
Thomas Haller
2c628e4762
libnmc-base:fix leak in NMSecretAgentSimple's request_secrets_from_ui()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772): [#def271] [important]
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: var_assign: Assigning: "ssid_utf8" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid, NULL), g_bytes_get_size(ssid))".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:877: noescape: Resource "ssid_utf8" is not freed or pointed-to in "g_strdup_printf".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:882: leaked_storage: Variable "ssid_utf8" going out of scope leaks the storage it points to.
  #  880|
  #  881|           if (!add_wireless_secrets(request, secrets))
  #  882|->             goto out_fail;
  #  883|       } else if (nm_connection_is_type(request->connection, NM_SETTING_WIRED_SETTING_NAME)) {
  #  884|           title = _("Wired 802.1X authentication");

  Error: RESOURCE_LEAK (CWE-772): [#def272] [important]
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: alloc_fn: Storage is returned from allocation function "nm_utils_ssid_to_utf8".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:874: var_assign: Assigning: "ssid_utf8" = storage returned from "nm_utils_ssid_to_utf8(g_bytes_get_data(ssid, NULL), g_bytes_get_size(ssid))".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:877: noescape: Resource "ssid_utf8" is not freed or pointed-to in "g_strdup_printf".
  NetworkManager-1.31.3/src/libnmc-base/nm-secret-agent-simple.c:883: leaked_storage: Variable "ssid_utf8" going out of scope leaks the storage it points to.
  #  881|           if (!add_wireless_secrets(request, secrets))
  #  882|               goto out_fail;
  #  883|->     } else if (nm_connection_is_type(request->connection, NM_SETTING_WIRED_SETTING_NAME)) {
  #  884|           title = _("Wired 802.1X authentication");
  #  885|           msg   = g_strdup_printf(_("Secrets are required to access the wired network %s"),

Fixes: 3fbabde4c3 ('libnm-core: replace GByteArray with pointer + length in some APIs')
2021-05-11 13:56:49 +02:00
Thomas Haller
d527d3874c
glib-aux: workaround coverty warning about comparing nm_json_int_t with int64
Error: CONSTANT_EXPRESSION_RESULT (CWE-569): [#def240]
  NetworkManager-1.31.3/src/libnm-glib-aux/nm-json-aux.h:260: result_independent_of_operands: "v < -9223372036854775808LL /* (gint64)(-9223372036854775807L - 1L) */" is always false regardless of the values of its operands. This occurs as the logical first operand of "||".
  #  258|
  #  259|       v = vt->nm_json_integer_value(elem);
  #  260|->     if (v < G_MININT64 || v > G_MAXINT64)
  #  261|           return -ERANGE;
  #  262|

  Error: CONSTANT_EXPRESSION_RESULT (CWE-569): [#def241]
  NetworkManager-1.31.3/src/libnm-glib-aux/nm-json-aux.h:279: result_independent_of_operands: "v > 18446744073709551615UL" is always false regardless of the values of its operands. This occurs as the logical second operand of "||".
  #  277|
  #  278|       v = vt->nm_json_integer_value(elem);
  #  279|->     if (v < 0 || v > G_MAXUINT64)
  #  280|           return -ERANGE;
  #  281|
2021-05-11 13:56:48 +02:00
Thomas Haller
463db1c7a6
tui: use internal _nm_utils_ip4_prefix_to_netmask() helper
nm_utils_ip4_prefix_to_netmask() is public API of libnm.
As we also want to have this function at a few places where
we don't have libnm, we have an internal variant
_nm_utils_ip4_prefix_to_netmask().

Use the internal variant consistently and everywhere.
2021-05-11 13:56:48 +02:00
Thomas Haller
b487cf30a9
glib-aux/tests: add test for _nm_utils_ip4_prefix_to_netmask() 2021-05-11 13:56:47 +02:00
Thomas Haller
64985beef8
dhcp/systemd: avoid using g_free() with buffers allocated with malloc() in lease_to_ip4_config()()
Coverity says:

  Error: ALLOC_FREE_MISMATCH (CWE-762):
  NetworkManager-1.31.3/src/core/dhcp/nm-dhcp-systemd.c:234: alloc: Allocation of memory which must be freed using "free".
  NetworkManager-1.31.3/src/core/dhcp/nm-dhcp-systemd.c:447: free: Calling "_nm_auto_g_free" frees "routes" using "g_free" but it should have been freed using "free".
  #  445|       }
  #  446|       NM_SET_OUT(out_options, g_steal_pointer(&options));
  #  447|->     return g_steal_pointer(&ip4_config);
  #  448|   }
  #  449|

Fixes: acc0d79224 ('systemd: merge branch 'systemd' into master')
2021-05-11 13:56:47 +02:00
Thomas Haller
44abe6d661
systemd/tests: avoid using g_free() with buffers allocated with malloc() in _test_unbase64mem_mem()
Coverity says:

  Error: ALLOC_FREE_MISMATCH (CWE-762):
  NetworkManager-1.31.3/src/core/tests/test-systemd.c:261: alloc: Allocation of memory which must be freed using "free".
  NetworkManager-1.31.3/src/core/tests/test-systemd.c:274: free: Calling "_nm_auto_g_free" frees "exp2_arr" using "g_free" but it should have been freed using "free".
  #  272|           g_assert_cmpmem(expected_arr, expected_len, exp3_arr, exp3_len);
  #  273|       }
  #  274|-> }
  #  275|
  #  276|   #define _test_unbase64mem(base64, expected_str) \

  Error: ALLOC_FREE_MISMATCH (CWE-762):
  NetworkManager-1.31.3/src/core/tests/test-systemd.c:270: alloc: Allocation of memory which must be freed using "free".
  NetworkManager-1.31.3/src/core/tests/test-systemd.c:274: free: Calling "_nm_auto_g_free" frees "exp3_arr" using "g_free" but it should have been freed using "free".
  #  272|           g_assert_cmpmem(expected_arr, expected_len, exp3_arr, exp3_len);
  #  273|       }
  #  274|-> }
  #  275|
  #  276|   #define _test_unbase64mem(base64, expected_str) \

Fixes: 0298d54078 ('systemd: expose unbase64mem() as nm_sd_utils_unbase64mem()')
2021-05-11 13:56:47 +02:00
Thomas Haller
936b60e00f
wifi: avoid large shift for calculating netmask in ip4_config_to_iwd_config()
Found by Coverity:

  Error: BAD_SHIFT (CWE-682): [#def53]
  NetworkManager-1.31.3/src/core/devices/wifi/nm-wifi-utils.c:1590: zero_return: Function call "nm_ip_address_get_prefix(addr)" returns 0.
  NetworkManager-1.31.3/src/core/devices/wifi/nm-wifi-utils.c:1590: assignment: Assigning: "prefix" = "nm_ip_address_get_prefix(addr)". The value of "prefix" is now 0.
  NetworkManager-1.31.3/src/core/devices/wifi/nm-wifi-utils.c:1591: large_shift: In expression "0xffffffffU << 32U - prefix", left shifting by more than 31 bits has undefined behavior.  The shift amount, "32U - prefix", is 32.
  # 1589|           NMIPAddress *addr    = nm_setting_ip_config_get_address(s_ip, 0);
  # 1590|           guint        prefix  = nm_ip_address_get_prefix(addr);
  # 1591|->         in_addr_t    netmask = htonl(0xffffffffu << (32 - prefix));
  # 1592|           char         buf[INET_ADDRSTRLEN];
  # 1593|

Fixes: 9d22ae7981 ('wifi: Add utilities for writing IWD connection profiles')
2021-05-11 13:56:46 +02:00
Thomas Haller
820ab364fd
tests: avoid Coverity warning UNINIT in nmtst_keyfile_get_num_keys()
A false positive:

  Error: UNINIT (CWE-457):
  NetworkManager-1.31.3/src/libnm-glib-aux/nm-test-utils.h:2816: var_decl: Declaring variable "l" without initializer.
  NetworkManager-1.31.3/src/libnm-glib-aux/nm-test-utils.h:2828: uninit_use: Using uninitialized value "l".
  # 2826|       nmtst_assert_success(keys, error);
  # 2827|
  # 2828|->     g_assert_cmpint(NM_PTRARRAY_LEN(keys), ==, l);
  # 2829|
  # 2830|       return l;
2021-05-11 13:56:46 +02:00
Thomas Haller
90f5d9114f
libnm: use nm_auto_unref_tc_action cleanup macro in nm_utils_tc_action_from_str() 2021-05-11 13:56:45 +02:00
Thomas Haller
3cd56e92d4
libnm: fix leak in nm_utils_tc_tfilter_from_str()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772):
  NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2772: alloc_fn: Storage is returned from allocation function "nm_utils_tc_action_from_str".
  NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2772: var_assign: Assigning: "action" = storage returned from "nm_utils_tc_action_from_str(extra_opts, error)".
  NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2785: leaked_storage: Variable "action" going out of scope leaks the storage it points to.
  # 2783|       tfilter = nm_tc_tfilter_new(kind, parent, error);
  # 2784|       if (!tfilter)
  # 2785|->         return NULL;
  # 2786|
  # 2787|       nm_tc_tfilter_set_handle(tfilter, handle);

Fixes: de41c45e61 ('libnm-core: add functionality for dealing with tc-style traffic filter specifiers')
2021-05-11 13:56:45 +02:00
Thomas Haller
272119d925
libnm: add nm_auto_unref_tc_action cleanup macro for NMTCAction 2021-05-11 13:56:45 +02:00
Thomas Haller
02dbba49d6
libnm: fix leak in nm_vpn_service_plugin_read_vpn_details()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772):
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:814: alloc_fn: Storage is returned from allocation function "g_string_new".
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:814: var_assign: Assigning: "key" = storage returned from "g_string_new(line->str + strlen("DATA_KEY="))".
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:815: var_assign: Assigning: "str" = "key".
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:855: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
  NetworkManager-1.31.3/src/libnm-client-impl/nm-vpn-service-plugin.c:855: leaked_storage: Variable "key" going out of scope leaks the storage it points to.
  #  853|           NM_SET_OUT(out_secrets, g_steal_pointer(&secrets));
  #  854|       }
  #  855|->     return success;
  #  856|   }
  #  857|

Fixes: 3dfb72b926 ('service-plugin: allow continuations in the auth-dialog protocol')
2021-05-11 13:56:45 +02:00
Thomas Haller
ceaa1c369f
core: fix leak in _config_data_get_main_auth_polkit()
Found by Coverity:

  Error: RESOURCE_LEAK (CWE-772):
  NetworkManager-1.31.3/src/core/nm-config-data.c:450: alloc_fn: Storage is returned from allocation function "nm_config_data_get_value".
  NetworkManager-1.31.3/src/core/nm-config-data.c:450: var_assign: Assigning: "str" = storage returned from "nm_config_data_get_value(self, "main", "auth-polkit", (enum [unnamed type of NMConfigGetValueFlags])6)".
  NetworkManager-1.31.3/src/core/nm-config-data.c:454: noescape: Resource "str" is not freed or pointed-to in "nm_auth_polkit_mode_from_string".
  NetworkManager-1.31.3/src/core/nm-config-data.c:465: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
  #  463|           NM_SET_OUT(out_invalid_config, FALSE);
  #  464|
  #  465|->     return auth_polkit_mode;
  #  466|   }
  #  467|

Fixes: 6d7446e52f ('core: add main.auth-polkit option "root-only"')
2021-05-11 13:56:44 +02:00
Thomas Haller
dd3aa1224a
ifcfg-rh: use NMStrBuf in svUnescape()
This is a popular, low-level function. Let's use NMStrBuf.

Also, Coverity wrongly things that there is a leak here. This change
should also avoid that:

    Error: RESOURCE_LEAK (CWE-772):
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/shvar.c:411: alloc_arg: "_gstr_init" allocates memory that is stored into "str".
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/shvar.c:423: noescape: Resource "str" is not freed or pointed-to in "g_string_append_len".
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/shvar.c:619: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
    #  617|           nm_assert(!str);
    #  618|           *to_free = NULL;
    #  619|->         return "";
    #  620|       }
    #  621|

Profile:

We run test-ifcfg-rh which calls svUnescape() under realistic circumstances.
However, the test does too many other things that svUnescape() would be
measurable. So use the following patch, to run the tested code more frequently:

    diff --git a/src/core/settings/plugins/ifcfg-rh/shvar.c b/src/core/settings/plugins/ifcfg-rh/shvar.c
    index c6099dd1731c..18a907113ea9 100644
    --- a/src/core/settings/plugins/ifcfg-rh/shvar.c
    +++ b/src/core/settings/plugins/ifcfg-rh/shvar.c
    @@ -645,6 +645,24 @@ out_error:
         return NULL;
     }

    +#define svUnescape(value, to_free)                   \
    +    ({                                               \
    +        const char *_value = (value);                \
    +        const char *_result;                         \
    +        int         _i;                              \
    +                                                     \
    +        for (_i = 0; TRUE; _i++) {                   \
    +            gs_free char *_to_free;                  \
    +                                                     \
    +            _result = svUnescape(_value, &_to_free); \
    +            if (_i < 1000)                           \
    +                continue;                            \
    +            *(to_free) = g_steal_pointer(&_to_free); \
    +            break;                                   \
    +        }                                            \
    +        _result;                                     \
    +    })
    +
     /*****************************************************************************/

     shvarFile *

Build:

    CFLAGS='-O2' ./autogen.sh --with-more-asserts=0
    make -j 10 src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh && \
        src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh &&
        perf stat -r 50 -B src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh

Before:

 Performance counter stats for 'src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh' (20 runs):

            590.56 msec task-clock:u              #    0.972 CPUs utilized            ( +-  0.48% )
                 0      context-switches:u        #    0.000 K/sec
                 0      cpu-migrations:u          #    0.000 K/sec
             1,091      page-faults:u             #    0.002 M/sec                    ( +-  0.12% )
     2,022,618,453      cycles:u                  #    3.425 GHz                      ( +-  0.33% )
     4,165,011,633      instructions:u            #    2.06  insn per cycle           ( +-  0.01% )
     1,168,673,648      branches:u                # 1978.910 M/sec                    ( +-  0.01% )
         8,279,364      branch-misses:u           #    0.71% of all branches          ( +-  0.14% )

           0.60739 +- 0.00292 seconds time elapsed  ( +-  0.48% )

After:

 Performance counter stats for 'src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh' (50 runs):

            580.19 msec task-clock:u              #    0.972 CPUs utilized            ( +-  0.33% )
                 0      context-switches:u        #    0.000 K/sec
                 0      cpu-migrations:u          #    0.000 K/sec
             1,092      page-faults:u             #    0.002 M/sec                    ( +-  0.08% )
     1,956,368,933      cycles:u                  #    3.372 GHz                      ( +-  0.22% )
     4,106,984,148      instructions:u            #    2.10  insn per cycle           ( +-  0.01% )
     1,087,931,864      branches:u                # 1875.143 M/sec                    ( +-  0.01% )
         7,731,041      branch-misses:u           #    0.71% of all branches          ( +-  0.15% )

           0.59680 +- 0.00193 seconds time elapsed  ( +-  0.32% )

The run time varies greatly. But it can be seen that the new code is consistently
faster.
2021-05-11 13:56:23 +02:00
Thomas Haller
1556732ef0
glib-aux: add nm_str_buf_append_unichar() helper 2021-05-11 13:53:52 +02:00
Thomas Haller
7065d75b91
ifcfg-rh: avoid cloning vlans array in write_bridge_vlans() 2021-05-11 13:53:52 +02:00
Thomas Haller
72e8336fdc
libnm: add internal accessors for _nm_setting_bridge{,_port}_get_vlans()
For internal code, when we control what we are doing, make it possible
to directly access the internal GPtrArray.
2021-05-11 13:53:52 +02:00
Thomas Haller
f5685e5bc9
ifcfg-rh: add comment about unreachable code in write_bridge_vlans() 2021-05-11 13:53:52 +02:00
Thomas Haller
5b36f215f4
ifcfg-rh: fix code that looks like a leak in write_bridge_vlans()
"string" is leaked in the error case. But in practice, this cannot
happen because nm_bridge_vlan_to_str() cannot fail.

While at it, replace GString by NMStrBuf.

Thanks Coverity:

    Error: RESOURCE_LEAK (CWE-772):
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1565: alloc_fn: Storage is returned from allocation function "g_string_new".
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1565: var_assign: Assigning: "string" = storage returned from "g_string_new("")".
    NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1572: leaked_storage: Variable "string" going out of scope leaks the storage it points to.
    # 1570|           vlan_str = nm_bridge_vlan_to_str(vlan, error);
    # 1571|           if (!vlan_str)
    # 1572|->             return FALSE;
    # 1573|           if (string->len > 0)
    # 1574|               g_string_append(string, ",");
2021-05-11 13:53:52 +02:00
Thomas Haller
8e5374512e
CONTRIBUTING: add section about unit tests 2021-05-11 12:24:38 +02:00
Fernando Fernandez Mancera
eee4332e8f
ifcfg: fix ACCEPT_ALL_MAC_ADDRESSES for virtual interfaces
The rh-ifcfg plugin was missing the writting code for
ACCEPT_ALL_MAC_ADDRESSES property in virtual interfaces.

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

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>

Fixes: d946aa0c50 ('wired-setting: add support to accept-all-mac-addresses')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/846
2021-05-10 22:54:50 +02:00
Wade Berrier
3839db5191
wifi: fix WPS PBC string typo
Fixes: b83f07916a ('supplicant: large rework of wpa_supplicant handling')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/845
2021-05-09 21:19:42 +02:00
Thomas Haller
661934e48e
firewall: merge branch 'th/firewall'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/841
2021-05-07 11:50:14 +02:00
Thomas Haller
aa859d85d9
firewall: rename NMUtilsShareRules to NMFirewallConfig
It's still not a very good name, but it seems better then
NMUtilsShareRules.

Currently, NMFirewallConfig is mostly about masquerading for shared
mode. But in practice, it's a piece of configuration for something to
configure in the firewall (the NAT and filter rules).
2021-05-07 11:42:51 +02:00
Thomas Haller
b1625697cb
firewall: move firewall code to new "nm-firewall-utils.c" file 2021-05-07 11:42:50 +02:00
Thomas Haller
e9c1d2a9dd
firewall: add new "nm-firewall-utils.[ch]" module 2021-05-07 11:42:50 +02:00
Thomas Haller
ad37120065
firewall: rework iptables rules for shared mode to use custom chain
- add our own rules to a separate custom change. This allows
  to simply flush and delete the chain. This is supposed to
  interfere less with what is already configured on the system.

- also use comments to our rules, so that we can delete them
  more explicitly and don't kill the wrong rule.

- rework the code how we call iptables. We no longer create a list
  of argv arguments that we iterate over. Instead, call functions that
  do the job. The actual arguments move further down the call stack.

- enabling masquerading is now more separate from our other shared
  rules. These two things are mostly independent and the code now
  reflects that.

Before:
  up:   /usr/sbin/iptables --table filter --insert INPUT --in-interface eth0 --protocol tcp --destination-port 53 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --insert INPUT --in-interface eth0 --protocol udp --destination-port 53 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --insert INPUT --in-interface eth0 --protocol tcp --destination-port 67 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --insert INPUT --in-interface eth0 --protocol udp --destination-port 67 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --insert FORWARD --in-interface eth0 --jump REJECT
  up:   /usr/sbin/iptables --table filter --insert FORWARD --out-interface eth0 --jump REJECT
  up:   /usr/sbin/iptables --table filter --insert FORWARD --in-interface eth0 --out-interface eth0 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --insert FORWARD --source 192.168.42.0/255.255.255.0 --in-interface eth0 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --insert FORWARD --destination 192.168.42.0/255.255.255.0 --out-interface eth0 --match state --state ESTABLISHED,RELATED --jump ACCEPT
  up:   /usr/sbin/iptables --table nat --insert POSTROUTING --source 192.168.42.0/255.255.255.0 ! --destination 192.168.42.0/255.255.255.0 --jump MASQUERADE
  down: /usr/sbin/iptables --table nat --delete POSTROUTING --source 192.168.42.0/255.255.255.0 ! --destination 192.168.42.0/255.255.255.0 --jump MASQUERADE
  down: /usr/sbin/iptables --table filter --delete FORWARD --destination 192.168.42.0/255.255.255.0 --out-interface eth0 --match state --state ESTABLISHED,RELATED --jump ACCEPT
  down: /usr/sbin/iptables --table filter --delete FORWARD --source 192.168.42.0/255.255.255.0 --in-interface eth0 --jump ACCEPT
  down: /usr/sbin/iptables --table filter --delete FORWARD --in-interface eth0 --out-interface eth0 --jump ACCEPT
  down: /usr/sbin/iptables --table filter --delete FORWARD --out-interface eth0 --jump REJECT
  down: /usr/sbin/iptables --table filter --delete FORWARD --in-interface eth0 --jump REJECT
  down: /usr/sbin/iptables --table filter --delete INPUT --in-interface eth0 --protocol udp --destination-port 67 --jump ACCEPT
  down: /usr/sbin/iptables --table filter --delete INPUT --in-interface eth0 --protocol tcp --destination-port 67 --jump ACCEPT
  down: /usr/sbin/iptables --table filter --delete INPUT --in-interface eth0 --protocol udp --destination-port 53 --jump ACCEPT
  down: /usr/sbin/iptables --table filter --delete INPUT --in-interface eth0 --protocol tcp --destination-port 53 --jump ACCEPT

Now:

  up:   /usr/sbin/iptables --table nat --insert POSTROUTING --source 192.168.42.0/24 ! --destination 192.168.42.0/24 --jump MASQUERADE -m comment --comment nm-shared-eth0
  up:   /usr/sbin/iptables --table filter --new-chain nm-sh-in-eth0
  up:   /usr/sbin/iptables --table filter --append nm-sh-in-eth0 --protocol tcp --destination-port 67 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --append nm-sh-in-eth0 --protocol udp --destination-port 67 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --append nm-sh-in-eth0 --protocol tcp --destination-port 53 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --append nm-sh-in-eth0 --protocol udp --destination-port 53 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --new-chain nm-sh-fw-eth0
  up:   /usr/sbin/iptables --table filter --append nm-sh-fw-eth0 --destination 192.168.42.0/24 --out-interface eth0 --match state --state ESTABLISHED,RELATED --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --append nm-sh-fw-eth0 --source 192.168.42.0/24 --in-interface eth0 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --append nm-sh-fw-eth0 --in-interface eth0 --out-interface eth0 --jump ACCEPT
  up:   /usr/sbin/iptables --table filter --append nm-sh-fw-eth0 --out-interface eth0 --jump REJECT
  up:   /usr/sbin/iptables --table filter --append nm-sh-fw-eth0 --in-interface eth0 --jump REJECT
  up:   /usr/sbin/iptables --table filter --insert INPUT --in-interface eth0 --jump nm-sh-in-eth0 -m comment --comment nm-shared-eth0
  up:   /usr/sbin/iptables --table filter --insert FORWARD --jump nm-sh-fw-eth0 -m comment --comment nm-shared-eth0
  down: /usr/sbin/iptables --table nat --delete POSTROUTING --source 192.168.42.0/24 ! --destination 192.168.42.0/24 --jump MASQUERADE -m comment --comment nm-shared-eth0
  down: /usr/sbin/iptables --table filter --delete INPUT --in-interface eth0 --jump nm-sh-in-eth0 -m comment --comment nm-shared-eth0
  down: /usr/sbin/iptables --table filter --delete FORWARD --jump nm-sh-fw-eth0 -m comment --comment nm-shared-eth0
  down: /usr/sbin/iptables --table filter --flush nm-sh-in-eth0
  down: /usr/sbin/iptables --table filter --delete-chain nm-sh-in-eth0
  down: /usr/sbin/iptables --table filter --flush nm-sh-fw-eth0
  down: /usr/sbin/iptables --table filter --delete-chain nm-sh-fw-eth0
2021-05-07 11:42:49 +02:00
Thomas Haller
c752de2237
firewall: extract _share_iptables_set_masquerade() helper
When we configure iptables rules, we really do two independent
steps: enable masquerading and do some filtering.

As such, introduce a helper method _share_iptables_set_masquerade() for
the masquerading part.

nm_utils_share_rules_apply() is at the moment a bit odd, because
of the order in which we add/remove the rule. This will get better next.
2021-05-07 11:42:37 +02:00
Thomas Haller
f5e12f3915
firewall: use prefix length instead of netmask for iptables arguments
The form "address/netmask" is unnecessarily verbose. iptables
supports prefix length notation just fine.
2021-05-07 11:42:37 +02:00
Thomas Haller
734c82d789
firewall: add _share_iptables_subnet_to_str() helper for creating string 2021-05-07 11:42:37 +02:00
Thomas Haller
27e325c6c2
firewall: extract helper function _shared_iptables_call() to spawn iptables 2021-05-07 11:42:36 +02:00
Thomas Haller
2277c9490a
firewall: rework NMUtilsShareRules to generate argv on demand
Previously, NMUtilsShareRules basically was tracking a list of command
line arguments, and during apply(), it would spawn the (iptables)
processes.

But in practice, this list was always pre-determined by a few
parameters, the interface name and the subnet. Instead of keeping the
list of arguments, only keep those few parameters. And generate the list
of arguments only for the short time when we need them.

The difference is that we will want to support nftables too. Later,
we can just generate a different list of commands, but there is no
need to keep this list around.
2021-05-07 11:42:36 +02:00
Thomas Haller
a9a33f2d12
firewall: downgrade info logging for iptables commands
Sure, it's interesting to see the actual iptables commands in the log.
But not at <info> level!
2021-05-07 11:42:36 +02:00
Thomas Haller
60744889e2
firewall: fix adding duplicate iptables rules for shared mode
nm_act_request_set_shared() already calls nm_utils_share_rules_apply().
Calling it twice, is pretty bad because during deactivate we will only
remove one of each duplicate rule.

Fixes: 701654b930 ('core: refactor tracking of shared-rules to use NMUtilsShareRules')
2021-05-07 11:42:35 +02:00
Thomas Haller
51899c3085
wifi: merge branch 'verdre:better-wifi-auth-selection'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/833
2021-05-06 22:26:54 +02:00
Jonas Dreßler
0e30a5256c
devices/wifi: Use wpa-psk key-mgmt for networks supporting WPA2 and WPA3
Networks offering WPA2 and WPA3/SAE at the same time are in WPA3 hybrid
mode. In this case the PSK passphrase rules that apply need to be the
WPA2 rules, so we shouldn't use "sae" as key-mgmt. Also our wifi card
might not support SAE and we want to make sure WPA2 eventually gets used
in that case.

So use "wpa-psk" as key-mgmt method in case an AP is in WPA3 hybrid
mode.
2021-05-06 22:23:29 +02:00
Jonas Dreßler
e019132033
supplicant/interface: Match more ciphers to determine AP security
There can also be APs which don't do wpa-psk, but do support
wpa-psk-sha256, so we should match all AKM suites the AP offers to
determine the security type we want to assign it.
2021-05-06 22:23:29 +02:00
Jonas Dreßler
60c2f6c596
supplicant/config: Allow using FT ciphers with WPA-EAP-SUITE-B-192
According to [1], the only suitable FT cipher suite for WPA3 Enterprise
192-bit mode is "FT over 802.1X, SHA-384", so enable that in case of
key-mgmt is WPA-EAP-SUITE-B-192 to support FT in that case too.

[1] https://mrncciew.com/2020/08/17/wpa3-enterprise/
2021-05-06 22:23:28 +02:00
Jonas Dreßler
2bc3cf0cb8
supplicant/config: Disallow SHA1 ciphers when using required PMF
As mentioned in the wpa_supplicant reference config, when setting PMF to
required with WPA2 (personal or enterprise) authentication, we want to
only enable SHA256 and upwards as HMAC. So enforce that by not passing
WPA-PSK and WPA-EAP to the config in case pmf is set to REQUIRED.
2021-05-06 22:23:28 +02:00
Jonas Dreßler
a4b95a9fcd
libnm-core: Don't allow disabling PMF when using WPA3 key management
Modern WPA3 authentication methods like SAE and WPA-EAP-SUITE-B-192 need
to have management frame protection set to required according to the
standard. Since the last commit, we enforce this automatically when
key-mgmt is set to 'owe', 'sae' or 'wpa-eap-suite-b-192', so disabling
it manually should not be possible.

Add a check to the pmf property that makes sure it can't be set to
'disabled' or 'optional' when one of those key-mgmt methods is used.
2021-05-06 22:23:28 +02:00
Jonas Dreßler
8816cfe736
supplicant/config: Require pmf for owe, sae and wpa-eap-suite-b-192
When using modern WPA3 encryption like owe, sae or wpa-eap-suite-b-192
without fallbacks (so not WPA3+WPA2), protected management frames are
required to be enabled by the specification.

For wpa-eap-suite-b-192 we already do this and force PMF to REQUIRED, we
should also do it for OWE and SAE.
2021-05-06 22:23:28 +02:00
Jonas Dreßler
e06f9508d1
libnm-core: Rewrite comment of key-mgmt property
The key-mgmt property of NMSettingWirelessSecurity is slightly confusing
when you know there's also a wpa_supplicant configuration option called
"key_mgmt". Our property is not the same as that supplicant option even
though they do have things in common. NMs key-mgmt is not exactly meant
to configure which AKM suites you want to use, but rather which method
of wifi security is being used (so "wpa2+wpa3 personal", "wpa3 personal
only" or "wpa3 enterprise only").

Try to make this a bit clearer in the documentation of the property by
rewriting it and listing those security methods.
2021-05-06 22:23:28 +02:00