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

nixos/hostapd: Disable insecure TKIP, enable 802.11n/ac, usability #91188

Open
wants to merge 3 commits into
base: master
from

Conversation

@gloaming
Copy link
Contributor

gloaming commented Jun 20, 2020

Motivation for this change
  • cfg.interface needs to be specified, so give an error that says so. Leaving the option blank leads to a confusing error about systemd unit dependencies:
Jun 19 20:47:57 samwise systemd[1]: Dependency failed for hostapd wireless AP.
Jun 19 20:47:57 samwise systemd[1]: hostapd.service: Job hostapd.service/start failed with result 'dependency'.
  • Secure default (no TKIP). It's possible that setting wpa=2 disables TKIP anyway, but on my machine I could not get 802.11n to work without setting rsn_pairwise=CCMP.
    N.B. rsn_pairwise takes its default from wpa_pairwise.
  • I don't see any reason not to enable PHY n/ac by default, as hostapd will still provide backwards-compatible service (I actually tested this with an old 802.11g adapter).

In summary, it's now much easier to set up a fast and secure AP. Note that iwlwifi's 5Ghz support on Intel radio chips is mostly nonexistent.

Things done

Tested against nixos-unstable 9480bae; 802.11n works at 80Mbit as expected.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
gloaming added 3 commits Jun 20, 2020
Leaving this blank leads to a confusing error about systemd unit dependencies.
It's possible that setting wpa=2 disables TKIP anyway, but on my machine
I could not get 802.11n to work without setting rsn_pairwise=CCMP.
N.B. rsn_pairwise takes its default from wpa_pairwise.
@gloaming
Copy link
Contributor Author

gloaming commented Jun 20, 2020

@mweinelt
Copy link
Member

mweinelt commented Jun 20, 2020

Secure default (no TKIP). It's possible that setting wpa=2 disables TKIP anyway, but on my machine I could not get 802.11n to work without setting rsn_pairwise=CCMP.

WFM. This is also compatible with (mixed and pure) WPA3-PSK setups.

I don't see any reason not to enable PHY n/ac by default, as hostapd will still provide backwards-compatible service (I actually tested this with an old 802.11g adapter).

On the condition that this is a backwards compatible change, which is apparently what you tested already.

@aanderse
Copy link
Contributor

aanderse commented Jun 20, 2020

I would really recommend adding a settings option here in addition to what you have to avoid hard coding options. #89662 is a recent example of what I'm talking about. See RFC 42 for more details.

@gloaming
Copy link
Contributor Author

gloaming commented Jun 22, 2020

To clarify, I tested that with n/ac enabled, a g-only client was able to access the network as one would expect. I realise now that there's also the question of whether hostapd will tolerate these settings if the AP hardware doesn't support them. I haven't (yet) tested this.

@aanderse well, in this case hostapd will accept options multiple times and take the latest value. But yes, that looks nicer. Can you point me to a canonical example of how to deprecate extraConfig?

If we find there is backwards-incompatibility:
If we're going to change the option structure, we should probably also consider the question of whether we should just provide a better interface, e.g. a phy option [a, b, g, n, ac] and set hw_mode depending on channel. That way we do not require the user to understand hostapd.conf.

If we find there is not:
We might as well remove the hwMode option and leave it to settings.

@mweinelt
Copy link
Member

mweinelt commented Jun 22, 2020

I realise now that there's also the question of whether hostapd will tolerate these settings if the AP hardware doesn't support them. I haven't (yet) tested this.

Yep, that is the corner case I'm most worried about here. I currently imagine it checks for the capability and dies. I'd be happy to be proven wrong though.

If we're going to change the option structure, we should probably also consider the question of whether we should just provide a better interface, e.g. a phy option [a, b, g, n, ac] and set hw_mode depending on channel. That way we do not require the user to understand hostapd.conf.

Indeed, thats an important undertaking for getting real hostapd usage under way. It was already started here: #49171

@aanderse
Copy link
Contributor

aanderse commented Jun 22, 2020

@gloaming if you feel confident that you write a settings option that covers every possibility in hostapd.conf then you can simply remove the extraConfig option with mkRemovedOptionModule. If you aren't confident that your settings can cover every edge case (yet) then taking the approach of keeping extraConfig around for a release, marking it as deprecated, will allow using to report any configuration they can't produce with settings but could with extraConfig.

See #73872 for a nice example of deprecating. At this point @filalex77 might consider making a simple PR removing the services.hardware.bluetooth.extraConfig as their original change was over a release ago and I haven't heard any reported issues over it.

To directly answer your question, though, the above mentioned RFC is currently the main discussion point around introducing settings options and will eventually introduce official process/documentation in the NixOS manual.

@lheckemann
Copy link
Member

lheckemann commented Jun 26, 2020

I can test this on an older device that has no 802.11ac.

EDIT: Never mind, the driver doesn't support AP mode at all.

@flokli flokli requested review from mweinelt and NinjaTrappeur Jul 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.