Re: [PATCH v3 05/13] netdev: support extended key IDs

* Re: [PATCH v3 05/13] netdev: support extended key IDs
@ 2021-10-08 14:11 Denis Kenzior
  0 siblings, 0 replies; 2+ messages in thread
From: Denis Kenzior @ 2021-10-08 14:11 UTC (permalink / raw)
  To: iwd

[-- Attachment #1: Type: text/plain, Size: 4050 bytes --]

Hi James,

On 10/7/21 3:49 PM, James Prestwood wrote:
> This implements the new handshake callback for setting a TK with
> an extended key ID. The procedure is different from legacy zero
> index TKs.
> 
> First the new TK is set as RX only. Then message 4 should be sent
> out (so it uses the existing TK). This poses a slight issue with
> PAE sockets since message order is not guaranteed. In this case
> the 4th message is stored and sent after the new TK is installed.
> Then the new TK is modified using SET_KEY to both send and
> receive.
> 
> In the case of control port over NL80211 the above can be avoided
> and we can simply install the new key, send message 4, and modify
> the TK as TX + RX all in sequence, without waiting for any callbacks.
> ---
>   src/netdev.c | 401 ++++++++++++++++++++++++++++++++++++++-------------
>   1 file changed, 297 insertions(+), 104 deletions(-)
> 

<snip>

> @@ -1823,7 +2047,8 @@ static void netdev_set_tk(struct handshake_state *hs,
>   		goto invalid_key;
>   
>   	msg = netdev_build_cmd_new_key_pairwise(netdev, cipher, addr, tk_buf,
> -						crypto_cipher_key_len(cipher));
> +						crypto_cipher_key_len(cipher),
> +						hs->active_tk_index);

This seems inconsistent?  You pass the key_idx in the ext_tk callback, but not 
here.  Bit of a pain to change the signature, but it may be nicer to just pass 
the desired index into this callback.

>   	nhs->pairwise_new_key_cmd_id =
>   		l_genl_family_send(nl80211, msg, netdev_new_pairwise_key_cb,
>   						nhs, NULL);
> @@ -1836,6 +2061,74 @@ invalid_key:
>   	netdev_setting_keys_failed(nhs, err);
>   }
>   
> +static void netdev_set_ext_tk(struct handshake_state *hs, uint8_t key_idx,
> +				const uint8_t *tk, uint32_t cipher,
> +				const struct eapol_frame *step4, uint16_t proto,
> +				bool noencrypt)
> +{
> +	struct netdev_handshake_state *nhs =
> +		l_container_of(hs, struct netdev_handshake_state, super);
> +	uint8_t tk_buf[32];
> +	struct netdev *netdev = nhs->netdev;
> +	struct l_genl_msg *msg;
> +	const uint8_t *addr = netdev_choose_key_address(nhs);
> +	int err;
> +	size_t frame_size = sizeof(struct eapol_header) +
> +				L_BE16_TO_CPU(step4->header.packet_len);
> +
> +	err = -ENOENT;
> +	if (!netdev_copy_tk(tk_buf, tk, cipher, false))
> +		goto error;
> +
> +	msg = netdev_build_cmd_new_rx_key_pairwise(netdev, cipher, addr, tk_buf,
> +						crypto_cipher_key_len(cipher),
> +						hs->active_tk_index);
> +	nhs->pairwise_new_key_cmd_id =
> +		l_genl_family_send(nl80211, msg, netdev_new_rx_pairwise_key_cb,
> +						nhs, NULL);
> +
> +	if (!nhs->pairwise_new_key_cmd_id)
> +		goto io_error;
> +
> +	/*
> +	 * Without control port we cannot guarantee the order that messages go
> +	 * out and must wait for NEW_KEY to call back before sending message 4
> +	 */
> +	if (!wiphy_control_port_enabled(netdev->wiphy)) {

nit: lets just use netdev->pae_over_nl80211 for consistency.

> +		netdev->ext_key_info = l_malloc(
> +					sizeof(struct netdev_ext_key_info) +
> +					frame_size);
> +		memcpy(netdev->ext_key_info->frame, step4, frame_size);
> +		netdev->ext_key_info->proto = proto;
> +		netdev->ext_key_info->noencrypt = noencrypt;
> +		return;
> +	}
> +
> +	/*
> +	 * Otherwise, order of messages will be guaranteed. Therefore we can
> +	 * send send message 4, and set the TK to TX (below) without waiting for
> +	 * any callbacks
> +	 */
> +	err = netdev_control_port_frame(netdev->index, netdev->handshake->aa,
> +					proto, step4, noencrypt, NULL);
> +	if (err < 0)
> +		goto error;
> +
> +	/* Then toggle to RX + TX */
> +	err = netdev_set_key_tx(netdev);
> +	if (err < 0)
> +		goto error;
> +
> +	return;
> +
> +io_error:
> +	err = -EIO;
> +	l_genl_msg_unref(msg);
> +
> +error:
> +	netdev_setting_keys_failed(nhs, err);
> +}
> +
>   static void netdev_set_pmk_cb(struct l_genl_msg *msg, void *user_data)
>   {
>   	struct netdev_handshake_state *nhs = user_data;

<snip>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 2+ messages in thread