Re: [PATCH] wifi: enable WPA-*-SHA256 AKMs only when the supplicant supports them

[Thomas Haller <thaller redhat com>]

On Wed, 2018-01-17 at 18:06 +0100, Beniamino Galvani wrote:
> Commit 87ec5e90fe79 ("supplicant: set key_mgmt independent of pmf
> value") enabled WPA-PSK-SHA256 or WPA-EAP-SHA256 even when the
> supplicant didn't support them, potentially causing connection
> failures.  Instead, use the 'pmf' capability to detect when they can
> be enabled.
>
> Fixes: 87ec5e90fe79fcb2ac315cf1604e757dcab60bb9
> ---
>
> Hi,
>
> this patch fixes regressions discovered after the merge of FILS
> patches by our nightly CI that runs on CentOS where wpa_supplicant is
> compiled without 802.11w support.
>
> This patch fixes those regression. Comments welcome.
>
> Thanks,
> Beniamino
>
>
>  src/devices/nm-device-ethernet.c              |  2 +-
>  src/devices/nm-device-macsec.c                |  2 +-
>  src/devices/wifi/nm-device-wifi.c             | 42 ++---------------
> ---
>  src/supplicant/nm-supplicant-config.c         | 55
> +++++++++++++++++++++++----

> diff --git a/src/supplicant/nm-supplicant-config.c
> b/src/supplicant/nm-supplicant-config.c
> index a2502bd7b..14f5cac82 100644
> --- a/src/supplicant/nm-supplicant-config.c
> +++ b/src/supplicant/nm-supplicant-config.c
> @@ -47,6 +47,8 @@ typedef struct {
>       guint32    ap_scan;
>       gboolean   fast_required;
>       gboolean   dispose_has_run;
> +     gboolean   support_pmf;
> +     gboolean   support_fils;

inside structs, could we use bool:1 bitfields?




>  NMSupplicantConfig *
> -nm_supplicant_config_new (void)
> +nm_supplicant_config_new (gboolean support_pmf, gboolean
> support_fils)
>  {
> -     return g_object_new (NM_TYPE_SUPPLICANT_CONFIG, NULL);
> +     NMSupplicantConfigPrivate *priv;
> +     NMSupplicantConfig *self;
> +
> +     self = g_object_new (NM_TYPE_SUPPLICANT_CONFIG, NULL);
> +     priv = NM_SUPPLICANT_CONFIG_GET_PRIVATE (self);
> +
> +     priv->support_pmf = support_pmf;
> +     priv->support_fils = support_fils;

I always find it nice if nm_*_new() only calls g_object_new() and
nothing else. But that would requie to introduce G_PARAM_CONSTRUCT_ONLY
properties, to pass in the parameters. Clearly that is more hazzle then
it's worth, so ok.



Rest lgtm.
Thomas

Attachment: signature.asc
Description: This is a digitally signed message part