1. 07 May, 2019 3 commits
    • Thomas Haller's avatar
      libnm: use macro and designated initializers for NMVariantAttributeSpec · 86dc50d4
      Thomas Haller authored
      I think initializing structs should (almost) be always done with designated
      initializers, because otherwise it's easy to get the order wrong. The
      problem is that otherwise the order of fields gets additional meaning
      not only for the memory layout, but also for the code that initialize
      the structs.
      
      Add a macro NM_VARIANT_ATTRIBUTE_SPEC_DEFINE() that replaces the other
      (duplicate) macros. This macro also gets it right to mark the struct as
      const.
      86dc50d4
    • Thomas Haller's avatar
      libnm: mark NMVariantAttributeSpec pointers as const · 4e3955e6
      Thomas Haller authored
      This actually allows the compiler/linker to mark the memory as read-only and any
      modification will cause a segmentation fault.
      
      I would also think that it allows the compiler to put the structure directly
      beside the outer constant array (in which this pointer is embedded). That is good
      locality-wise.
      4e3955e6
    • Thomas Haller's avatar
      libnm: cleanup _nm_utils_parse_tc_handle() · cc9f0716
      Thomas Haller authored
      - g_ascii_strtoll() accepts leading spaces, but it leaves
        the end pointer at the first space after the digit. That means,
        we accepted "1: 0" but not "1 :0". We should either consistently
        accept spaces around the digits/colon or reject it.
      
      - g_ascii_strtoll() accepts "\v" as a space (just like `man 3 isspace`
        comments that "\v" is a space in C and POSIX locale.
        For some reasons (unknown to me) g_ascii_isspace() does not treat
        "\v" as space. And neither does NM_ASCII_SPACES and
        nm_str_skip_leading_spaces().
        We should be consistent about what we consider spaces and what not.
        It's already odd to accept '\n' as spaces here, but well, lets do
        it for the sake of consistency (so that it matches with our
        understanding of ASCII spaces, albeit not POSIX's).
      
      - don't use bogus error domains in "g_set_error (error, 1, 0, ..."
        That is a bug and we have NM_UTILS_ERROR exactly for error instances
        with unspecified domain and code.
      
      - as before, accept a trailing ":" with omitted minor number.
      
      - reject all unexpected characters. strtoll() accepts '+' / '-'
        and a "0x" prefix of the numbers (and leading POSIX spaces). Be
        strict here and only accepts NM_ASCII_SPACES, ':', and hexdigits.
        In particular, don't accept the "0x" prefix.
      
      This parsing would be significantly simpler to implement, if we could
      just strdup() the string, split the string at the colon delimiter and
      use _nm_utils_ascii_str_to_int64() which gets leading/trailing spaces
      right. But let's save the "overhead" of an additional alloc.
      cc9f0716
  2. 01 May, 2019 1 commit
    • Thomas Haller's avatar
      libnm: unify property-to-dbus handling of NMSetting · 0d1b8ee9
      Thomas Haller authored
      Merge the function pointer get_func() into to_dbus_fcn().
      
      Previously, get_func() as handled separately from to_dbus_fnc()
      (formerly synth_func()). The notion was that synth-func would syntetize
      properties that are D-Bus only. But that distinction does not seem
      very helpful to me.
      
      Instaed, we want to convert a property to D-Bus. Period. The
      implementation should be handled uniformly. Hence, now that is
      all done by property_to_dbus().
      
      Note that property_to_dbus() is also called as default implementation
      for compare-property. At least, for properties that are backed by a
      GObject property.
      0d1b8ee9
  3. 30 Apr, 2019 2 commits
  4. 25 Apr, 2019 1 commit
  5. 20 Apr, 2019 2 commits
  6. 18 Apr, 2019 8 commits
    • Thomas Haller's avatar
      shared: build helper "libnm-libnm-core-{intern|aux}.la" library for libnm-core · 284ac92e
      Thomas Haller authored
      "libnm-core" implements common functionality for "NetworkManager" and
      "libnm".
      
      Note that clients like "nmcli" cannot access the internal API provided
      by "libnm-core". So, if nmcli wants to do something that is also done by
      "libnm-core", , "libnm", or "NetworkManager", the code would have to be
      duplicated.
      
      Instead, such code can be in "libnm-libnm-core-{intern|aux}.la".
      Note that:
      
        0) "libnm-libnm-core-intern.la" is used by libnm-core itsself.
           On the other hand, "libnm-libnm-core-aux.la" is not used by
           libnm-core, but provides utilities on top of it.
      
        1) they both extend "libnm-core" with utlities that are not public
           API of libnm itself. Maybe part of the code should one day become
           public API of libnm. On the other hand, this is code for which
           we may not want to commit to a stable interface or which we
           don't want to provide as part of the API.
      
        2) "libnm-libnm-core-intern.la" is statically linked by "libnm-core"
           and thus directly available to "libnm" and "NetworkManager".
           On the other hand, "libnm-libnm-core-aux.la" may be used by "libnm"
           and "NetworkManager".
           Both libraries may be statically linked by libnm clients (like
           nmcli).
      
        3) it must only use glib, libnm-glib-aux.la, and the public API
           of libnm-core.
           This is important: it must not use "libnm-core/nm-core-internal.h"
           nor "libnm-core/nm-utils-private.h" so the static library is usable
           by nmcli which couldn't access these.
      
      Note that "shared/nm-meta-setting.c" is an entirely different case,
      because it behaves differently depending on whether linking against
      "libnm-core" or the client programs. As such, this file must be compiled
      twice.
      
      (cherry picked from commit af07ed01)
      284ac92e
    • Thomas Haller's avatar
      shared: move most of "shared/nm-utils" to "shared/nm-glib-aux" · d984b2ce
      Thomas Haller authored
      From the files under "shared/nm-utils" we build an internal library
      that provides glib-based helper utilities.
      
      Move the files of that basic library to a new subdirectory
      "shared/nm-glib-aux" and rename the helper library "libnm-core-base.la"
      to "libnm-glib-aux.la".
      
      Reasons:
      
       - the name "utils" is overused in our code-base. Everything's an
         "utils". Give this thing a more distinct name.
      
       - there were additional files under "shared/nm-utils", which are not
         part of this internal library "libnm-utils-base.la". All the files
         that are part of this library should be together in the same
         directory, but files that are not, should not be there.
      
       - the new name should better convey what this library is and what is isn't:
         it's a set of utilities and helper functions that extend glib with
         funcitonality that we commonly need.
      
      There are still some files left under "shared/nm-utils". They have less
      a unifying propose to be in their own directory, so I leave them there
      for now. But at least they are separate from "shared/nm-glib-aux",
      which has a very clear purpose.
      
      (cherry picked from commit 80db06f7)
      d984b2ce
    • Thomas Haller's avatar
      shared: build helper "libnm-libnm-core-{intern|aux}.la" library for libnm-core · af07ed01
      Thomas Haller authored
      "libnm-core" implements common functionality for "NetworkManager" and
      "libnm".
      
      Note that clients like "nmcli" cannot access the internal API provided
      by "libnm-core". So, if nmcli wants to do something that is also done by
      "libnm-core", , "libnm", or "NetworkManager", the code would have to be
      duplicated.
      
      Instead, such code can be in "libnm-libnm-core-{intern|aux}.la".
      Note that:
      
        0) "libnm-libnm-core-intern.la" is used by libnm-core itsself.
           On the other hand, "libnm-libnm-core-aux.la" is not used by
           libnm-core, but provides utilities on top of it.
      
        1) they both extend "libnm-core" with utlities that are not public
           API of libnm itself. Maybe part of the code should one day become
           public API of libnm. On the other hand, this is code for which
           we may not want to commit to a stable interface or which we
           don't want to provide as part of the API.
      
        2) "libnm-libnm-core-intern.la" is statically linked by "libnm-core"
           and thus directly available to "libnm" and "NetworkManager".
           On the other hand, "libnm-libnm-core-aux.la" may be used by "libnm"
           and "NetworkManager".
           Both libraries may be statically linked by libnm clients (like
           nmcli).
      
        3) it must only use glib, libnm-glib-aux.la, and the public API
           of libnm-core.
           This is important: it must not use "libnm-core/nm-core-internal.h"
           nor "libnm-core/nm-utils-private.h" so the static library is usable
           by nmcli which couldn't access these.
      
      Note that "shared/nm-meta-setting.c" is an entirely different case,
      because it behaves differently depending on whether linking against
      "libnm-core" or the client programs. As such, this file must be compiled
      twice.
      af07ed01
    • Thomas Haller's avatar
      shared: move most of "shared/nm-utils" to "shared/nm-glib-aux" · 80db06f7
      Thomas Haller authored
      From the files under "shared/nm-utils" we build an internal library
      that provides glib-based helper utilities.
      
      Move the files of that basic library to a new subdirectory
      "shared/nm-glib-aux" and rename the helper library "libnm-core-base.la"
      to "libnm-glib-aux.la".
      
      Reasons:
      
       - the name "utils" is overused in our code-base. Everything's an
         "utils". Give this thing a more distinct name.
      
       - there were additional files under "shared/nm-utils", which are not
         part of this internal library "libnm-utils-base.la". All the files
         that are part of this library should be together in the same
         directory, but files that are not, should not be there.
      
       - the new name should better convey what this library is and what is isn't:
         it's a set of utilities and helper functions that extend glib with
         funcitonality that we commonly need.
      
      There are still some files left under "shared/nm-utils". They have less
      a unifying propose to be in their own directory, so I leave them there
      for now. But at least they are separate from "shared/nm-glib-aux",
      which has a very clear purpose.
      80db06f7
    • Thomas Haller's avatar
      libnm: minor refactoring of _nm_utils_bridge_vlan_verify_list() · 05a54713
      Thomas Haller authored
      - if there is only one vlan in the list, then we can return success
        early. That is, because one NMBridgeVlan instance is always valid
        due to the way how users must use the API to construct the element.
      
      - the implementation for check_normalizable is only correct, if there
        are no duplicate or overlapping ranges. Assert for that. In fact,
        all callers first check for errors and then for normalizable errors.
      
      - avoid duplicate calls to nm_bridge_vlan_get_vid_range(). There are
        duplicate assertions that we don't need.
      
      - only check for pvid once per range.
      
      - combine calls to g_hash_table_contains() and g_hash_table_add().
      
      (cherry picked from commit a358da09)
      05a54713
    • Beniamino Galvani's avatar
      all: support bridge vlan ranges · da204257
      Beniamino Galvani authored
      In some cases it is convenient to specify ranges of bridge vlans, as
      already supported by iproute2 and natively by kernel. With this commit
      it becomes possible to add a range in this way:
      
       nmcli connection modify eth0-slave +bridge-port.vlans "100-200 untagged"
      
      vlan ranges can't be PVIDs because only one PVID vlan can exist.
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1652910
      (cherry picked from commit 70935157)
      da204257
    • Thomas Haller's avatar
      libnm: minor refactoring of _nm_utils_bridge_vlan_verify_list() · a358da09
      Thomas Haller authored
      - if there is only one vlan in the list, then we can return success
        early. That is, because one NMBridgeVlan instance is always valid
        due to the way how users must use the API to construct the element.
      
      - the implementation for check_normalizable is only correct, if there
        are no duplicate or overlapping ranges. Assert for that. In fact,
        all callers first check for errors and then for normalizable errors.
      
      - avoid duplicate calls to nm_bridge_vlan_get_vid_range(). There are
        duplicate assertions that we don't need.
      
      - only check for pvid once per range.
      
      - combine calls to g_hash_table_contains() and g_hash_table_add().
      a358da09
    • Beniamino Galvani's avatar
      all: support bridge vlan ranges · 70935157
      Beniamino Galvani authored
      In some cases it is convenient to specify ranges of bridge vlans, as
      already supported by iproute2 and natively by kernel. With this commit
      it becomes possible to add a range in this way:
      
       nmcli connection modify eth0-slave +bridge-port.vlans "100-200 untagged"
      
      vlan ranges can't be PVIDs because only one PVID vlan can exist.
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1652910
      70935157
  7. 12 Apr, 2019 3 commits
  8. 10 Apr, 2019 1 commit
  9. 08 Apr, 2019 1 commit
    • Beniamino Galvani's avatar
      libnm-core: fix invalid memory access · 80a3031a
      Beniamino Galvani authored
      When we delete the runner.name property, the runner object itself gets
      deleted if that was the only property, and @runner becomes invalid.
      
       ==13818== Invalid read of size 1
       ==13818==    at 0x55EAF4: nm_streq (nm-macros-internal.h:869)
       ==13818==    by 0x55EAF4: _json_team_normalize_defaults (nm-utils.c:5573)
       ==13818==    by 0x566C89: _nm_utils_team_config_set (nm-utils.c:6057)
       ==13818==    by 0x5498A6: _nm_utils_json_append_gvalue (nm-utils-private.h:228)
       ==13818==    by 0x5498A6: set_property (nm-setting-team.c:1622)
       ==13818==  Address 0x182a9330 is 0 bytes inside a block of size 13 free'd
       ==13818==    at 0x4839A0C: free (vg_replace_malloc.c:530)
       ==13818==    by 0x4857868: json_delete_string (value.c:763)
       ==13818==    by 0x4857868: json_delete (value.c:975)
       ==13818==    by 0x4851FA1: UnknownInlinedFun (jansson.h:129)
       ==13818==    by 0x4851FA1: hashtable_do_del (hashtable.c:131)
       ==13818==    by 0x4851FA1: hashtable_del (hashtable.c:289)
       ==13818==    by 0x55DFDD: _json_del_object (nm-utils.c:5384)
       ==13818==    by 0x55EA70: _json_delete_object_on_string_match (nm-utils.c:5532)
       ==13818==    by 0x55EADB: _json_team_normalize_defaults (nm-utils.c:5549)
       ==13818==    by 0x566C89: _nm_utils_team_config_set (nm-utils.c:6057)
       ==13818==    by 0x5498A6: _nm_utils_json_append_gvalue (nm-utils-private.h:228)
       ==13818==    by 0x5498A6: set_property (nm-setting-team.c:1622)
       ==13818==  Block was alloc'd at
       ==13818==    at 0x483880B: malloc (vg_replace_malloc.c:299)
       ==13818==    by 0x4852E8C: lex_scan_string (load.c:389)
       ==13818==    by 0x4852E8C: lex_scan (load.c:620)
       ==13818==    by 0x4853458: parse_object (load.c:738)
       ==13818==    by 0x4853458: parse_value (load.c:862)
       ==13818==    by 0x4853466: parse_object (load.c:739)
       ==13818==    by 0x4853466: parse_value (load.c:862)
       ==13818==    by 0x4853655: parse_json.constprop.7 (load.c:899)
       ==13818==    by 0x48537CF: json_loads (load.c:959)
       ==13818==    by 0x566780: _nm_utils_team_config_set (nm-utils.c:5961)
       ==13818==    by 0x5498A6: _nm_utils_json_append_gvalue (nm-utils-private.h:228)
       ==13818==    by 0x5498A6: set_property (nm-setting-team.c:1622)
      
      Fixes: a5642fd9 ('libnm-core: team: rework defaults management on runner properties')
      80a3031a
  10. 29 Mar, 2019 1 commit
    • Francesco Giudici's avatar
      libnm-core: make compiler happy · a0d1971c
      Francesco Giudici authored
        ../libnm-core/nm-utils.c:6784:30: error: unused variable 'var_unref' [-Werror,-Wunused-variable]
                      gs_unref_variant GVariant *var_unref = vlan_var;
      a0d1971c
  11. 27 Mar, 2019 1 commit
  12. 26 Mar, 2019 3 commits
  13. 25 Mar, 2019 1 commit
  14. 24 Mar, 2019 6 commits
  15. 07 Mar, 2019 6 commits
    • Thomas Haller's avatar
      libnm: rename and expose nm_utils_base64secret_decode() in libnm · 0d178a96
      Thomas Haller authored
      A NetworkManager client requires an API to validate and decode
      a base64 secret -- like it is used by WireGuard. If we don't have
      this as part of the API, it's inconvenient. Expose it.
      
      Rename it from _nm_utils_wireguard_decode_key(), to give it a more
      general name.
      
      Also, rename _nm_utils_wireguard_normalize_key() to
      nm_utils_base64secret_normalize(). But this one we keep as internal
      API. The user will care more about validating and decoding the base64
      key. To convert the key back to base64, we don't need a public API in
      libnm.
      
      This is another ABI change since 1.16-rc1.
      
      (cherry picked from commit e46ba018)
      0d178a96
    • Thomas Haller's avatar
      wireguard: accept all-zero private-key, public-key and preshared-key · b680d64b
      Thomas Haller authored
      - For PSK, an all-zero PSK means to don't do symmetric encryption. As such,
        at first it seems a bit odd when the user sets
      
            - preshared-key-flags != "4 (not-required)"
      
            - preshared-key = AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
      
        Here the user indicates that a PSK is required, but then provides an
        all-zero PSK that effectively disables it. Still, we should not reject
        such a configuration. This has the benefit that it allos the user for
        being prompted for a PSK, only to disable it by entering the all-zero key.
      
      - For the private-key (and consequently the public-key), "public-key-flags=4"
        is rejected by libnm. A private key is always required for NetworkManager to
        configure the link. However, let's not care for all-zero keys either. If the user
        configures that, we just set that key. It's a valid setting as far as WireGuard
        (the kernel module) is concerned, so we shouldn't reject it.
      
      (cherry picked from commit 78dccb8b)
      b680d64b
    • Thomas Haller's avatar
      libnm: rename and expose nm_utils_base64secret_decode() in libnm · e46ba018
      Thomas Haller authored
      A NetworkManager client requires an API to validate and decode
      a base64 secret -- like it is used by WireGuard. If we don't have
      this as part of the API, it's inconvenient. Expose it.
      
      Rename it from _nm_utils_wireguard_decode_key(), to give it a more
      general name.
      
      Also, rename _nm_utils_wireguard_normalize_key() to
      nm_utils_base64secret_normalize(). But this one we keep as internal
      API. The user will care more about validating and decoding the base64
      key. To convert the key back to base64, we don't need a public API in
      libnm.
      
      This is another ABI change since 1.16-rc1.
      e46ba018
    • Thomas Haller's avatar
      wireguard: accept all-zero private-key, public-key and preshared-key · 78dccb8b
      Thomas Haller authored
      - For PSK, an all-zero PSK means to don't do symmetric encryption. As such,
        at first it seems a bit odd when the user sets
      
            - preshared-key-flags != "4 (not-required)"
      
            - preshared-key = AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
      
        Here the user indicates that a PSK is required, but then provides an
        all-zero PSK that effectively disables it. Still, we should not reject
        such a configuration. This has the benefit that it allos the user for
        being prompted for a PSK, only to disable it by entering the all-zero key.
      
      - For the private-key (and consequently the public-key), "public-key-flags=4"
        is rejected by libnm. A private key is always required for NetworkManager to
        configure the link. However, let's not care for all-zero keys either. If the user
        configures that, we just set that key. It's a valid setting as far as WireGuard
        (the kernel module) is concerned, so we shouldn't reject it.
      78dccb8b
    • Marco Trevisan's avatar
      nm: Fix syntax on introspection annotations · b5bbf8ed
      Marco Trevisan authored
      Various annotations were added using multiple colons, while only one has
      to be added or g-ir-introspect will consider them part of the description
      
      !94
      (cherry picked from commit 73005fcf)
      b5bbf8ed
    • Marco Trevisan's avatar
      nm: Fix syntax on introspection annotations · 73005fcf
      Marco Trevisan authored
      Various annotations were added using multiple colons, while only one has
      to be added or g-ir-introspect will consider them part of the description
      
      !94
      73005fcf