Messages in this thread | | | Date | Tue, 23 Aug 2022 17:45:34 +0300 | Subject | Re: [PATCH 08/31] net/tcp: Introduce TCP_AO setsockopt()s | From | Leonard Crestez <> |
| |
On 8/18/22 19:59, Dmitry Safonov wrote: > Add 3 setsockopt()s: > 1. to add a new Master Key Tuple (MKT) on a socket > 2. to delete present MKT from a socket > 3. to change flags of an MKT > > Userspace has to introduce keys on every socket it wants to use TCP-AO > option on, similarly to TCP_MD5SIG/TCP_MD5SIG_EXT. > RFC5925 prohibits definition of MKTs that would match the same peer, > so do sanity checks on the data provided by userspace. Be as > conservative as possible, including refusal of defining MKT on > an established connection with no AO, removing the key in-use and etc. > > (1) and (2) are to be used by userspace key manager to add/remove keys. > (3) main purpose is to set rnext_key, which (as prescribed by RFC5925) > is the key id that will be requested in TCP-AO header from the peer to > sign their segments with. > > At this moment the life of ao_info ends in tcp_v4_destroy_sock().
> +#define TCP_AO 38 /* (Add/Set MKT) */ > +#define TCP_AO_DEL 39 /* (Delete MKT) */ > +#define TCP_AO_MOD 40 /* (Modify MKT) */
The TCP_AO_MOD sockopt doesn't actually modify and MKT, it only controls per-socket properties. It is equivalent to my TCP_AUTHOPT sockopt while TCP_AO is equivalent to TCP_AUTHOPT_KEY. My equivalent of TCP_AO_DEL sockopt is a flag inside tcp_authopt_key.
> +struct tcp_ao { /* setsockopt(TCP_AO) */ > + struct __kernel_sockaddr_storage tcpa_addr; > + char tcpa_alg_name[64]; > + __u16 tcpa_flags;
This field accept TCP_AO_CMDF_CURR and TCP_AO_CMDF_NEXT which means that you are combining key addition with key selection. Not clear it shouldn't just always be a separate sockopt?
> + __u8 tcpa_prefix; > + __u8 tcpa_sndid; > + __u8 tcpa_rcvid; > + __u8 tcpa_maclen; > + __u8 tcpa_keyflags; > + __u8 tcpa_keylen; > + __u8 tcpa_key[TCP_AO_MAXKEYLEN]; > +} __attribute__((aligned(8))); > + > +struct tcp_ao_del { /* setsockopt(TCP_AO_DEL) */ > + struct __kernel_sockaddr_storage tcpa_addr; > + __u16 tcpa_flags; > + __u8 tcpa_prefix; > + __u8 tcpa_sndid; > + __u8 tcpa_rcvid; > + __u8 tcpa_current; > + __u8 tcpa_rnext; > +} __attribute__((aligned(8))); > + > +struct tcp_ao_mod { /* setsockopt(TCP_AO_MOD) */ > + __u16 tcpa_flags; > + __u8 tcpa_current; > + __u8 tcpa_rnext; > +} __attribute__((aligned(8)));
This is quite similar to my "struct tcp_authopt" in the fact that it is intented to support controlling the "current keys".
* tcpa_current is equivalent to send_keyid * tcpa_rnext is equivalent to send_rnextkeyid
I also have two fields called "recv_keyid" and "recv_rnextkeyid" which inform userspace about what the remote is sending, I'm not seeing an equivalent on your side.
The specification around send_keyid in the RFC is conflicting: * User must be able to control it * Implementation must respect rnextkeyid in incoming packet
I solved this apparent conflict by adding a "TCP_AUTHOPT_FLAG_LOCK_KEYID" flag so that user can choose if it wants to control the sending key or let it be controlled from the other side.
The "send_rnextkeyid" is also optional in my patch, if TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID is not passed then the recv_id of the sending key is sent.
Here's a link to my implementation of key selection controls:
https://lore.kernel.org/netdev/2956d99e7fbf9ff2a8cc720c67baaef35bc32343.1660852705.git.cdleonard@gmail.com/
> +static int tcp_ao_parse_crypto(struct tcp_ao *cmd, struct tcp_ao_key *key) > +{ > + unsigned int syn_tcp_option_space; > + struct crypto_pool_ahash hp; > + bool is_kdf_aes_128_cmac = false; > + struct crypto_ahash *tfm; > + int err, pool_id; > + > + /* Force null-termination of tcpa_alg_name */ > + cmd->tcpa_alg_name[ARRAY_SIZE(cmd->tcpa_alg_name) - 1] = '\0'; > + > + /* RFC5926, 3.1.1.2. KDF_AES_128_CMAC */ > + if (!strcmp("cmac(aes128)", cmd->tcpa_alg_name)) { > + strcpy(cmd->tcpa_alg_name, "cmac(aes)"); > + is_kdf_aes_128_cmac = (cmd->tcpa_keylen != 16); > + }
Only two algorithms are defined in RFC5926 and you have to treat one of them as a special case. I remain convinced that generic support for arbitrary algorithms is undesirable; it's better for the algorithm to be specified as an enum.
-- Regards, Leonard
| |