Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netplan: add support for WPA3-Enterprise (LP: #2029876) #402

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

daniloegea
Copy link
Collaborator

@daniloegea daniloegea commented Aug 22, 2023

These changes add two new EAP methods to Netplan: "eap-sha256" and "eap-suite-b-192". They are both used with WPA3-Enterprise.

We will implicitly set the pmf (Protected Management Frames) to either optional or required according to the method set by the user. It's currently "required" for sae and eap-suite-b-192 and optional for eap-sha256.

For reference, check "80211w" here https://w1.fi/cgit/hostap/tree/hostapd/hostapd.conf and here https://w1.fi/cgit/hostap/tree/wpa_supplicant/wpa_supplicant.conf and "pmf" here https://developer-old.gnome.org/NetworkManager/stable/nm-settings-nmcli.html

Description

Checklist

  • Runs make check successfully.
  • Retains 100% code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@daniloegea daniloegea added the schema change This PR includes a change to netplan's YAML schema, which needs sign-off label Aug 22, 2023
@slyon slyon changed the title netplan: add support for WPA3-Enterprise and PMF netplan: add support for WPA3-Enterprise and PMF (LP: #2029876) Aug 23, 2023
@slyon slyon mentioned this pull request Aug 23, 2023
5 tasks
Copy link
Collaborator

@slyon slyon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for providing all the relevant context! This is not yet a full review, but I'm providing some initial thoughts.

Schema wise, I feel like pmf is not a well-known abbreviation and protected_mgmt_frames doesn't feel very Netplan'y with the mgmt abbreviation. So we could consider spelling it out as protected-management-frames (or call it by its IEEE name, e.g. 802-11w).
But I'm a bit reluctant about introducing new YAML schema for this at all, but would prefer to make some opinionated decisions, setting PMF implicitly, depending on the key-management mode.

We have the simplified case of just providing a password. I feel like this should allow connecting to any WPA2-Personal or WPA3-Personal network by default (Transition Mode), therefore enable WPA-PSK & WPA-SAE in wpa_supplicant and make PMF optional:

network:
  version: 2
  wifis:
    wlp0s1:
      access-points:
        "network_ssid_name":
          password: "**********"

Similarly for WPA-Enterprise, if key-management: eap is selected in Netplan, we should enable WPA-EAP and WPA-EAP-SHA256 in wpa_supplicant and make PMF optional, to allow for WPA2 enterprise & WPA3 enterprise.

OTOH, if key-management: sae (or key-management: eap-suite-b-192) is given explicitly in the YAML, we might want to set PMF to "required", forcing WPA3-Personal (or WPA3 enterprise – I wonder if we can find a better name for "eap-suite-b-192"?).

Alternatively, we could make Netplan's key-management setting accept a list (in addition to accepting a scalar), taking any of the values and setting PMF to optional if any of the values provided doesn't work with PMF, keeping PMF required if all the given values support it. But that could lead to problems with the NetworkManager backend implementation, as that can consume only a single value, while wpa_supplicant can consume a list.

So we can keep the internal NETPLAN_AUTH_PMF implementation, but do not expose it to the YAML parser (for now).

PS: What about WPA-EAP-SUITE-B (128 bit) and WPA-PSK-SHA256? Do we want or need to support those key management modes?

PPS: IMO we don't need to think too hard about NetworkManager when writing wpa_supplicant configuration, as the wpa_supplicant settings are only used on the systemd-networkd renderer backend, while NM will come up with its own wpa_supplicant (or iwd) configuration, depending on the keyfile settings given.

src/parse-nm.c Show resolved Hide resolved
tests/generator/test_wifis.py Outdated Show resolved Hide resolved
src/abi.h Show resolved Hide resolved
@daniloegea
Copy link
Collaborator Author

Thanks, Lukas!

I also think it would be better to implicitly enable pmf for each case, but I thought it would be safer to set it manually to avoid issues due to how NM handles it.

I'll make some changes and test it a bit more here.

@daniloegea
Copy link
Collaborator Author

I followed my original plan and got rid of the new key.

@daniloegea daniloegea removed the schema change This PR includes a change to netplan's YAML schema, which needs sign-off label Aug 23, 2023
@daniloegea daniloegea marked this pull request as ready for review August 23, 2023 19:09
@daniloegea daniloegea changed the title netplan: add support for WPA3-Enterprise and PMF (LP: #2029876) netplan: add support for WPA3-Enterprise (LP: #2029876) Aug 23, 2023
Copy link
Collaborator

@slyon slyon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I like it better without introducing a new pmf YAML setting.

I left just one tiny nitpick inline, that we should fix before merging.

Also, I was wondering about making the simple case (WPA2/3-Personal) more simple, what's your thought on that? It could easily be part of a separate PR.

We have the simplified case of just providing a password. I feel like this should allow connecting to any WPA2-Personal or WPA3-Personal network by default (Transition Mode), therefore enable WPA-PSK & WPA-SAE in wpa_supplicant and make PMF optional:

network:
  version: 2
  wifis:
    wlp0s1:
      access-points:
        "network_ssid_name":
          password: "**********"

Also, do you have any opinion on this question?

PS: What about WPA-EAP-SUITE-B (128 bit) and WPA-PSK-SHA256? Do we want or need to support those key management modes?

src/nm.c Outdated Show resolved Hide resolved
src/parse-nm.c Show resolved Hide resolved
@daniloegea
Copy link
Collaborator Author

Thank you, Lukas. I like your idea. And it would be more aligned with Network Manager because apparently that's what it does. When only the password is provided we could use key_mgmt=WPA-PSK WPA-PSK-SHA256 SAE and set the pmf as optional. That sounds like a good idea. I'll work on that in a separate PR though, so I can unblock this task.

About the WPA-EAP-SUITE-B, I'm not completely sure in what scenario it's used. I'll do some research.

PMF (Protected Management Frames) is required by WPA3 and was
already implicitly set to "required" for WPA3-Personal (via SAE).

Network Manager will enable different EAP methods simultaneously when we
set it to "eap", such as WPA-EAP and WPA-EAP-SHA256. NM doesn't allow
the user to set the method as "eap-sha256" only.

These changes add two new EAP methods to Netplan: "eap-sha256" and
"eap-suite-b-192". They are both used with WPA3-Enterprise.

PMF is mandatory when using "eap-suite-b-192" so it's implicitly set to
"required". It's implicitly set to "optional" when eap-sha256 is used.
@daniloegea daniloegea merged commit ca230fc into canonical:main Aug 24, 2023
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants