1. 19 Dec, 2018 8 commits
    • Thomas Haller's avatar
      all: don't use static buffer for nm_utils_inet*_ntop() · a51c09dc
      Thomas Haller authored
      While nm_utils_inet*_ntop() accepts a %NULL buffer to fallback
      to a static buffer, don't do that.
      
      I find the possibility of using a static buffer here error prone
      and something that should be avoided. There is of course the downside,
      that in some cases it requires an additional line of code to allocate
      the buffer on the stack as auto-variable.
      a51c09dc
    • Thomas Haller's avatar
      device: drop rp_filter handling · a936086d
      Thomas Haller authored
      After commit b1082aa9 (device: disable
      rp_filter handling) drop the now unused code.
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1651097
      a936086d
    • Thomas Haller's avatar
      device: add sysctl-ip-conf getter and use it · 8bf6ae1b
      Thomas Haller authored
      - add nm_device_sysctl_ip_conf_get() and nm_device_sysctl_ip_conf_get_int_checked().
        These functions don't use nm_device_get_ip_iface(), but resolve the
        ifname from the platform cache.
      
      - in general, resolve the name first with nm_device_get_ip_iface_from_platform().
      8bf6ae1b
    • Thomas Haller's avatar
      device: add nm_device_get_ip_iface_from_platform() · f9077fa7
      Thomas Haller authored
      We have a cached nm_device_get_ip_iface() property. However, the interface
      name is not an identifier for a link because it can change at any time.
      
      Also, we already have the (ip) ifindex as proper identifier for the
      platform link. We shouldn't use two redundant identifiers to refer to
      a link.
      
      Clearly, sometimes we need an ifname. For example for ethtool ioctl or
      sysctl path names. For ethtool API, we resolve the actual name as late
      as possible, and for sysctl API we prefer NMP_SYSCTL_PATHID_NETDIR*().
      However, that is not always possible, for example for /proc/sys/net/ipv6/conf/
      sysctls.
      
      Add a function that resolves the ifname by looking into the cache. This
      of course is still racy, but it minimizes the time.
      
      Also, we should less and less rely on the ifname, and resolve it as late
      as possible. This patch adds a small wrapper going into that direction.
      f9077fa7
    • Thomas Haller's avatar
      core/trivial: rename nm_platform_sysctl_set_ip6_hop_limit_safe() · 91b5babf
      Thomas Haller authored
      Now that we have other helper function on platfrom for setting
      IP configuration sysctls, rename the function to set the hop-limit
      to match the pattern.
      91b5babf
    • Thomas Haller's avatar
      18a99e86
    • Thomas Haller's avatar
      device/trivial: rename device's sysctl function · 395374cf
      Thomas Haller authored
      These functions call platform's sysctl getter and setters.
      
      Note that the called platform functions are called nm_platform_sysctl_get()
      and nm_platform_sysctl_set(). Also, in this case they use the ip-conf path
      via nm_utils_sysctl_ip_conf_path().
      
      Also, next we will add API nm_platform_sysctl_ip_conf_get() and
      nm_platform_sysctl_ip_conf_set(), which will be wrappers around
      nm_platform_sysctl_get() and nm_platform_sysctl_set(), using the ip-conf
      paths as well.
      
      Rename the device functions, to be more similar to the existing and future
      naming in platform.
      395374cf
    • Thomas Haller's avatar
      device: merge IPv4 and IPv6 variants of nm_device_ipv4_sysctl_set() · d27fa362
      Thomas Haller authored
      For one, next we will drop setting rp_filter, hence there are no
      more users of an IPv4 variant and nm_device_ipv4_sysctl_set() would
      have to be dropped anyway.
      
      However, instead of doing that, merge the IPv4 and IPv6 variant.
      
      With this, the fallback to the default is now also supported for IPv6
      (though unused).
      
      Also, don't access nm_device_get_ip_iface(). The interface name might
      not be right, we should only rely on the ifindex. Load the interface
      name from platform cache instead.
      d27fa362
  2. 16 Dec, 2018 1 commit
  3. 14 Dec, 2018 2 commits
  4. 13 Dec, 2018 4 commits
  5. 12 Dec, 2018 6 commits
    • Beniamino Galvani's avatar
      e01a7c11
    • Beniamino Galvani's avatar
      device: reset SR-IOV VFs on deactivation · 529533a5
      Beniamino Galvani authored
      If the connection has a sriov setting we configure SR-IOV VFs on
      activation. We should also clear resources when the connection
      deactivates.
      529533a5
    • Beniamino Galvani's avatar
      device: configure static number of VFs in unavailable state · 75024e11
      Beniamino Galvani authored
      Don't configure the static number of VFs when the device is realized
      because the device could still be unmanaged. Instead, do it when the
      device becomes managed.
      75024e11
    • Thomas Haller's avatar
      core: never fail reading host-id timestamp and never change it · a68d027b
      Thomas Haller authored
      The timestamp of the host-id is the timestamp of the secret_key file.
      Under normal circumstances, reading the timestamp should never fail,
      and reading it multiple times should always yield the same result.
      
      If we unexpectedly fail to read the timestamp from the file we want:
      
      - log a warning, so that the user can find out what's wrong. But
        do so only once.
      
      - we don't want to handle errors or fail operation due to a missing
        timestamp. Remember, it's not supposed to ever fail, and if it does,
        just log a warning and proceed with a fake timestamp instead. In
        that case something is wrong, but using a non-stable, fake timestamp
        is the least of the problems here.
        We already have a stable identifier (the host-id) which we can use to
        generate a fake timestamp. Use it.
      
      In case the user would replace the secret_key file, we also don't want
      that accessing nm_utils_host_id_get_timestamp*() yields different
      results. It's not implemented (nor necessary) to support reloading a
      different timestamp. Hence, nm_utils_host_id_get_timestamp() should
      memoize the value and ensure that it never changes.
      a68d027b
    • Thomas Haller's avatar
      core/trivial: rename nm_utils_get_boot_id_*() · d693e03a
      Thomas Haller authored
      Rename to nm_utils_boot_id_*(), it matches nm_utils_machine_id_*()
      and nm_utils_host_id_get().
      d693e03a
    • Thomas Haller's avatar
      core/trivial: rename secret-key to host-key · 6ffcd263
      Thomas Haller authored
      Now that the secret-key is hashed with the machine-id, the name is
      no longer best.
      
      Sure, part of the key are persisted in /var/lib/NetworkManager/secret_key
      file, which the user is well advised to keep secret.
      
      But what nm_utils_secret_key_get() returns is first and foremost a binary
      key that is per-host and used for hashing a per-host component. It's
      really the "host-id". Compare that to what we also have, the
      "machine-id" and the "boot-id".
      
      Rename.
      6ffcd263
  6. 11 Dec, 2018 5 commits
  7. 10 Dec, 2018 3 commits
  8. 09 Dec, 2018 7 commits
    • Thomas Haller's avatar
      core: improve and fix keeping connection active based on "connection.permissions" · b635b4d4
      Thomas Haller authored
      By setting "connection.permissions", a profile is restricted to a
      particular user.
      That means for example, that another user cannot see, modify, delete,
      activate or deactivate the profile. It also means, that the profile
      will only autoconnect when the user is logged in (has a session).
      
      Note that root is always able to activate the profile. Likewise, the
      user is also allowed to manually activate the own profile, even if no
      session currently exists (which can easily happen with `sudo`).
      
      When the user logs out (the session goes away), we want do disconnect
      the profile, however there are conflicting goals here:
      
      1) if the profile was activate by root user, then logging out the user
         should not disconnect the profile. The patch fixes that by not
         binding the activation to the connection, if the activation is done
         by the root user.
      
      2) if the profile was activated by the owner when it had no session,
         then it should stay alive until the user logs in (once) and logs
         out again. This is already handled by the previous commit.
      
         Yes, this point is odd. If you first do
      
            $ sudo -u $OTHER_USER nmcli connection up $PROFILE
      
         the profile activates despite not having a session. If you then
      
            $ ssh guest@localhost nmcli device
      
         you'll still see the profile active. However, the moment the SSH session
         ends, a session closes and the profile disconnects. It's unclear, how to
         solve that any better. I think, a user who cares about this, should not
         activate the profile without having a session in the first place.
      
      There are quite some special cases, in particular with internal
      activations. In those cases we need to decide whether to bind the
      activation to the profile's visibility.
      
      Also, expose the "bind" setting in the D-Bus API. Note, that in the future
      this flag may be modified via D-Bus API. Like we may also add related API
      that allows to tweak the lifetime of the activation.
      
      Also, I think we broke handling of connection visiblity with 37e8c53e
      "core: Introduce helper class to track connection keep alive". This
      should be fixed now too, with improved behavior.
      
      Fixes: 37e8c53e
      
      https://bugzilla.redhat.com/show_bug.cgi?id=1530977
      b635b4d4
    • Thomas Haller's avatar
      device: arm keep-alive instance when queuing active-connection for activation · a4bdb161
      Thomas Haller authored
      Now that the keep-alive instance defaults to ALIVE by default, we can
      always arm it when starting to activate the active-connection.
      
      The keep-alive instance may have been armed earlier already:
      for example, when binding its lifetime to a D-Bus name or
      when watching the connection's visible state.
      
      However, at the moment when we queue the active-connection for
      activation, we also want to make sure that the keep-alive instance is
      armed. It is nicer for consistancy reasons.
      
      Note, that nm_keep_alive_arm() has no effect if nm_keep_alive_disarm()
      was called earlier already. Also note, that NMActiveConnection will
      disarm the keep-alive instance, when changing to a state greater than
      ACTIVATED. So, all works together nicely.
      
      Also, no longer arm the keep-alive instance in the constructor of
      NMActiveConnection. It would essentially mean, that the instances
      is aremd very early.
      
      Also, as alternative point of interest, arm the keep-alive instance
      when registering the signal handler in "nm-policy.c".
      a4bdb161
    • Thomas Haller's avatar
      device: always disconnect in nm_device_disconnect_active_connection() · e1b0451d
      Thomas Haller authored
      Previously, if @active referenced a device but was not currently queued
      or the current activation request, nothing was done.
      
      Now, in such a case still call nm_active_connection_set_state_fail().
      Note that nm_active_connection_set_state_fail() has no effects on
      active-connections that are already in disconnected state (which
      we would expect by such an active connection). Likely there is no
      visible change here, but it feels more correct to ensure the active
      connection is always failed.
      e1b0451d
    • Thomas Haller's avatar
      device: use correct active-connection's state-change reason in... · 71a090c9
      Thomas Haller authored
      device: use correct active-connection's state-change reason in nm_device_disconnect_active_connection()
      
      It just makes more sense, to let the caller decide on the reason.
      71a090c9
    • Thomas Haller's avatar
      8f360197
    • Thomas Haller's avatar
      device: pass active-connection's state-change reason to _clear_queued_act_request() · fe5f5f7a
      Thomas Haller authored
      No change in behavior, yet.
      fe5f5f7a
    • Thomas Haller's avatar
      device: add state-change reason argument to nm_device_disconnect_active_connection() · 461bf7aa
      Thomas Haller authored
      nm_device_disconnect_active_connection() is generally useful and a prefered
      form to fail an active connection. The device's state-change reason is important,
      so it needs to be injected.
      461bf7aa
  9. 06 Dec, 2018 1 commit
  10. 04 Dec, 2018 1 commit
  11. 03 Dec, 2018 2 commits
    • Thomas Haller's avatar
      core: avoid calling platform code with invalid ifindex · d45eed44
      Thomas Haller authored
      Since commit 945c904f "platform: assert against valid ifindex and
      remove duplicate assertions", it is no longer allowed to call certain
      platform functions with invalid ifindex.
      
      These trigger now an assertion. Note that the assertion is merely a
      g_return_val_if_fail(), hence in non-debug mode, this does not lead to
      a crash.
      
      Fixes: 945c904f
      d45eed44
    • Thomas Haller's avatar
      device/shared: set ANDROID_METERED option 43 for shared connections · 1a2e767f
      Thomas Haller authored
      The problem is that updating the metered value of a shared connection is
      not implemented. The user needs to fully reactivate the profile for changes
      to take effect.
      
      That is unfortunate, especially because reapplying the route metric
      works in other other cases.
      1a2e767f