[PATCH v2 09/33] FT: new rrb message format

[Jouni Malinen]

On Sat, Sep 24, 2016 at 10:53:50PM +0200, Michael Braun wrote:
> Convert FT RRB into new TLV based format.
> Encryption is unchanged.

This type of change should be made before doing any of the
non-compatible changes in the pull/push message formats. As such, I
dropped the previous not-yet-applied patches in the series from my
queue. It would probably be good to focus on the message format change
before resubmitting any of the other patches that depend on this.

> diff --git a/src/ap/wpa_auth.h b/src/ap/wpa_auth.h
> @@ -38,76 +38,63 @@ struct ft_rrb_frame {
>  /* Vendor-specific types for R0KH-R1KH protocol; not defined in 802.11r */
> -#define FT_PACKET_R0KH_R1KH_PULL 200
> -#define FT_PACKET_R0KH_R1KH_RESP 201
> -#define FT_PACKET_R0KH_R1KH_PUSH 202

> +/* 200, 201 und 202 are reserved by old protocol version */
> +#define FT_PACKET_R0KH_R1KH_PULL 203
> +#define FT_PACKET_R0KH_R1KH_RESP 204
> +#define FT_PACKET_R0KH_R1KH_PUSH 205

It makes sense to not use the old values, but it needs to be kept in
mind that the previous design was pretty bad misuse of the undefined
values and we should not try to continue using something very similar to
define the messages. The Ethertype 89-0d is managed by the IEEE 802.11
standard. Values 1-4 of the Payload Type field have been assigned so far
and all the other values (5-255) are reserved. We should not use them
for the new protocol messages here unless we can first somehow get IEEE
802.11 WG to assign those values for such uses.. And I'm not really sure
I would even try to ask for such allocation for this type of
implementation specific use case.

In other words, I think we need to find other ways of doing this either
by getting a properly assigned Ethertype (or subtype of one) or
alternatively building something on top of IP packets, etc., that would
not need a new Ethertype assignment.

> +/* new packet format */
> +
> +/* packet layout
> + *  | struct ft_rrb_frame | multiple of of struct ft_rrbv1_tlv |

Taken into account the comments above, this struct ft_rrb_frame part
would likely need to change to something else since it is not really
defined for this type of vendor specific use and while we could use the
exact same header definition in a completely different message, that may
not be the cleanest approach.

> + *  | padding for encryption (blocksize) | 8 byte extra for keywrap |
> + * where
> + *  packet_type = FT_PACKET_R0KH_R1KH*
> + *  action_length = size of all struct ft_rrbv1_tlv (without final padding)
> + *                  (aka plaintext length)
> + *  encryption: covers everything after struct ft_rrb_frame
> + */

The TLV part looks pretty reasonable, but I'd replace encryption to use
something quite different, e.g., with an AEAD cipher and additional
integrity protection for some of the unencrypted header fields.

-- 
Jouni Malinen                                            PGP id EFC895FA