Re: [PATCH 1/2] Revert "mac80211: support NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211_MAC_ADDRS"

[Markus Theil <markus.theil@tu-ilmenau.de>]

On 25.02.20 18:06, Denis Kenzior wrote:
> Hi Jouni,
>
> On 2/25/20 5:00 AM, Jouni Malinen wrote:
>> On Mon, Feb 24, 2020 at 01:35:51PM -0600, Denis Kenzior wrote:
>>> But it seems like the benefits outweigh the drawbacks?  At least we
>>> have
>>> been super happy with how control port works for us.  If you take the
>>> pre-auth path away, I'm really not sure there's any point in (at
>>> least for
>>> us) keeping support for the control port path.
>>
>> Do you use the control port for RX only or both TX and RX? The RX side
>
> Both.  For reasons already discussed.
>
>> is mostly harmless _if_ something filters unprotected RSN
>> pre-authentication frames that are received between the association and
>> the completion of 4-way handshake. That something would either need to
>> be the specific user space application using the interface or
>> potentially mac80211 with some special rules that are different between
>> EAPOL and RSN pre-authentication ethertypes.
>
> For mac80211, pre-authentication frames are already filtered, or at
> least that is the intent.  See ieee80211_frame_allowed().  Only
> control_port_protocol packets are allowed through if the station is
> not yet authorized.  Under normal circumstances that would only be
> EAPoL packets (or esoteric protocols like WAPI).  Pre-auth frames
> would be filtered.
>
> Furthermore, only the userspace daemon that initiated the association
> would get to see the packets flowing via
> NL80211_CMD_CONTROL_PORT_FRAME (by providing SOCKET_OWNER attribute). 
> In iwd, we would drop any pre-auth packets without an initiated
> session on the floor.  And we don't initiate pre-auth sessions until
> we are fully associated/authenticated/authorized.
>
> Last I checked, mac80211 is the only driver that supports this control
> port mechanism.  If other drivers obtain this support, then perhaps
> stricter checking of the packets flowing via
> cfg80211_rx_control_port() would be a good idea.  However, I'm not
> sure how much can be done here due to nl80211 being stateless.
>
>>
>> For TX, the control port path will likely result in more problematic
>> issues. I'd expect drivers to use higher priority and/or higher
>> reliability for delivering the frames. That is justifiable for EAPOL
>
> Fair enough, but the driver is notified of the protocol being sent in
> tx_control_port().  So it can easily choose not to prioritize pre-auth
> packets.
>
>> frames, but unnecessary for RSN pre-authentication frames. Being able to
>> bypass the port authorization control would be undesired from security
>> view point.
>
> TX_CONTROL_PORT does require administrative access for the caller. 
> The userspace management daemon is already trusted to do a lot of
> things by the kernel (including cleaning up things inside the kernel
> itself in certain cases).  If the userspace daemon cannot be trusted
> to send control port frames at the appropriate time, then the
> 'undesired from security view point' argument would apply quite
> broadly across the entire nl80211 API.  In fact, even the
> NL80211_ATTR_CONTROL_PORT_ETHERTYPE is implicitly trusted to be
> provided correctly by userspace.
>
> From a practical perspective, if you're worried about this, then
> perhaps stricter checking in nl80211_tx_control_port() is warranted.
> But that really implies peeking into the frames being sent...
>
> Another related issue is that NL80211_CMD_TX_CONTROL_PORT (amongst
> others) doesn't actually check whether the command is being called by
> the $SOCKET_OWNER process for the target netdev.  Realistically, only
> the $SOCKET_OWNER process has the necessary secrets to generate
> packets that go via this path.  I've submitted some RFC patches to
> lock this down, but they were rejected.
>
>>
>> The key point for me here is the concept of authorized/unauthorized port
>> for normal Data frames based on the IEEE 802.1X standard. Only the
>> frames critical for the authentication service (establishing protected
>> link with the current access point) are allowed to be transmitted and
>> received while the port used for normal data is unauthorized. For IEEE
>> 802.11, only the EAPOL frames are such Data frames that are needed
>> before the port can be authorized. RSN pre-authentication frames are
>> used to establish a new security association with a different access
>> point once the port with the current AP is authorized. As such, RSN
>> pre-authentication frames do not need to go through any special path
>> from the protocol view point and in fact, it would be incorrect to allow
>> them to be transmitted or received before the main port has been
>> authorized.
>>   The IEEE 802.11 standard describes this with "communication of all
>> non-IEEE-802.1X MSDUs sent or received" being authorized/not-authorized.
>> MSDU is a reference to Data frames and "non-IEEE-802.1X" in this context
>> to any ethertype other than the one defined in IEEE 802.1X (EAPOL).
>>
>> As a more specific example, the EAPOL frames are expected to be
>> transmitted unencrypted during the initial 4-way handshake (and with
>> some old IEEE 802.1X/WEP designs and some WPA(v1) implementation, even
>> during rekeying). On the other hand, RSN pre-authentication frames are
>> never supposed to go out unencrypted over the air (i.e., they must not
>> be sent or received before the encryption key has been established for
>> the link). The IEEE 802.11 standard describes this with "A STA shall not
>> use preauthentication except when pairwise keys are employed" and "As
>> preauthentication frames do not use the IEEE 802.1X EAPOL EtherType
>> field, the AP with which the STA is currently associated need not apply
>> any special handling. The AP and the MAC in the STA shall handle these
>> frames in the same way as other frames with arbitrary EtherType field
>> values that require distribution via the DS."
>
> No disagreement with anything you said here...
>
>>
>> I understand that there is a different view point for this from the
>> kernel--user space interface side and it may indeed look more
>> convenient to use the same path for both EAPOL and RSN
>> pre-authentication frames from that view point. If that mechanism is
>> used, it needs to be understood that the rules for EAPOL and RSN
>> pre-authentication frames are different, though, and it is not clear
>> where that difference is going to be enforced if the same interface path
>> is used.
>>
>
> I understand the purity argument you're making here, and I don't
> disagree with it.  Perhaps the naming of the commands, with
> CONTROL_PORT inside them, is unfortunate, but it seemed like a good
> idea at the time.  It is also unfortunate that pre-authentication was
> designed the way it was.  It is turning out to be somewhat of an
> anomaly that we nevertheless have to deal with.
>
> I do indeed view this through a different lens.  Only the wifi
> management daemon can realistically generate any frames that flow
> through the control port, including pre-auth frames.  So I think it
> makes very good sense to provide an optimized path for the userspace
> daemon to send/receive these frames and not force it to create sockets
> and use up resources needlessly.
>
> I also don't mind if RX forwarding of pre-auth frames via the control
> port is made optional.  What I would not want to happen is for this
> capability to be removed completely. 
Is there any consensus on how to move on in this discussion? In my
opinion a pragmatic way would be to make rx forwarding of pre-auth
frames optional with a flag which can be set whenever
NL80211_ATTR_CONTROL_PORT_OVER_NL80211 is included, like Denis has
already proposed. We could call this flag for example
NL80211_ATTR_CONTROL_PORT_NO_PREAUTH.