mbox series

[RFC,0/2] AP: make wpa_psk_file more dynamic

Message ID 20181121214205.443-1-kazikcz@gmail.com
Headers show
Series AP: make wpa_psk_file more dynamic | expand

Message

Michał Kazior Nov. 21, 2018, 9:42 p.m. UTC
From: Michal Kazior <michal@plume.com>

This has been prompted by the thread on the mailing list
"dynamically added/removed PSKs without MAC pairing".

I've originally did these patches for an older hostapd
tree quite some time ago. For various reasons I didn't
upstream them until now.

I've had to rebase and rework them a bit but hopefully I
didn't mess anything up.

I guess it's a little iffy to expose PMK in cli/logs. I was
considering using explicit tags/aliases in wpa_psk_file in
the format of `tag mac psk`, e.g.

 tag_1 00:00:00:00:00:00 secretpassword
 tag_2 00:00:00:00:00:00 different111

But asked myself if it's really more secure or is it just
unnecessarily complex. FWIW The tag could be made optional
so old wpa_psk_file format would remain working with no
changes. Thoughts?


Michal Kazior (2):
  AP: keep track and expose WPA-PSK PMK info of each station
  AP: add wpa_psk_file reloading in runtime

 hostapd/ctrl_iface.c  | 61 +++++++++++++++++++++++++++++++++++++++++++
 hostapd/hostapd_cli.c |  9 +++++++
 src/ap/sta_info.c     | 28 ++++++++++++++++++++
 src/ap/wpa_auth.c     | 18 +++++++++++++
 src/ap/wpa_auth.h     |  1 +
 src/ap/wpa_auth_ft.c  |  2 ++
 src/common/wpa_ctrl.h |  1 +
 7 files changed, 120 insertions(+)

Comments

Carlito Nueno Nov. 28, 2018, 9:57 p.m. UTC | #1
Hi Michał,

Thanks! I will give it a try and report back.
On Wed, Nov 21, 2018 at 1:43 PM Michal Kazior <kazikcz@gmail.com> wrote:
>
> From: Michal Kazior <michal@plume.com>
>
> This has been prompted by the thread on the mailing list
> "dynamically added/removed PSKs without MAC pairing".
>
> I've originally did these patches for an older hostapd
> tree quite some time ago. For various reasons I didn't
> upstream them until now.
>
> I've had to rebase and rework them a bit but hopefully I
> didn't mess anything up.
>
> I guess it's a little iffy to expose PMK in cli/logs. I was
> considering using explicit tags/aliases in wpa_psk_file in
> the format of `tag mac psk`, e.g.
>
>  tag_1 00:00:00:00:00:00 secretpassword
>  tag_2 00:00:00:00:00:00 different111
>
> But asked myself if it's really more secure or is it just
> unnecessarily complex. FWIW The tag could be made optional
> so old wpa_psk_file format would remain working with no
> changes. Thoughts?
>
>
> Michal Kazior (2):
>   AP: keep track and expose WPA-PSK PMK info of each station
>   AP: add wpa_psk_file reloading in runtime
>
>  hostapd/ctrl_iface.c  | 61 +++++++++++++++++++++++++++++++++++++++++++
>  hostapd/hostapd_cli.c |  9 +++++++
>  src/ap/sta_info.c     | 28 ++++++++++++++++++++
>  src/ap/wpa_auth.c     | 18 +++++++++++++
>  src/ap/wpa_auth.h     |  1 +
>  src/ap/wpa_auth_ft.c  |  2 ++
>  src/common/wpa_ctrl.h |  1 +
>  7 files changed, 120 insertions(+)
>
> --
> 2.19.1
>
Jouni Malinen Jan. 4, 2019, 10:59 p.m. UTC | #2
On Wed, Nov 21, 2018 at 10:42:03PM +0100, Michal Kazior wrote:
> This has been prompted by the thread on the mailing list
> "dynamically added/removed PSKs without MAC pairing".

> I guess it's a little iffy to expose PMK in cli/logs. I was
> considering using explicit tags/aliases in wpa_psk_file in
> the format of `tag mac psk`, e.g.
> 
>  tag_1 00:00:00:00:00:00 secretpassword
>  tag_2 00:00:00:00:00:00 different111
> 
> But asked myself if it's really more secure or is it just
> unnecessarily complex. FWIW The tag could be made optional
> so old wpa_psk_file format would remain working with no
> changes. Thoughts?

I don't think we should be exposing the PSK/PMK/passphrase over the
control interface (or accidentally in any debug log dumping control
interface commands/events). The use case here does not seem to have any
use for the particular PSK value, i.e., it depends on only knowing some
identifier for the particular key. As such, I'd go with a key
identifier. That could be added as an optional part before the MAC
address to maintain backwards compatibility of the file format (with the
old no-identifier versions not getting the new events/exposure through
STA command). E.g., "keyid=foo 02:00:00:00:00:00 password".

>   AP: keep track and expose WPA-PSK PMK info of each station

As noted above, I'd like to see changes in this design.

>   AP: add wpa_psk_file reloading in runtime

This makes mostly sense, but this could get more complex if the key
identifier is added and it is used for grouping devices. In such a case,
moving a device from one group (keyid) to another could be a dynamic
change that occurs when reloading the file. It might make most sense to
disconnect the station in such a case if the keyid ("group") changes
even though when the PSK/passphrase would still remain in the file.

The use case I'm mostly thinking about here is an extension of this to
allow VLAN binding based on the "keyid", e.g., in the form of VLAN ID or
a netdev ifname.
Michał Kazior Jan. 7, 2019, 9:01 a.m. UTC | #3
On Fri, 4 Jan 2019 at 23:59, Jouni Malinen <j@w1.fi> wrote:
>
> On Wed, Nov 21, 2018 at 10:42:03PM +0100, Michal Kazior wrote:
> > This has been prompted by the thread on the mailing list
> > "dynamically added/removed PSKs without MAC pairing".
>
> > I guess it's a little iffy to expose PMK in cli/logs. I was
> > considering using explicit tags/aliases in wpa_psk_file in
> > the format of `tag mac psk`, e.g.
> >
> >  tag_1 00:00:00:00:00:00 secretpassword
> >  tag_2 00:00:00:00:00:00 different111
> >
> > But asked myself if it's really more secure or is it just
> > unnecessarily complex. FWIW The tag could be made optional
> > so old wpa_psk_file format would remain working with no
> > changes. Thoughts?
>
> I don't think we should be exposing the PSK/PMK/passphrase over the
> control interface (or accidentally in any debug log dumping control
> interface commands/events). The use case here does not seem to have any
> use for the particular PSK value, i.e., it depends on only knowing some
> identifier for the particular key. As such, I'd go with a key
> identifier. That could be added as an optional part before the MAC
> address to maintain backwards compatibility of the file format (with the
> old no-identifier versions not getting the new events/exposure through
> STA command). E.g., "keyid=foo 02:00:00:00:00:00 password".

I don't really have a strong case for using PMK other than its just
convenient and straightforward.

I'm mostly okay with a key identifier though. I'll rework it and re-spin.

Does space-separated key=value list (with = being forbidden in
key/value strings) sound good? Or do you see the need for escaping to
allow expressing these characters? And if so, do you have any
preference about for escaping logic?


> >   AP: keep track and expose WPA-PSK PMK info of each station
>
> As noted above, I'd like to see changes in this design.
>
> >   AP: add wpa_psk_file reloading in runtime
>
> This makes mostly sense, but this could get more complex if the key
> identifier is added and it is used for grouping devices. In such a case,
> moving a device from one group (keyid) to another could be a dynamic
> change that occurs when reloading the file. It might make most sense to
> disconnect the station in such a case if the keyid ("group") changes
> even though when the PSK/passphrase would still remain in the file.
>
> The use case I'm mostly thinking about here is an extension of this to
> allow VLAN binding based on the "keyid", e.g., in the form of VLAN ID or
> a netdev ifname.

You mean to have something like `keyid=pwd0 vlan_ifname=wlan0sta1
vlan_bridge=br2 00:11:22:33:44:55 secretpassphrase`? That would
technically make sense even without wpa, wouldn't it? It sounds like a
more generic attribute list with flexible matching. Maybe kind of like
udev rules. Not sure if it's worth making it that complex though.


Michał
Jouni Malinen Jan. 8, 2019, 11:11 a.m. UTC | #4
On Mon, Jan 07, 2019 at 10:01:44AM +0100, Michał Kazior wrote:
> I don't really have a strong case for using PMK other than its just
> convenient and straightforward.
> 
> I'm mostly okay with a key identifier though. I'll rework it and re-spin.

Thanks.

> Does space-separated key=value list (with = being forbidden in
> key/value strings) sound good? Or do you see the need for escaping to
> allow expressing these characters? And if so, do you have any
> preference about for escaping logic?

I think it's fine to keep this simpler and not require escaping some
characters.

> > The use case I'm mostly thinking about here is an extension of this to
> > allow VLAN binding based on the "keyid", e.g., in the form of VLAN ID or
> > a netdev ifname.
> 
> You mean to have something like `keyid=pwd0 vlan_ifname=wlan0sta1
> vlan_bridge=br2 00:11:22:33:44:55 secretpassphrase`? That would
> technically make sense even without wpa, wouldn't it? It sounds like a
> more generic attribute list with flexible matching. Maybe kind of like
> udev rules. Not sure if it's worth making it that complex though.

I'm not sure I thought about hostapd handling that internally, but
rather, having some external component look for the events and assign
parameters to STA filtering/routing/etc. based on the "keyid" values
which might actually be shared by multiple keys, so it is would be more
of a group rather than an identifier of a specific key. No need to make
this any more complex than the currently identified real use cases
require, though. vlan ifname/bridge is already available through other
means, so this should probably not try to override those with a
completely different mechanism (if someone is interested in extending
hostapd to do these VLAN configurations internally based on the used
PSK or "keyid" group).