1. 08 Oct, 2019 1 commit
    • apatard's avatar
      Add support for wpa_supplicant engine-id / key-id, engine2-id / key2-id · c1a93f9f
      apatard authored
      There's no need to try to set the engine/engine2 parameters, they're
      automatically set.
      
      Support tested with :
      - eap-tls on libvirt/kvm virtual machine and real system
      - wpa2-eap-peap-tls on real system. (TODO: setting test VM with mac80211_hwsim)
      The certificate key is protected by tpm2. No pin.
      
      Modifications done to  libnm-core/nm-setting-8021x.c :
      o verify_tls() to make sure that if engine_id/engine2_id are specified
        key_id/key2_id are specified too. If engine_id/engine2_id not specified, behaves
        as before.
      o need_secrets_tls() modified to not look for a passphrase for a certificate if
        an engine id is set for phase 1 or phase 2.
      o verify_ttls() to work in my phase 2 peap-tls case. Could have used a new fonction
        but was a little bit easier to adapt verify_ttls(). The function nows check:
        - we're using phase2 auth or autheap
        - there's an identity set
        - in case of ttls(), check that anonymous identity is set.
      
      Example of 802-1x section for ethernet eap-tls case:
      [802-1x]
      ca-cert=/home/rtp/ca.pem
      client-cert=/home/rtp/tpm2/csr2/client-tpm-qemu.crt
      eap=tls;
      identity=nm-tpm2
      phase1-engine-id=tpm2tss
      phase1-key-id=/home/rtp/tpm2/csr2/pri_pub_blob.key
      Signed-off-by: apatard's avatarArnaud Patard <apatard@hupstream.com>
      c1a93f9f
  2. 07 Oct, 2019 1 commit
  3. 04 Oct, 2019 2 commits
  4. 03 Oct, 2019 5 commits
    • Thomas Haller's avatar
      5a24ad53
    • Ilya Shipitsin's avatar
      src/devices/nm-device.c: resolve possible null pointer dereference · e8588d0c
      Ilya Shipitsin authored
      found by cppcheck
      
      [src/devices/nm-device.c:3032] -> [src/devices/nm-device.c:3025]: (warning) Either the condition '!handle' is redundant or there is possible null pointer dereference: handle.
      
      https://github.com/NetworkManager/NetworkManager/pull/352
      e8588d0c
    • Thomas Haller's avatar
    • Thomas Haller's avatar
      libnm: deprecate nm_client_check_connectivity() in 1.22 · f45aeba4
      Thomas Haller authored
      The previous commit marks all synchronous libnm API as deprecated.
      In practice, the macro _NM_DEPRECATED_SYNC_METHOD expands to
      nothing, because there is no immediate urgency to force users
      to migrate.
      
      However nm_client_check_connectivity() is especially bad: it
      makes a synchronous call and then updates the content of the
      cache artificially. Usually, NMClient's cache of D-Bus objects
      is only updated by "PropertiesChanged" D-Bus signals.
      nm_client_check_connectivity() instead will act on the response to
      the "CheckConnectivity" D-Bus call -- a response that is picked
      out of order from the ordered sequence of messages --  and will
      update the cache instead of honoring the usual "PropertiesChanged"
      signal.
      
      I think such behavior is fundamentally broken. For a trivial property like
      NM_CLIENT_CONNECTIVITY such behavior is odd at best. Note how applying
      this approach to other functions (like nm_client_deactivate_connection(),
      which would affect a much larger state) would not be feasible.
      
      I also imagine it to be complicate to preserve this behavior when
      reworking libnm, as I plan to do.
      
      See also commit b799de28 ('libnm: update property in the manager
      after connectivity check'), which introduced this behavior to "fix"
      bgo#784629.
      f45aeba4
    • Thomas Haller's avatar
      libnm: deprecate synchronous/blocking API in libnm · e90684a1
      Thomas Haller authored
      Note that D-Bus is fundamentally asynchronous. Doing blocking calls
      on top of D-Bus is odd, especially for libnm's NMClient. That is because
      NMClient essentially is a client-side cache of the objects from the D-Bus
      interface. This cache should be filled exclusively by (asynchronous) D-Bus
      events (PropertiesChanged). So, making a blocking D-Bus call means to wait
      for a response and return it, while queuing all messages that are received
      in the meantime.
      Basically there are three ways how a synchronous API on NMClient could behave:
      
       1) the call just calls g_dbus_connection_call_sync(). This means
          that libnm sends a D-Bus request via GDBusConnection, and blockingly
          waits for the response. All D-Bus messages that get received in the
          meantime are queued in the GMainContext that belongs to NMClient.
          That means, none of these D-Bus events are processed until we
          iterate the GMainContext after the call returns. The effect is,
          that NMClient (and all cached objects in there) are unaffected by
          the D-Bus request.
          Most of the synchronous API calls in libnm are of this kind.
          The problem is that the strict ordering of D-Bus events gets
          violated.
          For some API this is not an immediate problem. Take for example
          nm_device_wifi_request_scan(). The call merely blockingly tells
          NetworkManager to start scanning, but since NetworkManager's D-Bus
          API does not directly expose any state that tells whether we are
          currently scanning, this out of order processing of the D-Bus
          request is a small issue.
          The problem is more obvious for nm_client_networking_set_enabled().
          After calling it, NM_CLIENT_NETWORKING_ENABLED is still unaffected
          and unchanged, because the PropertiesChanged signal from D-Bus
          is not yet processed.
          This means, while you make such a blocking call, NMClient's state
          does not change. But usually you perform the synchronous call
          to change some state. In this form, the blocking call is not useful,
          because NMClient only changes the state after iterating the GMainContext,
          and not after the blocking call returns.
      
       2) like 1), but after making the blocking g_dbus_connection_call_sync(),
          update the NMClient cache artificially. This is what
          nm_manager_check_connectivity() does, to "fix" bgo#784629.
          This also has the problem of out-of-order events, but it kinda
          solves the problem of not changing the state during the blocking
          call. But it does so by hacking the state of the cache. I think
          this is really wrong because the state should only be updated from
          the ordered stream of D-Bus messages (PropertiesChanged signal and
          similar). When libnm decides to modify the state, there may be already
          D-Bus messages queued that affect this very state.
      
       3) instead of calling g_dbus_connection_call_sync(), use the
          asynchronous g_dbus_connection_call(). If we would use a sepaate
          GMainContext for all D-Bus related calls, we could ensure that
          while we block for the response, we iterate that internal main context.
          This might be nice, because all events are processed in order and
          after the blocking call returns, the NMClient state is up to date.
          The are problems however: current blocking API does not do this,
          so it's a significant change in behavior. Also, it might be
          unexpected to the user that during the blocking call the entire
          content of NMClient's cache might change and all pointers to the
          cache might be invalidated. Also, of course NMClient would invoke
          signals for all the changes that happen.
          Another problem is that this would be more effort to implement
          and it involves a small performance overhead for all D-Bus related
          calls (because we have to serialize all events in an internal
          GMainContext first and then invoke them on the caller's context).
          Also, if the users wants this behavior, they could implement it themself
          by running libnm in their own GMainContext. Note that libnm might
          have bugs to make that really working, but that should be fixed
          instead of adding such synchrnous API behavior.
      
      Read also [1], for why blocking calls are wrong.
      
      [1] https://smcv.pseudorandom.co.uk/2008/11/nonblocking/
      
      So, all possible behaviors for synchronous API have severe behavioural
      issues.  Mark all this API as deprecated. Also, this serves the purpose of
      identifying blocking D-Bus calls in libnm.
      
      Note that "deprecated" here does not really mean that the API is going
      to be removed. We don't break API. The user may:
      
        - continue to use this API. It's deprecated, awkward and discouraged,
          but if it works, by all means use it.
      
        - use asynchronous API. That's the only sensible way to use D-Bus.
          If libnm lacks a certain asynchronous counterpart, it should be
          added.
      
        - use GDBusConnection directly. There really isn't anything wrong
          with D-Bus or GDBusConnection. This deprecated API is just a wrapper
          around g_dbus_connection_call_sync(). You may call it directly
          without feeling dirty.
      
      ---
      
      The only other remainging API is the synchronous GInitable call for
      NMClient. That is an entirely separate beast and not particularly
      wrong (from an API point of view).
      
      Note that synchronous API in NMSecretAgentOld, NMVpnPluginOld and
      NMVpnServicePlugin as not deprecated here. These types are not part
      of the D-Bus cache and while they have similar issues, it's less severe
      because they have less state.
      e90684a1
  5. 02 Oct, 2019 16 commits
  6. 01 Oct, 2019 15 commits