Our reader/writer has flaws. We easily write out something that
is re-read differently. That is a problem and should be fixed.
Add API to re-read the connection after writing.
Extend the tests to check that the re-read value is identical
to what we wrote. In some cases, this does not hold. That is
usually a bug which needs fixing. Note that for certificate
blobs and paths we may intentionally mutate the connection
during writing, so there are valid cases where a connection
is re-read differently.
Avoid using new_settings when they are none. Also, don't shortcut when
the connection hasn't been changed -- let the settings plugin decide if
it needs to rewrite the connection.
Some keys, such as MASTER may still be different as they may depend on
other connections. svWriteFile() checks if the resulting file is
different already anyway.
The DNS configuration for VPN connections is associated to the VPN
device (tun, ppp, etc.) and that device can be unmanaged by NM: don't
ignore such configuration. We do the same for other DNS plugins.
https://bugzilla.gnome.org/show_bug.cgi?id=779087
The state-change of a device has a reason argument, which is mostly for information
only.
There are many places in code that are the source of a state-reason.
Mostly these are calls to:
- nm_device_state_changed()
- nm_device_queue_state()
- nm_device_queue_recheck_available()
- nm_device_set_unmanaged_by_*()
- nm_device_master_release_one_slave()
- nm_device_ip_method_failed()
- nm_modem_emit_prepare_result()
- nm_modem_emit_ppp_failed()
- nm_manager_deactivate_connection()
- NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_*);
However, there are a few places in code that look at the reason
to decide how to proceed. I think this is a bad pattern, because
cause and effect are decoupled and it gets hard to understand where
a certain reason is set and what consequences that has.
Add a nop-function nm_device_state_reason_check() to mark all uses
of the device state reason that derive decisions from it. That is,
highlight the "effect" part.
Don't reuse NMDeviceStateReason for the autoconnect-blocked-reason. There are
only two cases we care: blocked-due-to-no-secrets, blocked-otherwise.
Encode these values in a new enum type.
... instead of emitting the signal by name. For one,
we get the casting of the NMDeviceStateReason enum right.
Also, emitting by the guint signal-id is faster then
emitting by name.
The only reliable way of setting a MAC address for the team is through
the "hwaddr" property in the configuration passed to teamd. In order
to rewrite the configuration we need Jansson support; since it is
already a requirement for teamd, let the team plugin depend on it.
The out-reason is only set to NM_DEVICE_STATE_REASON_CONFIG_FAILED.
And there is only one caller who cares about the reason.
If we one day decide to return a more distinguished error reasons,
we can revert this patch. Until then, drop the code.
This argument is only relevant when the NMActStageReturn argument
indicates NM_ACT_STAGE_RETURN_FAILURE. In all other cases it is ignored.
Rename the argument to make the meaning clearer. The argument is passed
through several layers of code, it isn't obvious that this argument only
matters for the failure case. Also, the distinct name makes it easier
to distinguish from other uses of the "reason" name.
While at it, do some drive-by cleanup:
- use g_return_*() instead of g_assert() to have a more graceful
assertion.
- functions like dhcp4_start() don't need to return a failure reason.
Most callers don't care, and the caller who does can determine the
proper reason.
- allow omitting the out-argument via NM_SET_OUT().
Add support for creating dummy devices. This commit adds a D-Bus
interface 'org.freedesktop.NetworkManager.Device.Dummy' which is used
primarily for determining the device type but does not carry any
properties.
Add svGetValue_cp() and svGetValueStr() for completeness.
Currently, we mostly use svGetValueStr_cp(), which I think is wrong
because for most cases we should instead not ignore empty values -- that
is, svGetValue_cp() would be a better choice.
Also, I think that the non *_cp() API should be preferred in many cases
because it avoids cloning the value in many cases. The API is not
necessarily less favorable either:
gs_free char *value = NULL;
value = svGetValue_cp (s, key);
if (value)
...
vs.
gs_free char *value_to_free = NULL;
const char *value;
value = svGetValue (s, key, &value_to_free);
if (value)
...
Add the two missing variants, so that future code can use what fits
best, not following undesired practices because seemingly there is
no alternative.
We have
- svGetValue()
- returns the original string
- avoids copying the string unless necessary
- svGetValueStr_cp() (formerly svGetValueString())
- returns the original string, unless it is empty ""
- always clones the string
I think the behavior svGetValueStr*() of coercing "" to NULL is wrongly
used in most places. We should better handle "" like any other value,
not treat it as unset.
That would require another function svGetValue_cp(), which is like svGetValue()
but always copies the string. Rename svGetValueString() so that there is a place
for names like
- svGetValue_cp()
- svGetValueStr()
Also rename svSetValueString() to svSetValueStr().
Ifcfg reader now properly handles escaping and quoting. We don't
need to stip whitespace, if somebody explicitly configures
prop=" value"
it is a configuration error.
NMDeviceEthernet and NMDeviceMacsec implement their own retry policy
for connection using 802.1X, and consider the credentials wrong when
the authentication fails for 3 times. In such case, they also disable
autoconnection for the device by setting the state reason NO_SECRETS.
This means that it's not possible at the moment to choose how many
times the authentication will be retried since they don't use the
standard reconnection logic.
Change NMDeviceEthernet and NMDeviceMacsec to use the number of
retries from connection.autoconnect-retries instead of a hardcoded
value to decide how many times the authentication must be restarted.
Use the per-connection authentication timeout for 802.1X Ethernet,
MACsec and Wi-Fi connections. In case the value is not defined, fall
back to the global one.
The wpa_supplicant API supports to enable/disable each TLS version
individually, or leave it at the default. Currently, the default
means to enable a TLS version, thus, the only meaningful option
for the momemnt means to explicitly disable it.
In the future, supplicant may disable options by default, and
the inverse option can become interesting to configure
"tls_disable_tlsv1_0=0". When that happens, we can solve it by
adding another flag NM_SETTING_802_1X_AUTH_FLAGS_TLS_1_0_ENABLE.
Change the previous behavior of the NMSetting8021xAuthFlags.
Previously, when not specifying TLS_DISABLE_DEFAULT, all
options were unspecified. On the other hand, when specifying
a single TLS disable flag, all versions were explicitly enabled
or disabled.
Instead, change the meaning of the disable flags. When present,
it explicitly disables an option. But it does not explicitly enable
it.
Some AAA servers have issues interoperating with select TLS versions,
which wpa_supplicant negotiates by default.
This commit allows disabling troubling versions of TLS so that
connecting to broken authentication servers could be possible.
We log updates of the Wi-Fi AP with a separate logging domain LOGD_WIFI_SCAN.
However, there is ony "update" message that is triggered every 6 seconds, which
becomes especially verbose.
Suppress this one and only log it when compiled --with-more-logging. And then
only log with level LOGL_TRACE, so the user still can filter this one out.
